Skip to content

Commit

Permalink
Move navigation classes to the <nav>
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
36degrees committed Dec 3, 2021
1 parent e0e4731 commit d085e96
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 16 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ This change was introduced in [pull request #2323: Avoid invalid nesting of `<sp

We've updated the HTML for the header. This update only affects you if your header includes navigation.

Any additional classes passed using the `navigationClasses` Nunjucks option are now applied to the `<nav>` rather than the `<ul>`. Check that the additional classes are still doing what you expect.

If you're not using Nunjucks macros, then you should move:

- the `<button>` inside the `<nav>`, immediately before the `<ul>`
Expand Down
16 changes: 9 additions & 7 deletions src/govuk/components/header/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -206,16 +206,18 @@
}

.govuk-header__navigation {
display: block;
margin: 0;
padding: 0;
list-style: none;

@include govuk-media-query ($from: desktop) {
margin-bottom: govuk-spacing(2);
}
}

.govuk-header__navigation-list {
// Reset user-agent default list styles
margin: 0;
padding: 0;
list-style: none;
}

.js-enabled {
.govuk-header__menu-button {
display: block;
Expand All @@ -224,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
8 changes: 4 additions & 4 deletions src/govuk/components/header/header.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ describe('Header navigation', () => {
})

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')
const hasOpenClass = await page.$eval('.govuk-header__navigation-list',
el => el.classList.contains('govuk-header__navigation-list--open')
)

expect(hasOpenClass).toBeTruthy()
Expand Down Expand Up @@ -102,8 +102,8 @@ describe('Header navigation', () => {
})

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

expect(hasOpenClass).toBeFalsy()
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 @@ -54,10 +54,10 @@
</a>
{% endif %}
{% if params.navigation %}
<nav aria-label="{{ params.navigationLabel | default('Menu') }}">
<nav aria-label="{{ params.navigationLabel | default('Menu') }}" class="govuk-header__navigation {{ params.navigationClasses if params.navigationClasses }}">
<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 }}" >
<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
2 changes: 1 addition & 1 deletion src/govuk/components/header/template.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('header', () => {
const $ = render('header', examples['with navigation'])

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

0 comments on commit d085e96

Please sign in to comment.