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

fix(dropdown): restore space bar and enter key item selection #3926

Merged

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Sep 6, 2019

Closes #3913

This PR fixes a regression introduced in #3586 and restores enter key and space bar item selection for the dropdown

Previously (pre-v10.5) individual items in the dropdown list fired click events on Enter keypress when selected, and the selected item information could be extracted from the event object. This behavior was changed so that focus is no longer placed on the individual dropdown items and is instead kept fixed on the dropdown menu. So in order to get the currently selected item information, we need to explicitly get the selected item info (getCurrentNavigation) and call .select()

on a side note, it seems that dropdowns now have 2 tab stops due to a tabindex on the dropdown menu even while hidden. this should probably be addressed in a separate PR

Changelog

New

  • NPE check for focused item before removing focus class
  • keyboard navigation tests for semantic markup introduced in Dropdown a11y updates #3586

Changed

  • select item after space/enter keypress
  • ignore nonrelevant events in toggle method
  • focus on trigger after item selection with keyboard

Testing / Reviewing

Ensure that keyboard selection of dropdown items functions properly

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for the-carbon-components ready!

Built with commit 9f66dc1

https://deploy-preview-3926--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for carbon-elements failed.

Built with commit 9f66dc1

https://app.netlify.com/sites/carbon-elements/deploys/5d7269bde4780e0008ed2e08

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for carbon-components-react failed.

Built with commit 9f66dc1

https://app.netlify.com/sites/carbon-components-react/deploys/5d7269bd84959f0008e67e08

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for the-carbon-components ready!

Built with commit 6c893e1

https://deploy-preview-3926--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for carbon-elements ready!

Built with commit 6c893e1

https://deploy-preview-3926--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Sep 6, 2019

Deploy preview for carbon-components-react ready!

Built with commit 6c893e1

https://deploy-preview-3926--carbon-components-react.netlify.com

@tw15egan
Copy link
Collaborator

tw15egan commented Sep 6, 2019

Should we add something to prevent the screen from scrolling when the spacebar is pressed when the menu is open?

@emyarod emyarod force-pushed the 3913-dropdown-keyboard-navigation branch from bcd3277 to dcf4b8d Compare September 6, 2019 18:11
@emyarod
Copy link
Member Author

emyarod commented Sep 6, 2019

added in latest commit @tw15egan

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 - Thanks alot @emyarod for taking a look at this issue!

Copy link
Collaborator

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅

@asudoh asudoh merged commit fc22773 into carbon-design-system:master Sep 7, 2019
@emyarod emyarod deleted the 3913-dropdown-keyboard-navigation branch September 11, 2019 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dropdown]: cannot select item with keyboard
5 participants