-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(multiselect): add useEffect to selection hook to update externally #16139
fix(multiselect): add useEffect to selection hook to update externally #16139
Conversation
DCO Assistant Lite bot All contributors have signed the DCO. |
I have read the DCO document and I hereby sign the DCO. |
recheck |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 ✅
the setter function of a useState hook returns undefined, this was mistakenly used as the value to be set instead of sending the value directly
Hi, I noticed that an error in a different part of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this!
@guidari @preetibansalui This is something to look out for in #16147 |
0bc9c78
Thanks @tay1orjones, We will look into this. I have one doubt. For this PR, I know it is to provide controlled state only, but aren't we giving a wrong example here in storybook by selecting all items (including disabled item). |
Closes #16124
Update MultiSelect's
useSelection
hook to include auseEffect
that updates the selected items when they are updated externally (controlled component), since the recently addeduseState
is only executed on initial render.Changelog
Changed
Testing / Reviewing
Added a Storybook to test controlled MultiSelect components. The MultiSelect in this storybook should update its selected values when the included buttons are pressed, and a storybook action should be triggered.
Also added a unit test that tests this behavior.