Skip to content

Commit

Permalink
Associate menu button with nav element in header
Browse files Browse the repository at this point in the history
The menu button is currently associated with the list inside the menu's navigation element through the use of `aria-controls`.

However, it's really the navigation itself that is being controlled, so it makes more sense to apply the `navigation` id to the nav than the list.

Because of the way the JavaScript uses aria-controls to find the toggled element, this also means that the `govuk-header__navigation--open` modifier class is being applied to the `<nav>`, so we also need to move the `govuk-header__navigation` and any `navigationClasses` passed to the component's macro to the `<nav>`.

Because we're now toggling the visibility of the navigation region itself, rather than the list within it, this also avoids having an empty nav region present on the page when on a mobile viewport with the menu closed.

Add a new `govuk-header__navigation-list` class to the list, and move the CSS required to reset the default user-agent list styles to this new class.

The `govuk-responsive-margin(2, "bottom")` mixin call is redundant as it was previously being overridden by the `margin: 0` that followed, so that needs to be removed in order to maintain the existing design.
  • Loading branch information
36degrees committed Nov 9, 2021
1 parent 494840b commit 817c90b
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 6 deletions.
3 changes: 1 addition & 2 deletions src/govuk/components/header/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,7 @@
}
}

.govuk-header__navigation {
@include govuk-responsive-margin(2, "bottom");
.govuk-header__navigation-list {
display: block;
margin: 0;
padding: 0;
Expand Down
4 changes: 2 additions & 2 deletions src/govuk/components/header/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@
{% 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 aria-label="{{ params.navigationLabel | default('Menu') }}">
<ul id="navigation" class="govuk-header__navigation {{ params.navigationClasses if params.navigationClasses }}">
<nav id="navigation" class="govuk-header__navigation {{ params.navigationClasses if params.navigationClasses }}" aria-label="{{ params.navigationLabel | default('Menu') }}">
<ul 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
3 changes: 1 addition & 2 deletions src/govuk/components/header/template.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ describe('header', () => {
const $ = render('header', examples['with navigation'])

const $component = $('.govuk-header')
const $list = $component.find('ul.govuk-header__navigation')
const $items = $list.find('li.govuk-header__navigation-item')
const $items = $component.find('.govuk-header__navigation-item')
const $firstItem = $items.find('a.govuk-header__link:first-child')
expect($items.length).toEqual(4)
expect($firstItem.attr('href')).toEqual('#1')
Expand Down

0 comments on commit 817c90b

Please sign in to comment.