Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix header navigation overflow on desktop #1062

Merged
merged 3 commits into from
Oct 31, 2024
Merged

Fix header navigation overflow on desktop #1062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e173741
Fix header navigation overflow on desktop
paulrobertlloyd Oct 24, 2024
1c3a251
Update header drop-down navigation styles for desktop breakpoint
paulrobertlloyd Oct 31, 2024
d43f337
Update CHANGELOG
paulrobertlloyd Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

:wrench: **Fixes**

- Fix navigation items not flowing into the overflow drop-down menu on desktop ([PR 1062](https://github.com/nhsuk/nhsuk-frontend/pull/1062))
- Update header styles so that `.nhsuk-header__search-no-nav` class is no longer needed when header contains a search field but no navigation ([PR 1046](https://github.com/nhsuk/nhsuk-frontend/pull/1046))
- Update navigation list item padding to vertically align navigation items with width container ([PR 1033](https://github.com/nhsuk/nhsuk-frontend/pull/1033))

Expand Down
16 changes: 3 additions & 13 deletions packages/components/header/_header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,10 +412,6 @@
padding-right: 23px;
}

@include mq($from: desktop) {
display: none;
}

.nhsuk-icon__chevron-down {
right: -3px;
}
Expand Down Expand Up @@ -498,19 +494,13 @@
left: 0;

.nhsuk-header__navigation-link {
color: $color_nhsuk-blue;
padding: 12px nhsuk-spacing(3);
@include govuk-width-container;
@include nhsuk-link-style-no-visited-state;
padding: 12px 0;
}

.nhsuk-header__navigation-item {
border-top: 1px solid $color_nhsuk-grey-5;
margin-bottom: 0;

@include mq($from: large-desktop) {
border-top: 0;
margin: 0;
text-align: center;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/components/header/header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ class Header {
*/

updateNavigation() {
const availableSpace = this.navigation.offsetWidth
const availableSpace = this.navigationList.offsetWidth
let itemsVisible = this.navigationList.children.length

if (availableSpace < this.breakpoints[itemsVisible - 1]) {
Expand Down
50 changes: 24 additions & 26 deletions tests/integration/jsdom/header.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,21 @@ import Header from '../../../packages/components/header/header.js'
describe('Header class', () => {
beforeEach(() => {
document.body.innerHTML = `
<div class="nhsuk-navigation"></div>
<div class="nhsuk-header__navigation-list">
<li style="width: 50px;">Health A-Z</li>
<li style="width: 75px;">NHS services</li>
<li style="width: 100px;">Live Well</li>
<li style="width: 125px;">Mental health</li>
<li style="width: 150px;">Care and support</li>
<li style="width: 275px;">Pregnancy</li>
<li style="width: 200px;">Home</li>
<li style="width: 225px;">More</li>
</div>
<div class="nhsuk-header__menu-toggle"></div>
<div class="nhsuk-mobile-menu-container"></div>
<div class="nhsuk-header__menu-toggle"></div>
`
<div class="nhsuk-navigation"></div>
<div class="nhsuk-header__navigation-list">
<li style="width: 50px;">Health A-Z</li>
<li style="width: 75px;">NHS services</li>
<li style="width: 100px;">Live Well</li>
<li style="width: 125px;">Mental health</li>
<li style="width: 150px;">Care and support</li>
<li style="width: 275px;">Pregnancy</li>
<li style="width: 200px;">Home</li>
<li style="width: 225px;">More</li>
</div>
<div class="nhsuk-header__menu-toggle"></div>
<div class="nhsuk-mobile-menu-container"></div>
<div class="nhsuk-header__menu-toggle"></div>
`
})

it('Should create navigation elements in the DOM', async () => {
Expand Down Expand Up @@ -50,7 +50,7 @@ describe('Header class', () => {
})

it('Should close menu when escape key is pressed', async () => {
//define a event for the escape key
// Define a event for the escape key
const escapeKeyEvent = new KeyboardEvent('keydown', {
key: 'Escape',
code: 'Escape',
Expand All @@ -62,18 +62,18 @@ describe('Header class', () => {

await Header()

//Expect the menu to be hidden initially
// Expect the menu to be hidden initially
expect(
document.querySelector('.nhsuk-header__drop-down').classList.contains('nhsuk-header__drop-down--hidden')
).toBe(true)

//Toogle the menu - open it
// Toggle the menu - open it
toggleButton.click()
expect(
document.querySelector('.nhsuk-header__drop-down').classList.contains('nhsuk-header__drop-down--hidden')
).toBe(false)

//Press the escape key to close it
// Press the escape key to close it
document.dispatchEvent(escapeKeyEvent)
expect(
document.querySelector('.nhsuk-header__drop-down').classList.contains('nhsuk-header__drop-down--hidden')
Expand All @@ -88,16 +88,16 @@ describe('Header class', () => {
})

it('Should setup the Mobile Menu List during initialization', async () => {
//Initially there won't be any ul elements inside the container- it gets added in the setupMobileMenu method
// Initially there won't be any ul elements inside the container- it gets added in the setupMobileMenu method
let mobileMenuList = document.querySelector('.nhsuk-mobile-menu-container ul')

//So we expect that to be null until it gets created
// So we expect that to be null until it gets created
expect(mobileMenuList).toBe(null)

// Call the Header initialization function
await Header()

//We update the variable to hold the ul element from the container that has been created
// We update the variable to hold the ul element from the container that has been created
mobileMenuList = document.querySelector('.nhsuk-mobile-menu-container ul')

expect(mobileMenuList).not.toBeNull()
Expand All @@ -108,15 +108,14 @@ describe('Header class', () => {
it('Should not update navigation when the available space is enough for all elements', async () => {
const mobileMenuToggleButton = document.querySelector('.nhsuk-header__menu-toggle')
const mobileMenuContainer = document.querySelector('.nhsuk-mobile-menu-container')
const navigationElement = document.querySelector('.nhsuk-navigation')
const navigationList = document.querySelector('.nhsuk-header__navigation-list')
let mobileMenuList = document.querySelector('.nhsuk-mobile-menu-container ul')

// Spy on offsetWidth property for navigation element
const navigationOffsetWidthSpy = jest.spyOn(HTMLElement.prototype, 'offsetWidth', 'get')
// Mock offsetWidth for navigation element
navigationOffsetWidthSpy.mockImplementation(function () {
if (this === navigationElement) {
if (this === navigationList) {
return 1000 // Mock navigation element offsetWidth
}
return 50 // Mock children offsetWidth
Expand All @@ -140,15 +139,14 @@ describe('Header class', () => {
it('Should update navigation when the available space is not enough for all elements', async () => {
const mobileMenuToggleButton = document.querySelector('.nhsuk-header__menu-toggle')
const mobileMenuContainer = document.querySelector('.nhsuk-mobile-menu-container')
const navigationElement = document.querySelector('.nhsuk-navigation')
const navigationList = document.querySelector('.nhsuk-header__navigation-list')
let mobileMenuList = document.querySelector('.nhsuk-mobile-menu-container ul')

// Spy on offsetWidth property for navigation element
const navigationOffsetWidthSpy = jest.spyOn(HTMLElement.prototype, 'offsetWidth', 'get')
// Mock offsetWidth for navigation element
navigationOffsetWidthSpy.mockImplementation(function () {
if (this === navigationElement) {
if (this === navigationList) {
return 700 // Mock navigation element offsetWidth
}
return 100 // Mock children offsetWidth
Expand Down