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 47e79a9
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 16 deletions.
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 47e79a9

Please sign in to comment.