Improve the screen reader experience for the header menu #2427
Conversation
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2427
|
Raised as a draft for now – still need to do testing and write up an entry for the release notes. |
36degrees
force-pushed
the
header-navigation
branch
from
817c90b to
3f97233
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2427
|
Still needs testing with Android / TalkBack. |
|
|
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2427
|
Testing commit Move button inside navigation region with Talkback / Chrome (Android).
Note that after swiping to the menu using the landmark swipe, the next item in reading swipe order is the button, but this is not necessarily obvious from the focus border.
|
Move the 'Navigation menu' label from the `<ul>` to the parent `<nav>` element. This has previously been flagged as a 'possible misuse of `aria-label`.' According to WAI ARIA Practises 1.2 [2], it's recommended to label navigation regions, especially as 'if a page includes more than one navigation landmark, each should have a unique label' [3]. Lists can optionally be labelled, but this is only '_potentially_ beneficial for users of screen readers that support both list names and navigation among lists on a page'. Applying the label to both the list and the parent navigation could be noisy – "Potentially a source of distracting or undesirable screen reader verbosity, especially if nested within a named container, such as a navigation region." The provided examples [4] also apply the label consistently to the navigation region (albeit using `aria-labelledby`) [1]: #1340 (comment) [2]: https://www.w3.org/TR/wai-aria-practices-1.1/#naming_role_guidance [3]: https://www.w3.org/TR/wai-aria-practices-1.1/#aria_lh_navigation [4]: https://www.w3.org/TR/wai-aria-practices-1.1/examples/landmarks/navigation.html
In line with the section 'Composing Effective and User-friendly Accessible Names' in WAI-ARIA Authoring Practices 1.2 [1], remove the word navigation from the aria-label on the navigation. 'Do NOT include a WAI-ARIA role name in the accessible name. For example, do not include [..] the word "navigation" in the name of a navigation region. Doing so would create duplicate screen reader output since screen readers convey the role of an element in addition to its name.' Without this change, assistive technologies report this as 'Navigation menu navigation'. Also remove the word 'navigation' from the button that shows or hides the menu, as it also seems redundant – the word 'menu' should be descriptive enough for users to understand what it represents, especially when encountered inside a header landmark. [1]: https://www.w3.org/TR/wai-aria-practices-1.2/#naming_effectively
Move the menu button inside the navigation region, so that it is discoverable by screen reader users even when the nav is closed: > Place mobile open/close buttons within the <nav> element and use them to toggle state of another child wrapper of the nav. This will ensure that the navigation landmark is still discoverable by screen readers, even if it is in a closed/hidden state. https://a11y-style-guide.com/style-guide/section-navigation.html#kssref-navigation-navigation-mobile > Where a lot of people go wrong is by placing the button outside the region. This would mean screen reader users who move to the <nav> using a shortcut would find it to be empty, which isn’t very helpful. https://inclusive-components.design/menus-menu-buttons/#placement > For instance, a navigation on small screen devices may initially contain only a single toggle button. But that toggle button would reveal additional links and other navigational elements, when activated. https://www.scottohara.me/blog/2018/03/03/landmarks.html#navigation
36degrees
force-pushed
the
header-navigation
branch
from
537651d to
53a6c00
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2427
36degrees
changed the base branch from
main
to
header-navigation-bottom-border
36degrees
changed the title
<nav> the labelled and controlled element in the header, rather than the <ul> within it
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2427
| <nav aria-label="{{ params.navigationLabel | default('Menu') }}"> | ||
| <button type="button" class="govuk-header__menu-button govuk-js-header-toggle" aria-controls="navigation" aria-label="{{ params.menuButtonLabel | default('Show or hide menu') }}">Menu</button> | ||
|
|
||
| <ul id="navigation" class="govuk-header__navigation {{ params.navigationClasses if params.navigationClasses }}" > |
There was a problem hiding this comment.
I've left the govuk-header__navigation class and any other passed navigationClasses on the <ul>. There's an argument for moving them to the <nav> as well, but I tried to make the minimum set of changes to fix the issue. Happy to revisit though – it does seem odd to have the govuk-header__navigation element class not on the <nav>.
There was a problem hiding this comment.
Hm, tricky 🤔 Like you said, it seems a bit odd to be applying them to the <ul>. But I'm also not sure what people are using the navigationClasses for, which makes it difficult to know if they'd expect/find it useful for them to be applied to the <ul> or the <nav>. If we move it to the <nav> element, will we also need to provide a way for people to pass classes to the <ul>?
I'm not sure if we can separate the two - move the govuk-header__navigation class to the <nav> but keep the navigationClasses on the <ul>? Perhaps that's even more confusing though.
There was a problem hiding this comment.
Yeah, I think it'd make sense to keep the two together.
Arguments for keeping govuk-header__navigation and navigationClasses on the <ul>:
- Preserves existing behaviour
- We don't know what people are using
navigationClassesfor, so it's hard to reason about what impact the change would have
Arguments for moving govuk-header__navigation and navigationClasses to the <nav>:
- Seems more 'correct' / what users would expect
- Users can still style the list if they need to by passing e.g.
app-header__navigationas anavigationClassand then using.app-header__navigation ulto target it, but without this change would have no equivalent way to style the nav - We could make it possible to add classes to the
<ul>by introducing a new param, without having to make a breaking change - If we didn't do this and later wanted to make it possible to style the
<nav>we'd have to introduce a new param with a suboptimal name (e.g.navigationWrapperClasses) or come back and make this (breaking) change
Have I missed anything?
There was a problem hiding this comment.
Thanks for summarising, @36degrees, that's really helpful! I don't think you've missed anything. It's a really good point about being able to style the <ul> by using the navigationClasses but not being able to do it the other way around - I think that's convinced me that we should make the change. I think we might need to provide some additional guidance or examples somewhere to help people make the change though, maybe in the Changelog
There was a problem hiding this comment.
Yeah, I think I've talked myself into it too. Will push a commit…
36degrees
changed the title
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2427
Co-authored-by: EoinShaughnessy <72507742+EoinShaughnessy@users.noreply.github.com>
36degrees
force-pushed
the
header-navigation
branch
from
d0ae7e4 to
e0e4731
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2427
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2427
|
This change looks really good! Think we just need to add something to the Changelog about the classes change? |
Move the `govuk-header__navigation` class and any additional classes provided by the `navigationClasses` param to the `<nav>` element rather than the list. Create a new `govuk-header__navigation-list` for the list, and move the CSS properties that reset the user-agent default list styles and the properties that show and hide the list to this new class. The `--open` modifier class is now applied to the list (the nav itself now contains the menu button is always visible) so rename it so that it's clear it's applied to the list rather than the nav. We're making this change because it's more 'correct' and less 'surprising' – users would logically expect the `navigation` BEM element and the `navigationClasses` to be applied to the nav element itself. There may be a need to allow users to pass classes to apply to the list – but we can do that if and when it comes up. In the meantime, users will still be able to style the list by passing a class `navigationClasses` and targeting the `ul` as a descendant.
36degrees
force-pushed
the
header-navigation
branch
from
47e79a9 to
d085e96
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2427
I've added a section to the changelog entry which I think should cover it?
|
|
@EoinShaughnessy are you happy with that latest addition to the release notes? Happy for this to be merged once Eoin has reviewed. |
This brings the use of
aria-labelandaria-controlsin the menu navigation in the header in line with other 'best practise' examples and guidance from W3C.Move the label from the
<ul>to the<nav>Move the 'Navigation menu' label from the
<ul>to the parent<nav>element - this has previously been flagged as a 'possible misuse ofaria-label'.According to WAI ARIA Practises 1.2, it's recommended to label navigation regions, especially as 'if a page includes more than one navigation landmark, each should have a unique label'.
Lists can optionally be labelled, but this is only 'potentially beneficial for users of screen readers that support both list names and navigation among lists on a page'.
Applying the label to both the list and the parent navigation could be noisy – "Potentially a source of distracting or undesirable screen reader verbosity, especially if nested within a named container, such as a navigation region."
The provided examples also apply the label consistently to the navigation region (albeit using
aria-labelledby).Remove redundant 'navigation' from the nav label
In line with the section 'Composing Effective and User-friendly Accessible Names' in WAI-ARIA Authoring Practices 1.2, remove the word navigation from the aria-label on the navigation.
Without this change, assistive technologies report this as 'Navigation menu navigation'.
Also remove the word 'navigation' from the button that shows or hides the menu, as it also seems redundant – the word 'menu' should be descriptive enough for users to understand what it represents, especially when encountered inside a header landmark.
Move the button inside the
<nav>Move the menu button inside the navigation region, so that it is discoverable by screen reader users even when the nav is closed:
https://a11y-style-guide.com/style-guide/section-navigation.html#kssref-navigation-navigation-mobile
https://inclusive-components.design/menus-menu-buttons/#placement
https://www.scottohara.me/blog/2018/03/03/landmarks.html#navigation
Summary of changes to markup
Before
After
Summary of changes to AT experience
JAWS / Chrome
NVDA / Firefox
VoiceOver / Safari (macOS)
VoiceOver / Safari (iOS)
TalkBack (Android)
Tested by @lfdebrux in #2427 (comment)
Closes #1340