Reacreare PR-5801 to remove aria activedescendant and add a roving tab index

Author: pksjce

packages/react/.changeset/four-cars-attend.md

@@ -0,0 +1,10 @@
+---
+'@primer/react': minor
+---
+
+SelectPanel: Remove `aria-activedescendant` from SelectPanel
+
+- Remove aria-activedescendant pattern from modern ActionList implementation (when `primer_react_select_panel_with_modern_action_list` feature flag is enabled)
+- Replace with roving tabindex behavior for better accessibility 
+- Simplify announcement logic since roving tabindex provides focus announcements automatically
+- This change only affects SelectPanel when using the modern ActionList feature flag

packages/react/src/ActionList/ActionListContainerContext.tsx

@@ -6,7 +6,7 @@ import type {AriaRole} from '../utils/types'
 type ContextProps = {
   container?: string
   listRole?: AriaRole
-  selectionVariant?: 'single' | 'multiple'
+  selectionVariant?: 'single' | 'multiple' | 'radio'
   selectionAttribute?: 'aria-selected' | 'aria-checked'
   listLabelledBy?: string
   // This can be any function, we don't know anything about the arguments

packages/react/src/ActionList/Item.tsx

@@ -91,7 +91,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     const inactive = Boolean(inactiveText)
     // TODO change `menuContext` check to ```listRole !== undefined && ['menu', 'listbox'].includes(listRole)```
     // once we have a better way to handle existing usage in dotcom that incorrectly use ActionList.TrailingAction
-    const menuContext = container === 'ActionMenu' || container === 'SelectPanel'
+    const menuContext = container === 'ActionMenu' || container === 'SelectPanel' || container === 'FilteredActionList'
     // TODO: when we change `menuContext` to check `listRole` instead of `container`
     const showInactiveIndicator = inactive && !(listRole !== undefined && ['menu', 'listbox'].includes(listRole))
 
@@ -118,7 +118,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       if (selectionVariant === 'single') inferredItemRole = 'menuitemradio'
       else if (selectionVariant === 'multiple') inferredItemRole = 'menuitemcheckbox'
       else inferredItemRole = 'menuitem'
-    } else if (listRole === 'listbox') {
+    } else if (container && ['SelectPanel', 'FilteredActionList'].includes(container) && listRole === 'listbox') {
       if (selectionVariant !== undefined && !role) inferredItemRole = 'option'
     }
 

packages/react/src/ActionList/List.tsx

@@ -28,6 +28,7 @@ export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
       listLabelledBy,
       selectionVariant: containerSelectionVariant, // TODO: Remove after DropdownMenu2 deprecation
       enableFocusZone: enableFocusZoneFromContainer,
+      container,
     } = React.useContext(ActionListContainerContext)
 
     const ariaLabelledBy = slots.heading ? (slots.heading.props.id ?? headingId) : listLabelledBy
@@ -42,7 +43,7 @@ export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
       disabled: !enableFocusZone,
       containerRef: listRef,
       bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | FocusKeys.PageUpDown,
-      focusOutBehavior: listRole === 'menu' ? 'wrap' : undefined,
+      focusOutBehavior: listRole === 'menu' || container === 'SelectPanel' || container === 'FilteredActionList' ? 'wrap' : undefined,
     })
 
     return (

packages/react/src/FilteredActionList/FilteredActionListEntry.tsx

@@ -3,11 +3,11 @@ import {FilteredActionList as WithDeprecatedActionList} from './FilteredActionLi
 import {FilteredActionList as WithStableActionList} from './FilteredActionListWithModernActionList'
 import {useFeatureFlag} from '../FeatureFlags'
 
-export function FilteredActionList(props: FilteredActionListProps): JSX.Element {
+export function FilteredActionList({onListContainerRefChanged, ...props}: FilteredActionListProps) {
   const enabled = useFeatureFlag('primer_react_select_panel_with_modern_action_list')
 
   if (enabled) return <WithStableActionList {...props} />
-  else return <WithDeprecatedActionList {...props} />
+  else return <WithDeprecatedActionList onListContainerRefChanged={onListContainerRefChanged} {...props} />
 }
 
 FilteredActionList.displayName = 'FilteredActionList'

packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx

@@ -1,5 +1,4 @@
-import type {ScrollIntoViewOptions} from '@primer/behaviors'
-import {scrollIntoView, FocusKeys} from '@primer/behaviors'
+import {FocusKeys} from '@primer/behaviors'
 import type {KeyboardEventHandler} from 'react'
 import type React from 'react'
 import {useCallback, useEffect, useRef, useState} from 'react'
@@ -10,7 +9,6 @@ import TextInput from '../TextInput'
 import {get} from '../constants'
 import {ActionList} from '../ActionList'
 import type {GroupedListProps, ListPropsBase, ItemInput} from '../SelectPanel/types'
-import {useFocusZone} from '../hooks/useFocusZone'
 import {useId} from '../hooks/useId'
 import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
 import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
@@ -20,14 +18,12 @@ import type {SxProp} from '../sx'
 import type {FilteredActionListLoadingType} from './FilteredActionListLoaders'
 import {FilteredActionListLoadingTypes, FilteredActionListBodyLoader} from './FilteredActionListLoaders'
 import classes from './FilteredActionList.module.css'
-
+import {ActionListContainerContext} from '../ActionList/ActionListContainerContext'
 import {isValidElementType} from 'react-is'
 import type {RenderItemFn} from '../deprecated/ActionList/List'
 import {useAnnouncements} from './useAnnouncements'
 import {clsx} from 'clsx'
 
-const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
-
 export interface FilteredActionListProps
   extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
     ListPropsBase,
@@ -37,7 +33,6 @@ export interface FilteredActionListProps
   placeholderText?: string
   filterValue?: string
   onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
-  onListContainerRefChanged?: (ref: HTMLElement | null) => void
   onInputRefChanged?: (ref: React.RefObject<HTMLInputElement>) => void
   textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
   inputRef?: React.RefObject<HTMLInputElement>
@@ -58,7 +53,6 @@ export function FilteredActionList({
   filterValue: externalFilterValue,
   loadingType = FilteredActionListLoadingTypes.bodySpinner,
   onFilterChange,
-  onListContainerRefChanged,
   onInputRefChanged,
   items,
   textInputProps,
@@ -68,6 +62,7 @@ export function FilteredActionList({
   showItemDividers,
   message,
   className,
+  selectionVariant,
   announcementsEnabled = true,
   fullScreenOnNarrow,
   ...listProps
@@ -82,72 +77,66 @@ export function FilteredActionList({
     [onFilterChange, setInternalFilterValue],
   )
 
+  const [enableAnnouncements, setEnableAnnouncements] = useState(false)
+  const [selectedItems, setSelectedItems] = useState<(string | number | undefined)[]>([])
+
   const scrollContainerRef = useRef<HTMLDivElement>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
-  const [listContainerElement, setListContainerElement] = useState<HTMLUListElement | null>(null)
-  const activeDescendantRef = useRef<HTMLElement>()
+  const listRef = useRef<HTMLUListElement>(null)
   const listId = useId()
   const inputDescriptionTextId = useId()
-  const onInputKeyPress: KeyboardEventHandler = useCallback(
-    event => {
-      if (event.key === 'Enter' && activeDescendantRef.current) {
-        event.preventDefault()
-        event.nativeEvent.stopImmediatePropagation()
+  const keydownListener = useCallback(
+    (event: React.KeyboardEvent<HTMLDivElement>) => {
+      if (event.key === 'ArrowDown') {
+        if (listRef.current) {
+          const firstSelectedItem = listRef.current.querySelector('[role="option"]') as HTMLElement | undefined
+          firstSelectedItem?.focus()
 
-        // Forward Enter key press to active descendant so that item gets activated
-        const activeDescendantEvent = new KeyboardEvent(event.type, event.nativeEvent)
-        activeDescendantRef.current.dispatchEvent(activeDescendantEvent)
-      }
-    },
-    [activeDescendantRef],
-  )
+          event.preventDefault()
+        }
+      } else if (event.key === 'Enter') {
+        let firstItem
+        // If there are groups, it's not guaranteed that the first item is the actual first item in the first -
+        // as groups are rendered in the order of the groupId provided
+        if (groupMetadata) {
+          const firstGroup = groupMetadata[0].groupId
+          firstItem = items.filter(item => item.groupId === firstGroup)[0]
+        } else {
+          firstItem = items[0]
+        }
 
-  const listContainerRefCallback = useCallback(
-    (node: HTMLUListElement | null) => {
-      setListContainerElement(node)
-      onListContainerRefChanged?.(node)
+        if (firstItem.onAction) {
+          firstItem.onAction(firstItem, event)
+          event.preventDefault()
+        }
+      }
     },
-    [onListContainerRefChanged],
+    [items, groupMetadata],
   )
 
   useEffect(() => {
     onInputRefChanged?.(inputRef)
   }, [inputRef, onInputRefChanged])
 
-  useFocusZone(
-    {
-      containerRef: {current: listContainerElement},
-      bindKeys: FocusKeys.ArrowVertical | FocusKeys.PageUpDown,
-      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 container isn't in the DOM while loading.  Need to re-bind focus zone when it changes.
-      listContainerElement,
-    ],
-  )
-
   useEffect(() => {
-    // if items changed, we want to instantly move active descendant into view
-    if (activeDescendantRef.current && scrollContainerRef.current) {
-      scrollIntoView(activeDescendantRef.current, scrollContainerRef.current, {
-        ...menuScrollMargins,
-        behavior: 'auto',
-      })
+    if (items.length === 0) {
+      inputRef.current?.focus()
+    } else {
+      const itemIds = items.filter(item => item.selected).map(item => item.id)
+      const removedItem = selectedItems.find(item => !itemIds.includes(item))
+      if (removedItem && document.activeElement !== inputRef.current) {
+        const list = listRef.current
+        if (list) {
+          const firstSelectedItem = list.querySelector('[role="option"]') as HTMLElement
+          firstSelectedItem.focus()
+        }
+      }
     }
-  }, [items])
+  }, [items, inputRef, selectedItems])
 
-  useAnnouncements(items, {current: listContainerElement}, inputRef, announcementsEnabled, loading)
+  useEffect(() => {
+    setEnableAnnouncements(announcementsEnabled)
+  }, [announcementsEnabled])
   useScrollFlash(scrollContainerRef)
 
   function getItemListForEachGroup(groupId: string) {
@@ -170,36 +159,40 @@ export function FilteredActionList({
     }
 
     return (
-      <ActionList
-        ref={listContainerRefCallback}
-        showDividers={showItemDividers}
-        {...listProps}
-        role="listbox"
-        id={listId}
-        sx={{flexGrow: 1}}
+      <ActionListContainerContext.Provider
+        value={{
+          container: 'FilteredActionList',
+          listRole: 'listbox',
+          selectionAttribute: 'aria-selected',
+          selectionVariant,
+          enableFocusZone: true,
+        }}
       >
-        {groupMetadata?.length
-          ? groupMetadata.map((group, index) => {
-              return (
-                <ActionList.Group key={index}>
-                  <ActionList.GroupHeading variant={group.header?.variant ? group.header.variant : undefined}>
-                    {group.header?.title ? group.header.title : `Group ${group.groupId}`}
-                  </ActionList.GroupHeading>
-                  {getItemListForEachGroup(group.groupId).map(({key: itemKey, ...item}, index) => {
-                    const key = itemKey ?? item.id?.toString() ?? index.toString()
-                    return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
-                  })}
-                </ActionList.Group>
-              )
-            })
-          : items.map(({key: itemKey, ...item}, index) => {
-              const key = itemKey ?? item.id?.toString() ?? index.toString()
-              return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
-            })}
-      </ActionList>
+        <ActionList ref={listRef} showDividers={showItemDividers} {...listProps} id={listId} sx={{flexGrow: 1}}>
+          {groupMetadata?.length
+            ? groupMetadata.map((group, index) => {
+                return (
+                  <ActionList.Group key={index}>
+                    <ActionList.GroupHeading variant={group.header?.variant ? group.header.variant : undefined}>
+                      {group.header?.title ? group.header.title : `Group ${group.groupId}`}
+                    </ActionList.GroupHeading>
+                    {getItemListForEachGroup(group.groupId).map(({key: itemKey, ...item}, index) => {
+                      const key = itemKey ?? item.id?.toString() ?? index.toString()
+                      return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
+                    })}
+                  </ActionList.Group>
+                )
+              })
+            : items.map(({key: itemKey, ...item}, index) => {
+                const key = itemKey ?? item.id?.toString() ?? index.toString()
+                return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
+              })}
+        </ActionList>
+      </ActionListContainerContext.Provider>
     )
   }
 
+  useAnnouncements(items, listRef, inputRef, enableAnnouncements)
   return (
     <Box
       display="flex"
@@ -217,7 +210,7 @@ export function FilteredActionList({
           color="fg.default"
           value={filterValue}
           onChange={onInputChange}
-          onKeyPress={onInputKeyPress}
+          onKeyDown={keydownListener}
           placeholder={placeholderText}
           role="combobox"
           aria-expanded="true"