Skip to content

Commit f69bd5f

Browse files
colinrotherhamcolinrotherham
committed
Align error summary with NHS.UK frontend
1 parent f01e5f6 commit f69bd5f

File tree

4 files changed

+283
-148
lines changed

4 files changed

+283
-148
lines changed

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

Lines changed: 87 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,86 +1,120 @@
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+
Title.displayName = 'ErrorSummary.Title';
1926

20-
const ErrorSummaryBody: FC<HTMLProps<HTMLDivElement>> = ({ className, ...rest }) => (
21-
<div className={classNames('nhsuk-error-summary__body', className)} {...rest} />
22-
);
27+
const List: FC<ComponentProps<'ul'>> = ({ children, className, ...rest }) => {
28+
if (!children) {
29+
return null;
30+
}
2331

24-
const ErrorSummaryList: FC<HTMLProps<HTMLUListElement>> = ({ className, ...rest }) => (
25-
<ul className={classNames('nhsuk-list', 'nhsuk-error-summary__list', className)} {...rest} />
26-
);
32+
return (
33+
<ul className={classNames('nhsuk-list', 'nhsuk-error-summary__list', className)} {...rest}>
34+
{children}
35+
</ul>
36+
);
37+
};
2738

28-
const ErrorSummaryListItem: FC<HTMLProps<HTMLAnchorElement>> = (props) => (
29-
<li>
30-
<a {...props} />
31-
</li>
32-
);
39+
List.displayName = 'ErrorSummary.List';
40+
41+
const ListItem: FC<ComponentProps<'a'>> = ({ children, href, ...rest }) => {
42+
if (!children) {
43+
return null;
44+
}
45+
46+
return (
47+
<li>
48+
{href ? (
49+
<a href={href} {...rest}>
50+
{children}
51+
</a>
52+
) : (
53+
<>{children}</>
54+
)}
55+
</li>
56+
);
57+
};
58+
59+
ListItem.displayName = 'ErrorSummary.ListItem';
60+
61+
interface ErrorSummaryProps extends ComponentProps<'div'> {
62+
disableAutoFocus?: boolean;
63+
}
3364

3465
interface ErrorSummaryComponent
3566
extends ForwardRefExoticComponent<
36-
PropsWithoutRef<HTMLProps<HTMLDivElement>> & RefAttributes<HTMLDivElement>
67+
PropsWithoutRef<ErrorSummaryProps> & RefAttributes<HTMLDivElement>
3768
> {
38-
Title: FC<HTMLProps<HTMLHeadingElement>>;
39-
Body: FC<HTMLProps<HTMLDivElement>>;
40-
List: FC<HTMLProps<HTMLUListElement>>;
41-
Item: FC<HTMLProps<HTMLAnchorElement>>;
69+
Title: FC<ComponentProps<'h2'>>;
70+
List: FC<ComponentProps<'ul'>>;
71+
ListItem: FC<ComponentProps<'a'>>;
4272
}
4373

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-
);
74+
const ErrorSummaryDiv = forwardRef<HTMLDivElement, ErrorSummaryProps>(
75+
({ children, className, disableAutoFocus, ...rest }, forwardedRef) => {
76+
const [moduleRef] = useState(() => forwardedRef || createRef<HTMLDivElement>());
77+
const [instance, setInstance] = useState<ErrorSummary>();
78+
79+
useEffect(() => {
80+
if (!moduleRef || !('current' in moduleRef) || instance) {
81+
return;
82+
}
83+
84+
setInstance(new ErrorSummary(moduleRef.current));
85+
}, [moduleRef, instance]);
86+
87+
const items = Children.toArray(children);
88+
const [title] = items.filter((child) => childIsOfComponentType(child, Title));
89+
const bodyItems = items.filter((child) => !childIsOfComponentType(child, Title));
6390

6491
return (
6592
<div
6693
className={classNames('nhsuk-error-summary', className)}
67-
ref={ref}
68-
tabIndex={tabIndex}
69-
role={role}
70-
aria-labelledby={ariaLabelledBy}
94+
data-module="nhsuk-error-summary"
95+
data-disable-auto-focus={
96+
typeof disableAutoFocus !== 'undefined' ? disableAutoFocus : undefined
97+
}
98+
ref={moduleRef}
7199
{...rest}
72-
/>
100+
>
101+
{/* Keep the role="alert" in a seperate child container to prevent a race condition between
102+
the focusing js at the alert, resulting in information getting missed in screen reader announcements */}
103+
<div role="alert">
104+
{title}
105+
<div className="nhsuk-error-summary__body">{bodyItems}</div>
106+
</div>
107+
</div>
73108
);
74109
},
75110
);
76111

77112
ErrorSummaryDiv.displayName = 'ErrorSummary';
78113

79114
const ErrorSummaryComponent: ErrorSummaryComponent = Object.assign(ErrorSummaryDiv, {
80-
Title: ErrorSummaryTitle,
81-
Body: ErrorSummaryBody,
82-
List: ErrorSummaryList,
83-
Item: ErrorSummaryListItem,
115+
Title,
116+
List,
117+
ListItem,
84118
});
85119

86120
export default ErrorSummaryComponent;

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

Lines changed: 123 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -10,36 +10,47 @@ describe('ErrorSummary', () => {
1010
});
1111

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

16-
expect(ref.current).not.toBeNull();
16+
const errorSummaryEl = container.querySelector('div');
17+
18+
expect(inputRef.current).toBe(errorSummaryEl);
1719
});
1820

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

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-
);
24+
const errorSummaryEl = container.querySelector('div');
25+
26+
expect(document.activeElement).toBe(errorSummaryEl);
2727
});
2828

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-
);
29+
it('is focused automatically with forwarded ref', () => {
30+
const inputRef = createRef<HTMLDivElement>();
31+
const { container } = render(<ErrorSummary ref={inputRef} />);
32+
33+
const errorSummaryEl = container.querySelector('div');
34+
35+
expect(document.activeElement).toBe(errorSummaryEl);
3536
});
3637

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-
);
38+
it('has default props', () => {
39+
const { container } = render(<ErrorSummary />);
40+
41+
const errorSummaryEl = container.querySelector('div');
42+
43+
expect(errorSummaryEl?.getAttribute('tabindex')).toBe('-1');
44+
expect(errorSummaryEl?.firstElementChild?.getAttribute('role')).toBe('alert');
45+
});
46+
47+
it('has default props with forwarded ref', () => {
48+
const { container } = render(<ErrorSummary />);
49+
50+
const errorSummaryEl = container.querySelector('div');
51+
52+
expect(errorSummaryEl?.getAttribute('tabindex')).toBe('-1');
53+
expect(errorSummaryEl?.firstElementChild?.getAttribute('role')).toBe('alert');
4354
});
4455

4556
describe('ErrorSummary.Title', () => {
@@ -52,44 +63,117 @@ describe('ErrorSummary', () => {
5263
it('renders a title', () => {
5364
const { container } = render(<ErrorSummary.Title>Title</ErrorSummary.Title>);
5465

55-
expect(container.textContent).toBe('Title');
66+
expect(container).toHaveTextContent('Title');
5667
});
5768
});
5869

59-
describe('ErrorSummary.Body', () => {
70+
describe('ErrorSummary.List', () => {
6071
it('matches snapshot', () => {
61-
const { container } = render(<ErrorSummary.Body>Body</ErrorSummary.Body>);
72+
const { container } = render(
73+
<ErrorSummary>
74+
<ErrorSummary.List>List</ErrorSummary.List>
75+
</ErrorSummary>,
76+
);
6277

63-
expect(container.textContent).toBe('Body');
64-
expect(container).toMatchSnapshot('ErrorSummary.Body');
78+
const errorListEl = container.querySelector('ul');
79+
80+
expect(errorListEl).toMatchSnapshot('ErrorSummary.List');
6581
});
66-
});
6782

68-
describe('ErrorSummary.List', () => {
69-
it('matches snapshot', () => {
70-
const { container } = render(<ErrorSummary.List>List</ErrorSummary.List>);
83+
it('renders children', () => {
84+
const { container } = render(
85+
<ErrorSummary>
86+
<ErrorSummary.List>List</ErrorSummary.List>
87+
</ErrorSummary>,
88+
);
89+
90+
const errorListEl = container.querySelector('ul');
7191

72-
expect(container).toMatchSnapshot('ErrorSummary.List');
92+
expect(errorListEl).toHaveTextContent('List');
7393
});
7494

75-
it('renders children', () => {
76-
const { container } = render(<ErrorSummary.List>List</ErrorSummary.List>);
95+
it('skips render without children', () => {
96+
const { container } = render(
97+
<ErrorSummary>
98+
<ErrorSummary.List />
99+
</ErrorSummary>,
100+
);
101+
102+
const errorListEl = container.querySelector('ul');
77103

78-
expect(container.textContent).toBe('List');
104+
expect(errorListEl).toBeNull();
79105
});
80106
});
81107

82108
describe('ErrorSummary.ListItem', () => {
83-
it('matches snapshot', () => {
84-
const { container } = render(<ErrorSummary.Item>ListItem</ErrorSummary.Item>);
109+
it('matches snapshot for items without links', () => {
110+
const { container } = render(
111+
<ErrorSummary>
112+
<ErrorSummary.List>
113+
<ErrorSummary.ListItem>List item without link</ErrorSummary.ListItem>
114+
</ErrorSummary.List>
115+
</ErrorSummary>,
116+
);
117+
118+
const errorListEl = container.querySelector('ul');
119+
120+
expect(errorListEl).toMatchSnapshot();
121+
});
122+
123+
it('matches snapshot for items with links', () => {
124+
const { container } = render(
125+
<ErrorSummary>
126+
<ErrorSummary.List>
127+
<ErrorSummary.ListItem href="#example-error-1">
128+
List item with link
129+
</ErrorSummary.ListItem>
130+
</ErrorSummary.List>
131+
</ErrorSummary>,
132+
);
133+
134+
const errorListEl = container.querySelector('ul');
85135

86-
expect(container).toMatchSnapshot('ErrorSummary.ListItem');
136+
expect(errorListEl).toMatchSnapshot();
87137
});
88138

89139
it('renders children', () => {
90-
const { container } = render(<ErrorSummary.Item>ListItem</ErrorSummary.Item>);
140+
const { container } = render(
141+
<ErrorSummary>
142+
<ErrorSummary.List>
143+
<ErrorSummary.ListItem href="#example-error-1">
144+
List item with link 1
145+
</ErrorSummary.ListItem>
146+
<ErrorSummary.ListItem href="#example-error-2">
147+
List item with link 2
148+
</ErrorSummary.ListItem>
149+
</ErrorSummary.List>
150+
</ErrorSummary>,
151+
);
152+
153+
const errorListEl = container.querySelector('ul');
154+
const errorLinkEls = errorListEl?.querySelectorAll('a');
155+
156+
expect(errorLinkEls?.[0]).toHaveTextContent('List item with link 1');
157+
expect(errorLinkEls?.[0]).toHaveAttribute('href', '#example-error-1');
158+
159+
expect(errorLinkEls?.[1]).toHaveTextContent('List item with link 2');
160+
expect(errorLinkEls?.[1]).toHaveAttribute('href', '#example-error-2');
161+
});
162+
163+
it('skips render without children', () => {
164+
const { container } = render(
165+
<ErrorSummary>
166+
<ErrorSummary.List>
167+
<ErrorSummary.ListItem />
168+
</ErrorSummary.List>
169+
</ErrorSummary>,
170+
);
171+
172+
const errorListEl = container.querySelector('ul');
173+
const errorListItemEls = errorListEl?.querySelectorAll('li');
91174

92-
expect(container.querySelector('a')?.textContent).toBe('ListItem');
175+
expect(errorListEl).toHaveTextContent('');
176+
expect(errorListItemEls).toHaveLength(0);
93177
});
94178
});
95179
});

0 commit comments

Comments
 (0)