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: Allow null / "" / [] / {} again as option values #7370

Closed
VentuzTammoHinrichs opened this issue Oct 24, 2024 · 9 comments · Fixed by #7371
Closed

Dropdown: Allow null / "" / [] / {} again as option values #7370

VentuzTammoHinrichs opened this issue Oct 24, 2024 · 9 comments · Fixed by #7371
Assignees
Labels
Type: Bug Issue contains a defect related to a specific component.
Milestone

Comments

@VentuzTammoHinrichs
Copy link
Contributor

Describe the bug

Before PrimeReact 10, the Dropdown component was able to handle option values like null or empty strings or array or objects, and they worked like any other value.

Since updating to 10, this isn't the case anymore:

  • The documentation states "If optionValue is omitted and the object has no value property, the object itself becomes the value of an option" but the ObjectUtils.isNotEmpty call at
    return props.optionValue || ObjectUtils.isNotEmpty(optionValue) ? optionValue : option;
    makes sure that this falsely also applies to null and otherwise empty but defined values.
  • The code at
    if (props.value != null && options) {
    now clears the option selection whenever props.value is null and so prohibits the user from having an option with a null value. I get the reasoning behind this but it would be wiser to do the check only if the search for the option fails, so a null option value would work again.

Reproducer

No response

System Information

System:
    OS: Windows 11 10.0.22631
    CPU: (32) x64 AMD Ryzen 9 7950X 16-Core Processor
    Memory: 12.28 GB / 31.14 GB
  Binaries:
    Node: 20.15.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.7.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (130.0.2849.52)
    Internet Explorer: 11.0.22621.3527
  npmPackages:
    primereact: ^10.8.2 => 10.8.2
    react: ^18.3.1 => 18.3.1

Steps to reproduce the behavior

Try a Dropdown with the following options:

const options = {
  { label: "auto", value: null },
  { label: "none", value: 0 },
  { label: "one", value: 1 },
  { label: "many", value: 9001 },
}

Expected behavior

IMO null, "", [] and {} should be accepted as option values again - these are perfectly fine, distinct values, and needed in our use case. (as a workaround, I had add two functions that replace those values with dummy strings and then back, and you might agree that's not really elegant)

@VentuzTammoHinrichs VentuzTammoHinrichs added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Oct 24, 2024
@melloware
Copy link
Member

Please try 10.8.4 you are using 10.8.2 and i think this was fixed?

@melloware melloware added Status: Needs Reproducer Issue needs a runnable reproducer and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Oct 24, 2024
Copy link

Please fork the Stackblitz project and create a case demonstrating your bug report. This issue will be closed if no activities in 20 days.

@melloware
Copy link
Member

I believe was fixed with this: #7081

@VentuzTammoHinrichs
Copy link
Contributor Author

Please try 10.8.4 you are using 10.8.2 and i think this was fixed?

I'm sorry but the proposed fix for #7081 did sadly not fix our case (note that my source links are form 10.8.4):

  1. You added a check for the optionValue property there. If that property isn't set, and the component defaults back to "value", that isNotEmpty() path still gets taken. Yes, we could work around that by specifying optionValue="value" in all of our numerous DropDown incarnations, but I'd suggest changing this to something like typeof optionValue !== 'undefined' to make the code behave like documented.
  2. The null check in getSelectedOptionIndex still prohibits us from having a null value with a label.

I'll try to create a repro case as suggested.

@melloware
Copy link
Member

Oh in your printout above it says " primereact: ^10.8.2 => 10.8.2"

@melloware
Copy link
Member

PR is welcome

@melloware melloware added Type: Bug Issue contains a defect related to a specific component. and removed Status: Needs Reproducer Issue needs a runnable reproducer labels Oct 24, 2024
@melloware melloware added this to the 10.8.5 milestone Oct 24, 2024
@melloware
Copy link
Member

I assigned to you if you want to submit a PR please

@VentuzTammoHinrichs
Copy link
Contributor Author

https://stackblitz.com/edit/vitejs-vite-da87c5?file=src%2FApp.tsx

Ok seems that the first part is indeed fixed in 10.8.4 (seems the optionValue prop is getting filled in with a default so it's always there. I didn't see that, sorry), but the second still remains - if you click on "Auto" in the Dropdown the selection is cleared instead.

@melloware
Copy link
Member

OK cool fix is welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants