fix(dropdown): container body keyboard navigation #3791
Merged
Conversation
This file contains 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
benouat
force-pushed
the
fix/dropdown/container-body-tab
branch
from
ae9c8d5
to
bc04267
Compare
maxokorokov
requested changes
Jul 7, 2020
There was a problem hiding this comment.
Hey, @benouat, LGTM!
Just have:
- a short question about
tabindex - a short request to refactor accessing to
anchorElviagetNativeElement()
Haven't checked the test coverage on my side.
Cheers,
Max
benouat
force-pushed
the
fix/dropdown/container-body-tab
branch
from
bc04267
to
6fda4bc
Compare
|
I created a new commit regarding the refactoring of how to access |
accross anchor/toggle and menu
benouat
force-pushed
the
fix/dropdown/container-body-tab
branch
from
6fda4bc
to
567ec09
Compare
maxokorokov
approved these changes
Jul 8, 2020
There was a problem hiding this comment.
Hey, @benouat, LGTM!
Just a small detail → I guess the second commit should be refactor, not fix, but I'm not sure if you want to squash them in the end or not.
Cheers,
Max
I'll squash it! |
maxokorokov
pushed a commit
that referenced
this pull request
Jul 8, 2020
Dropdown keyboard navigation is now fully supporting the "container="body" mode. Navigating a page with dropdowns using `Tab` does not break anymore, and follow the visual order of elements.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR is a follow up PR to #3625 which introduced the ability to close the dropdown when leaving it by the last elements.
Unfortunately, it did not fix the keyboard navigation when using
container="body"is used.Leaving the dropdown menu from the bottom when container is set to "body" leaves the user in a broken state, where the browser window url bar gets the focus.
The PR introduces a mechanism to catch the last Tab from the dropdown menu, and temporary set the focus back in a synchronous fashion to the dropdown anchor, hence letting the browser placing the focus on the next focusable element.
It also makes sure now that whenever the user wants to close the dropdown from the keyboard with Esc, the focus gets back to the dropdown toggle element.
This used to be the broken behaviour that now works
