SelectPanel: Accessibility remediation (#2138)

* Change deprecated/ActionList semantics * Remove item selection via Enter key * Handle focus on deprecated/ActionList * Adapt deprecated/ActionMenu prop * Update deprecated/ActionList tests and stories * Update deprecated/ActionList docs * Remove focusZone from the FilteredActionList component * Add sr-only description for input in the FilteredActionList component * Adapt SelectPanel: semantics, visual layout, add title, close button, save/cancel button and functionality. Update tests, docs, and stories. * Update SelectPanel test snap * Update deprecated/ActionList axe test * Update deprecated/ActionList axe test * Delete commented code * Update stories, Add ESC reset functionality * Add changeset (breaking change) * Preselect items on the SelectPanel docs that aren't the first ones * Use Heading instead of h1 on SelectPanel * Don't use selected items as the SelectPanel anchor label * Replace custom SrOnly with VisuallyHidden * Make the SelectPanel example dimensions larger * Support setting the selected items asyncronously * Fix new select panel issues (#2147) * Fix onClose being called with event instead of gesture Co-authored-by: Hector Garcia <hectahertz@github.com> * Remove unsupported PageUpDown bindkeys from useFocusZone * Remove console.debug * Make `title` and `inputLabel` optional with defaults * Remove unused prop * Add a placeholder prop for the search input * Make footer buttons smaller * Use `ButtonClose` * Better default title for multiselect `SelectPanel` * Fix lint warnings * Fix the AutocompleteMenu tests * Fix lint error * Fix MarkdownEditor tests, remove one for old behavior Co-authored-by: Hector Garcia <hectahertz@github.com> Co-authored-by: Cameron McHenry <camchenry@users.noreply.github.com>
primer · Aug 25, 2022 · ace38af · ace38af
1 parent 7b8bc81
commit ace38af
Show file tree

Hide file tree

Showing 20 changed files with 2,710 additions and 2,548 deletions.
diff --git a/.changeset/grumpy-carrots-admire.md b/.changeset/grumpy-carrots-admire.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+Accessibility fixes for SelectPanel.
diff --git a/docs/content/SelectPanel.mdx b/docs/content/SelectPanel.mdx
@@ -37,27 +37,28 @@ const items = [
 ]
 
 function DemoComponent() {
-  const [selected, setSelected] = React.useState([items[0], items[1]])
+  const [selected, setSelected] = React.useState([items[2], items[4]])
   const [filter, setFilter] = React.useState('')
   const filteredItems = items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))
   const [open, setOpen] = React.useState(false)
 
   return (
     <SelectPanel
-      renderAnchor={({children, 'aria-labelledby': ariaLabelledBy, ...anchorProps}) => (
-        <Button trailingIcon={TriangleDownIcon} aria-labelledby={` ${ariaLabelledBy}`} {...anchorProps}>
-          {children || 'Select Labels'}
+      renderAnchor={({...anchorProps}) => (
+        <Button trailingIcon={TriangleDownIcon} {...anchorProps}>
+          Select Labels
         </Button>
       )}
-      placeholderText="Filter Labels"
+      title="Select Items"
+      inputLabel="Select Items"
       open={open}
       onOpenChange={setOpen}
       items={filteredItems}
       selected={selected}
       onSelectedChange={setSelected}
       onFilterChange={setFilter}
       showItemDividers={true}
-      overlayProps={{width: 'small', height: 'xsmall'}}
+      overlayProps={{width: 'medium', height: 'medium'}}
     />
   )
 }

diff --git a/docs/content/deprecated/ActionList.mdx b/docs/content/deprecated/ActionList.mdx
@@ -14,25 +14,26 @@ Use [new version of ActionList](/ActionList) with composable API, design updates
 
 ```jsx
 <ActionList
+  role="listbox"
   items={[
-    {text: 'New file'},
-    {text: 'Copy link'},
-    {text: 'Edit file'},
+    {text: 'New file', role: 'option'},
+    {text: 'Copy link', role: 'option'},
+    {text: 'Edit file', role: 'option'},
     ActionList.Divider,
-    {text: 'Delete file', variant: 'danger'}
+    {text: 'Delete file', variant: 'danger', role: 'option'}
   ]}
 />
 ```
 
 **After**
 
 ```jsx
-<ActionList>
-  <ActionList.Item>New file</ActionList.Item>
-  <ActionList.Item>Copy link</ActionList.Item>
-  <ActionList.Item>Edit file</ActionList.Item>
+<ActionList role="listbox">
+  <ActionList.Item role="option">New file</ActionList.Item>
+  <ActionList.Item role="option">Copy link</ActionList.Item>
+  <ActionList.Item role="option">Edit file</ActionList.Item>
   <ActionList.Divider />
-  <ActionList.Item variant="danger">Delete file</ActionList.Item>
+  <ActionList.Item variant="danger" role="option">Delete file</ActionList.Item>
 </ActionList>
 ```
 
@@ -46,12 +47,13 @@ import {ActionList} from '@primer/react/deprecated'
 
 ```jsx live deprecated
 <ActionList
+  role="listbox"
   items={[
-    {text: 'New file'},
+    {text: 'New file', role: 'option'},
     ActionList.Divider,
-    {text: 'Copy link'},
-    {text: 'Edit file'},
-    {text: 'Delete file', variant: 'danger'}
+    {text: 'Copy link', role: 'option'},
+    {text: 'Edit file', role: 'option'},
+    {text: 'Delete file', variant: 'danger', role: 'option'}
   ]}
 />
 ```
@@ -60,6 +62,7 @@ import {ActionList} from '@primer/react/deprecated'
 
 ```jsx live deprecated
 <ActionList
+  role="listbox"
   groupMetadata={[
     {groupId: '0'},
     {groupId: '1', header: {title: 'Live query', variant: 'subtle'}},
@@ -68,34 +71,37 @@ import {ActionList} from '@primer/react/deprecated'
     {groupId: '4'}
   ]}
   items={[
-    {key: '1', leadingVisual: TypographyIcon, text: 'Rename', groupId: '0', trailingVisual: '⌘R'},
-    {key: '2', leadingVisual: VersionsIcon, text: 'Duplicate', groupId: '0'},
-    {key: '3', leadingVisual: SearchIcon, text: 'repo:github/github', groupId: '1'},
+    {key: '1', leadingVisual: TypographyIcon, text: 'Rename', groupId: '0', trailingVisual: '⌘R', role: 'option'},
+    {key: '2', leadingVisual: VersionsIcon, text: 'Duplicate', groupId: '0', role: 'option'},
+    {key: '3', leadingVisual: SearchIcon, text: 'repo:github/github', groupId: '1', role: 'option'},
     {
       key: '4',
       leadingVisual: NoteIcon,
       text: 'Table',
       description: 'Information-dense table optimized for operations across teams',
       descriptionVariant: 'block',
-      groupId: '2'
+      groupId: '2',
+      role: 'option'
     },
     {
       key: '5',
       leadingVisual: ProjectIcon,
       text: 'Board',
       description: 'Kanban-style board focused on visual states',
       descriptionVariant: 'block',
-      groupId: '2'
+      groupId: '2',
+      role: 'option'
     },
     {
       key: '6',
       leadingVisual: FilterIcon,
       text: 'Save sort and filters to current view',
       disabled: true,
-      groupId: '3'
+      groupId: '3',
+      role: 'option'
     },
-    {key: '7', leadingVisual: FilterIcon, text: 'Save sort and filters to new view', groupId: '3'},
-    {key: '8', leadingVisual: GearIcon, text: 'View settings', groupId: '4', trailingVisual: ArrowRightIcon}
+    {key: '7', leadingVisual: FilterIcon, text: 'Save sort and filters to new view', groupId: '3', role: 'option'},
+    {key: '8', leadingVisual: GearIcon, text: 'View settings', groupId: '4', trailingVisual: ArrowRightIcon, role: 'option'}
   ]}
 />
 ```
@@ -104,17 +110,21 @@ import {ActionList} from '@primer/react/deprecated'
 
 ```jsx deprecated
 <ActionList
+  role="listbox"
   items={[
     {
       text: 'Vanilla link',
+      role: 'option',
       renderItem: props => <ActionList.Item as="a" href="/about" {...props} />
     },
     {
       text: 'React Router link',
+      role: 'option',
       renderItem: props => <ActionList.Item as={ReactRouterLikeLink} to="/about" {...props} />
     },
     {
       text: 'NextJS style',
+      role: 'option',
       renderItem: props => (
         <NextJSLikeLink href="/about">
           <ActionList.Item as="a" {...props} />

diff --git a/src/Autocomplete/AutocompleteMenu.tsx b/src/Autocomplete/AutocompleteMenu.tsx
@@ -151,6 +151,7 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
       items.map(selectableItem => {
         return {
           ...selectableItem,
+          _legacyEnterSupport: true, //TODO: Change behaviour, the enter key should not be used here.
           role: 'option',
           id: selectableItem.id,
           selected: selectionVariant === 'multiple' ? selectedItemIds.includes(selectableItem.id) : undefined,
@@ -217,6 +218,7 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
         ? [
             {
               ...addNewItem,
+              _legacyEnterSupport: true, //TODO: Change behaviour, the enter key should not be used here.
               leadingVisual: () => <PlusIcon />,
               onAction: (item: T) => {
                 // TODO: make it possible to pass a leadingVisual when using `addNewItem`

diff --git a/src/FilteredActionList/FilteredActionList.tsx b/src/FilteredActionList/FilteredActionList.tsx
@@ -5,7 +5,6 @@ import TextInput, {TextInputProps} from '../TextInput'
 import Box from '../Box'
 import {ActionList} from '../deprecated/ActionList'
 import Spinner from '../Spinner'
-import {useFocusZone} from '../hooks/useFocusZone'
 import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
 import styled from 'styled-components'
 import {get} from '../constants'
@@ -14,6 +13,7 @@ import useScrollFlash from '../hooks/useScrollFlash'
 import {scrollIntoView} from '@primer/behaviors'
 import type {ScrollIntoViewOptions} from '@primer/behaviors'
 import {SxProp} from '../sx'
+import VisuallyHidden from '../_VisuallyHidden'
 
 const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
 
@@ -22,7 +22,7 @@ export interface FilteredActionListProps
     ListPropsBase,
     SxProp {
   loading?: boolean
-  placeholderText: string
+  placeholderText?: string
   filterValue?: string
   onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
   textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
@@ -56,10 +56,10 @@ export function FilteredActionList({
   )
 
   const scrollContainerRef = useRef<HTMLDivElement>(null)
-  const listContainerRef = useRef<HTMLDivElement>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
   const activeDescendantRef = useRef<HTMLElement>()
   const listId = useSSRSafeId()
+  const inputDescriptionTextId = useSSRSafeId()
   const onInputKeyPress: KeyboardEventHandler = useCallback(
     event => {
       if (event.key === 'Enter' && activeDescendantRef.current) {
@@ -74,28 +74,6 @@ export function FilteredActionList({
     [activeDescendantRef]
   )
 
-  useFocusZone(
-    {
-      containerRef: listContainerRef,
-      focusOutBehavior: 'wrap',
-      focusableElementFilter: element => {
-        return !(element instanceof HTMLInputElement)
-      },
-      activeDescendantFocus: inputRef,
-      onActiveDescendantChanged: (current, previous, directlyActivated) => {
-        activeDescendantRef.current = current
-
-        if (current && scrollContainerRef.current && directlyActivated) {
-          scrollIntoView(current, scrollContainerRef.current, menuScrollMargins)
-        }
-      }
-    },
-    [
-      // List ref isn't set while loading.  Need to re-bind focus zone when it changes
-      loading
-    ]
-  )
-
   useEffect(() => {
     // if items changed, we want to instantly move active descendant into view
     if (activeDescendantRef.current && scrollContainerRef.current) {
@@ -119,16 +97,18 @@ export function FilteredActionList({
           placeholder={placeholderText}
           aria-label={placeholderText}
           aria-controls={listId}
+          aria-describedby={inputDescriptionTextId}
           {...textInputProps}
         />
+        <VisuallyHidden id={inputDescriptionTextId}>Items will be filtered as you type</VisuallyHidden>
       </StyledHeader>
       <Box ref={scrollContainerRef} overflow="auto">
         {loading ? (
           <Box width="100%" display="flex" flexDirection="row" justifyContent="center" pt={6} pb={7}>
             <Spinner />
           </Box>
         ) : (
-          <ActionList ref={listContainerRef} items={items} {...listProps} role="listbox" id={listId} />
+          <ActionList items={items} {...listProps} role="listbox" id={listId} />
         )}
       </Box>
     </Box>