-
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): fix phantom selection on MenuKeyDownEnter 👻 #9765
fix(MultiSelect): fix phantom selection on MenuKeyDownEnter 👻 #9765
Conversation
✔️ Deploy Preview for carbon-react-next ready! 🔨 Explore the source changes: 85674db 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/615d4e043a43d70007832a71 😎 Browse the preview: https://deploy-preview-9765--carbon-react-next.netlify.app |
✔️ Deploy Preview for carbon-elements ready! 🔨 Explore the source changes: a37cd32 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/6153b02cd985d900074425ba 😎 Browse the preview: https://deploy-preview-9765--carbon-elements.netlify.app |
✔️ Deploy Preview for carbon-components-react ready! 🔨 Explore the source changes: a37cd32 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/6153b02ccba63e0008df2832 😎 Browse the preview: https://deploy-preview-9765--carbon-components-react.netlify.app/ |
✔️ Deploy Preview for carbon-elements ready! 🔨 Explore the source changes: 85674db 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/615d4e04ae94660008882668 😎 Browse the preview: https://deploy-preview-9765--carbon-elements.netlify.app |
✔️ Deploy Preview for carbon-components-react ready! 🔨 Explore the source changes: 85674db 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/615d4e046730830007b5c158 😎 Browse the preview: https://deploy-preview-9765--carbon-components-react.netlify.app |
@dakahn looks good to me after testing :) Just one thing: https://react.carbondesignsystem.com/?path=/story/components-select--default Is the expectation for default Also - do we still need this PR? #9522 |
ea568a0
to
92308db
Compare
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. doesnt close or choose anything on enter
Closes #9367
Adds a conditional to the switch case on l193 that checks that an item was actually highlighted and selected. Otherwise it just calls
break
Testing / Reviewing
In the MultiSelect storybook hi tab to focus the form element. Then hit enter to open the element. Hit enter again and no selection should be made