Skip to content

Add notification banner component#284

Merged
colinrotherham merged 13 commits intoNHSDigital:rollup-directivesfrom
robkerrybjss:add-notification-banner-component
Oct 13, 2025
Merged

Add notification banner component#284
colinrotherham merged 13 commits intoNHSDigital:rollup-directivesfrom
robkerrybjss:add-notification-banner-component

Conversation

@robkerrybjss
Copy link
Contributor

No description provided.

@robkerrybjss
Copy link
Contributor Author

Fixes #283

Copy link
Collaborator

@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.

Love seeing this PR opened!

I've added a few comments but nothing huge. We need to carefully review the design system guidance and add a few missing things.

Verifying all the NHS.UK frontend examples work would be really useful too

Think it's only the task list component missing after this 😎

@robkerrybjss
Copy link
Contributor Author

Thanks @colinrotherham, I've learned a lot by doing this! Really appreciate you taking the time to give this a thorough review.

@robkerrybjss
Copy link
Contributor Author

Forgot to say on Friday, all the changes you've asked for are in now!

@colinrotherham
Copy link
Collaborator

Thanks @robkerrybjss, I'm going to have a look at this now 🙌

I've had to adjust childIsOfComponentType() slightly for React Server Components (RSC) in #285 (see commit) but I'll check everything still works here in your PR too


```html
<main class="nhsuk-main-wrapper id="maincontent">
<main class="nhsuk-main-wrapper" id="maincontent">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for spotting this!

Copy link
Collaborator

@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.

Excellent work here!

I'm just going to rebase onto #285 so we can get it into the next beta

I'm going to fix a few problems I've caused, bear with

@colinrotherham colinrotherham changed the base branch from main to rollup-directives October 13, 2025 14:08
@colinrotherham colinrotherham force-pushed the add-notification-banner-component branch from 46149e3 to 1c43ecd Compare October 13, 2025 14:13
@colinrotherham colinrotherham self-requested a review October 13, 2025 16:07
@sonarqubecloud
Copy link

Copy link
Collaborator

@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 @robkerrybjss

Will merge this into #285 and get another beta released

@colinrotherham colinrotherham merged commit 37641b4 into NHSDigital:rollup-directives Oct 13, 2025
3 checks passed
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.

2 participants