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 margins can be simplified slightly – there was previously a `margin: 0` in the same ruleset as the call to `govuk-responsive-margin` which meant that the margin was always 0 on mobile and 2 spacing points only on tablet and above, rather than being 'responsive'.

The extra margin being applied on tablet also appears to be a mistake, so switch to applying just applying the margin on desktop and above.
  • Loading branch information
36degrees committed Nov 17, 2021
1 parent 494840b commit 02ce19f
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
7 changes: 6 additions & 1 deletion src/govuk/components/header/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,12 @@
}

.govuk-header__navigation {
@include govuk-responsive-margin(2, "bottom");
@include govuk-media-query ($from: desktop) {
margin-bottom: govuk-spacing(2);
}
}

.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 02ce19f

Please sign in to comment.