Skip to content

Commit 6ebc628

Browse files
robkerrybjssrobkerrybjss
committed
Address further review comments
1 parent 1c43ecd commit 6ebc628

File tree

6 files changed

+51
-40
lines changed

6 files changed

+51
-40
lines changed

‎docs/upgrade-to-6.0.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ The [notification banner](https://service-manual.nhs.uk/design-system/components
3737

3838
```jsx
3939
<NotificationBanner>
40-
<NotificationBanner.Heading>Upcoming Maintenance</NotificationBanner.Heading>
40+
<NotificationBanner.Heading>Upcoming maintenance</NotificationBanner.Heading>
4141
<p>The service will be unavailable from 8pm to 9pm on Thursday 1 January 2025.</p>
4242
</NotificationBanner>
4343
```

‎src/components/content-presentation/notification-banner/NotificationBanner.tsx‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,14 @@ import { childIsOfComponentType } from '#util/types/TypeGuards.js';
2020
export interface NotificationBannerProps extends ComponentPropsWithoutRef<'div'> {
2121
success?: boolean;
2222
disableAutoFocus?: boolean;
23+
titleId?: string;
2324
}
2425

2526
const NotificationBannerComponent = forwardRef<HTMLDivElement, NotificationBannerProps>(
26-
({ children, className, title, success, role, disableAutoFocus, ...rest }, forwardedRef) => {
27+
(
28+
{ children, className, title, titleId, success, role, disableAutoFocus, ...rest },
29+
forwardedRef,
30+
) => {
2731
const [moduleRef] = useState(() => forwardedRef || createRef<HTMLDivElement>());
2832
const [instanceError, setInstanceError] = useState<Error>();
2933
const [instance, setInstance] = useState<NotificationBannerModule>();
@@ -59,7 +63,7 @@ const NotificationBannerComponent = forwardRef<HTMLDivElement, NotificationBanne
5963
{ 'nhsuk-notification-banner--success': success },
6064
className,
6165
)}
62-
aria-labelledby={titleElement?.props.id || 'nhsuk-notification-banner-title'}
66+
aria-labelledby={titleId || titleElement?.props.id || 'nhsuk-notification-banner-title'}
6367
data-module="nhsuk-notification-banner"
6468
data-disable-auto-focus={disableAutoFocus}
6569
ref={moduleRef}
@@ -69,7 +73,9 @@ const NotificationBannerComponent = forwardRef<HTMLDivElement, NotificationBanne
6973
{titleElement ? (
7074
<>{titleElement}</>
7175
) : (
72-
<NotificationBannerTitle success={success}>{title}</NotificationBannerTitle>
76+
<NotificationBannerTitle success={success} id={titleId}>
77+
{title}
78+
</NotificationBannerTitle>
7379
)}
7480
</div>
7581
<div className={'nhsuk-notification-banner__content'} {...rest}>

‎src/components/content-presentation/notification-banner/__tests__/NotificationBanner.test.tsx‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ describe('NotificationBanner', () => {
3030

3131
it('matches snapshot custom title', async () => {
3232
const { container } = await renderClient(
33-
<NotificationBanner title={'Upcoming Maintenance'}>
33+
<NotificationBanner title={'Upcoming maintenance'}>
3434
<NotificationBanner.Heading>
3535
The service will be unavailable from 8pm to 9pm on Thursday 1 January 2025.
3636
</NotificationBanner.Heading>
@@ -62,7 +62,7 @@ describe('NotificationBanner', () => {
6262
<NotificationBanner>
6363
<NotificationBanner.Heading>
6464
You have 7 days left to send your application.{' '}
65-
<NotificationBanner.Link href={'#'}>View application</NotificationBanner.Link>.
65+
<NotificationBanner.Link href="#">View application</NotificationBanner.Link>.
6666
</NotificationBanner.Heading>
6767
</NotificationBanner>,
6868
{ className: 'nhsuk-notification-banner' },
@@ -77,7 +77,7 @@ describe('NotificationBanner', () => {
7777
<NotificationBanner.Heading>Patient record updated</NotificationBanner.Heading>
7878
<p>
7979
Contact{' '}
80-
<NotificationBanner.Link href={'#'}>[email protected]</NotificationBanner.Link> if
80+
<NotificationBanner.Link href="#">[email protected]</NotificationBanner.Link> if
8181
you think there&#39;s a problem.
8282
</p>
8383
</NotificationBanner>,
@@ -146,8 +146,8 @@ describe('NotificationBanner', () => {
146146
</NotificationBanner.Heading>
147147
<p>
148148
You will have to apply the{' '}
149-
<NotificationBanner.Link>reverse charge</NotificationBanner.Link> if the applicant
150-
supplies any of these services:
149+
<NotificationBanner.Link href="#">reverse charge</NotificationBanner.Link> if the
150+
applicant supplies any of these services:
151151
</p>
152152
<ul>
153153
<li>
@@ -194,7 +194,7 @@ describe('NotificationBanner', () => {
194194

195195
it('matches snapshot custom title (via server)', async () => {
196196
const { container } = await renderServer(
197-
<NotificationBanner title={'Upcoming Maintenance'}>
197+
<NotificationBanner title={'Upcoming maintenance'}>
198198
<NotificationBanner.Heading>
199199
The service will be unavailable from 8pm to 9pm on Thursday 1 January 2025.
200200
</NotificationBanner.Heading>
@@ -226,7 +226,7 @@ describe('NotificationBanner', () => {
226226
<NotificationBanner>
227227
<NotificationBanner.Heading>
228228
You have 7 days left to send your application.{' '}
229-
<NotificationBanner.Link href={'#'}>View application</NotificationBanner.Link>.
229+
<NotificationBanner.Link href="#">View application</NotificationBanner.Link>.
230230
</NotificationBanner.Heading>
231231
</NotificationBanner>,
232232
{ className: 'nhsuk-notification-banner' },
@@ -241,7 +241,7 @@ describe('NotificationBanner', () => {
241241
<NotificationBanner.Heading>Patient record updated</NotificationBanner.Heading>
242242
<p>
243243
Contact{' '}
244-
<NotificationBanner.Link href={'#'}>[email protected]</NotificationBanner.Link> if
244+
<NotificationBanner.Link href="#">[email protected]</NotificationBanner.Link> if
245245
you think there&#39;s a problem.
246246
</p>
247247
</NotificationBanner>,
@@ -310,8 +310,8 @@ describe('NotificationBanner', () => {
310310
</NotificationBanner.Heading>
311311
<p>
312312
You will have to apply the{' '}
313-
<NotificationBanner.Link>reverse charge</NotificationBanner.Link> if the applicant
314-
supplies any of these services:
313+
<NotificationBanner.Link href="#">reverse charge</NotificationBanner.Link> if the
314+
applicant supplies any of these services:
315315
</p>
316316
<ul>
317317
<li>

‎src/components/content-presentation/notification-banner/__tests__/__snapshots__/NotificationBanner.test.tsx.snap‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ exports[`NotificationBanner matches snapshot custom title (via server) 1`] = `
7878
class="nhsuk-notification-banner__title"
7979
id="nhsuk-notification-banner-title"
8080
>
81-
Upcoming Maintenance
81+
Upcoming maintenance
8282
</h2>
8383
</div>
8484
<div
@@ -110,7 +110,7 @@ exports[`NotificationBanner matches snapshot custom title 1`] = `
110110
class="nhsuk-notification-banner__title"
111111
id="nhsuk-notification-banner-title"
112112
>
113-
Upcoming Maintenance
113+
Upcoming maintenance
114114
</h2>
115115
</div>
116116
<div
@@ -643,6 +643,7 @@ exports[`NotificationBanner matches snapshot with lots of content (via server) 1
643643
644644
<a
645645
class="nhsuk-notification-banner__link"
646+
href="#"
646647
>
647648
reverse charge
648649
</a>
@@ -693,6 +694,7 @@ exports[`NotificationBanner matches snapshot with lots of content 1`] = `
693694
694695
<a
695696
class="nhsuk-notification-banner__link"
697+
href="#"
696698
>
697699
reverse charge
698700
</a>

‎src/components/content-presentation/notification-banner/components/NotificationBannerHeading.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
'use client';
2+
13
import { HeadingLevel, type HeadingLevelProps } from '#components/utils/HeadingLevel.js';
24
import type { FC } from 'react';
35

‎stories/Content Presentation/NotificationBanner.stories.tsx‎

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { NotificationBannerLink } from '#components/content-presentation/notific
2525
* <NotificationBanner>
2626
* <NotificationBanner.Heading>Patient record updated</Details.Summary>
2727
* <p>
28-
* Contact <NotificationBanner.Link href={'#'}>[email protected]</NotificationBanner.Link> if you think there&#39;s a problem.
28+
* Contact <NotificationBanner.Link href="#">example@department.nhs.uk</NotificationBanner.Link> if you think there&#39;s a problem.
2929
* </p>
3030
* </NotificationBanner>
3131
* );
@@ -41,8 +41,8 @@ type Story = StoryObj<typeof NotificationBanner>;
4141

4242
export const StandardPanel: Story = {
4343
args: {},
44-
render: (args) => (
45-
<NotificationBanner {...args}>
44+
render: () => (
45+
<NotificationBanner>
4646
<NotificationBanner.Heading>
4747
The service will be unavailable from 8pm to 9pm on Thursday 1 January 2025.
4848
</NotificationBanner.Heading>
@@ -52,25 +52,25 @@ export const StandardPanel: Story = {
5252

5353
export const SuccessPanel: Story = {
5454
args: {},
55-
render: (args) => (
56-
<NotificationBanner success {...args}>
55+
render: () => (
56+
<NotificationBanner success>
5757
<NotificationBanner.Heading>Patient record updated</NotificationBanner.Heading>
5858
<p>
5959
Contact{' '}
60-
<NotificationBanner.Link href={'#'}>[email protected]</NotificationBanner.Link> if
61-
you think there&#39;s a problem.
60+
<NotificationBanner.Link href="#">[email protected]</NotificationBanner.Link> if you
61+
think there&#39;s a problem.
6262
</p>
6363
</NotificationBanner>
6464
),
6565
};
6666

6767
export const StandardPanelWithLink: Story = {
6868
args: {},
69-
render: (args) => (
70-
<NotificationBanner {...args}>
69+
render: () => (
70+
<NotificationBanner>
7171
<NotificationBanner.Heading>
72-
You have 7 days left to send your application.
73-
<NotificationBanner.Link href={'#'}>View application</NotificationBanner.Link>.
72+
You have 7 days left to send your application.{' '}
73+
<NotificationBanner.Link href="#">View application</NotificationBanner.Link>.
7474
</NotificationBanner.Heading>
7575
</NotificationBanner>
7676
),
@@ -80,31 +80,31 @@ export const StandardPanelWithCustomTitle: Story = {
8080
args: {
8181
title: 'Important Message',
8282
},
83-
render: (args) => (
84-
<NotificationBanner {...args}>
85-
<NotificationBanner.Heading>Upcoming Maintenance</NotificationBanner.Heading>
83+
render: () => (
84+
<NotificationBanner>
85+
<NotificationBanner.Heading>Upcoming maintenance</NotificationBanner.Heading>
8686
<p>The service will be unavailable from 8pm to 9pm on Thursday 1 January 2025.</p>
8787
</NotificationBanner>
8888
),
8989
};
9090

9191
export const StandardPanelWithCustomTitleElement: Story = {
9292
args: {},
93-
render: (args) => (
94-
<NotificationBanner {...args}>
93+
render: () => (
94+
<NotificationBanner>
9595
<NotificationBanner.Title>
96-
<span style={{ textDecoration: 'underline' }}>Very</span> Important Message
96+
Maintenance <time dateTime={'2025-01-01'}>January 1</time>
9797
</NotificationBanner.Title>
98-
<NotificationBanner.Heading>Upcoming Maintenance</NotificationBanner.Heading>
98+
<NotificationBanner.Heading>Upcoming maintenance</NotificationBanner.Heading>
9999
<p>The service will be unavailable from 8pm to 9pm on Thursday 1 January 2025.</p>
100100
</NotificationBanner>
101101
),
102102
};
103103

104104
export const StandardPanelWithList: Story = {
105105
args: {},
106-
render: (args) => (
107-
<NotificationBanner {...args}>
106+
render: () => (
107+
<NotificationBanner>
108108
<NotificationBanner.Heading>4 files uploaded</NotificationBanner.Heading>
109109
<ul>
110110
<li>
@@ -126,14 +126,15 @@ export const StandardPanelWithList: Story = {
126126

127127
export const StandardPanelWithLotsOfContent: Story = {
128128
args: {},
129-
render: (args) => (
130-
<NotificationBanner {...args}>
129+
render: () => (
130+
<NotificationBanner>
131131
<NotificationBanner.Heading>
132132
Check if you need to apply the reverse charge to this application
133133
</NotificationBanner.Heading>
134134
<p>
135-
You will have to apply the <NotificationBanner.Link>reverse charge</NotificationBanner.Link>{' '}
136-
if the applicant supplies any of these services:
135+
You will have to apply the{' '}
136+
<NotificationBanner.Link href="#">reverse charge</NotificationBanner.Link> if the applicant
137+
supplies any of these services:
137138
</p>
138139
<ul>
139140
<li>

0 commit comments

Comments
 (0)