From d0338069ea151aa1675bc02d35054796ea72ba60 Mon Sep 17 00:00:00 2001
From: Thomas Judd-Cooper <thomas.judd-cooper1@nhs.net>
Date: Mon, 10 Jun 2024 14:26:30 +0100
Subject: [PATCH] Fix aria attributes on error messages (#230)

* Fix aria attributes on error messages
* Fix behaviour of aria-describedby in form components
---
 .../__snapshots__/Checkboxes.test.tsx.snap    |  2 ++
 src/util/FormGroup.tsx                        |  8 +++--
 src/util/__tests__/FormGroup.test.tsx         | 35 ++++++++++++++++---
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/src/components/form-elements/checkboxes/__tests__/__snapshots__/Checkboxes.test.tsx.snap b/src/components/form-elements/checkboxes/__tests__/__snapshots__/Checkboxes.test.tsx.snap
index 214668c8..2a800007 100644
--- a/src/components/form-elements/checkboxes/__tests__/__snapshots__/Checkboxes.test.tsx.snap
+++ b/src/components/form-elements/checkboxes/__tests__/__snapshots__/Checkboxes.test.tsx.snap
@@ -178,6 +178,7 @@ exports[`Checkboxes matches snapshot with boolean error 1`] = `
       class="nhsuk-form-group nhsuk-form-group--error"
     >
       <div
+        aria-describedby="example--error-message"
         class="nhsuk-checkboxes"
         id="example"
       >
@@ -262,6 +263,7 @@ exports[`Checkboxes matches snapshot with string error 1`] = `
         Example error
       </span>
       <div
+        aria-describedby="example--error-message"
         class="nhsuk-checkboxes"
         id="example"
       >
diff --git a/src/util/FormGroup.tsx b/src/util/FormGroup.tsx
index 8324147a..f08006d7 100644
--- a/src/util/FormGroup.tsx
+++ b/src/util/FormGroup.tsx
@@ -62,9 +62,13 @@ const FormGroup = <T extends BaseFormElementRenderProps>(props: FormGroupProps<T
   const errorID = `${elementID}--error-message`;
   const hintID = `${elementID}--hint`;
 
+  const ariaDescribedBy = [
+    hint ? hintID : undefined,
+    error ? errorID : undefined,
+  ].filter(Boolean);
+
   const childProps = {
-    'aria-describedby': hint ? hintID : undefined,
-    'aria-labelledby': label ? labelID : undefined,
+    'aria-describedby': ariaDescribedBy.join(' ') || undefined,
     error,
     name: name || elementID,
     id: elementID,
diff --git a/src/util/__tests__/FormGroup.test.tsx b/src/util/__tests__/FormGroup.test.tsx
index eefa1bfa..9c04be3f 100644
--- a/src/util/__tests__/FormGroup.test.tsx
+++ b/src/util/__tests__/FormGroup.test.tsx
@@ -108,9 +108,6 @@ describe('FormGroup', () => {
     expect(renderProps!.id).toHaveLength(11);
     expect(renderProps!.id).toContain('input');
 
-    expect(container.querySelector('input')?.getAttribute('aria-labelledby')).toBe(
-      `${renderProps!.id}--label`,
-    );
     expect(container.querySelector('.nhsuk-label')?.getAttribute('id')).toBe(
       `${renderProps!.id}--label`,
     );
@@ -133,7 +130,6 @@ describe('FormGroup', () => {
     expect(renderProps).not.toBe(null);
     expect(renderProps!.id).toBe('testID');
 
-    expect(container.querySelector('input')?.getAttribute('aria-labelledby')).toBe('testID--label');
     expect(container.querySelector('.nhsuk-label')?.getAttribute('id')).toBe('testID--label');
     expect(container.querySelector('.nhsuk-label')?.getAttribute('for')).toBe('testID');
     expect(container.querySelector('.nhsuk-label')?.textContent).toBe('This is a test label');
@@ -155,6 +151,7 @@ describe('FormGroup', () => {
     expect(renderProps).not.toBe(null);
     expect(renderProps!.id).toHaveLength(11);
     expect(renderProps!.id).toContain('input');
+    expect(renderProps!['aria-describedby']).toBe(`${renderProps!.id}--error-message`);
 
     expect(container.querySelector('.nhsuk-error-message')?.getAttribute('id')).toBe(
       `${renderProps!.id}--error-message`,
@@ -182,6 +179,8 @@ describe('FormGroup', () => {
 
     expect(renderProps).not.toBe(null);
     expect(renderProps!.id).toBe('testID');
+    expect(renderProps!['aria-describedby']).toBe(`testID--error-message`);
+
 
     expect(container.querySelector('.nhsuk-error-message')?.getAttribute('id')).toBe(
       'testID--error-message',
@@ -240,4 +239,32 @@ describe('FormGroup', () => {
 
     expect(await axe(html)).toHaveNoViolations();
   });
+
+  it('should add hint ID and error ID to the aria-describedby of the input', () => {
+    const { container } = renderFormGroupComponent({
+      inputType: 'input',
+      id: 'error-and-hint',
+      error: 'This is an error',
+      hint: 'This is a hint',
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      children: ({ error, ...rest }) => <input {...rest} />,
+    });
+
+    const inputElement = container.querySelector('input');
+    expect(inputElement).not.toBeNull();
+    expect(inputElement?.getAttribute('aria-describedby')).toBe('error-and-hint--hint error-and-hint--error-message');
+  })
+
+  it('should have no aria-describedby when there is no hint or label', () => {
+    const { container } = renderFormGroupComponent({
+      inputType: 'input',
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      children: ({ error, ...rest }) => <input {...rest} />,
+    });
+
+    const inputElement = container.querySelector('input');
+    expect(inputElement).not.toBeNull();
+
+    expect(inputElement?.getAttribute('aria-describedby')).toBe(null);
+  });
 });