fix(aria-describedby): associated inline error messages with a specif…

…ic field. (#5464)
aws-amplify · Aug 3, 2024 · 29f11a5 · 29f11a5
1 parent e3a6a45
commit 29f11a5
Show file tree

Hide file tree

Showing 25 changed files with 608 additions and 52 deletions.
diff --git a/.changeset/shaggy-planes-teach.md b/.changeset/shaggy-planes-teach.md
@@ -0,0 +1,5 @@
+---
+'@aws-amplify/ui-react': patch
+---
+
+fix(accessibility): associated inline error messages with all form fields in aria-describedby.
diff --git a/packages/react/package.json b/packages/react/package.json
@@ -83,13 +83,13 @@
       "name": "Authenticator",
       "path": "dist/esm/index.mjs",
       "import": "{ Authenticator }",
-      "limit": "76 kB"
+      "limit": "77 kB"
     },
     {
       "name": "AccountSettings",
       "path": "dist/esm/index.mjs",
       "import": "{ AccountSettings }",
-      "limit": "29 kB"
+      "limit": "30 kB"
     }
   ]
 }
diff --git a/packages/react/src/helpers/constants.ts b/packages/react/src/helpers/constants.ts
@@ -4,3 +4,6 @@ export const AMPLIFY_SYMBOL = (
     ? Symbol.for('amplify_default')
     : '@@amplify_default'
 ) as Symbol;
+
+export const ERROR_SUFFIX = 'error';
+export const DESCRIPTION_SUFFIX = 'description';
diff --git a/packages/react/src/primitives/Checkbox/Checkbox.tsx b/packages/react/src/primitives/Checkbox/Checkbox.tsx
@@ -10,7 +10,7 @@ import { Text } from '../Text';
 import { VisuallyHidden } from '../VisuallyHidden';
 import { BaseCheckboxProps, CheckboxProps } from '../types/checkbox';
 import { ForwardRefPrimitive, Primitive } from '../types/view';
-import { getTestId } from '../utils/getTestId';
+import { getUniqueComponentId } from '../utils/getUniqueComponentId';
 import { useStableId } from '../utils/useStableId';
 import { splitPrimitiveProps } from '../utils/splitPrimitiveProps';
 import { classNameModifierByFlag } from '../shared/utils';
@@ -88,9 +88,18 @@ const CheckboxPrimitive: Primitive<CheckboxProps, 'input'> = (
     }
   }, [dataId, isIndeterminate]);
 
-  const buttonTestId = getTestId(testId, ComponentClassName.CheckboxButton);
-  const iconTestId = getTestId(testId, ComponentClassName.CheckboxIcon);
-  const labelTestId = getTestId(testId, ComponentClassName.CheckboxLabel);
+  const buttonTestId = getUniqueComponentId(
+    testId,
+    ComponentClassName.CheckboxButton
+  );
+  const iconTestId = getUniqueComponentId(
+    testId,
+    ComponentClassName.CheckboxIcon
+  );
+  const labelTestId = getUniqueComponentId(
+    testId,
+    ComponentClassName.CheckboxLabel
+  );
   const flexClasses = classNames(
     ComponentClassName.CheckboxButton,
     classNameModifierByFlag(

diff --git a/packages/react/src/primitives/CheckboxField/CheckboxField.tsx b/packages/react/src/primitives/CheckboxField/CheckboxField.tsx
@@ -11,7 +11,7 @@ import {
 } from '../types';
 import { ComponentClassName } from '@aws-amplify/ui';
 import { FieldErrorMessage } from '../Field';
-import { getTestId } from '../utils/getTestId';
+import { getUniqueComponentId } from '../utils/getUniqueComponentId';
 import { primitiveWithForwardRef } from '../utils/primitiveWithForwardRef';
 import { Flex } from '../Flex';
 
@@ -28,7 +28,10 @@ const CheckboxFieldPrimitive: Primitive<CheckboxFieldProps, 'input'> = (
   },
   ref
 ) => {
-  const checkboxTestId = getTestId(testId, ComponentClassName.Checkbox);
+  const checkboxTestId = getUniqueComponentId(
+    testId,
+    ComponentClassName.Checkbox
+  );
   return (
     <Flex
       className={classNames(

diff --git a/packages/react/src/primitives/HighlightMatch/HighlightMatch.tsx b/packages/react/src/primitives/HighlightMatch/HighlightMatch.tsx
@@ -3,7 +3,7 @@ import * as React from 'react';
 
 import { View } from '../View';
 import { strHasLength } from '../shared/utils';
-import { getTestId } from '../utils/getTestId';
+import { getUniqueComponentId } from '../utils/getUniqueComponentId';
 import { ComponentClassName } from '@aws-amplify/ui';
 import type {
   BaseHighlightMatchProps,
@@ -17,7 +17,7 @@ export const HighlightMatchPrimitive: Primitive<HighlightMatchProps, 'span'> = (
   { children, className, query, testId, ...rest },
   ref
 ) => {
-  const matchTestId = getTestId(testId, 'match');
+  const matchTestId = getUniqueComponentId(testId, 'match');
   const startIndex = children
     ?.toLocaleLowerCase()
     .indexOf(query?.toLocaleLowerCase());

diff --git a/packages/react/src/primitives/RadioGroupField/RadioGroupField.tsx b/packages/react/src/primitives/RadioGroupField/RadioGroupField.tsx
@@ -13,9 +13,11 @@ import {
   ForwardRefPrimitive,
   Primitive,
 } from '../types';
-import { getTestId } from '../utils/getTestId';
+import { getUniqueComponentId } from '../utils/getUniqueComponentId';
 import { useStableId } from '../utils/useStableId';
 import { primitiveWithForwardRef } from '../utils/primitiveWithForwardRef';
+import { createSpaceSeparatedIds } from '../utils/createSpaceSeparatedIds';
+import { DESCRIPTION_SUFFIX, ERROR_SUFFIX } from '../../helpers/constants';
 
 const RadioGroupFieldPrimitive: Primitive<RadioGroupFieldProps, 'fieldset'> = (
   {
@@ -43,9 +45,18 @@ const RadioGroupFieldPrimitive: Primitive<RadioGroupFieldProps, 'fieldset'> = (
   ref
 ) => {
   const fieldId = useStableId(id);
-  const descriptionId = useStableId();
-  const ariaDescribedBy = descriptiveText ? descriptionId : undefined;
-  const radioGroupTestId = getTestId(testId, ComponentClassName.RadioGroup);
+  const stableId = useStableId();
+  const descriptionId = descriptiveText
+    ? getUniqueComponentId(stableId, DESCRIPTION_SUFFIX)
+    : undefined;
+  const errorId = hasError
+    ? getUniqueComponentId(stableId, ERROR_SUFFIX)
+    : undefined;
+  const ariaDescribedBy = createSpaceSeparatedIds([errorId, descriptionId]);
+  const radioGroupTestId = getUniqueComponentId(
+    testId,
+    ComponentClassName.RadioGroup
+  );
 
   const radioGroupContextValue: RadioGroupContextType = React.useMemo(
     () => ({
@@ -107,7 +118,11 @@ const RadioGroupFieldPrimitive: Primitive<RadioGroupFieldProps, 'fieldset'> = (
           {children}
         </RadioGroupContext.Provider>
       </Flex>
-      <FieldErrorMessage hasError={hasError} errorMessage={errorMessage} />
+      <FieldErrorMessage
+        id={errorId}
+        hasError={hasError}
+        errorMessage={errorMessage}
+      />
     </Fieldset>
   );
 };

diff --git a/packages/react/src/primitives/RadioGroupField/__tests__/RadioGroupField.test.tsx b/packages/react/src/primitives/RadioGroupField/__tests__/RadioGroupField.test.tsx
@@ -10,6 +10,8 @@ import {
   testFlexProps,
   expectFlexContainerStyleProps,
 } from '../../Flex/__tests__/Flex.test';
+import { ERROR_SUFFIX, DESCRIPTION_SUFFIX } from '../../../helpers/constants';
+import { getUniqueComponentId } from '../../utils/getUniqueComponentId';
 
 const basicProps = { legend: 'testLabel', name: 'testName', testId: 'testId' };
 const radioGroupTestId = `${basicProps.testId}-${ComponentClassName.RadioGroup}`;
@@ -310,4 +312,78 @@ describe('RadioFieldGroup', () => {
       expect(errorText).toContainHTML(errorMessage);
     });
   });
+
+  describe('aria-describedby test', () => {
+    const errorMessage = 'This is an error message';
+    const descriptiveText = 'Description';
+    it('when hasError, include id of error component and describe component in the aria-describedby', async () => {
+      render(
+        RadioFieldGroup({
+          ...basicProps,
+          descriptiveText,
+          hasError: true,
+          errorMessage,
+        })
+      );
+
+      const field = await screen.findByTestId(
+        getUniqueComponentId(
+          basicProps.testId,
+          ComponentClassName.RadioGroup
+        ) || ''
+      );
+      const ariaDescribedBy = field.getAttribute('aria-describedby');
+      const descriptiveTextElement = screen.queryByText(descriptiveText);
+      const errorTextElement = screen.queryByText(errorMessage);
+      expect(
+        errorTextElement?.id && errorTextElement?.id.endsWith(ERROR_SUFFIX)
+      ).toBe(true);
+      expect(
+        descriptiveTextElement?.id &&
+          descriptiveTextElement?.id.endsWith(DESCRIPTION_SUFFIX)
+      ).toBe(true);
+      expect(
+        errorTextElement?.id &&
+          descriptiveTextElement?.id &&
+          ariaDescribedBy ===
+            `${errorTextElement.id} ${descriptiveTextElement.id}`
+      ).toBe(true);
+    });
+
+    it('only show id of describe component in aria-describedby when hasError is false', async () => {
+      render(
+        RadioFieldGroup({
+          ...basicProps,
+          descriptiveText,
+          errorMessage,
+        })
+      );
+
+      const field = await screen.findByTestId(
+        getUniqueComponentId(
+          basicProps.testId,
+          ComponentClassName.RadioGroup
+        ) || ''
+      );
+      const ariaDescribedBy = field.getAttribute('aria-describedby');
+      const descriptiveTextElement = screen.queryByText(descriptiveText);
+      expect(
+        descriptiveTextElement?.id &&
+          ariaDescribedBy?.startsWith(descriptiveTextElement?.id)
+      ).toBe(true);
+    });
+
+    it('aria-describedby should be empty when hasError is false and descriptiveText is empty', async () => {
+      render(
+        RadioFieldGroup({
+          ...basicProps,
+          errorMessage,
+        })
+      );
+
+      const field = await screen.findByTestId(basicProps.testId);
+      const ariaDescribedBy = field.getAttribute('aria-describedby');
+      expect(ariaDescribedBy).toBeNull();
+    });
+  });
 });
diff --git a/packages/react/src/primitives/SearchField/__tests__/SearchField.test.tsx b/packages/react/src/primitives/SearchField/__tests__/SearchField.test.tsx
@@ -226,7 +226,7 @@ describe('SearchField component', () => {
 
       const searchField = await screen.findByLabelText(label);
 
-      await user.type(searchField, searchQuery);
+      await act(async () => user.type(searchField, searchQuery));
 
       const clearButton = await screen.findByLabelText(clearButtonLabel);
       expect(clearButton).not.toHaveFocus();

diff --git a/packages/react/src/primitives/SelectField/SelectField.tsx b/packages/react/src/primitives/SelectField/SelectField.tsx
@@ -16,6 +16,9 @@ import {
 import { splitPrimitiveProps } from '../utils/splitPrimitiveProps';
 import { useStableId } from '../utils/useStableId';
 import { primitiveWithForwardRef } from '../utils/primitiveWithForwardRef';
+import { createSpaceSeparatedIds } from '../utils/createSpaceSeparatedIds';
+import { DESCRIPTION_SUFFIX, ERROR_SUFFIX } from '../../helpers/constants';
+import { getUniqueComponentId } from '../utils/getUniqueComponentId';
 
 interface SelectFieldChildrenProps {
   children?: React.ReactNode;
@@ -64,8 +67,14 @@ const SelectFieldPrimitive: Primitive<SelectFieldProps, 'select'> = (
   } = props;
 
   const fieldId = useStableId(id);
-  const descriptionId = useStableId();
-  const ariaDescribedBy = descriptiveText ? descriptionId : undefined;
+  const stableId = useStableId();
+  const descriptionId = descriptiveText
+    ? getUniqueComponentId(stableId, DESCRIPTION_SUFFIX)
+    : undefined;
+  const errorId = hasError
+    ? getUniqueComponentId(stableId, ERROR_SUFFIX)
+    : undefined;
+  const ariaDescribedBy = createSpaceSeparatedIds([errorId, descriptionId]);
 
   const { styleProps, rest } = splitPrimitiveProps(_rest);
 
@@ -99,7 +108,11 @@ const SelectFieldPrimitive: Primitive<SelectFieldProps, 'select'> = (
       >
         {selectFieldChildren({ children, options })}
       </Select>
-      <FieldErrorMessage hasError={hasError} errorMessage={errorMessage} />
+      <FieldErrorMessage
+        id={errorId}
+        hasError={hasError}
+        errorMessage={errorMessage}
+      />
     </Flex>
   );
 };

diff --git a/packages/react/src/primitives/SelectField/__tests__/SelectField.test.tsx b/packages/react/src/primitives/SelectField/__tests__/SelectField.test.tsx
@@ -9,6 +9,7 @@ import {
   expectFlexContainerStyleProps,
 } from '../../Flex/__tests__/Flex.test';
 import { AUTO_GENERATED_ID_PREFIX } from '../../utils/useStableId';
+import { ERROR_SUFFIX, DESCRIPTION_SUFFIX } from '../../../helpers/constants';
 
 describe('SelectField', () => {
   const className = 'my-select';
@@ -353,4 +354,77 @@ describe('SelectField', () => {
       expect(consoleWarnSpy).toHaveBeenCalledWith(warningMessage);
     });
   });
+
+  describe('aria-describedby test', () => {
+    const errorMessage = 'This is an error message';
+    const descriptiveText = 'Description';
+    it('when hasError, include id of error component and describe component in the aria-describedby', async () => {
+      render(
+        <SelectField
+          label={label}
+          descriptiveText={descriptiveText}
+          errorMessage={errorMessage}
+          hasError
+        >
+          <option value="1">1</option>
+          <option value="2">2</option>
+          <option value="3">3</option>
+        </SelectField>
+      );
+
+      const select = await screen.findByLabelText(label);
+      const ariaDescribedBy = select.getAttribute('aria-describedby');
+      const descriptiveTextElement = screen.queryByText(descriptiveText);
+      const errorTextElement = screen.queryByText(errorMessage);
+      expect(
+        errorTextElement?.id && errorTextElement?.id.endsWith(ERROR_SUFFIX)
+      ).toBe(true);
+      expect(
+        descriptiveTextElement?.id &&
+          descriptiveTextElement?.id.endsWith(DESCRIPTION_SUFFIX)
+      ).toBe(true);
+      expect(
+        errorTextElement?.id &&
+          descriptiveTextElement?.id &&
+          ariaDescribedBy ===
+            `${errorTextElement.id} ${descriptiveTextElement.id}`
+      ).toBe(true);
+    });
+
+    it('only show id of describe component in aria-describedby when hasError is false', async () => {
+      render(
+        <SelectField
+          label={label}
+          descriptiveText={descriptiveText}
+          errorMessage={errorMessage}
+        >
+          <option value="1">1</option>
+          <option value="2">2</option>
+          <option value="3">3</option>
+        </SelectField>
+      );
+
+      const select = await screen.findByLabelText(label);
+      const ariaDescribedBy = select.getAttribute('aria-describedby');
+      const descriptiveTextElement = screen.queryByText(descriptiveText);
+      expect(
+        descriptiveTextElement?.id &&
+          ariaDescribedBy?.startsWith(descriptiveTextElement?.id)
+      ).toBe(true);
+    });
+
+    it('aria-describedby should be empty when hasError is false and descriptiveText is empty', async () => {
+      render(
+        <SelectField label={label} errorMessage={errorMessage}>
+          <option value="1">1</option>
+          <option value="2">2</option>
+          <option value="3">3</option>
+        </SelectField>
+      );
+
+      const select = await screen.findByLabelText(label);
+      const ariaDescribedBy = select.getAttribute('aria-describedby');
+      expect(ariaDescribedBy).toBeNull();
+    });
+  });
 });