From efddee7ffa584481d8b23e9b98d8acccef16e7fd Mon Sep 17 00:00:00 2001
From: Ed Horsford <mail@edwardhorsford.com>
Date: Fri, 27 Sep 2024 13:24:49 +0100
Subject: [PATCH] Update error summary markup following nhs frontend #1036

---
 .../error-summary/ErrorSummary.tsx            | 21 ++++++++++---------
 .../__tests__/ErrorSummary.test.tsx           | 17 ++++++++-------
 .../__snapshots__/ErrorSummary.test.tsx.snap  |  9 ++++----
 3 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/src/components/form-elements/error-summary/ErrorSummary.tsx b/src/components/form-elements/error-summary/ErrorSummary.tsx
index 83fceb4..a74d3a0 100644
--- 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
index 097142d..eb90417 100644
--- 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/src/components/form-elements/error-summary/__tests__/__snapshots__/ErrorSummary.test.tsx.snap b/src/components/form-elements/error-summary/__tests__/__snapshots__/ErrorSummary.test.tsx.snap
index 7e3f548..96f6dae 100644
--- a/src/components/form-elements/error-summary/__tests__/__snapshots__/ErrorSummary.test.tsx.snap
+++ b/src/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>
 `;