Fix focus issues #381
Merged
Fix focus issues #381
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
tombye
force-pushed
the
fix-focus-issues
branch
from
565ab84 to
d7f16a6
Compare
|
seems like an issue with latest ubuntu image, ssl version and phantomjs |
When using the dialog with a keyboard, focus is not set when you close it so the browser has to guess what to focus. This is usually on the document itself or the main content area, both of which are confusing to users. This focuses the button that opens the dialog when it closes, as per WAI ARIA advice from the W3C: https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/
The main content area has the following markup: ``` <div class="app-pane__content" tabindex="0"> <main> ... </main> </div> ``` `div.app-pane__content` has `tabindex=0` to make it focusable (`div`s are not by default). It is focusable so keyboard users can scroll it, using their arrow keys when it is tabbed to. This is important because the docs' page is split into 2 panes that scroll independently. `main` is focused when you click the 'skip to main content' link, at the start of the page, using the design system skip link component: https://design-system.service.gov.uk/components/skip-link/ These changes don't try to merge these tags but rather make the visuals show a single focus style for both, because users shouldn't care which one is focused. Both alow you to scroll the pane and represent the main content area. This commit also includes a change that removes the outline from links in the table of contents. Testing the other changes in this commit, I saw the outline style from the browser styles applied to table of contents links. focus-visible styles are displayed based on browser heuristics, which seem to kick in when the other changes are applied. This cancels them on the link without removing them from the child `<span>`.
tombye
force-pushed
the
fix-focus-issues
branch
from
d7f16a6 to
bcc23c5
Compare
tombye
added a commit
that referenced
this pull request
Jan 15, 2025
- [Fix focus issues with table of contents and main content pane](#381)
Open
tombye
added a commit
that referenced
this pull request
Jan 21, 2025
- [Fix focus issues with table of contents and main content pane](#381)
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.
What’s changed
Fixes the following accessibility issues discovered when using Notify's tech docs with a keyboard:
Identifying a user need
Something always needs to be in focus when using the keyboard, so you know what you're interacting with. These issues directly affect keyboard users by breaking that in a few places.
What happens to focus when you close the toc dialog
Before
toc_dialog_close_focus_issue_before.mov
After
toc_dialog_close_focus_issue_after.mov
Main content area being focusable on mobile
Before
main_content_area_focusable_on_mobile_before.mov
After
main_content_area_focusable_on_mobile_after.mov
Main area focus style
Before
main_content_area_focus_fix_before.mov
After
main_content_focus_fix_after.mov
Notes for reviewers
Better to read this commit-by-commit.