feat(PF4-Dropdown): Enhance keyboard interaction for a11y #815
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michaelkro
force-pushed
the
557-dropdown-keyboard-interaction
branch
from
d9542a3 to
8f7f6f1
Compare
Pull Request Test Coverage Report for Build 2779
💛 - Coveralls |
- Fixes esc key interaction - Introduces focus-trap-react Fixes patternfly#557
michaelkro
force-pushed
the
557-dropdown-keyboard-interaction
branch
from
8f7f6f1 to
27faf90
Compare
Collaborator
|
PatternFly-React preview: https://815-pr-patternfly-react-patternfly.surge.sh |
dgutride
previously approved these changes
Oct 19, 2018
michaelkro
changed the title
Contributor
|
Thanks for sharing the resources you found! I tested this for the sighted keyboard-only experience, and it works as expected. There are still some updates pending in #788 that alter the screen reader experience, which is blocking me from reviewing and confirming this works as expected across all screen readers. |
tlabaj
approved these changes
Oct 23, 2018
tlabaj
requested changes
Oct 23, 2018
| const { parentRef } = this.props; | ||
| const keyCode = event.keyCode || event.which; | ||
| if (keyCode === 27) { | ||
| if (keyCode === 27 && parentRef && parentRef.contains(event.target)) { |
Contributor
There was a problem hiding this comment.
There is a constant for the escape key defined in the internal/constants.js can we use that here?
jeff-phillips-18
previously approved these changes
Oct 24, 2018
michaelkro
dismissed stale reviews from jeff-phillips-18 and dgutride
via
81d79ea
Collaborator
|
Deploy preview for patternfly-react-gone ready! Built with commit 81d79ea https://deploy-preview-815--patternfly-react-gone.netlify.com |
dlabaj
approved these changes
Oct 24, 2018
This was referenced Oct 24, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #557
What
focus-trap-reactdependency (documentation)esckey interaction when there are multipleDropdowns on the same page. Hitting theesckey will close the menu and return focus to that particularDropdown's toggle. Previously, focus would get shifted to the lastDropdownon the screenfocus-trap-reactfocus-trap-reactwhile reading the Programmatically Managing Focus section in the React docs regarding a11yfocus-trap-reactFocus Trap Resources