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

perf(dropdown): get option value judgment logic #7083

Merged
merged 6 commits into from
Aug 24, 2024

Conversation

betavs
Copy link
Contributor

@betavs betavs commented Aug 24, 2024

Defect Fixes

When submitting a PR, please also create an issue documenting the error and manually link to an issue or mention it in the description using #<issue_id>.

Feature Requests

Due to company policy, we are unable to accept feature request PRs with significant changes as such cases has to be implemented by our team following our own processes.
Smaller scaled feature implementations such as adding a property to a component will be considered for merging.

close #7064
Fix #7081
Fix #6617

Copy link

vercel bot commented Aug 24, 2024

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

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Aug 24, 2024 11:34am
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Aug 24, 2024 11:34am

@mtsh1000
Copy link

Defect Fixes

When submitting a PR, please also create an issue documenting the error and manually link to an issue or mention it in the description using #<issue_id>.

Feature Requests

Due to company policy, we are unable to accept feature request PRs with significant changes as such cases has to be implemented by our team following our own processes. Smaller scaled feature implementations such as adding a property to a component will be considered for merging.

close #7064

This won't fix the issue, optionValue should always be returned if it was defined in props even if its null or ""

@melloware
Copy link
Member

@mtsh1000 is correct this doesn't fix it. however what if the original optionValue was also NULL or "" how would the component know that is what you are using?

components/lib/dropdown/Dropdown.js Outdated Show resolved Hide resolved
components/lib/dropdown/Dropdown.js Outdated Show resolved Hide resolved
@melloware
Copy link
Member

Actually I changed it to !== undefined because null and "" would be treated as falsy.

@mtsh1000
Copy link

Actually I changed it to !== undefined because null and "" would be treated as falsy.

Great, thats the proper fix, thank you:)

components/lib/dropdown/Dropdown.js Outdated Show resolved Hide resolved
components/lib/dropdown/Dropdown.js Outdated Show resolved Hide resolved
@melloware
Copy link
Member

I think now i actually have the best correct fix. If either props.optionValue is set or the value is not empty it will use optionValue

components/lib/dropdown/Dropdown.js Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants