Skip to content

Add notification banner component#1408

Merged
colinrotherham merged 23 commits into
mainfrom
add-notification-banner-component
Jul 24, 2025
Merged

Add notification banner component#1408
colinrotherham merged 23 commits into
mainfrom
add-notification-banner-component

Conversation

@edwardhorsford
Copy link
Copy Markdown
Contributor

@edwardhorsford edwardhorsford commented Jun 17, 2025

Adds a new component 'notification banner' adapted from GOV.UK Frontend.

Steps taken:

  • Copied all files from /govuk-frontend/src/govuk/components/nofication-banner, removing those we don't use.
  • Updated namespace from govuk to nhsuk
  • Adapted mixins and other classes to nhs equivalents
  • Added new nhsuk-success-color sass variable.

Default example:
Screenshot of default notification banner

Success example:
Screenshot of success banner

@anandamaryon1 anandamaryon1 temporarily deployed to nhsuk-frontend-pr-1408 June 17, 2025 13:45 Inactive
@paulrobertlloyd
Copy link
Copy Markdown
Contributor

Could you add some screenshots?

One question I have is whether the background should be white or the page background colour, the faded blue/grey.

@edwardhorsford edwardhorsford temporarily deployed to nhsuk-frontend-pr-1408 June 17, 2025 13:58 Inactive
@edwardhorsford
Copy link
Copy Markdown
Contributor Author

Could you add some screenshots?

One question I have is whether the background should be white or the page background colour, the faded blue/grey.

I hadn't thought about that yet, but here are two examples from my service with clear or white background:

Screenshot of success banner with white background
Screenshot of success banner with clear background

@edwardhorsford edwardhorsford temporarily deployed to nhsuk-frontend-pr-1408 June 18, 2025 08:53 Inactive
@edwardhorsford edwardhorsford marked this pull request as ready for review June 18, 2025 09:02
@colinrotherham colinrotherham force-pushed the add-notification-banner-component branch from 7401993 to 1b57550 Compare June 18, 2025 09:16
@colinrotherham colinrotherham temporarily deployed to nhsuk-frontend-pr-1408 June 18, 2025 09:16 Inactive
@colinrotherham colinrotherham force-pushed the add-notification-banner-component branch from 1b57550 to d932ca1 Compare June 18, 2025 09:21
@colinrotherham colinrotherham temporarily deployed to nhsuk-frontend-pr-1408 June 18, 2025 09:21 Inactive
@colinrotherham
Copy link
Copy Markdown
Contributor

Other than cache issues in actions/cache#1621 it's all wired up now 🙌

E.g. Auto focus is working in notification-banner/success.html

@edwardhorsford edwardhorsford temporarily deployed to nhsuk-frontend-pr-1408 June 18, 2025 11:59 Inactive
@anandamaryon1
Copy link
Copy Markdown
Contributor

Looking good.

I notice that, like the error summary, we have a visible focus outline on the success version. Shall we try to fix these, or wait for the GOV extension work to progress @colinrotherham?

@colinrotherham
Copy link
Copy Markdown
Contributor

Ah I've come to like the focus outline, it only kicks in for alert roles (same as error summary)

The focus outline is visible in GOV.UK Frontend too:

Other than the background colour (fixed) it seems like a sensible addition

@anandamaryon1
Copy link
Copy Markdown
Contributor

Ok fair enough 👍

@edwardhorsford
Copy link
Copy Markdown
Contributor Author

edwardhorsford commented Jun 18, 2025

After some discussion on slack we feel these should likely have a transparent background (page colour) to match how the error summary works. I've updated the screenshots in the initial PR to match.

@colinrotherham colinrotherham changed the base branch from main to support/9.x June 20, 2025 13:11
@colinrotherham colinrotherham force-pushed the add-notification-banner-component branch from 306be13 to 4a12e2e Compare June 20, 2025 13:16
@colinrotherham colinrotherham temporarily deployed to nhsuk-frontend-pr-1408 June 20, 2025 13:16 Inactive
@colinrotherham
Copy link
Copy Markdown
Contributor

@edwardhorsford I've updated this PR to target support/9.x

Should unblock you from installing it into the service manual website

@colinrotherham colinrotherham temporarily deployed to nhsuk-frontend-pr-1408 June 20, 2025 13:21 Inactive
@colinrotherham colinrotherham temporarily deployed to nhsuk-frontend-pr-1408 July 10, 2025 10:04 Inactive
@edwardhorsford edwardhorsford changed the title WIP Add notification banner component Add notification banner component Jul 14, 2025
@edwardhorsford edwardhorsford temporarily deployed to nhsuk-frontend-pr-1408 July 15, 2025 11:55 Inactive
@colinrotherham colinrotherham changed the base branch from support/9.x to main July 15, 2025 12:57
@colinrotherham colinrotherham force-pushed the add-notification-banner-component branch from b58cb11 to 92af00e Compare July 15, 2025 12:57
@colinrotherham colinrotherham self-requested a review July 15, 2025 12:58
@colinrotherham colinrotherham temporarily deployed to nhsuk-frontend-pr-1408 July 15, 2025 12:58 Inactive
Copy link
Copy Markdown
Contributor

@colinrotherham colinrotherham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for this @edwardhorsford

I've switched it back to v10.x by targetting main

This PR currently adds support for Nunjucks data-disable-auto-focus but it only works via the JavaScript config at the moment, similarly over in GOV.UK Frontend this same feature works in both Nunjucks and JavaScript

What shall we do?

I'm tempted to either drop support for Nunjucks data-disable-auto-focus or fix it and make sure we add it to Error Summary too so it's not left behind

@colinrotherham colinrotherham force-pushed the add-notification-banner-component branch from 9f5d3da to f0764b7 Compare July 23, 2025 22:16
@colinrotherham colinrotherham temporarily deployed to nhsuk-frontend-pr-1408 July 23, 2025 22:16 Inactive
@sonarqubecloud
Copy link
Copy Markdown

@colinrotherham colinrotherham self-requested a review July 24, 2025 08:16
@colinrotherham
Copy link
Copy Markdown
Contributor

@edwardhorsford This is now ready to go if you can have a look please @anandamaryon1 🙌

I've split out work to configure all components by either Nunjucks or JavaScript in:

@colinrotherham colinrotherham merged commit f9c1fbc into main Jul 24, 2025
13 checks passed
@colinrotherham colinrotherham deleted the add-notification-banner-component branch July 24, 2025 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants