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

Remove bottom border from primary navigation #193

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

paulrobertlloyd
Copy link
Contributor

Adds a tint of the bottom border on a current items (which currently uses the link colour) as a 1px box shadow below it. This overlaps the bottom border of the primary navigation background which, when set against a dark colour creates a light dividing line.

Before After
Screenshot of current item before. Screenshot of current item after.

(Am also going to suggest this change on the GOV.UK One Login service header, at some point…)

@paulrobertlloyd paulrobertlloyd added this to the v3.0 milestone Dec 18, 2023
@paulrobertlloyd paulrobertlloyd added the enhancement New feature or request label Dec 18, 2023
@frankieroberto
Copy link
Contributor

Hmm, I'm not sure about the shadow - it maybe looks a bit subtle for the GOV.UK brand, which is otherwise all solid colours?

An alternative might be just have the blue border overlap the grey line, like this:

Screenshot 2023-12-19 at 10 45 33

This is what the (draft) secondary navigation current does.

Another alternative would be to remove the 1px border from the bottom of the primary nav altogether. It's arguably not needed, as the primary nav already has a background colour.

In my current service, I already removed this 1px grey border in an attempt to make the primary nav more visible (some users weren’t spotting it when it was introduced).

Teaching Vacancies, State of the Nation, Register trainee teachers and the MOJ Design System don't have the 1px border either...

@paulrobertlloyd
Copy link
Contributor Author

Yeah, removing the overall navigation border would be my preference too. I also agree that the bottom border doesn’t feel in line with the ‘sparseness’ of the GOV.UK aesthetic. The MOD Design System website also doesn’t have this border, either. I wonder if it was added more for the purposes of High Contrast mode, where a border would help distinguish the navigation from the rest of the page? In which case we can keep the border, but have it be transparent.

More than happy to do that in this PR; make the navigation border transparent. I can then add a PR upstream on the One Login service header with that suggestion (including a list of existing services that have decided to not include a dark grey bottom border).

@frankieroberto
Copy link
Contributor

Yeah, removing the overall navigation border would be my preference too. I also agree that the bottom border doesn’t feel in line with the ‘sparseness’ of the GOV.UK aesthetic. The MOD Design System website also doesn’t have this border, either. I wonder if it was added more for the purposes of High Contrast mode, where a border would help distinguish the navigation from the rest of the page? In which case we can keep the border, but have it be transparent.

More than happy to do that in this PR; make the navigation border transparent. I can then add a PR upstream on the One Login service header with that suggestion (including a list of existing services that have decided to not include a dark grey bottom border).

Sounds good - I feel like the consensus is moving to no-border. Visually, my suspicion is that the border makes the nav slightly less noticeable, as it looks a bit more like it belongs to global header stuff (which gets ignored), rather than the service itself. But that's purely a hunch and hard to measure!

@paulrobertlloyd paulrobertlloyd force-pushed the primary-navigation-current-item branch from 6767ae7 to 9ab5cf1 Compare December 19, 2023 15:53
@paulrobertlloyd paulrobertlloyd changed the title Add bottom shadow to current item in primary navigation Remove bottom border from primary navigation Dec 19, 2023
@paulrobertlloyd
Copy link
Contributor Author

Updated to remove dark grey bottom border, replacing it with a transparent 1px border.

The bottom margin of the component is adjusted to be -1px to prevent a transparent space appearing between this and any component that follows directly below it. Adding utility margin classes to the component can be used to add any (bottom) margin if needed, and override this behaviour.

@paulrobertlloyd paulrobertlloyd merged commit f2b3c40 into main Dec 19, 2023
2 checks passed
@paulrobertlloyd paulrobertlloyd deleted the primary-navigation-current-item branch December 19, 2023 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants