Add notification banner component#284
Add notification banner component#284colinrotherham merged 13 commits intoNHSDigital:rollup-directivesfrom
Conversation
|
Fixes #283 |
colinrotherham
left a comment
There was a problem hiding this comment.
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 😎
src/components/content-presentation/notification-banner/NotificationBanner.tsx
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/NotificationBanner.tsx
Outdated
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/__tests__/NotificationBanner.test.tsx
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/components/NotificationBannerLink.tsx
Outdated
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/components/NotificationBannerLink.tsx
Outdated
Show resolved
Hide resolved
|
Thanks @colinrotherham, I've learned a lot by doing this! Really appreciate you taking the time to give this a thorough review. |
|
Forgot to say on Friday, all the changes you've asked for are in now! |
|
Thanks @robkerrybjss, I'm going to have a look at this now 🙌 I've had to adjust |
|
|
||
| ```html | ||
| <main class="nhsuk-main-wrapper id="maincontent"> | ||
| <main class="nhsuk-main-wrapper" id="maincontent"> |
There was a problem hiding this comment.
Thanks for spotting this!
colinrotherham
left a comment
There was a problem hiding this comment.
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
src/components/content-presentation/notification-banner/__tests__/NotificationBanner.test.tsx
Outdated
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/NotificationBanner.tsx
Outdated
Show resolved
Hide resolved
...components/content-presentation/notification-banner/components/NotificationBannerHeading.tsx
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/NotificationBanner.tsx
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/NotificationBanner.tsx
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/NotificationBanner.tsx
Show resolved
Hide resolved
46149e3 to
1c43ecd
Compare
src/components/content-presentation/notification-banner/NotificationBanner.tsx
Outdated
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/NotificationBanner.tsx
Outdated
Show resolved
Hide resolved
src/components/content-presentation/notification-banner/NotificationBanner.tsx
Outdated
Show resolved
Hide resolved
|
colinrotherham
left a comment
There was a problem hiding this comment.
Thanks @robkerrybjss
Will merge this into #285 and get another beta released



No description provided.