Header breaking changes nunjucks params #1109

frankieroberto · 2025-01-31T11:15:40Z

This updates the params for the Header nunjucks component to no longer differentiate between 'transactional' and non-transactional services, and instead use the same Nunjucks params for both.

A separate additional option has been added for the logo param: includeWithServiceLink: true - this would enable the behaviour of combining the logo and the service name as a single link (matching the current non-transactional service name option). We’d create some separate guidance on when to use this option, although it will likely be context-dependent and up to teams to decide when to do this based on their research.

This also removes the default link target of / for the service name and NHS logo. This allows either of them to be un-linked if necessary.

Finally, there’s also some smaller changes to the nunjucks params to make them more consistent and logical, such as changing homeHref to logo.href, service.name to service.text and organisation.logoURL to logo.src.

Full list of Nunjucks changes

Before	After
`transactionalService.name`	`service.text`
`transactionalService.href`	`service.href`
`service.name`	`service.text` and optionally `logo.includeWithServiceLink: true`
`organisation.logoURL`	`logo.src`
`homeHref`	`logo.href`

Update HTML and CSS classes

The HTML and CSS classes have been updated to more closely follow the Block Element Modifier (BEM) convention.

The previous nhsuk-header__link and nhsuk-header__link--service have been removed and replaced with:

Element	CSS class
Logo container	`.nhsuk-header__logo`
Logo container (if there’s no service name sibling)	`.nhsuk-header__logo--no-service-name`
Logo link	`.nhsuk-header__logo__link`
NHS logo itself	`.nhsuk-logo`
Service name container	`.nhsuk-header__service-name`
Service name link	`.nhsuk-header__service-name__link`

Remaining questions

Should we also support service.html if you needed to add some <span>s or something? (No known use case so far)
There’s a bunch of slightly-confusing param names under organisation - eg organisation.split. Should we change those too, or leave as-is?

Checklist

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

…unjucks-params

paulrobertlloyd · 2025-02-02T23:37:18Z

Perhaps you could outline what the expected params would be for the following headers:

Logo only
Logo only (linked)
Logo and service name (only logo linked)
Logo and service name (only service name linked – would this variant be possible?)
Logo and service name (logo and service name have separate links)
Logo and service name (logo and service name share the same link)

And in all cases ‘logo’ would be either the NHS logo, or an organisation logo.

Might we want to add/update the examples to include these different variants? (Might want to do this anyway to test that the suggested CSS changes work across these different scenarios, too).

paulrobertlloyd · 2025-02-02T23:38:57Z

Perhaps in a separate PR we can also rename primaryLinks to navigation? Given we’ve essentially changing a fair number of variables, might as well take the hit and address others parameter names, too.

frankieroberto · 2025-02-03T10:33:12Z

Perhaps you could outline what the expected params would be for the following headers:

@paulrobertlloyd here’s how that would look at the moment. There’s not currently (either in the current release or the work-in-progres) possible to not have either the NHS logo or the service name be a link, as they both default to /. We could remove the default and allow either to be un-linked though?

Example	Params
Logo only	Not currently possible - logo is always linked, defaults to `/`
Logo only (linked)	`logo: { href: "#" }`
Logo and service name (only logo linked)	Not currently possible - service name is always linked and defaults to `/`
Logo and service name (only service name linked – would this variant be possible?)	Not currently possible
Logo and service name (logo and service name have separate links)	`logo: { href: "#" }, service: { text: "Service name", href: "#" }`
Logo and service name (logo and service name share the same link)	`logo: { includeWithServiceLink: true }, service: { text: "Service name", href: "#" }`

Might we want to add/update the examples to include these different variants? (Might want to do this anyway to test that the suggested CSS changes work across these different scenarios, too).

I think most variations are already included, but we could add a few more.

frankieroberto · 2025-02-03T12:38:56Z

Perhaps in a separate PR we can also rename primaryLinks to navigation? Given we’ve essentially changing a fair number of variables, might as well take the hit and address others parameter names, too.

@paulrobertlloyd I don’t mind primaryLinks that much. Arguably it's clearer than navigation, which could be anything? Could shorten to primary though?

app/components/header/header-service-name.njk

anandamaryon1 · 2025-02-05T16:33:57Z

Thanks for this @frankieroberto.

I can be tempted either way on navigation vs primaryLinks so maybe keep as-is for now.

On the defaulting of the logo to be linked: currently you'd have to customise the header to make the logo/name have no link right? (as is done on NHS Login). Not sure whether there are other examples (pages that act like dialogs, e.g. confirm delete)? So, maybe it would be helpful to not force a link. Interested in any other thinking on it though.

frankieroberto · 2025-02-07T16:24:13Z

@anandamaryon1:

I can be tempted either way on navigation vs primaryLinks so maybe keep as-is for now.

👍

On the defaulting of the logo to be linked: currently you'd have to customise the header to make the logo/name have no link right? (as is done on NHS Login). Not sure whether there are other examples (pages that act like dialogs, e.g. confirm delete)? So, maybe it would be helpful to not force a link. Interested in any other thinking on it though.

An option for no-link on either the NHS logo or the service name could be useful - and probably better to avoid teams having to do custom HTML to do this (which is harder to maintain).

To enable this we could just remove the default / href from the logo and service objects. That might break things for anyone currently relying on this, but then it’s a breaking change anyway...

…ader-breaking-changes-nunjucks-params

frankieroberto · 2025-02-13T15:36:53Z

@anandamaryon1 @paulrobertlloyd @colinrotherham I’ve updated this to remove the default / href for the service name and logo - this now allows all possible combinations of:

NHS logo only, linked
NHS logo only, un-linked
NHS logo and service name, each with separate links
NHS logo and service name combined as a single link
NHS logo and service name, neither linked
NHS logo and service name, only logo is a link
NHS logo and service name, only service name is a link

Probably not all these combinations have a valid use case and would need to be explained in guidance, but they’re all there if needed.

The only CSS change needed was moving a font size declaration from the service name link to the parent element (for where there’s no <a> tag).

…unjucks-params

frankieroberto · 2025-02-17T11:55:14Z

@paulrobertlloyd @anandamaryon1 @colinrotherham updated this with a bit of refactoring to the CSS (updated the description to try and explain the thinking). It's a bit complicated as you always have the logo, sometimes have a separate logo and service name, and sometimes have a combined logo + service name container – so not sure how best to name these elements in the CSS BEM convention...

Currently the Organisational logo variant is also broken and I can't work out why yet.

…unjucks-params

frankieroberto · 2025-02-22T22:26:26Z

Fixed the organisational logo variant in 0a33d85 (I’d forgotten to update the class name there).

frankieroberto · 2025-02-22T22:37:28Z

packages/components/header/template.njk

+      {%- if params.service.href -%}
+        <a class="nhsuk-header__service-name__link" href="{{ params.service.href }}">
+      {% endif %}
+        {{ params.service.text }}
+      {%- if params.service.href -%}
+        </a>
+      {% endif %}


This currently has to repeat the same if condition twice, once to add the opening tag, and once to add the closing tag.

An alternative would be something like this, which has the condition only once, but repeats the {{ params.service.text }} line instead:

Suggested change

{%- if params.service.href -%}

<a class="nhsuk-header__service-name__link" href="{{ params.service.href }}">

{% endif %}

{{ params.service.text }}

{%- if params.service.href -%}

</a>

{% endif %}

{%- if params.service.href -%}

<a class="nhsuk-header__service-name__link" href="{{ params.service.href }}">

{{ params.service.text }}

</a>

{% else %}

{{ params.service.text }}

{% endif %}

The same pattern is used above - although there there is even more content which can appear within the link... 🤔

Not a strong opinion but your updated example seems clearer to me, easier to see what I'll end up with between the if and else.

colinrotherham · 2025-02-26T14:59:12Z

app/pages/examples.njk

@@ -82,11 +82,12 @@
    <li><a href="../components/header/header-account-logged-in.html">Header with account (logged in)</a></li>
    <li><a href="../components/header/header-account-logged-out.html">Header with account (logged out)</a></li>
    <li><a href="../components/header/header-account-rbac.html">Header with account (logged in, RBAC)</a></li>
-    <li><a href="../components/header/header-logo.html">Header with logo only</a></li>
+    <li><a href="../components/header/header-logo.html">Header with un-linked logo only</a></li>


Unless, unlikely, unlinked?

This is a fab PR, perhaps the only bit of feedback I could find 😆

Suggested change

<li><a href="../components/header/header-logo.html">Header with un-linked logo only</a></li>

<li><a href="../components/header/header-logo.html">Header with unlinked logo only</a></li>

colinrotherham · 2025-02-26T15:01:10Z

packages/components/header/README.md

-    <div class="nhsuk-header__transactional-service-name">
-      <a class="nhsuk-header__transactional-service-name--link" href="https://www.nhs.uk/nhs-services/online-services/find-nhs-number/">Find your NHS number</a>
+    <div class="nhsuk-header__service-name">
+      <a class="nhsuk-header__service-name__link" href="https://www.nhs.uk/nhs-services/online-services/find-nhs-number/">Find your NHS number</a>


Could probably say this is BEEM with an extra element

Shall we stick to one-level only?

Suggested change

<a class="nhsuk-header__service-name__link" href="https://www.nhs.uk/nhs-services/online-services/find-nhs-number/">Find your NHS number</a>

<a class="nhsuk-header__service-link" href="https://www.nhs.uk/nhs-services/online-services/find-nhs-number/">Find your NHS number</a>

colinrotherham · 2025-02-26T15:09:22Z

packages/components/header/template.njk

 {%- if params.organisation %} nhsuk-header--organisation{% endif %}
 {%- if params.classes %} {{ params.classes }}{% endif %}" role="banner"
 {{- nhsukAttributes(params.attributes) }}>
  <div class="nhsuk-header__container">
-    <div class="nhsuk-header__logo">
+    <div class="nhsuk-header__logo {% if not params.service %}nhsuk-header__logo--no-service-name{% endif %}">


Watch out for trailing spaces by default

<div class="nhsuk-header__logo ">

Suggested change

<div class="nhsuk-header__logo {% if not params.service %}nhsuk-header__logo--no-service-name{% endif %}">

<div class="nhsuk-header__logo{% if not params.service %} nhsuk-header__logo--no-service-name{% endif %}">

colinrotherham · 2025-02-26T15:11:45Z

packages/components/header/template.njk

-      {%- if params.organisation.logoURL %}
-        <img class="nhsuk-organisation-logo" src="{{ baseUrl }}{{ params.organisation.logoURL }}" alt=""/>
+      {%- if params.logo.href -%}
+      <a class="nhsuk-header__logo__link" href="{{ params.logo.href }}" aria-label="{{ params.organisation.name }} {{ params.organisation.split }} {{ params.organisation.descriptor }} homepage">


Will this leave multiple empty spaces?

E.g. If params.organisation.split is empty

colinrotherham · 2025-02-26T15:14:19Z

packages/components/header/template.njk

+      {%- if params.logo.href -%}
+      <a class="nhsuk-header__logo__link" href="{{ params.logo.href }}" aria-label="{{ params.organisation.name }} {{ params.organisation.split }} {{ params.organisation.descriptor }} homepage">
+      {%- endif -%}
+      {%- if params.logo.src %}


Some lines end -%} but others %}

Not sure how wonky the output HTML looks already, or whether this is intentional or not

colinrotherham · 2025-02-26T15:25:37Z

packages/components/header/template.njk

+      {% if params.logo.href or params.logo.includeWithServiceLink %}
+      <a class="nhsuk-header__logo__link" href="{%- if params.logo.includeWithServiceLink %}{{ params.service.href }}{% else %}{{ params.logo.href }}{% endif %}" aria-label="{{ params.logo.ariaLabel | default('NHS homepage') }}">
+      {% endif %}
        {{ nhsLogo | safe }}
-        {%- if params.service %}
-        <span class="nhsuk-header__service-name">{{ params.service.name }}</span>
+        {%- if params.logo.includeWithServiceLink %}
+        <span class="nhsuk-header__service-name">{{ params.service.text }}</span>
        {%- endif %}
-      </a>
+        {%- if params.logo.href or params.logo.includeWithServiceLink -%}
+        </a>
+        {%- endif -%}


I'm not too keen on a optional </a> closing tags

It's very easy to have the logic not match the opening tag, and out comes invalid HTML

Would more macros in the same template help?

{# Service only #} {% set macro _service(params) %} <span class="service"> {{ caller() }} </span> {% endset %} {# Logo only #} {% set macro _logo(params) %} <span class="logo"> {{ caller() }} </span> {% endset %} {% if scenarioA %}  <a href="#"> {{ _logo(params) }} {{ _service(params) }} </a> {% else %}  <a href="#"> {{ _logo(params) }} </a> <a href="#"> {{ _service(params) }} </a> {% endif %}

Ah this is also related to https://github.com/nhsuk/nhsuk-frontend/pull/1109/files#r1966615647

colinrotherham · 2025-02-26T15:33:19Z

app/components/header/header-service-name-combined-with-logo.njk

+  {{ header({
+    logo: {
+      includeWithServiceLink: true
+    },
+    service: {
+      text: "Prototype kit",
+      href: "#"
+    }
+  }) }}


Are there any manual includeWithServiceLink scenarios we couldn't work out automatically?

E.g. If both logo.href and service.href are identical

Or only split them out automatically when the (optional) logo.href differs?

Logo included by default

Included because logo.href is not set

{{ header({ service: { text: "Prototype kit", href: "#" } }) }}

Logo included automatically

Included because logo.href matches service.href

{{ header({ logo: { href: "#1" }, service: { text: "Prototype kit", href: "#1" } }) }}

Logo split out automatically

Split out because logo.href and service.href differ

{{ header({ logo: { href: "#1" }, service: { text: "Prototype kit", href: "#2" } }) }}

frankieroberto added 9 commits January 31, 2025 09:19

Update nunjucks params for logo and service

4bd7d27

Merge branch 'header-breaking-changes' into header-breaking-changes-n…

167f369

…unjucks-params

Merge branch 'header-breaking-changes' into header-breaking-changes-n…

efec30d

…unjucks-params

style fix

b391315

Update CSS classes to remove 'transactional'

fde7e1f

Update example names

96297d3

Use service href if logo combined with service name

8e56724

Update docs in component README

2efdc9e

Move src to logo object.

7b3dbae

frankieroberto requested review from paulrobertlloyd, anandamaryon1, edwardhorsford and sarawilcox and removed request for paulrobertlloyd January 31, 2025 11:15

anandamaryon1 reviewed Feb 5, 2025

View reviewed changes

app/components/header/header-service-name.njk Outdated Show resolved Hide resolved

frankieroberto mentioned this pull request Feb 7, 2025

Header updates - breaking changes #1058

Draft

15 tasks

colinrotherham and others added 2 commits February 13, 2025 12:40

Merge remote-tracking branch 'origin/header-breaking-changes' into he…

c08850c

…ader-breaking-changes-nunjucks-params

Allow logo or service name to be unlinked

e738c8b

frankieroberto added 3 commits February 13, 2025 15:39

Remove 'transactional' references

c6b664c

Update documentation in README

37271b8

Code style fix

3ab5ae7

frankieroberto added the header label Feb 17, 2025

frankieroberto added the 💥 breaking change label Feb 17, 2025

frankieroberto added 6 commits February 17, 2025 10:49

Merge branch 'header-breaking-changes' into header-breaking-changes-n…

f932bb8

…unjucks-params

Update backstop references

61548a4

Rename css to follow BEM convention

5a89bd9

Change CSS names

f6e7673

CSS fixes

7ee325a

Update backstop images

b5933b1

frankieroberto mentioned this pull request Feb 18, 2025

Only underline links in header upon hover #1136

Open

3 tasks

colinrotherham and others added 2 commits February 22, 2025 09:13

Merge branch 'header-breaking-changes' into header-breaking-changes-n…

9801de7

…unjucks-params

Fix

0a33d85

Update class name

9b5cb9c

frankieroberto commented Feb 22, 2025

View reviewed changes

colinrotherham reviewed Feb 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Header breaking changes nunjucks params #1109

Header breaking changes nunjucks params #1109

frankieroberto commented Jan 31, 2025 •

edited

Loading

paulrobertlloyd commented Feb 2, 2025

paulrobertlloyd commented Feb 2, 2025

frankieroberto commented Feb 3, 2025

frankieroberto commented Feb 3, 2025

anandamaryon1 commented Feb 5, 2025

frankieroberto commented Feb 7, 2025

frankieroberto commented Feb 13, 2025

frankieroberto commented Feb 17, 2025

frankieroberto commented Feb 22, 2025

frankieroberto Feb 22, 2025

anandamaryon1 Feb 26, 2025

colinrotherham Feb 26, 2025

colinrotherham Feb 26, 2025

colinrotherham Feb 26, 2025

colinrotherham Feb 26, 2025

colinrotherham Feb 26, 2025

colinrotherham Feb 26, 2025

colinrotherham Feb 26, 2025

colinrotherham Feb 26, 2025

	<li><a href="../components/header/header-logo.html">Header with un-linked logo only</a></li>
	<li><a href="../components/header/header-logo.html">Header with unlinked logo only</a></li>

	<a class="nhsuk-header__service-name__link" href="https://www.nhs.uk/nhs-services/online-services/find-nhs-number/">Find your NHS number</a>
	<a class="nhsuk-header__service-link" href="https://www.nhs.uk/nhs-services/online-services/find-nhs-number/">Find your NHS number</a>

	<div class="nhsuk-header__logo {% if not params.service %}nhsuk-header__logo--no-service-name{% endif %}">
	<div class="nhsuk-header__logo{% if not params.service %} nhsuk-header__logo--no-service-name{% endif %}">

Header breaking changes nunjucks params #1109

Are you sure you want to change the base?

Header breaking changes nunjucks params #1109

Conversation

frankieroberto commented Jan 31, 2025 • edited Loading

Full list of Nunjucks changes

Update HTML and CSS classes

Remaining questions

Checklist

paulrobertlloyd commented Feb 2, 2025

paulrobertlloyd commented Feb 2, 2025

frankieroberto commented Feb 3, 2025

frankieroberto commented Feb 3, 2025

anandamaryon1 commented Feb 5, 2025

frankieroberto commented Feb 7, 2025

frankieroberto commented Feb 13, 2025

frankieroberto commented Feb 17, 2025

frankieroberto commented Feb 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Logo included by default

Logo included automatically

Logo split out automatically

frankieroberto commented Jan 31, 2025 •

edited

Loading