Refactor SelectNext into [role=combobox] to improve a11y

hypothesis · Oct 23, 2023 · 7c89d48 · 7c89d48
1 parent 795843c
commit 7c89d48
Show file tree

Hide file tree

Showing 4 changed files with 174 additions and 109 deletions.
diff --git a/src/components/input/SelectNext.tsx b/src/components/input/SelectNext.tsx
@@ -15,8 +15,8 @@ import { useFocusAway } from '../../hooks/use-focus-away';
 import { useKeyPress } from '../../hooks/use-key-press';
 import { useSyncedRef } from '../../hooks/use-synced-ref';
 import type { PresentationalProps } from '../../types';
+import { downcastRef } from '../../util/typing';
 import { MenuCollapseIcon, MenuExpandIcon } from '../icons';
-import Button from './Button';
 import { inputGroupStyles } from './InputGroup';
 import SelectContext from './SelectContext';
 
@@ -149,6 +149,9 @@ export type SelectProps<T> = PresentationalProps & {
    */
   buttonId?: string;
 
+  'aria-label'?: string;
+  'aria-labelledby'?: string;
+
   /** @deprecated Use buttonContent instead */
   label?: ComponentChildren;
 };
@@ -163,6 +166,8 @@ function SelectMain<T>({
   elementRef,
   classes,
   buttonId,
+  'aria-label': ariaLabel,
+  'aria-labelledby': ariaLabelledBy,
 }: SelectProps<T>) {
   const [listboxOpen, setListboxOpen] = useState(false);
   const closeListbox = useCallback(() => setListboxOpen(false), []);
@@ -212,11 +217,11 @@ function SelectMain<T>({
       className={classnames('relative w-full border rounded', inputGroupStyles)}
       ref={wrapperRef}
     >
-      <Button
+      <button
         id={buttonId ?? defaultButtonId}
-        variant="custom"
-        classes={classnames(
-          'w-full flex justify-between',
+        className={classnames(
+          'focus-visible-ring transition-colors whitespace-nowrap',
+          'w-full flex items-center justify-between gap-x-2 p-2',
           'bg-grey-0 disabled:bg-grey-1 disabled:text-grey-6',
           // Add inherited rounded corners so that the toggle is consistent with
           // the wrapper, which is the element rendering borders.
@@ -225,11 +230,15 @@ function SelectMain<T>({
           'rounded-[inherit]',
           classes,
         )}
-        expanded={listboxOpen}
+        type="button"
+        role="combobox"
         disabled={disabled}
+        aria-expanded={listboxOpen}
         aria-haspopup="listbox"
         aria-controls={listboxId}
-        elementRef={buttonRef}
+        aria-label={ariaLabel}
+        aria-labelledby={ariaLabelledBy}
+        ref={downcastRef(buttonRef)}
         onClick={() => setListboxOpen(prev => !prev)}
         onKeyDown={e => {
           if (e.key === 'ArrowDown' && !listboxOpen) {
@@ -243,7 +252,7 @@ function SelectMain<T>({
         <div className="text-grey-6">
           {listboxOpen ? <MenuCollapseIcon /> : <MenuExpandIcon />}
         </div>
-      </Button>
+      </button>
       <SelectContext.Provider value={{ selectValue, value }}>
         <ul
           className={classnames(

diff --git a/src/components/input/test/SelectNext-test.js b/src/components/input/test/SelectNext-test.js
@@ -289,15 +289,15 @@ describe('SelectNext', () => {
         name: 'Closed Select listbox',
         content: () =>
           createComponent(
-            { buttonContent: 'Select' },
+            { buttonContent: 'Select', 'aria-label': 'Select' },
             { optionsChildrenAsCallback: false },
           ),
       },
       {
         name: 'Open Select listbox',
         content: () => {
           const wrapper = createComponent(
-            { buttonContent: 'Select' },
+            { buttonContent: 'Select', 'aria-label': 'Select' },
             { optionsChildrenAsCallback: false },
           );
           toggleListbox(wrapper);

diff --git a/src/pattern-library/components/Library.tsx b/src/pattern-library/components/Library.tsx
@@ -171,7 +171,7 @@ export type LibraryDemoProps = {
   classes?: string | string[];
   /** Inline styles to apply to the demo container */
   style?: JSX.CSSProperties;
-  title?: string;
+  title?: ComponentChildren;
   /**
    * Should the demo also render the source? When true, a "Source" tab will be
    * rendered, which will display the JSX source of the Demo's children.

diff --git a/src/pattern-library/components/patterns/prototype/SelectNextPage.tsx b/src/pattern-library/components/patterns/prototype/SelectNextPage.tsx
@@ -1,7 +1,8 @@
 import classnames from 'classnames';
-import { useCallback, useMemo, useState } from 'preact/hooks';
+import { useCallback, useId, useMemo, useState } from 'preact/hooks';
 
 import { ArrowLeftIcon, ArrowRightIcon } from '../../../../components/icons';
+import type { SelectNextProps } from '../../../../components/input';
 import { IconButton, InputGroup } from '../../../../components/input';
 import SelectNext from '../../../../components/input/SelectNext';
 import Library from '../../Library';
@@ -23,65 +24,77 @@ const defaultItems: ItemType[] = [
 function SelectExample({
   disabled,
   textOnly,
-  classes,
   items = defaultItems,
-}: {
-  disabled?: boolean;
+  ...rest
+}: Pick<
+  SelectNextProps<ItemType>,
+  'aria-label' | 'aria-labelledby' | 'classes' | 'disabled'
+> & {
   textOnly?: boolean;
-  classes?: string;
-  items?: typeof defaultItems;
+  items?: ItemType[];
 }) {
   const [value, setValue] = useState<ItemType>();
+  const buttonId = useId();
 
   return (
-    <SelectNext
-      value={value}
-      onChange={setValue}
-      classes={classes}
-      disabled={disabled}
-      buttonContent={
-        value ? (
-          <>
-            {textOnly && value.name}
-            {!textOnly && (
-              <div className="flex">
-                <div className="truncate">{value.name}</div>
-                <div className="rounded px-2 ml-2 bg-grey-7 text-white">
-                  {value.id}
-                </div>
-              </div>
-            )}
-          </>
-        ) : disabled ? (
-          <>This is disabled</>
-        ) : (
-          <>Select one...</>
-        )
-      }
-    >
-      {items.map(item => (
-        <SelectNext.Option value={item} key={item.id} disabled={item.disabled}>
-          {({ disabled }) =>
-            textOnly ? (
-              item.name
-            ) : (
-              <>
-                {item.name}
-                <div className="grow" />
-                <div
-                  className={classnames('rounded px-2 ml-2 text-white', {
-                    'bg-grey-7': !disabled,
-                    'bg-grey-4': disabled,
-                  })}
-                >
-                  {item.id}
+    <>
+      {!rest['aria-label'] && !rest['aria-labelledby'] && (
+        <label htmlFor={buttonId}>Select a person</label>
+      )}
+      <SelectNext
+        {...rest}
+        buttonId={buttonId}
+        value={value}
+        onChange={setValue}
+        disabled={disabled}
+        buttonContent={
+          value ? (
+            <>
+              {textOnly && value.name}
+              {!textOnly && (
+                <div className="flex">
+                  <div className="truncate">{value.name}</div>
+                  <div className="rounded px-2 ml-2 bg-grey-7 text-white">
+                    {value.id}
+                  </div>
                 </div>
-              </>
-            )
-          }
-        </SelectNext.Option>
-      ))}
-    </SelectNext>
+              )}
+            </>
+          ) : disabled ? (
+            <>This is disabled</>
+          ) : (
+            <>Select one…</>
+          )
+        }
+      >
+        {items.map(item => (
+          <SelectNext.Option
+            value={item}
+            key={item.id}
+            disabled={item.disabled}
+          >
+            {({ disabled }) =>
+              textOnly ? (
+                item.name
+              ) : (
+                <>
+                  {item.name}
+                  <div className="grow" />
+                  <div
+                    className={classnames('rounded px-2 ml-2 text-white', {
+                      'bg-grey-7': !disabled,
+                      'bg-grey-4': disabled,
+                    })}
+                  >
+                    {item.id}
+                  </div>
+                </>
+              )
+            }
+          </SelectNext.Option>
+        ))}
+      </SelectNext>
+    </>
   );
 }
 
@@ -99,53 +112,58 @@ function InputGroupSelectExample({ classes }: { classes?: string }) {
     const newIndex = selectedIndex - 1;
     setSelected(defaultItems[newIndex] ?? selected);
   }, [selected, selectedIndex]);
+  const buttonId = useId();
 
   return (
-    <InputGroup>
-      <IconButton
-        icon={ArrowLeftIcon}
-        title="Previous student"
-        variant="dark"
-        onClick={previous}
-        disabled={selectedIndex <= 0}
-      />
-      <SelectNext
-        value={selected}
-        onChange={setSelected}
-        classes={classes}
-        buttonContent={
-          selected ? (
-            <div className="flex">
-              <div className="truncate">{selected.name}</div>
-              <div className="rounded px-2 ml-2 bg-grey-7 text-white">
-                {selected.id}
+    <>
+      <label htmlFor={buttonId}>Select a person</label>
+      <InputGroup>
+        <IconButton
+          icon={ArrowLeftIcon}
+          title="Previous student"
+          variant="dark"
+          onClick={previous}
+          disabled={selectedIndex <= 0}
+        />
+        <SelectNext
+          buttonId={buttonId}
+          value={selected}
+          onChange={setSelected}
+          classes={classes}
+          buttonContent={
+            selected ? (
+              <div className="flex">
+                <div className="truncate">{selected.name}</div>
+                <div className="rounded px-2 ml-2 bg-grey-7 text-white">
+                  {selected.id}
+                </div>
               </div>
-            </div>
-          ) : (
-            <>Select one...</>
-          )
-        }
-      >
-        {defaultItems.map(item => (
-          <SelectNext.Option value={item} key={item.id}>
-            {item.name}
-            <div className="grow" />
-            <div
-              className={classnames('rounded px-2 ml-2 text-white bg-grey-7')}
-            >
-              {item.id}
-            </div>
-          </SelectNext.Option>
-        ))}
-      </SelectNext>
-      <IconButton
-        icon={ArrowRightIcon}
-        title="Next student"
-        variant="dark"
-        onClick={next}
-        disabled={selectedIndex >= defaultItems.length - 1}
-      />
-    </InputGroup>
+            ) : (
+              <>Select one…</>
+            )
+          }
+        >
+          {defaultItems.map(item => (
+            <SelectNext.Option value={item} key={item.id}>
+              {item.name}
+              <div className="grow" />
+              <div
+                className={classnames('rounded px-2 ml-2 text-white bg-grey-7')}
+              >
+                {item.id}
+              </div>
+            </SelectNext.Option>
+          ))}
+        </SelectNext>
+        <IconButton
+          icon={ArrowRightIcon}
+          title="Next student"
+          variant="dark"
+          onClick={next}
+          disabled={selectedIndex >= defaultItems.length - 1}
+        />
+      </InputGroup>
+    </>
   );
 }
 
@@ -228,6 +246,44 @@ export default function SelectNextPage() {
             </Library.Demo>
           </Library.Example>
 
+          <Library.Example title="Labelling SelectNext">
+            <p>
+              There are three ways to label a <code>SelectNext</code>. Make sure
+              you always use one of them.
+            </p>
+
+            <Library.Demo
+              title={
+                <>
+                  Via{' '}
+                  <code>
+                    {'<'}label {'/>'}
+                  </code>{' '}
+                  linked to <code>buttonId</code>
+                </>
+              }
+            >
+              <div className="w-96 mx-auto">
+                <SelectExample />
+              </div>
+            </Library.Demo>
+
+            <Library.Demo title="Via aria-label">
+              <div className="w-96 mx-auto">
+                <SelectExample aria-label="Select a person with aria label" />
+              </div>
+            </Library.Demo>
+
+            <Library.Demo title="Via aria-labelledby">
+              <div className="w-96 mx-auto">
+                <p id="select-next-meta-label">
+                  Select a person with aria labelledby
+                </p>
+                <SelectExample aria-labelledby="select-next-meta-label" />
+              </div>
+            </Library.Demo>
+          </Library.Example>
+
           <Library.Example title="Select with long content">
             <p>
               <code>SelectNext</code> makes sure the button content never
@@ -403,7 +459,7 @@ export default function SelectNextPage() {
             </div>
           </>
         ) : (
-          <>Select one...</>
+          <>Select one…</>
         )
       }
     >