RFC: Combine service and transactionalService header options and remove layout modifiers

[paulrobertlloyd]

Follow up to #996 and fixes #997

Description

Prior to any work on adding account links to the header, or refining its design, this PR aims to reduce the complexity of the component template by refactoring the different variants and reducing the number of different layout variants and associated modifiers in the markup and CSS.

Note

In general, the header component appears to have suffered from a sticking plaster approach to adding features, accumulating tactical fixes that over time have increased the complexity of the Nunjucks template, its public API, the markup and CSS.

Before adding account links, we should aim to simplify and fix what is already here.

This PR:

• Removes the transactional boolean option and the transactionalService option. The component API now accepts organisation and service options, these being the 2 significant variants of the header. This means there are now only 2 sizes of NHS logo, of which only the organisation variant uses the smaller size.
• Removes the following layout modifiers:
  • .nhsuk-header__logo--only
  • .nhsuk-header__transactional--logo
  • .nhsuk-header__link--service
  • .nhsuk-header__search-no-nav (fixed in Update header styles so that .nhsuk-header__search-no-nav class is no longer needed #1046)
• Removes the .nhsuk-header__content#content-header container
• Removes the following transactional containers and styles:
  • .nhsuk-header__transactional-service-name
  • .nhsuk-header__transactional-service-name--link
• Adds a new .nhsuk-header__service-link style
• Uses a single path NHS logo, allowing it to inherit link and focus styles (broken out into Increase contrast of focused NHS logo in header #1047)
• Fixes weird clipping that occurs around the hover and/or focus state on the NHS logo
• Fixes additional padding appearing at the bottom of the header when there is no navigation (fixed in Update header styles so that .nhsuk-header__search-no-nav class is no longer needed #1046)
• Ensures style rules for the white variant only modify colour properties
• Removes a number of media queries and uses flex layout a bit more – there’s a lot more that can be achieved using this approach, but tried not to go overboard! (Addressed in Refactor header component styles #1059)

Some of these changes can (and probably should) be pulled out into smaller, non-breaking PRs, if required.

Handling long service names

Here’s a preview of a header with both navigation, search and a long service name, which addresses #997:

280px

|

280px (organisation)

400px

640px

800px

800px (organisation)

1024px

I have added the text-wrap: balance to .nhsuk-header__service-link; this is a relatively new CSS value, but can be used as a progressive enhancement; the screenshots above show it being used, but if a browser doesn’t support this value, a (long) service name will break later in its title.

Logo focus styles

Before	After

Checklist

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

[anandamaryon1]

This looks great @paulrobertlloyd. Nice work integrating the service name wrapping and the bonus feature of logo focus styles.

I suppose the most controversial part is removing the transactional (smaller logo) variant, so I'll try get some thoughts/reviews from others on this too.

Happy for you to split out some of these a smaller non-breaking PRs, if you can, but also happy to wait and release all of this together with the logged in header.

Note to self: It'll need corresponding service manual guidance updates.

[anandamaryon1]

Can I suggest something a bit ugly?

If we add
margin-top: -2px; margin-bottom: -3px;
to this class.

Then the logo won't be pulled down slightly when the service name wraps. (it's only a couple pixels so perhaps not worth this ugly code?)

[frankieroberto]

I like this approach! The difference between the "service" and "transactional service" is visually very small, and it's not clear what the difference between those 2 types of services is.

One other thing to consider is whether or not there should be an option to have a single link which wraps both the logo and the service name, like the current header with service name?

You could link both the NHS logo and the service name to the same page separately, but that’s probably not ideal accessibility-wise?

I don’t think there's any clear rule about when services should link the NHS logo to the NHS homepage vs their service homepage, but I imagine that one might make more sense for patient-facing services and the other for staff-facing ones (although might not be a 1:1 match).

[frankieroberto]

Rather than defaulting to /, I wonder if this should not link at all if no params.homeHref is present? Or alternatively it could link to https://www.nhs.uk but perhaps having no link at all would be safer, to avoid it being overlooked?

Or I guess a third option would be to display a visible error message to force a link to be specified?

[frankieroberto]

Defaulting to / feels safer here (should generally work for most services!) but again it might be worth having an option for no link at all, especially as the service name doesn't visually look like a link, and perhaps for some simple transactional services, a link isn't needed?

[frankieroberto]

Possibly one for a follow-up, but perhaps it would now make sense to recombine all the header CSS into a single file?

[paulrobertlloyd]

Many of the changes in this PR have been addressed in other PRs; merging transactional and service options needs more discussion, and best addressed in a more focused PR.