Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dropdown option fix #14080

Closed
wants to merge 5 commits into from
Closed

Conversation

rosenthalj
Copy link
Contributor

Fixes #14055
Fixes #14054

The pull request changes to dropdown.ts are slightly different then the changes I proposed in issue, I added logic for placeHolder:

&& !this.placeholder

The first video shows this pull request fixes the reproducer provided for issue #14055

DropdownTableFilterFix.mov

The second video shows this pull request fixes the reproducer provided for issue #14054. Note to be consistent with the "demo" reactive form example, I added '[autoDisplayFirst]="false"' to the reproducer html

DropdownOptionFix.mov

I have also tested the code changes with the PrimeNG dropdown demos. They appear to work properly. Note: I had to update two demo files because of missing attributes?

Copy link

vercel bot commented Nov 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
primeng-ssr-test ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 9, 2023 9:04pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primeng ⬜️ Ignored (Inspect) Visit Preview Nov 9, 2023 9:04pm

@vercel vercel bot temporarily deployed to Preview – primeng-ssr-test November 9, 2023 21:04 Inactive
@Jmurga16
Copy link

Now, if you include optionValue the dropdown not work, i select any option and this response blank.

@pkdigital
Copy link

Now, if you include optionValue the dropdown not work, i select any option and this response blank.

I think we just need to check for an optionValue and use the selectedOptionIndex, otherwise it'll try and resolve the label just from the value?

e.g.

 label = computed(() => {
        let selectedOptionIndex;
        this.autoDisplayFirst ? (!this.modelValue() ? (selectedOptionIndex = -1) : (selectedOptionIndex = this.findFirstOptionIndex())) : (selectedOptionIndex = this.findSelectedOptionIndex());
        return this.modelValue() && !this.optionValue ? this.getOptionLabel(this.modelValue()) : selectedOptionIndex !== -1 ? this.getOptionLabel(this.visibleOptions()[selectedOptionIndex]) : this.placeholder || 'p-emptylabel';
    });

@ashikjs
Copy link
Contributor

ashikjs commented Nov 13, 2023

@rosenthalj can you make single commit for a single issue

@cetincakiroglu
Copy link
Contributor

Hi,

Related code piece is refactored and the mentioned issue is fixed by the Core Team, thanks a lot for the effort and support!

Closing the PR

@rosenthalj rosenthalj deleted the dropdownOptionFix branch January 19, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants