Skip to content

Commit 2cf6b5a

Browse files
colinrotherhamcolinrotherham
committed
Align error summary with NHS.UK frontend
1 parent 3d0900f commit 2cf6b5a

File tree

4 files changed

+212
-136
lines changed

4 files changed

+212
-136
lines changed

‎src/components/form-elements/error-summary/ErrorSummary.tsx‎

Lines changed: 84 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,86 +1,117 @@
11
import React, {
2+
Children,
3+
ComponentProps,
24
FC,
35
ForwardRefExoticComponent,
4-
forwardRef,
5-
HTMLProps,
66
PropsWithoutRef,
77
RefAttributes,
8+
createRef,
9+
forwardRef,
10+
useState,
11+
useEffect,
812
} from 'react';
913
import classNames from 'classnames';
10-
import useDevWarning from '@util/hooks/UseDevWarning';
14+
import { childIsOfComponentType } from '@util/types/TypeGuards';
15+
import { ErrorSummary } from 'nhsuk-frontend';
1116

12-
const DefaultErrorSummaryTitleID = 'error-summary-title';
17+
const Title: FC<ComponentProps<'h2'>> = ({ children, className, ...rest }) => {
18+
return (
19+
<h2 className={classNames('nhsuk-error-summary__title', className)} {...rest}>
20+
{children}
21+
</h2>
22+
);
23+
};
1324

14-
const ErrorSummaryTitle: FC<HTMLProps<HTMLHeadingElement>> = ({
15-
className,
16-
id = DefaultErrorSummaryTitleID,
17-
...rest
18-
}) => <h2 className={classNames('nhsuk-error-summary__title', className)} id={id} {...rest} />;
25+
const List: FC<ComponentProps<'ul'>> = ({ children, className, ...rest }) => {
26+
if (!children) {
27+
return null;
28+
}
1929

20-
const ErrorSummaryBody: FC<HTMLProps<HTMLDivElement>> = ({ className, ...rest }) => (
21-
<div className={classNames('nhsuk-error-summary__body', className)} {...rest} />
22-
);
30+
return (
31+
<ul className={classNames('nhsuk-list', 'nhsuk-error-summary__list', className)} {...rest}>
32+
{children}
33+
</ul>
34+
);
35+
};
2336

24-
const ErrorSummaryList: FC<HTMLProps<HTMLUListElement>> = ({ className, ...rest }) => (
25-
<ul className={classNames('nhsuk-list', 'nhsuk-error-summary__list', className)} {...rest} />
26-
);
37+
const ListItem: FC<ComponentProps<'a'>> = ({ children, href, ...rest }) => {
38+
if (!children) {
39+
return null;
40+
}
2741

28-
const ErrorSummaryListItem: FC<HTMLProps<HTMLAnchorElement>> = (props) => (
29-
<li>
30-
<a {...props} />
31-
</li>
32-
);
42+
return (
43+
<li>
44+
{href ? (
45+
<a href={href} {...rest}>
46+
{children}
47+
</a>
48+
) : (
49+
<>{children}</>
50+
)}
51+
</li>
52+
);
53+
};
54+
55+
interface ErrorSummaryProps extends ComponentProps<'div'> {
56+
disableAutoFocus?: boolean;
57+
}
3358

3459
interface ErrorSummaryComponent
3560
extends ForwardRefExoticComponent<
36-
PropsWithoutRef<HTMLProps<HTMLDivElement>> & RefAttributes<HTMLDivElement>
61+
PropsWithoutRef<ErrorSummaryProps> & RefAttributes<HTMLDivElement>
3762
> {
38-
Title: FC<HTMLProps<HTMLHeadingElement>>;
39-
Body: FC<HTMLProps<HTMLDivElement>>;
40-
List: FC<HTMLProps<HTMLUListElement>>;
41-
Item: FC<HTMLProps<HTMLAnchorElement>>;
63+
Title: FC<ComponentProps<'h2'>>;
64+
List: FC<ComponentProps<'ul'>>;
65+
ListItem: FC<ComponentProps<'a'>>;
4266
}
4367

44-
const ErrorSummaryDiv = forwardRef<HTMLDivElement, HTMLProps<HTMLDivElement>>(
45-
(
46-
{
47-
className,
48-
tabIndex = -1,
49-
role = 'alert',
50-
'aria-labelledby': ariaLabelledBy = DefaultErrorSummaryTitleID,
51-
...rest
52-
},
53-
ref,
54-
) => {
55-
useDevWarning(
56-
'The ErrorSummary component should always have a tabIndex of -1',
57-
() => tabIndex !== -1,
58-
);
59-
useDevWarning(
60-
'The ErrorSummary component should always have a role of alert',
61-
() => role !== 'alert',
62-
);
68+
const ErrorSummaryDiv = forwardRef<HTMLDivElement, ErrorSummaryProps>(
69+
({ children, className, disableAutoFocus, ...rest }, forwardedRef) => {
70+
const [moduleRef] = useState(() => forwardedRef || createRef<HTMLDivElement>());
71+
const [instance, setInstance] = useState<ErrorSummary>();
72+
73+
useEffect(() => {
74+
if (!moduleRef || !('current' in moduleRef) || instance) {
75+
return;
76+
}
77+
78+
setInstance(new ErrorSummary(moduleRef.current));
79+
}, [moduleRef, instance]);
80+
81+
const items = Children.toArray(children);
82+
const [title] = items.filter((child) => childIsOfComponentType(child, Title));
83+
const bodyItems = items.filter((child) => !childIsOfComponentType(child, Title));
6384

6485
return (
6586
<div
6687
className={classNames('nhsuk-error-summary', className)}
67-
ref={ref}
68-
tabIndex={tabIndex}
69-
role={role}
70-
aria-labelledby={ariaLabelledBy}
88+
data-module="nhsuk-error-summary"
89+
data-disable-auto-focus={
90+
typeof disableAutoFocus !== 'undefined' ? disableAutoFocus : undefined
91+
}
92+
ref={moduleRef}
7193
{...rest}
72-
/>
94+
>
95+
{/* Keep the role="alert" in a seperate child container to prevent a race condition between
96+
the focusing js at the alert, resulting in information getting missed in screen reader announcements */}
97+
<div role="alert">
98+
{title}
99+
<div className="nhsuk-error-summary__body">{bodyItems}</div>
100+
</div>
101+
</div>
73102
);
74103
},
75104
);
76105

106+
Title.displayName = 'ErrorSummary.Title';
107+
List.displayName = 'ErrorSummary.List';
108+
ListItem.displayName = 'ErrorSummary.ListItem';
77109
ErrorSummaryDiv.displayName = 'ErrorSummary';
78110

79111
const ErrorSummaryComponent: ErrorSummaryComponent = Object.assign(ErrorSummaryDiv, {
80-
Title: ErrorSummaryTitle,
81-
Body: ErrorSummaryBody,
82-
List: ErrorSummaryList,
83-
Item: ErrorSummaryListItem,
112+
Title,
113+
List,
114+
ListItem,
84115
});
85116

86117
export default ErrorSummaryComponent;

‎src/components/form-elements/error-summary/__tests__/ErrorSummary.test.tsx‎

Lines changed: 66 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,35 +11,47 @@ describe('ErrorSummary', () => {
1111

1212
it('forwards refs', () => {
1313
const ref = createRef<HTMLDivElement>();
14-
render(<ErrorSummary ref={ref} />);
14+
const { container } = render(<ErrorSummary ref={ref} />);
1515

16-
expect(ref.current).not.toBeNull();
16+
const errorSummaryEl = container.querySelector('div');
17+
18+
expect(ref.current).toBe(errorSummaryEl);
19+
expect(ref.current).toHaveClass('nhsuk-error-summary');
1720
});
1821

19-
it('has default props', () => {
22+
it('is focused automatically', () => {
2023
const { container } = render(<ErrorSummary />);
2124

22-
expect(container.querySelector('div')?.getAttribute('tabindex')).toBe('-1');
23-
expect(container.querySelector('div')?.getAttribute('role')).toBe('alert');
24-
expect(container.querySelector('div')?.getAttribute('aria-labelledby')).toBe(
25-
'error-summary-title',
26-
);
25+
const errorSummaryEl = container.querySelector('div');
26+
27+
expect(document.activeElement).toBe(errorSummaryEl);
28+
});
29+
30+
it('is focused automatically with forwarded ref', () => {
31+
const ref = createRef<HTMLDivElement>();
32+
const { container } = render(<ErrorSummary ref={ref} />);
33+
34+
const errorSummaryEl = container.querySelector('div');
35+
36+
expect(document.activeElement).toBe(errorSummaryEl);
2737
});
2838

29-
it('throws a dev warning if tabIndex is not -1', () => {
30-
jest.spyOn(console, 'warn').mockImplementation(() => {});
31-
render(<ErrorSummary tabIndex={0} />);
32-
expect(console.warn).toHaveBeenCalledWith(
33-
'The ErrorSummary component should always have a tabIndex of -1',
34-
);
39+
it('has default props', () => {
40+
const { container } = render(<ErrorSummary />);
41+
42+
const errorSummaryEl = container.querySelector('div');
43+
44+
expect(errorSummaryEl?.getAttribute('tabindex')).toBe('-1');
45+
expect(errorSummaryEl?.firstElementChild?.getAttribute('role')).toBe('alert');
3546
});
3647

37-
it('throws a dev warning if role is not alert', () => {
38-
jest.spyOn(console, 'warn').mockImplementation(() => {});
39-
render(<ErrorSummary role="status" />);
40-
expect(console.warn).toHaveBeenCalledWith(
41-
'The ErrorSummary component should always have a role of alert',
42-
);
48+
it('has default props with forwarded ref', () => {
49+
const { container } = render(<ErrorSummary />);
50+
51+
const errorSummaryEl = container.querySelector('div');
52+
53+
expect(errorSummaryEl?.getAttribute('tabindex')).toBe('-1');
54+
expect(errorSummaryEl?.firstElementChild?.getAttribute('role')).toBe('alert');
4355
});
4456

4557
describe('ErrorSummary.Title', () => {
@@ -52,16 +64,7 @@ describe('ErrorSummary', () => {
5264
it('renders a title', () => {
5365
const { container } = render(<ErrorSummary.Title>Title</ErrorSummary.Title>);
5466

55-
expect(container.textContent).toBe('Title');
56-
});
57-
});
58-
59-
describe('ErrorSummary.Body', () => {
60-
it('matches snapshot', () => {
61-
const { container } = render(<ErrorSummary.Body>Body</ErrorSummary.Body>);
62-
63-
expect(container.textContent).toBe('Body');
64-
expect(container).toMatchSnapshot('ErrorSummary.Body');
67+
expect(container).toHaveTextContent('Title');
6568
});
6669
});
6770

@@ -75,21 +78,48 @@ describe('ErrorSummary', () => {
7578
it('renders children', () => {
7679
const { container } = render(<ErrorSummary.List>List</ErrorSummary.List>);
7780

78-
expect(container.textContent).toBe('List');
81+
expect(container).toHaveTextContent('List');
82+
});
83+
84+
it('renders null with no children', () => {
85+
const { container } = render(<ErrorSummary.List />);
86+
87+
expect(container.querySelector('ul')).toBeNull();
7988
});
8089
});
8190

8291
describe('ErrorSummary.ListItem', () => {
83-
it('matches snapshot', () => {
84-
const { container } = render(<ErrorSummary.Item>ListItem</ErrorSummary.Item>);
92+
it('matches snapshot for items without links', () => {
93+
const { container } = render(
94+
<ErrorSummary.ListItem>List item without link</ErrorSummary.ListItem>,
95+
);
8596

86-
expect(container).toMatchSnapshot('ErrorSummary.ListItem');
97+
expect(container).toMatchSnapshot();
98+
});
99+
100+
it('matches snapshot for items with links', () => {
101+
const { container } = render(
102+
<ErrorSummary.ListItem href="#example-error-1">List item with link</ErrorSummary.ListItem>,
103+
);
104+
105+
expect(container).toMatchSnapshot();
87106
});
88107

89108
it('renders children', () => {
90-
const { container } = render(<ErrorSummary.Item>ListItem</ErrorSummary.Item>);
109+
const { container } = render(
110+
<ErrorSummary.ListItem href="#example-error-1">List item with link</ErrorSummary.ListItem>,
111+
);
112+
113+
const errorLinkEls = container.querySelectorAll('a');
114+
115+
expect(errorLinkEls?.[0]).toHaveTextContent('List item with link');
116+
expect(errorLinkEls?.[0]).toHaveAttribute('href', '#example-error-1');
117+
});
118+
119+
it('renders null with no children', () => {
120+
const { container } = render(<ErrorSummary.ListItem />);
91121

92-
expect(container.querySelector('a')?.textContent).toBe('ListItem');
122+
expect(container.querySelector('li')).toBeNull();
93123
});
94124
});
95125
});

‎src/components/form-elements/error-summary/__tests__/__snapshots__/ErrorSummary.test.tsx.snap‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,5 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`ErrorSummary ErrorSummary.Body matches snapshot: ErrorSummary.Body 1`] = `
4-
<div>
5-
<div
6-
class="nhsuk-error-summary__body"
7-
>
8-
Body
9-
</div>
10-
</div>
11-
`;
12-
133
exports[`ErrorSummary ErrorSummary.List matches snapshot: ErrorSummary.List 1`] = `
144
<div>
155
<ul
@@ -20,21 +10,30 @@ exports[`ErrorSummary ErrorSummary.List matches snapshot: ErrorSummary.List 1`]
2010
</div>
2111
`;
2212

23-
exports[`ErrorSummary ErrorSummary.ListItem matches snapshot: ErrorSummary.ListItem 1`] = `
13+
exports[`ErrorSummary ErrorSummary.ListItem matches snapshot for items with links 1`] = `
2414
<div>
2515
<li>
26-
<a>
27-
ListItem
16+
<a
17+
href="#example-error-1"
18+
>
19+
List item with link
2820
</a>
2921
</li>
3022
</div>
3123
`;
3224

25+
exports[`ErrorSummary ErrorSummary.ListItem matches snapshot for items without links 1`] = `
26+
<div>
27+
<li>
28+
List item without link
29+
</li>
30+
</div>
31+
`;
32+
3333
exports[`ErrorSummary ErrorSummary.Title matches snapshot: ErrorSummary.Title 1`] = `
3434
<div>
3535
<h2
3636
class="nhsuk-error-summary__title"
37-
id="error-summary-title"
3837
>
3938
Title
4039
</h2>
@@ -44,10 +43,18 @@ exports[`ErrorSummary ErrorSummary.Title matches snapshot: ErrorSummary.Title 1`
4443
exports[`ErrorSummary matches snapshot: ErrorSummary 1`] = `
4544
<div>
4645
<div
47-
aria-labelledby="error-summary-title"
4846
class="nhsuk-error-summary"
49-
role="alert"
47+
data-module="nhsuk-error-summary"
48+
data-nhsuk-error-summary-init=""
5049
tabindex="-1"
51-
/>
50+
>
51+
<div
52+
role="alert"
53+
>
54+
<div
55+
class="nhsuk-error-summary__body"
56+
/>
57+
</div>
58+
</div>
5259
</div>
5360
`;

0 commit comments

Comments
 (0)