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.

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

Left align header navigation items by default #1138

Open
wants to merge 5 commits into
base: header-breaking-changes
Choose a base branch
from

Conversation

paulrobertlloyd
Copy link
Contributor

@paulrobertlloyd paulrobertlloyd commented Feb 14, 2025

Description

  • Left align items in the header navigation
  • Provide a new modifier class, .nhsuk-header__navigation-list--justified which restores the previous behaviour.
  • Remove the previous modifier class, .nhsuk-header__navigation-list--left-aligned, as this presentation is now the default.
  • Increase the space between navigation items from 24px to 32px above the desktop breakpoint, which is a little closer the previous design (items look a bit too closer together above this breakpoint without making this change)
  • Fixes Header responsive layout breaks with JS disabled #1093, navigation items not wrapping if JavaScript is not enabled
  • Fixes issue with menu button being shown too early or not early enough on smaller screens
  • Fixes issue with older browsers (namely Safari 14 device-locked to older iPads) where column-gap is not supported on flex containers. Items are now separated by inline padding on navigation list items

Checklist

@paulrobertlloyd paulrobertlloyd changed the title Header navigation spacing Left align header navigation items by default Feb 14, 2025
@paulrobertlloyd paulrobertlloyd force-pushed the header-navigation-spacing branch from 792369f to f6c5a8c Compare February 14, 2025 19:21
@paulrobertlloyd
Copy link
Contributor Author

FYI, visual regression tests are failing because I only added those related to this change; the text is the search input changed in a previous commit, which is causing the failures.

Copy link
Contributor

@frankieroberto frankieroberto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it.

@anandamaryon1
Copy link
Collaborator

Thanks @paulrobertlloyd this is great, and bonus that it fixes #1093 !

Testing it, I find that items are being hidden behind the 'More' link earlier than they should? (Pretty sure there is enough room to not hide the last item)

Maybe a corresponding change to the JS needed?

This branch:
image

Header breaking branch (without this chnage):
image

@paulrobertlloyd paulrobertlloyd force-pushed the header-navigation-spacing branch from f6c5a8c to 0e61a66 Compare February 21, 2025 23:22
@paulrobertlloyd
Copy link
Contributor Author

paulrobertlloyd commented Feb 21, 2025

@anandamaryon1 Good catch! There was a simple fix to this, but the more I played around resizing the browser window, the more related issues I uncovered.

So, this PR now also addresses a number of additional issues relating to navigation items, namely showing the menu button correctly (not too early nor too late), occasionally showing more navigation items on smaller screens, and separates items by using padding instead of column-gap, meaning this part of the header should appear less broken in Safari 14/older iPads.

This prevents items appearing next to each other in browsers that do not support column-gap on flex containers and removes the need to use gap when calculating breakpoint widths in component JS.
@paulrobertlloyd paulrobertlloyd force-pushed the header-navigation-spacing branch from 0e61a66 to 6f0c5bf Compare February 21, 2025 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants