From 29e890bf60a842ce84797cd598145a030865009b Mon Sep 17 00:00:00 2001
From: Thomas Judd-Cooper <thomas.judd-cooper1@nhs.net>
Date: Mon, 10 Jun 2024 14:05:32 +0100
Subject: [PATCH] Add default ErrorSummary attributes (#229)

* Add default ErrorSummary attributes
* Remove attributes from ErrorSummary in storybook
---
 .../error-summary/ErrorSummary.tsx            | 41 ++++++++++++++++---
 .../__tests__/ErrorSummary.test.tsx           | 20 +++++++++
 .../__snapshots__/ErrorSummary.test.tsx.snap  |  4 ++
 .../Form Elements/ErrorSummary.stories.tsx    |  8 ++--
 4 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/src/components/form-elements/error-summary/ErrorSummary.tsx b/src/components/form-elements/error-summary/ErrorSummary.tsx
index a7fd1c42..83fceb49 100644
--- a/src/components/form-elements/error-summary/ErrorSummary.tsx
+++ b/src/components/form-elements/error-summary/ErrorSummary.tsx
@@ -7,11 +7,19 @@ import React, {
   RefAttributes,
 } from 'react';
 import classNames from 'classnames';
+import useDevWarning from '@util/hooks/UseDevWarning';
 
-const ErrorSummaryTitle: FC<HTMLProps<HTMLHeadingElement>> = ({ className, ...rest }) => (
-  <h2 className={classNames('nhsuk-error-summary__title', className)} {...rest} />
+const DefaultErrorSummaryTitleID = 'error-summary-title';
+
+const ErrorSummaryTitle: FC<HTMLProps<HTMLHeadingElement>> = ({
+  className,
+  id = DefaultErrorSummaryTitleID,
+  ...rest
+}) => (
+  <h2 className={classNames('nhsuk-error-summary__title', className)} id={id} {...rest} />
 );
 
+
 const ErrorSummaryBody: FC<HTMLProps<HTMLDivElement>> = ({ className, ...rest }) => (
   <div className={classNames('nhsuk-error-summary__body', className)} {...rest} />
 );
@@ -37,10 +45,31 @@ interface ErrorSummary
 }
 
 const ErrorSummaryDiv = forwardRef<HTMLDivElement, HTMLProps<HTMLDivElement>>(
-  ({ className, ...rest }, ref) => (
-    <div className={classNames('nhsuk-error-summary', className)} ref={ref} {...rest} />
-  ),
-);
+  ({
+    className,
+    tabIndex = -1,
+    role = 'alert',
+    'aria-labelledby': ariaLabelledBy = DefaultErrorSummaryTitleID,
+    ...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}
+      />
+    )
+});
+
+
 ErrorSummaryDiv.displayName = 'ErrorSummary';
 
 const ErrorSummary: ErrorSummary = Object.assign(ErrorSummaryDiv, {
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 ca0ac516..097142d7 100644
--- a/src/components/form-elements/error-summary/__tests__/ErrorSummary.test.tsx
+++ b/src/components/form-elements/error-summary/__tests__/ErrorSummary.test.tsx
@@ -16,6 +16,26 @@ describe('ErrorSummary', () => {
     expect(ref.current).not.toBeNull();
   });
 
+  it('has default props', () => {
+    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');
+  })
+
+  it('throws a dev warning if tabIndex is not -1', () => {
+    jest.spyOn(console, 'warn').mockImplementation(() => {});
+    render(<ErrorSummary tabIndex={0} />);
+    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 f48af453..7e3f5481 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,6 +34,7 @@ exports[`ErrorSummary ErrorSummary.Title matches snapshot: ErrorSummary.Title 1`
 <div>
   <h2
     class="nhsuk-error-summary__title"
+    id="error-summary-title"
   >
     Title
   </h2>
@@ -43,7 +44,10 @@ 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>
 `;
diff --git a/stories/Form Elements/ErrorSummary.stories.tsx b/stories/Form Elements/ErrorSummary.stories.tsx
index 8a6032e2..a18cede2 100644
--- a/stories/Form Elements/ErrorSummary.stories.tsx	
+++ b/stories/Form Elements/ErrorSummary.stories.tsx	
@@ -23,8 +23,8 @@ import { Meta, StoryObj } from '@storybook/react';
  *
  * const Element = () => {
  *     return (
- *         <ErrorSummary aria-labelledby="error-summary-title" role="alert" tabIndex={-1}>
- *             <ErrorSummary.Title id="error-summary-title">There is a problem</ErrorSummary.Title>
+ *         <ErrorSummary>
+ *             <ErrorSummary.Title>There is a problem</ErrorSummary.Title>
  *             <ErrorSummary.Body>
  *                 <p>Optional description of the errors and how to correct them</p>
  *                 <ErrorSummary.List>
@@ -55,8 +55,8 @@ ErrorSummary.Item.displayName = 'ErrorSummary.Item';
 
 export const Standard: Story = {
   render: (args) => (
-    <ErrorSummary aria-labelledby="error-summary-title" role="alert" tabIndex={-1}>
-      <ErrorSummary.Title id="error-summary-title">There is a problem</ErrorSummary.Title>
+    <ErrorSummary>
+      <ErrorSummary.Title>There is a problem</ErrorSummary.Title>
       <ErrorSummary.Body>
         <p>Optional description of the errors and how to correct them</p>
         <ErrorSummary.List>