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

Line up navigation items with header #999

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

frankieroberto
Copy link
Contributor

Description

Whilst working on the 'Logged in header' (nhsuk/nhsuk-service-manual-community-backlog#192) with colleagues, we discussed how the navigation items don't currently align on the left with the NHS logo and the right with the end of the horizontal line, due to padding within the navigation link items.

This becomes more noticeable if additional items (such as account navigation and sign out links) are added to the header.

This could be resolved by adding some negative margin to the navigation container, so that the first and navigation items move into the gutter slightly, but because of the padding the text now lines up.

This does mean they are now out of alignment when focused, but this seems ok?

As a small extra bonus, this also buys 32 extra pixels of width for the navigation list.

Current Proposed
before after
before-focused afer-focused

Checklist

Needed for absolute positioning of the drop-down nav
@frankieroberto frankieroberto marked this pull request as ready for review August 28, 2024 14:01
@frankieroberto frankieroberto changed the title line up navigation items with header Line up navigation items with header Aug 28, 2024
@anandamaryon1
Copy link
Contributor

The mobile styling still needs a little tweak to make it line up properly.

Wondering about the best way to do this…

Options I can think of:

  • removing the left and right padding on the links completely (like the gov service nav) and using the nhsuk-navigation-container's 16px margins
  • increasing the padding on the nav links to 16px so they align (but this spaces them out slightly more than currently, 32px vs 24px)
  • adding back in a slight margin left to the nav container nhsuk-navigation to line it up (4px)

Any thoughts? @frankieroberto

I'm leaning towards the gov service nav approach now, as it makes it much simpler, no need to add negative margins or tweak anything for responsive.

@frankieroberto
Copy link
Contributor Author

@anandamaryon1 the first option (removing left and right padding on the links) means a slightly smaller target area, but does seem simpler, and also means the yellow focus state also aligns with the header. Is that ok, do you think?

Would still be 56 pixels high (or 64 pixels high on mobile), and should always be at least 44 pixels wide so long as the labels are around 4 characters or more. The shortest one on NHS.UK ("Health A-Z" would be 80px wide without horizontal padding).

@paulrobertlloyd
Copy link
Contributor

There is another option of course…

Screenshot 2024-09-27 at 10 51 45

Benefits of this approach being:

  • Consistent with focus state used for links elsewhere in the design system
  • Doesn’t obscure any current section indicators
  • Much less wrangling of CSS (just took a quick peak, and there’s a lot that could go away if this were simpler)

@frankieroberto
Copy link
Contributor Author

I had a quick go at another option here: #1033.

@paulrobertlloyd worth considering that too! It does potentially mean either reducing the height of the target area (which service navigation does) or keeping the taller target area but only having the focus yellow background colour behind the text rather than the full height.

I'm also not totally convinced that having both an underline on the focused item AND the current item is necessary, and it does look a bit odd to me, but maybe it's ok?

Screenshot 2024-09-27 at 10 00 34

@anandamaryon1
Copy link
Contributor

Hmm, playing with the service nav example, I do think our increased target area is helpful, given that the links are within a navigation menu and will be clicked a lot. If we were to go with an option that reduces the visible focus state, I'd hope to maintain the biggest possible click area.

I don't think it's a big issue for the focus state to be misaligned, it only shows fleetingly in usage and the alignment of the default state has a bigger benefit.

Also, on the potential obscuring of a current page highlight, I don't think this is an issue, since on page load the focus state will not be showing, leaving any current page highlight visible (until a user focusses an item).

So, I now vote that we keep this as-is (as currently implemented in this PR).

@frankieroberto
Copy link
Contributor Author

@anandamaryon1 makes sense and I'd agree that bigger click areas areas are better!

I think I fixed the alignment on mobile on this PR, but definitely worth double-checking and doing some testing on different browsers.

Suggestions on the CSS also welcome - I'm still getting my head around how the CSS works for the header with all the different combinations of screen widths and number of nav items...

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