Update error summary markup following nhs frontend #1036

NHSDigital · Sep 27, 2024 · efddee7 · efddee7
1 parent 6d09861
commit efddee7
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 22 deletions.
diff --git a/src/components/form-elements/error-summary/ErrorSummary.tsx b/src/components/form-elements/error-summary/ErrorSummary.tsx
@@ -9,14 +9,15 @@ import React, {
 import classNames from 'classnames';
 import useDevWarning from '@util/hooks/UseDevWarning';
 
-const DefaultErrorSummaryTitleID = 'error-summary-title';
-
 const ErrorSummaryTitle: FC<HTMLProps<HTMLHeadingElement>> = ({
   className,
-  id = DefaultErrorSummaryTitleID,
+  id,
   ...rest
 }) => (
-  <h2 className={classNames('nhsuk-error-summary__title', className)} id={id} {...rest} />
+  <h2 className={classNames('nhsuk-error-summary__title', className)}
+    {...(id && { id })}
+    {...rest}
+  />
 );
 
 
@@ -48,24 +49,24 @@ const ErrorSummaryDiv = forwardRef<HTMLDivElement, HTMLProps<HTMLDivElement>>(
   ({
     className,
     tabIndex = -1,
-    role = 'alert',
-    'aria-labelledby': ariaLabelledBy = DefaultErrorSummaryTitleID,
+    children,
     ...rest
   },
   ref
 ) => {
     useDevWarning('The ErrorSummary component should always have a tabIndex of -1', () => tabIndex !== -1)
-    useDevWarning('The ErrorSummary component should always have a role of alert', () => role !== 'alert')
 
     return (
       <div
         className={classNames('nhsuk-error-summary', className)}
         ref={ref}
         tabIndex={tabIndex}
-        role={role}
-        aria-labelledby={ariaLabelledBy}
         {...rest}
-      />
+      >
+        <div role="alert">
+          {children}
+        </div>
+      </div>
     )
 });
 

diff --git a/src/components/form-elements/error-summary/__tests__/ErrorSummary.test.tsx b/src/components/form-elements/error-summary/__tests__/ErrorSummary.test.tsx
@@ -20,8 +20,15 @@ describe('ErrorSummary', () => {
     const { container } = render(<ErrorSummary />);
 
     expect(container.querySelector('div')?.getAttribute('tabindex')).toBe('-1');
-    expect(container.querySelector('div')?.getAttribute('role')).toBe('alert');
-    expect(container.querySelector('div')?.getAttribute('aria-labelledby')).toBe('error-summary-title');
+    expect(container.querySelector('div')?.getAttribute('role')).toBeNull();
+    expect(container.querySelector('div')?.getAttribute('aria-labelledby')).toBeNull();
+  })
+
+  it('has role="alert" on the inner div', () => {
+    const { container } = render(<ErrorSummary />);
+
+    const alertDiv = container.querySelector('.nhsuk-error-summary > div[role="alert"]');
+    expect(alertDiv).not.toBeNull();
   })
 
   it('throws a dev warning if tabIndex is not -1', () => {
@@ -30,12 +37,6 @@ describe('ErrorSummary', () => {
     expect(console.warn).toHaveBeenCalledWith('The ErrorSummary component should always have a tabIndex of -1');
   })
 
-  it('throws a dev warning if role is not alert', () => {
-    jest.spyOn(console, 'warn').mockImplementation(() => {});
-    render(<ErrorSummary role="status" />);
-    expect(console.warn).toHaveBeenCalledWith('The ErrorSummary component should always have a role of alert');
-  })
-
   describe('ErrorSummary.Title', () => {
     it('matches snapshot', () => {
       const { container } = render(<ErrorSummary.Title>Title</ErrorSummary.Title>);

diff --git a/...components/form-elements/error-summary/__tests__/__snapshots__/ErrorSummary.test.tsx.snap b/...components/form-elements/error-summary/__tests__/__snapshots__/ErrorSummary.test.tsx.snap
@@ -34,7 +34,6 @@ exports[`ErrorSummary ErrorSummary.Title matches snapshot: ErrorSummary.Title 1`
 <div>
   <h2
     class="nhsuk-error-summary__title"
-    id="error-summary-title"
   >
     Title
   </h2>
@@ -44,10 +43,12 @@ exports[`ErrorSummary ErrorSummary.Title matches snapshot: ErrorSummary.Title 1`
 exports[`ErrorSummary matches snapshot: ErrorSummary 1`] = `
 <div>
   <div
-    aria-labelledby="error-summary-title"
     class="nhsuk-error-summary"
-    role="alert"
     tabindex="-1"
-  />
+  >
+    <div
+      role="alert"
+    />
+  </div>
 </div>
 `;