[ChoiceList] Connecting the ChoiceList error the input with errors via describedByProp

UNRELEASED.md

@@ -14,6 +14,8 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 ### Bug fixes
 
+- Fixed accessibility issue with ChoiceList errors not being correctly connected to the inputs ([#1824](https://github.com/Shopify/polaris-react/pull/1824));
+
 ### Documentation
 
 ### Development workflow

src/components/Checkbox/Checkbox.tsx

@@ -5,12 +5,15 @@ import {createUniqueIDFactory} from '@shopify/javascript-utilities/other';
 
 import {withAppProvider, WithAppProviderProps} from '../AppProvider';
 import Choice, {helpTextID} from '../Choice';
+import {errorTextID} from '../InlineError';
 import Icon from '../Icon';
 import {Error, Key} from '../../types';
 
 import styles from './Checkbox.scss';
 
 export interface BaseProps {
+  /** Indicates the ID of the element that describes the checkbox*/
+  ariaDescribedBy?: string;
   /** Label for the checkbox */
   label: React.ReactNode;
   /** Visually hide the label */
@@ -65,6 +68,7 @@ class Checkbox extends React.PureComponent<CombinedProps, never> {
 
   render() {
     const {
+      ariaDescribedBy: ariaDescribedByProp,
       id = getUniqueID(),
       label,
       labelHidden,
@@ -78,12 +82,15 @@ class Checkbox extends React.PureComponent<CombinedProps, never> {
       value,
     } = this.props;
     const describedBy: string[] = [];
-    if (error) {
-      describedBy.push(`${id}Error`);
+    if (error && typeof error !== 'boolean') {
+      describedBy.push(errorTextID(id));
     }
     if (helpText) {
       describedBy.push(helpTextID(id));
     }
+    if (ariaDescribedByProp) {
+      describedBy.push(ariaDescribedByProp);
+    }
     const ariaDescribedBy = describedBy.length
       ? describedBy.join(' ')
       : undefined;

src/components/Checkbox/tests/Checkbox.test.tsx

@@ -189,30 +189,27 @@ describe('<Checkbox />', () => {
       const checkbox = shallowWithAppProvider(
         <Checkbox error={<span>Error</span>} label="Checkbox" />,
       );
-      expect(checkbox.find('input').prop<string>('aria-invalid')).toBe(true);
+      expect(checkbox.find('input').prop('aria-invalid')).toBe(true);
 
       checkbox.setProps({error: 'Some error'});
-      expect(checkbox.find('input').prop<string>('aria-invalid')).toBe(true);
+      expect(checkbox.find('input').prop('aria-invalid')).toBe(true);
     });
 
-    it('connects the input to the error', () => {
+    it('connects the input to the error if the error is not boolean', () => {
       const checkbox = mountWithAppProvider(
         <Checkbox label="Checkbox" error="Some error" />,
       );
-      const errorID = checkbox.find('input').prop<string>('aria-describedby');
+      const errorID = checkbox.find('input').prop('aria-describedby');
       expect(typeof errorID).toBe('string');
       expect(checkbox.find(`#${errorID}`).text()).toBe('Some error');
     });
 
-    it('marks the input as invalid but avoids rendering an error message when provided a boolean', () => {
+    it('does not connect the input to the error if the error is boolean', () => {
       const checkbox = mountWithAppProvider(
-        <Checkbox error={Boolean(true)} label="Checkbox" />,
+        <Checkbox label="Checkbox" error />,
       );
-      const errorID = checkbox.find('input').prop<string>('aria-describedby');
-
-      expect(checkbox.find('input').prop<string>('aria-invalid')).toBe(true);
-      expect(typeof errorID).toBe('string');
-      expect(checkbox.find(`#${errorID}`)).toHaveLength(0);
+      const errorID = checkbox.find('input').prop('aria-describedby');
+      expect(errorID).toBeUndefined();
     });
 
     it('connects the input to both an error and help text', () => {
@@ -234,21 +231,32 @@ describe('<Checkbox />', () => {
       const checkbox = shallowWithAppProvider(
         <Checkbox label="Checkbox" checked="indeterminate" />,
       );
-      expect(checkbox.find('input').prop<string>('indeterminate')).toBe('true');
+      expect(checkbox.find('input').prop('indeterminate')).toBe('true');
     });
 
     it('sets the aria-checked attribute on the input as mixed when checked is "indeterminate"', () => {
       const checkbox = shallowWithAppProvider(
         <Checkbox label="Checkbox" checked="indeterminate" />,
       );
-      expect(checkbox.find('input').prop<string>('aria-checked')).toBe('mixed');
+      expect(checkbox.find('input').prop('aria-checked')).toBe('mixed');
     });
 
     it('sets the checked attribute on the input to false when checked is "indeterminate"', () => {
       const checkbox = shallowWithAppProvider(
         <Checkbox label="Checkbox" checked="indeterminate" />,
       );
-      expect(checkbox.find('input').prop<string>('checked')).toBe(false);
+      expect(checkbox.find('input').prop('checked')).toBe(false);
+    });
+  });
+
+  describe('ariaDescribedBy', () => {
+    it('sets the aria-describedBy attribute on the input', () => {
+      const checkBox = mountWithAppProvider(
+        <Checkbox label="checkbox" ariaDescribedBy="SomeId" />,
+      );
+      const ariaDescribedBy = checkBox.find('input').prop('aria-describedby');
+
+      expect(ariaDescribedBy).toBe('SomeId');
     });
   });
 });

src/components/ChoiceList/ChoiceList.tsx

@@ -5,7 +5,8 @@ import {createUniqueIDFactory} from '@shopify/javascript-utilities/other';
 import {withAppProvider, WithAppProviderProps} from '../AppProvider';
 import Checkbox from '../Checkbox';
 import RadioButton from '../RadioButton';
-import InlineError from '../InlineError';
+import InlineError, {errorTextID} from '../InlineError';
+
 import {Error} from '../../types';
 
 import styles from './ChoiceList.scss';
@@ -19,6 +20,8 @@ export interface ChoiceDescriptor {
   disabled?: boolean;
   /** Additional text to aide in use */
   helpText?: React.ReactNode;
+  /** Indicates that the choice is aria-describedBy the error message*/
+  describedByError?: boolean;
   /**  Method to render children with a choice */
   renderChildren?(isSelected: boolean): React.ReactNode | false;
 }
@@ -78,7 +81,13 @@ function ChoiceList({
   ) : null;
 
   const choicesMarkup = choices.map((choice) => {
-    const {value, label, helpText, disabled: choiceDisabled} = choice;
+    const {
+      value,
+      label,
+      helpText,
+      disabled: choiceDisabled,
+      describedByError,
+    } = choice;
 
     function handleChange(checked: boolean) {
       onChange(
@@ -105,6 +114,9 @@ function ChoiceList({
           checked={choiceIsSelected(choice, selected)}
           helpText={helpText}
           onChange={handleChange}
+          ariaDescribedBy={
+            error && describedByError ? errorTextID(finalName) : null
+          }
         />
         {children}
       </li>
@@ -118,12 +130,7 @@ function ChoiceList({
   );
 
   return (
-    <fieldset
-      className={className}
-      id={finalName}
-      aria-invalid={error != null}
-      aria-describedby={`${finalName}Error`}
-    >
+    <fieldset className={className} id={finalName} aria-invalid={error != null}>
       {titleMarkup}
       <ul className={styles.Choices}>{choicesMarkup}</ul>
       {errorMarkup}

src/components/ChoiceList/README.md

@@ -218,6 +218,40 @@ class ChoiceListExample extends React.Component {
 
 <!-- /content-for -->
 
+### Single choice list with error
+
+Allows for accessible error handling by connecting the error message to the field with the error.
+
+```jsx
+class ChoiceListExample extends React.Component {
+  state = {
+    selected: ['hidden'],
+  };
+
+  render() {
+    const {selected} = this.state;
+
+    return (
+      <ChoiceList
+        title="Company name"
+        choices={[
+          {label: 'Hidden', value: 'hidden', describedByError: true},
+          {label: 'Optional', value: 'optional'},
+          {label: 'Required', value: 'required'},
+        ]}
+        selected={selected}
+        onChange={this.handleChange}
+        error="Company name cannot be hidden at this time"
+      />
+    );
+  }
+
+  handleChange = (value) => {
+    this.setState({selected: value});
+  };
+}
+```
+
 ### Multi-choice list
 
 Allows merchants to select multiple options from a list.

src/components/ChoiceList/tests/ChoiceList.test.tsx

@@ -1,17 +1,12 @@
 import * as React from 'react';
 import {ReactWrapper} from 'enzyme';
 import {shallowWithAppProvider, mountWithAppProvider} from 'test-utilities';
-import {RadioButton, Checkbox, InlineError} from 'components';
-import ChoiceList from '../ChoiceList';
+import {RadioButton, Checkbox, InlineError, errorTextID} from 'components';
+
+import ChoiceList, {ChoiceDescriptor} from '../ChoiceList';
 
 describe('<ChoiceList />', () => {
-  let choices: ({
-    label: string;
-    value: string;
-    helpText?: React.ReactNode;
-    disabled?: boolean;
-    renderChildren?(): React.ReactNode;
-  })[];
+  let choices: ChoiceDescriptor[];
 
   beforeEach(() => {
     choices = [
@@ -332,34 +327,59 @@ describe('<ChoiceList />', () => {
   });
 
   describe('error', () => {
+    beforeEach(() => {
+      choices = [
+        ...choices,
+        {label: 'Choice with error', value: 'Four', describedByError: true},
+      ];
+    });
+
     it('marks the fieldset as invalid', () => {
       const element = mountWithAppProvider(
         <ChoiceList selected={[]} choices={choices} error="Error message" />,
       );
-
       expect(element.find('fieldset').prop<string>('aria-invalid')).toBe(true);
     });
 
-    it('connects the fieldset to the error', () => {
+    it('renders an InlineError markup when truthy', () => {
       const element = mountWithAppProvider(
         <ChoiceList selected={[]} choices={choices} error="Error message" />,
       );
 
-      const errorID = element.find('fieldset').prop<string>('aria-describedby');
-      expect(typeof errorID).toBe('string');
-      expect(element.find(`#${errorID}`).text()).toBe('Error message');
+      const error = element.find(InlineError);
+      expect(error.prop('message')).toBe('Error message');
     });
 
-    it('renders error markup when truthy', () => {
+    it("connects the InlineError to the choice, with the describedByError key's, ariaDescribedBy prop", () => {
       const element = mountWithAppProvider(
         <ChoiceList selected={[]} choices={choices} error="Error message" />,
       );
 
-      const error = element.find(InlineError);
-      expect(error.prop('message')).toBe('Error message');
+      const fieldId = element.find(InlineError).prop('fieldID');
+      const expectedErrorFieldId = errorTextID(fieldId);
+
+      const radioButtonDescribeBy = element
+        .find(RadioButton)
+        .last()
+        .prop('ariaDescribedBy');
+
+      expect(radioButtonDescribeBy).toBe(expectedErrorFieldId);
+    });
+
+    it('does not provide the choice, with the describedByError key, with ariaDescribedBy prop if no error is provided', () => {
+      const element = mountWithAppProvider(
+        <ChoiceList selected={[]} choices={choices} />,
+      );
+
+      const radioButtonDescribeBy = element
+        .find(RadioButton)
+        .last()
+        .prop('ariaDescribedBy');
+
+      expect(radioButtonDescribeBy).toBeNull();
     });
 
-    it('renders no error markup when falsy', () => {
+    it('does not render an InlineError when falsy', () => {
       const element = mountWithAppProvider(
         <ChoiceList selected={[]} choices={choices} error="" />,
       );

src/components/InlineError/InlineError.tsx

@@ -19,11 +19,15 @@ export default function InlineError({message, fieldID}: Props) {
   }
 
   return (
-    <div id={`${fieldID}Error`} className={styles.InlineError}>
+    <div id={errorTextID(fieldID)} className={styles.InlineError}>
       <div className={styles.Icon}>
         <Icon source={AlertMinor} />
       </div>
       {message}
     </div>
   );
 }
+
+export function errorTextID(id: string) {
+  return `${id}Error`;
+}

src/components/InlineError/index.ts

@@ -1,4 +1,4 @@
 import InlineError from './InlineError';
 
-export {Props} from './InlineError';
+export {Props, errorTextID} from './InlineError';
 export default InlineError;

src/components/InlineError/tests/InlineError.test.tsx

@@ -1,15 +1,18 @@
 import * as React from 'react';
 import {mountWithAppProvider} from 'test-utilities';
-import InlineError from '../InlineError';
+import InlineError, {errorTextID} from '../InlineError';
 
 describe('<InlineError />', () => {
   describe('fieldID', () => {
     it('renders with an ID generated from the fieldID', () => {
+      const fieldId = 'ProductTitle';
+      const expectedId = `#${errorTextID(fieldId)}`;
+
       const error = mountWithAppProvider(
-        <InlineError message="Title can’t be blank" fieldID="ProductTitle" />,
+        <InlineError message="Title can’t be blank" fieldID={fieldId} />,
       );
 
-      expect(error.find('#ProductTitleError')).toHaveLength(1);
+      expect(error.find(expectedId)).toHaveLength(1);
     });
   });