Skip to content

Commit

Permalink
Move button inside navigation region
Browse files Browse the repository at this point in the history
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

This means we need to make the nav region always visible, so that the menu button is always visible (on mobile) – and therefore we need to revert some of the changes from 02ce19f:

- toggling the visibility of the list rather than the nav
- moving the aria-controls assocation back to the list, by moving the `navigation` id in the HTML
  • Loading branch information
36degrees committed Nov 23, 2021
1 parent 3f97233 commit 537651d
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 10 deletions.
4 changes: 2 additions & 2 deletions src/govuk/components/header/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,14 @@
}
}

.govuk-header__navigation {
.govuk-header__navigation-list {
display: none;
@include govuk-media-query ($from: desktop) {
display: block;
}
}

.govuk-header__navigation--open {
.govuk-header__navigation-list--open {
display: block;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/govuk/components/header/header.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Header.prototype.init = function () {
return
}

this.syncState(this.$menu.classList.contains('govuk-header__navigation--open'))
this.syncState(this.$menu.classList.contains('govuk-header__navigation-list--open'))
this.$menuButton.addEventListener('click', this.handleMenuButtonClick.bind(this))
}

Expand All @@ -45,7 +45,7 @@ Header.prototype.syncState = function (isVisible) {
* sync the accessibility state and menu button state
*/
Header.prototype.handleMenuButtonClick = function () {
var isVisible = this.$menu.classList.toggle('govuk-header__navigation--open')
var isVisible = this.$menu.classList.toggle('govuk-header__navigation-list--open')
this.syncState(isVisible)
}

Expand Down
6 changes: 3 additions & 3 deletions src/govuk/components/header/header.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ describe('Header navigation', () => {
await page.click('.govuk-js-header-toggle')
})

it('adds the --open modifier class to the menu, making it visible', async () => {
const hasOpenClass = await page.$eval('.govuk-header__navigation',
el => el.classList.contains('govuk-header__navigation--open')
it('adds the --open modifier class to the list, making it visible', async () => {
const hasOpenClass = await page.$eval('.govuk-header__navigation-list',
el => el.classList.contains('govuk-header__navigation-list--open')
)

expect(hasOpenClass).toBeTruthy()
Expand Down
7 changes: 4 additions & 3 deletions src/govuk/components/header/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@
</a>
{% endif %}
{% if params.navigation %}
<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>
<nav id="navigation" class="govuk-header__navigation {{ params.navigationClasses if params.navigationClasses }}" aria-label="{{ params.navigationLabel | default('Menu') }}">
<ul class="govuk-header__navigation-list">
<nav class="govuk-header__navigation {{ params.navigationClasses if params.navigationClasses }}" 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-list">
{% for item in params.navigation %}
{% if item.text or item.html %}
<li class="govuk-header__navigation-item{{ ' govuk-header__navigation-item--active' if item.active }}">
Expand Down

0 comments on commit 537651d

Please sign in to comment.