Add current item indicator for header navigation #1067
Conversation
|
I think this is a non-breaking change, and there aren’t any cross-dependencies on the header breaking change PR, so we could choose to either merge it in with those or ship it earlier as a minor release, depending on how things go? |
|
@paulrobertlloyd @anandamaryon1 I've updated this now so that it's merging into The 'current' item in the expanded menu is unchanged, using a blue box-shadow on the left. It does feel maybe a bit weird to use grey in one context and blue in the other, but then they're completely different designs so 🤷♂️? Possibly the expanded navigation would feel like more of a natural extension of the main navigation if it was also blue instead of white - but that'd be a much bigger change... |
|
Hey @frankieroberto, loving this work. I was just wondering do we know why the underlines were added to the nav items? The original nav didn't have underlines, I think they got added when the header was updated last year?! It feels a bit odd to me, and makes it a little harder to read, but maybe that's just me? Other websites don't tend do underline nav items. GOV.UK Design system 
BBC 
DAC 
W3.org
So it feels like if we have have evidence from users that underlines are needed on the main nav, then we should include them (and also feed that evidence back to gov.uk). But if we don't have any evidence, then maybe consider removing them again? |
Good question. I’d be up for removing the underlines on the links in the nav. Don't know if @anandamaryon1 has any history on this? |
Hmm I've had a look around and can't find anything definitive… I can see from some documentation that in June 2022 the header still had underlines. But in November 2020 the (nhs.uk website) header had no underlines. 🤔 I can try to do more digging and ask around. |
|
@frankieroberto how were you expecting the focus style to work on a current link? Currently the styling for the indicator overrides the focus style: Should it instead be overridden by the focus style, so it looks like this? i.e. same as other links. Or some custom style? Not sure that it's important to see visually that you're focussed on the current link? |
Opps, thought I’d checked this. Might have got un-done when I updated from the header breaking changes branch and had to switch from border-bottom to the box-shadow. Fixed in 860df16. 👍 |
|
Wonder whether we should add some guidance around this in the service manual. I notice that the GOV service nav doesn't, but I think it could be useful to give an example to show it's an option and explain that there are two different aria labels. Anything else we'd wanna cover? Nunjucks macros may be enough if not. |
|
@anandamaryon1 agree to having some good guidance around it - the Nunjucks macro options are often missed. What do you think to the thickness of the line at the bottom? I did wonder if it was too subtle and should be increased, but maybe it’s ok as is. Not quite sure how this PR also picked up the task list changes - will try and unpick that. |
|
@frankieroberto hmmI thought it was showing task list stuff because the header-breaking-changes branch was out of date with main so I updated it, but it's still showing them. Maybe swapping the base branch out and back again will fix it? I feel the underline thickness is enough as it is, since it's breaks up such a big strong shape (the blue header). |
Removes the link to `"/"` labelled `"Home"`, which is currently hardcoded, and only shows up within the navigation menu on mobile screen widths. This link may not be appropriate for all services, which might not have a homepage, or might use a different path for it. It is also unclear whether having a homepage link is always needed in the navigation if the NHS logo also goes to the homepage.
paulrobertlloyd
force-pushed
the
header-breaking-changes
branch
from
f5a7da9 to
08add40
Compare
|
@anandamaryon1 @paulrobertlloyd mobile current item indicator bumped up to 8px in ae3aaba:
Possibly a bit too close to the text but maybe it’s just about passable? |
|
Current item indicator on the white header navigation variation changed from grey to blue in 74b72c8:
|
|
...and just for good measure, here’s how it currently looks in the variant with the white header but the blue nav (which I only just discovered was an option): 
|
@frankieroberto What about 6px? |
paulrobertlloyd
force-pushed
the
header-breaking-changes
branch
from
9ab62ce to
a1ee8c1
Compare
|
@paulrobertlloyd @anandamaryon1 6px looks better to me. Means not being able to use the spacing scale but in this instance that seems ok? Changed in 3dc45ab 
|
There was a problem hiding this comment.
Nice work, thank you @frankieroberto
One tiny thing I spotted reviewing this:
Focus state's black underline is being overriden by the current status indicator's blue
|
@anandamaryon1 well spotted! Fixed in 6cd2e33. It was caused by the ordering of CSS and the I think this should now be mergeable into the header breaking changes branch. |
This adds the new current and active boolean options for primaryLinks in the header added in #1067 to the documentation in the component README.
Add support for indicating the current section, using either
current: true(if the user is currently on this exact page) oractive: true(if the user is in that section but not necessarily that exact page). Both are styled the same but have different ARIA attributes.When displayed in the regular view within the blue header, the active links have a light-grey border at the bottom, and no text underline:
When displayed in the expanded menu view (eg on mobile), they have a blue border to their left instead, and are slightly indented:
In addition, a
<strong>tag wraps the current item text, to give some indication in any scenarios where CSS is unavailable - based on the GOVUK service navigation component.Checklist