fix: selectable box set props forwarding (#2461)

BREAKING CHANGE: aria-label is required when not using aria-labelledby in the SelectableBoxSet component. An associated label for the component is a WCAG requirement. Co-authored-by: Maxwell Frank <[email protected]>
openedx · Aug 15, 2023 · 1d6f95e · 1d6f95e
1 parent d9ccbbc
commit 1d6f95e
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 2 deletions.
diff --git a/src/SelectableBox/README.md b/src/SelectableBox/README.md
@@ -42,6 +42,7 @@ As ``Checkbox``
         onChange={handleChange}
         name="cheeses"
         columns={isExtraSmall ? 1 : 2}
+        ariaLabel="cheese selection"
       >
         <SelectableBox value="swiss" type={type} aria-label="swiss checkbox">
           <div>
@@ -83,6 +84,7 @@ As ``Checkbox``
       onChange={handleChange}
       name="colors"
       columns={isExtraSmall ? 1 : 3}
+      ariaLabel="color selection"
     >
       <SelectableBox value="red" type={type} aria-label="red checkbox">
         <div>
@@ -144,6 +146,7 @@ As ``Checkbox`` with ``isIndeterminate``
         onChange={handleChange}
         name="cheeses"
         columns={isExtraSmall ? 1 : 3}
+        ariaLabel="cheese selection"
       >
         <SelectableBox value="swiss" type={type} aria-label="swiss checkbox">
           <div>
@@ -162,3 +165,51 @@ As ``Checkbox`` with ``isIndeterminate``
   );
 }
 ```
+
+As ``Checkbox`` with ``ariaLabelledby``
+
+```jsx live
+() => {
+  const type = 'checkbox';
+  const allCheeseOptions = ['swiss', 'cheddar', 'pepperjack'];
+  const [checkedCheeses, { add, remove, set, clear }] = useCheckboxSetValues(['swiss']);
+
+  const handleChange = e => {
+    e.target.checked ? add(e.target.value) : remove(e.target.value);
+  };
+
+  const isExtraSmall = useMediaQuery({ maxWidth: breakpoints.extraSmall.maxWidth });
+
+  return (
+    <div className="bg-light-200 p-3">
+      <h3 id="cheese selection" className="mb-4">
+        Select your favorite cheese
+      </h3>
+      <SelectableBox.Set
+        value={checkedCheeses}
+        type={type}
+        onChange={handleChange}
+        name="cheeses"
+        columns={isExtraSmall ? 1 : 3}
+        ariaLabelledby="cheese selection"
+      >
+        <SelectableBox value="swiss" inputHidden={false} type={type} aria-label="swiss checkbox">
+          <h3>
+            Swiss
+          </h3>
+        </SelectableBox>
+        <SelectableBox value="cheddar" inputHidden={false} type={type} aria-label="cheddar checkbox">
+          <h3>
+            Cheddar
+          </h3>
+        </SelectableBox>
+        <SelectableBox value="pepperjack" inputHidden={false} type={type} aria-label="pepperjack checkbox">
+          <h3>
+            Pepperjack
+          </h3>
+        </SelectableBox>
+      </SelectableBox.Set>
+    </div>
+  );
+}
+```
diff --git a/src/SelectableBox/SelectableBoxSet.jsx b/src/SelectableBox/SelectableBoxSet.jsx
@@ -2,6 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
 import { getInputType } from './utils';
+import { requiredWhenNot } from '../utils/propTypes';
 
 const INPUT_TYPES = [
   'radio',
@@ -19,6 +20,9 @@ const SelectableBoxSet = React.forwardRef(({
   type,
   columns,
   className,
+  ariaLabel,
+  ariaLabelledby,
+  ...props
 }, ref) => {
   const inputType = getInputType('SelectableBoxSet', type);
 
@@ -35,6 +39,9 @@ const SelectableBoxSet = React.forwardRef(({
         `pgn__selectable_box-set--${columns || DEFAULT_COLUMNS_NUMBER}`,
         className,
       ),
+      'aria-label': ariaLabel,
+      'aria-labelledby': ariaLabelledby,
+      ...props,
     },
     children,
   );
@@ -62,6 +69,17 @@ SelectableBoxSet.propTypes = {
   columns: PropTypes.number,
   /** A class that is be appended to the base element. */
   className: PropTypes.string,
+  /**
+   * The ID of the label for the `SelectableBoxSet`.
+   *
+   * An accessible label must be provided to the `SelectableBoxSet`.
+   */
+  ariaLabelledby: PropTypes.string,
+  /**
+   * A label for the `SelectableBoxSet`.
+   *
+   * If not using `ariaLabelledby`, then `ariaLabel` must be provided */
+  ariaLabel: requiredWhenNot(PropTypes.string, 'ariaLabelledby'),
 };
 
 SelectableBoxSet.defaultProps = {
@@ -72,6 +90,8 @@ SelectableBoxSet.defaultProps = {
   type: 'radio',
   columns: DEFAULT_COLUMNS_NUMBER,
   className: undefined,
+  ariaLabelledby: undefined,
+  ariaLabel: undefined,
 };
 
 export default SelectableBoxSet;
diff --git a/src/SelectableBox/tests/SelectableBoxSet.test.jsx b/src/SelectableBox/tests/SelectableBoxSet.test.jsx
@@ -1,6 +1,8 @@
 import React from 'react';
 import { mount } from 'enzyme';
 import renderer from 'react-test-renderer';
+import { render, screen } from '@testing-library/react';
+import '@testing-library/jest-dom/extend-expect';
 
 import { Form } from '../..';
 import SelectableBox from '..';
@@ -11,9 +13,11 @@ const checkboxText = (text) => `SelectableCheckbox${text}`;
 const radioType = 'radio';
 const radioText = (text) => `SelectableRadio${text}`;
 
+const ariaLabel = 'test-default-label';
+
 function SelectableCheckboxSet(props) {
   return (
-    <SelectableBox.Set name={radioType} type={checkboxType} {...props}>
+    <SelectableBox.Set name={radioType} type={checkboxType} ariaLabel={ariaLabel} {...props}>
       <SelectableBox value={1} type={checkboxType}>{checkboxText(1)}</SelectableBox>
       <SelectableBox value={2} type={checkboxType}>{checkboxText(2)}</SelectableBox>
       <SelectableBox value={3} type={checkboxType}>{checkboxText(3)}</SelectableBox>
@@ -23,7 +27,7 @@ function SelectableCheckboxSet(props) {
 
 function SelectableRadioSet(props) {
   return (
-    <SelectableBox.Set name={radioType} type={radioType} {...props}>
+    <SelectableBox.Set name={radioType} type={radioType} ariaLabel={ariaLabel} {...props}>
       <SelectableBox value={1} type={radioType}>{radioText(1)}</SelectableBox>
       <SelectableBox value={2} type={radioType}>{radioText(2)}</SelectableBox>
       <SelectableBox value={3} type={radioType}>{radioText(3)}</SelectableBox>
@@ -37,6 +41,10 @@ describe('<SelectableBox.Set />', () => {
       const tree = renderer.create((<SelectableRadioSet name="testName" />)).toJSON();
       expect(tree).toMatchSnapshot();
     });
+    it('forwards props', () => {
+      render((<SelectableRadioSet name="testName" data-testid="test-radio-set-name" />));
+      expect(screen.getByTestId('test-radio-set-name')).toBeInTheDocument();
+    });
     it('correct render when type prop is changed', () => {
       const setWrapper = mount(<SelectableRadioSet name="set" />);
       expect(setWrapper.find(Form.RadioSet).length).toBeGreaterThan(0);
@@ -84,5 +92,21 @@ describe('<SelectableBox.Set />', () => {
       const selectableBoxSet = wrapper.find(Form.RadioSet);
       expect(selectableBoxSet.hasClass(`pgn__selectable_box-set--${columns}`)).toBe(true);
     });
+    it('renders with an aria-label attribute', () => {
+      render((<SelectableRadioSet name="testName" ariaLabel="test-radio-set-label" />));
+      expect(screen.getByLabelText('test-radio-set-label')).toBeInTheDocument();
+    });
+    it('renders with an aria-labelledby attribute', () => {
+      render((
+        <>
+          <h2 id="test-radio-set-label">Radio Set Label text</h2>
+          <SelectableRadioSet
+            name="testName"
+            ariaLabelledby="test-radio-set-label"
+          />
+        </>
+      ));
+      expect(screen.getByLabelText('Radio Set Label text')).toBeInTheDocument();
+    });
   });
 });
diff --git a/src/SelectableBox/tests/__snapshots__/SelectableBoxSet.test.jsx.snap b/src/SelectableBox/tests/__snapshots__/SelectableBoxSet.test.jsx.snap
@@ -2,6 +2,7 @@
 
 exports[`<SelectableBox.Set /> correct rendering renders without props 1`] = `
 <div
+  aria-label="test-default-label"
   className="pgn__selectable_box-set pgn__selectable_box-set--2 pgn__form-control-set"
   role="radiogroup"
 >