Skip to content

NCRS-3933 add header with logo on same row as nav items #35

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

Merged
merged 30 commits into from
May 22, 2025

Conversation

alice-bell
Copy link
Contributor

@alice-bell alice-bell commented May 14, 2025

NCRS-3933 is a ticket to solve a problem with the text overlapping in the Masthead when the page is first loaded. This was causing failures in the integration tests, because the links get hidden behind the logo where they can't be clicked.

There is also a small PR in NCRS which includes further changes.

This branch is currently published to NPM as 2.1.0-test.4. To test the changes, you can pull my branch from the NCRS repo and run yarn install && yarn serve to play around with it locally.

See also my comment on the NCRS-3933 ticket explaining why it's been done this way.

image

The problem

All the headers supported by nhsuk-frontend have the nav links on a separate row, where they use the full width of the screen. However, we have a layout with everything on one row, due to space constraints.

image

When the nav-bar uses only part of the width, it doesn't know how much space there is until the other components (e.g. logo) have loaded. This meant that when the page is refreshed at a small size, the nav items don't automatically move into a drop-down menu. As soon as the page is resized, even by one pixel, the layout is recalculated and everything snaps into place.

Screenshare.-.2025-03-11.1_48_20.PM.3.mp4

The solution

This pull request copies the various files necessary for the header component into nhsuk-react-components-extensions, so that the header style can be reused by other repos.

The changes to the code are tiny. The original code ran calculateBreakpoints() followed by updateNavigation(), to work out the widths of the items in the header and then push them into the drop-down menu if needed. The issue was that calculateBreakpoints() didn't pick up the width of the logo on its first pass, so the logo ended up being overlapped by the nav items. All I've done is get calculateBreakpoints() and updateNavigation() to run twice, so that the second pass picks up the size of the logo and everything gets moved to the correct place.

Code changes:

  • run commands twice in header.js
  • a few minor edits to test files
  • added the component to Storybook (CSS modelled loosely on the current NCRS setup)

I've marked the important changes with comments. 90% of this work is an unchanged copy of existing code.

This sometimes gives a split-second view of the wrong layout before the page loads and it snaps into the right position. I can't think of a way to work around this (since the nav bar won't know the layout until the page has loaded, and therefore won't be able to put the items into the correct location) but I am open to suggestions if anyone can think of a workaround for this.

I've added a comment to the NCRS-3933 ticket, explaining why I've cloned all this into nhsuk-react-components-extensions instead of editing it at source.

alice-bell added 30 commits May 7, 2025 12:00
…does not exist on type JestMatchers' error in HeaderWithLogo test
Copy link

@alice-bell alice-bell changed the title Feature/ncrs 3933 add new header types NCRS-3933 add header with logo on same row as nav items May 22, 2025
@alice-bell alice-bell marked this pull request as ready for review May 22, 2025 07:43
@alice-bell alice-bell merged commit 983be7c into master May 22, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants