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

MultiSelect: not selecting correct value when "options" objects contain property "value" #3392

Closed
erik-vanlankvelt opened this issue Sep 29, 2022 · 22 comments · Fixed by #3613
Assignees
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Milestone

Comments

@erik-vanlankvelt
Copy link
Contributor

Describe the Bug
I am not seeing selected "values" as checked in the dropdown. When checkboxes are checked, incorrect value is returned when "onChange" is invoked. "options" objects passed in contain a property/key named "value".

Reproducer
https://codesandbox.io/s/kind-bouman-2wobns?file=/src/App.tsx

PrimeReact version
8.5.0

React version
17.x

Language
React/TypeScript

Build / Runtime
Storybook

Browser(s)
Chrome

Steps to reproduce the behavior

  • Using above link to reproduce, expand dropdown. Notice that none of the checkboxes are checked.
  • Select an item in the dropdown. Notice that what's populated as a selection in the input is incorrect.

Steps to reproduce correct behavior

  • Uncomment out const [value, setValue] = useState(defaultValueGood);
  • Comment out const [value, setValue] = useState(defaultValueBad);
  • Uncomment out const optionLabel = "name";
  • Comment out const optionLabel = "label";
  • Uncomment out options={optionsGood};
  • Comment out options={optionsBad};
  • Refresh

Screen Shot 2022-09-29 at 12 58 23 PM
Screen Shot 2022-09-29 at 12 58 11 PM

@github-actions github-actions bot added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Sep 29, 2022
@melloware
Copy link
Member

Fixed your sandbox you were setting default values incorrectly: https://codesandbox.io/s/mystifying-einstein-9bezvf?file=/src/App.tsx

@melloware melloware added Resolution: Invalid Issue or pull request is not valid in the latest version and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Sep 29, 2022
@erik-vanlankvelt
Copy link
Contributor Author

erik-vanlankvelt commented Sep 29, 2022

@melloware I appreciate you fixing my sandbox, apologies for that. This is still an issue however. Can we reopen it?
https://codesandbox.io/s/tender-engelbart-ho1o1t?file=/src/App.tsx

@erik-vanlankvelt
Copy link
Contributor Author

erik-vanlankvelt commented Sep 29, 2022

@melloware should I not be using selectedItemTemplate? I see what your change did, however, it won't work if there isn't a key of value in the options object. Is there some way to specify what the value selected should be? I would like the ability to have the value selected be the entire object, instead of a single value. Is this possible or am I confined to having to specify the value key in the options object? Thanks!
https://codesandbox.io/s/tender-engelbart-ho1o1t?file=/src/App.tsx

@melloware
Copy link
Member

Fixed your sandbox again to use selectedItemTemplate https://codesandbox.io/s/mystifying-einstein-9bezvf?file=/src/App.tsx

@erik-vanlankvelt
Copy link
Contributor Author

Thanks @melloware. Based on all the updates you've made in sandbox, it looks like the options object needs a value key every time. Would you please confirm if this is the case or not? See examples below. I appreciate you taking the time to review this.

For example:

This works

const optionsBad = [
  {
    label: "Enamel pin letterpress",
    value: 1
  },
  {
    label: "Farm-to-table man bun",
    value: 2
  },
  {
    label: "Plaid locavore typewriter",
    value: 3
  }
];

This will never work:

const optionsBad = [
  {
    label: "Enamel pin letterpress",
    id: 1
  },
  {
    label: "Farm-to-table man bun",
    id: 2
  },
  {
    label: "Plaid locavore typewriter",
    id: 3
  }
];

@melloware
Copy link
Member

melloware commented Sep 29, 2022

That will work just fine you just need to use 'optionValue="id"' to tell it to use the id field instead of default value field.

@erik-vanlankvelt
Copy link
Contributor Author

erik-vanlankvelt commented Sep 29, 2022

@melloware I see, thanks! So it really boils down to the format of the value prop. It can't be an array of objects. That's what I was hoping for but sounds like that's not an option, and value should be an array of string or number.

@melloware
Copy link
Member

I don't understand? Your use case makes no sense?

Can you update your code sandbox to show what you are actually trying to do?

You want the value to be instead of a single number and array of numbers?

@erik-vanlankvelt
Copy link
Contributor Author

Here is my use case that works: https://codesandbox.io/s/friendly-bardeen-14m1u5?file=/src/App.tsx.

Here is my use case that doesn't: https://codesandbox.io/s/hidden-field-p7ivin?file=/src/App.tsx.

I think it is related to this line, where if the option has a value key, then it uses that instead of the whole option object.

Sorry for the confusion.

@melloware
Copy link
Member

Yes but in your example that doesn't work you are not using "optionValue="id" to tell it to use the Id value as your option?

@erik-vanlankvelt
Copy link
Contributor Author

In the example that doesn't work https://codesandbox.io/s/hidden-field-p7ivin?file=/src/App.tsx, setting optionValue="id" doesn't work and neither does optionValue="value".

@melloware
Copy link
Member

Ok let me take a look. I think I see what you are saying.

@erik-vanlankvelt
Copy link
Contributor Author

@melloware just checking in, any updates? Should I submit a PR?

@melloware
Copy link
Member

Feel free.

@melloware melloware added Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add and removed Resolution: Invalid Issue or pull request is not valid in the latest version labels Oct 4, 2022
@melloware melloware reopened this Oct 4, 2022
@github-actions github-actions bot 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 4, 2022
@melloware melloware removed the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Oct 4, 2022
@erik-vanlankvelt
Copy link
Contributor Author

@melloware PR submitted

@melloware melloware added this to the 8.6.2 milestone Oct 10, 2022
@mertsincan mertsincan modified the milestones: 8.7.0, 8.7.1 Oct 25, 2022
@erik-vanlankvelt
Copy link
Contributor Author

@melloware @mertsincan any chance this can be added to the next release?

@melloware
Copy link
Member

I was waiting for PrimeTek to review before merging.

@erik-vanlankvelt
Copy link
Contributor Author

Awesome, thanks! I wasn't sure what your process was, so just checking in.

@mertsincan mertsincan modified the milestones: 8.7.1, 8.7.2 Nov 1, 2022
@mertsincan mertsincan modified the milestones: 8.7.2, 8.7.3 Nov 10, 2022
@erik-vanlankvelt
Copy link
Contributor Author

@mertsincan is there any particular reason why this is not being included in releases? I can't use this project without this change being released and may have to use a different library if this isn't going to be released soon. Thanks

@melloware
Copy link
Member

@erik-vanlankvelt sent you a note in your PR there is a merge conflict.

@erik-vanlankvelt
Copy link
Contributor Author

@melloware I mucked up the git history so created a new PR for this that's clean. I'm sorry for the inconvenience.
#3613

@melloware
Copy link
Member

Thanks for the PR! Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Projects
None yet
3 participants