Skip to content
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

Closed
36degrees opened this issue Sep 22, 2020 · 1 comment · Fixed by #1967
Closed

Header navigation is hidden from assistive technologies on tablet/desktop view #1966

36degrees opened this issue Sep 22, 2020 · 1 comment · Fixed by #1967
Labels

Comments

@36degrees
Copy link
Contributor

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 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 not available to screen reader users.

@36degrees 36degrees added accessibility ⚠️ high priority Used by the team when triaging header labels Sep 22, 2020
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.
@36degrees
Copy link
Contributor Author

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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant