-
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(Dropdown, ComboBox): adds listener to open/close menu on enter #3862
fix(Dropdown, ComboBox): adds listener to open/close menu on enter #3862
Conversation
Deploy preview for the-carbon-components ready! Built with commit f17071b https://deploy-preview-3862--the-carbon-components.netlify.com |
Deploy preview for carbon-components-react ready! Built with commit d45fac4 https://deploy-preview-3862--carbon-components-react.netlify.com |
Deploy preview for carbon-elements ready! Built with commit d45fac4 |
Deploy preview for carbon-components-react ready! Built with commit f17071b https://deploy-preview-3862--carbon-components-react.netlify.com |
Deploy preview for carbon-elements ready! Built with commit f17071b |
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! 🎉
@@ -12,6 +12,8 @@ import React from 'react'; | |||
import { settings } from 'carbon-components'; | |||
import { WarningFilled16 } from '@carbon/icons-react'; | |||
import ListBox, { PropTypes as ListBoxPropTypes } from '../ListBox'; | |||
import { match } from '../../internal/keyboard/match'; | |||
import * as keys from '../../internal/keyboard/keys'; |
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.
Can we simplify this to be import { match, keys } from '../../internal/keyboard';
?
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.
Yeah for sure, made the change!
@@ -12,6 +12,8 @@ import React from 'react'; | |||
import { settings } from 'carbon-components'; | |||
import { WarningFilled16 } from '@carbon/icons-react'; | |||
import ListBox, { PropTypes as ListBoxPropTypes } from '../ListBox'; | |||
import { match } from '../../internal/keyboard/match'; | |||
import * as keys from '../../internal/keyboard/keys'; |
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.
Same as above
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.
Tiny style nits for consistency otherwise looks great! 👍✅
… into fix-dropdown-enter-doesnt-collapse-list
…hub.com/abbeyhrt/my-carbon-fork into fix-dropdown-enter-doesnt-collapse-list
Closes #2823
Components using Downshift, like Dropdown and ComboBox, were not opening on enter, this adds
toggleMenu
from Downshift to toggle the menu whenisOpen
changes.*Note: this problem also affects MultiSelect, since it uses Downshift but is more complicated to fix. I've made an issue (#3861) and will work on a solution for that separately.
Changelog
Changed
ListBox.Field
to toggleMenu on enterTesting / Reviewing
Go to ComboBox and Dropdown, tab into them and make sure they toggle open/close with Enter press.