-
Notifications
You must be signed in to change notification settings - Fork 269
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
[ACS-4257] - Resolved a11y issue around jumping focus #8168
Conversation
…tches focus from button to first element
I've also noticed a similar around the permissions-list.component, when clicking on the Show/Hide button for inherited permissions. Clicking on the show button switches focus from the button, to the table header for permission name. There, it is happening because we are providing the selector for the element to which the focus needs to be switched. See template and directive The solution I am proposing for that is to remove the selector from the template itself. Note that this is not a reported issue yet, just something I noticed while working on the same area |
@@ -46,7 +46,6 @@ export class SearchFacetChipComponent { | |||
onMenuOpen() { | |||
if (this.menuContainer && !this.focusTrap) { | |||
this.focusTrap = this.focusTrapFactory.create(this.menuContainer.nativeElement); | |||
this.focusTrap.focusInitialElement(); |
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.
if you remove this line, the whole sense in having a focusTrab attribute gets lost... the only thing that is left is assigning this property, but not using it...
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.
The focusTrap does seem to work even after removing this line. I suspect that is because we are passing it the native element on which the focusTrap is to be initialised (see line num 48, right above this change). The user experience after this change came out to be -
- User performs a search (same as before, no change here)
- Once search results are received, he clicks on one of these (search filters?) chips (same as before, no change here)
- On clicking on the chip, the focus remains on the chip itself (changed from before, where the focus was getting jumped to the first element in the panel/popover)
- User presses Tab. The focus is switched to the first element in the popover (changed from before, this focus was received as soon as step#3 was performed)
- Now the user can keep on pressing 'Tab'. The focus remains within the fields in the popover (same as before, no change here)
Also, we do need to initialise the focusTrap and store it in a const reference. This is because we are using that reference when the popover is closed, to unregister the focusTrap (see line num 52 onwards)
@@ -47,7 +47,6 @@ export class SearchWidgetChipComponent { | |||
onMenuOpen() { | |||
if (this.menuContainer && !this.focusTrap) { | |||
this.focusTrap = this.focusTrapFactory.create(this.menuContainer.nativeElement); | |||
this.focusTrap.focusInitialElement(); |
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, you are essentially disabling this whole feature
… keyboard only usability
… into ACS-4257-swapnil-a11y-focus-jump
…tches focus from button to first element
… keyboard only usability
5137a5a
to
7b0ca30
Compare
… into ACS-4257-swapnil-a11y-focus-jump
Clicking on filter buttons on search results no longer switches focus from button to first element
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x")
What is the current behaviour? (You can also link to an open issue here)
Clicking on any of the filter button chips on the search results page would instantly switch focus from the button to the first element in the filter layover
What is the new behaviour?
Focus no longer switches to the first element. Instead, it remains on the button. Pressing tab moves and then traps the focus within the layover.
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: