Fix appearance of focused logo and service name in header

[paulrobertlloyd]

Description

Fixes #1369.

Reviewing the focused header states, a few issues become apparent:

• The focused NHS logo looks broken, with the bottom shadow merging with the logo, and only the top part of the logo showing a yellow outline
• A linked service name stretches the full width of the header

Note

The commentary below is out of date. I’m now using a different approach by adding a mixin to use for focusing images. See the updated fix described in #1361 (comment)

This PR fixes these issues by:

• Incorporating padding into the NHS logo SVG (this ensures consistent rendering, and saves having to make multiple adjustments in CSS)
• Removing the top yellow box shadow on the focused state for a linked service logo to ensure the yellow outline around the logo is evenly spaced
• Adding hover and active background colour to a linked service logo (except for in the white header variant) – these match the blue/darker blue hover/active colours used by the search submit button
• Ensuring the NHS logo is resized with the correct aspect ratio when resized for use alongside an organisation name
• Updating the padding for the organisation name and descriptor to match that of the NHS logo

and finally:

• Preventing a service name from growing to fill any remaining space in the header

Focused NHS logo

Before, focused:

After, focused:

After, hover:

After, active:

Focused organiaation logo

Before:

After:

Focused service name

Before:

After:

Checklist

• Tested against our testing policy (Resolution, Browser & Accessibility)
• Follows our coding standards and style guide
• CHANGELOG entry

[colinrotherham]

Incredible work 😮

I'm working odd hours tomorrow so had a look straight away!

Can I double check the logo padding inside the SVG?

It pushes the logo right/down a little bit—do we need to offset that?

[paulrobertlloyd]

A few notes:

• Adding padding to the NHS logo directly is one approach, downside being that when it is scaled down the padding decreases too… think we just about get away with it, but it does mean the padding is not guaranteed to always be 2px.
• The focus ring for form elements (including the search input) is 4px, not 2px. However, 2px matches the space we’ve added inside navigation items. So… 🤷🏼‍♂️
• The alternative approach I tried got complicated quickly, namely because it involves trying to override mixins for link/focus styles, in certain contexts only. Maybe there’s an approach where we lean on our good friend the pseudo element, and style that, independently of any link styles. Lots of different ways to skin this particular cat.

[paulrobertlloyd]

It pushes the logo right/down a little bit—do we need to offset that?

It does, this is more of an issue for the organisational logos perhaps than when we use the NHS logo on its own. Open to other approaches to fix this!

[colinrotherham]

Would the GOV.UK Frontend @include govuk-link-image mixin help?

Have a look at the image links example with double box shadow:

[paulrobertlloyd]

Ooooh, I’ll give it a try.

[paulrobertlloyd]

Updated this PR to add a new @nhsuk-link-image mixin (copied over from GOV.UK Frontend). This simplifies things greatly, and no longer requires messing about with the SVG.

I’ve removed the hover and active styles from the NHS logo however, as it gets weird when this is combined with a service name.

New comparison images below.

Focused NHS logo

Before:

After:

Focused NHS logo and service name combined

Before:

After:

Focused NHS organisation header

No change, baring one pixel increase in gap between logo and organisation name (thus the updated visual regression images, now only 22 down from 72 in the initial PR!)

Before:

After:

Focused service name

This remains unchanged from the initial commit for this PR.

Before:

After:

---

I have also added some extra visual regression tests for the focused NHS/service name/organisation name link.

[frankieroberto]

Nice!

There’s no hover state for the logo, do we think that’s ok? Currently (in production) there’s a dark blue border, although I’ve just noticed that this doesn’t work in Safari properly for some reason...

[colinrotherham]

Forgot to mention, we need :not(:active) on any focus styles (buttons included)

Until we align more with GOV.UK Frontend at least

[colinrotherham]

I've fixed @include nhsuk-link-image being overridden in the white header

Think we need a design opinion on whether the "expanded focus box" should have a solid outline all around

[colinrotherham]

Nice!

There’s no hover state for the logo, do we think that’s ok? Currently (in production) there’s a dark blue border, although I’ve just noticed that this doesn’t work in Safari properly for some reason...

@frankieroberto I gave this a try by adding:

  &:hover,
  &:focus,
  &:active {
    .nhsuk-header__organisation-name,
    .nhsuk-header__organisation-name-descriptor,
    .nhsuk-header__service-name {
      color: inherit;
    }
  }
+
+ &:hover {
+   .nhsuk-header__logo {
+     outline: $nhsuk-focus-width solid transparent;
+     box-shadow: 0 0 0 4px #003d78;
+   }
+ }

Do you know where #003d78 comes from, or what we'd use for the white header?

See what you mean about the Safari :hover issue though

[anandamaryon1]

Happy with the focus states, thanks for these @paulrobertlloyd

Seems a pity to drop the hover state, even though it was low-contrast. Could do a subtle colour or opacity change on hover, similar to buttons?

[paulrobertlloyd]

Added a hover state that attempts to replicate the underline used in the service/organisation name. Some notes:

• Requires using a pseudo element because Safari refuses to render anything outside the bounding box of the SVG on hover
• Uses currentcolor to improve maintainability
• The metrics for the underline can be tweaked later to match the decision made in Add link underline offset and make underline thicker on hover #1366

Combined logo and service name

Organisation name

[colinrotherham]

Obviously @paulrobertlloyd found a brilliant workaround, but!

What if we use his same ::before workaround to stretch a border around the SVG?

Just pushed the old hover style back up, but feel free to drop the commit if we prefer the underline

[paulrobertlloyd]

I tried to recreate the previous hover state originally once I figured out the work around, but then decided on the underline because it:

1. is visually distinguishable (squint and you can see the underline whereas the dark border looks blurred)
2. requires less CSS
3. mimics the hover style used elsewhere in NHS frontend, but also mimics that used for the logo hover state on GOV.UK

[colinrotherham]

No worries, it's gone. Like it never happened 😶

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[colinrotherham]

Happy for these hover/focus styles to go in @anandamaryon1?

Saw a thumbs up in #1361 (comment)

[paulrobertlloyd]

All I’ll add is that, when the link underline stuff gets merged, we should update the line-weight for the logo hover to match.

[anandamaryon1]

Great stuff