-
Notifications
You must be signed in to change notification settings - Fork 338
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
Header navigation is hidden from assistive technologies on tablet/desktop view #1966
Labels
Comments
36degrees
added a commit
that referenced
this issue
Sep 22, 2020
In 18771ce (released as part of v3.9.0) we changed the logic of the header javascript to set the aria-expanded and aria-hidden attributes on the menu button and navigation respectively. However, the logic is applied at all breakpoints, not just the mobile breakpoint where the navigation is hidden, and so the navigation is currently not visible to screen reader users. As noted by @adamsilver, in this instance when the navigation is hidden it is already removed from the accessibility tree through the use of `display: none;`. As such, the `aria-hidden` attribute is redundant and can be removed: > If an interactive element is hidden using display:none or visibility:hidden (either on the element itself, or any of the element's ancestors), it won't be focusable, and it will also be removed from the accessibility tree. This makes the addition of aria-hidden="true" or explicitly setting tabindex="-1" unnecessary. > > https://www.w3.org/TR/using-aria/#4thrule This means that the only aria attribute we need to keep in sync is the `aria-expanded` attribute – which is applied to the menu control which is hidden when the menu is not collapsible, so it being ‘out of sync’ on the desktop breakpoint is not an issue. Fixes #1966.
36degrees
added a commit
that referenced
this issue
Sep 22, 2020
In 18771ce (released as part of v3.9.0) we changed the logic of the header javascript to set the aria-expanded and aria-hidden attributes on the menu button and navigation respectively. However, the logic is applied at all breakpoints, not just the mobile breakpoint where the navigation is hidden, and so the navigation is currently not visible to screen reader users. As noted by @adamsilver, in this instance when the navigation is hidden it is already removed from the accessibility tree through the use of `display: none;`. As such, the `aria-hidden` attribute is redundant and can be removed: > If an interactive element is hidden using display:none or visibility:hidden (either on the element itself, or any of the element's ancestors), it won't be focusable, and it will also be removed from the accessibility tree. This makes the addition of aria-hidden="true" or explicitly setting tabindex="-1" unnecessary. > > https://www.w3.org/TR/using-aria/#4thrule This means that the only aria attribute we need to keep in sync is the `aria-expanded` attribute – which is applied to the menu control which is hidden when the menu is not collapsible, so it being ‘out of sync’ on the desktop breakpoint is not an issue. Fixes #1966.
The behaviour here is even worse than it sounds, as the elements in the navigation are still focusable, but nothing is announced to the user. |
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What
The navigation in the header is hidden from screen readers and other assistive technologies because it includes
aria-hidden="true"
on the tablet/desktop view, despite being visible.Why
In #1942 (released in 3.9.0) we changed the logic of the header javascript to set the
aria-expanded
andaria-hidden
attributes on the menu button and navigation respectively. However, the logic is applied at all breakpoints, not just the mobile breakpoint where the navigation is hidden, and so the navigation is not available to screen reader users.The text was updated successfully, but these errors were encountered: