Feat(SelectPanel) remove aria active-descendant pattern in favor of roving tab index under FF

.changeset/pink-worms-kneel.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+feat(SelectPanel): remove aria activedescendant and add a roving tab index

e2e/components/SelectPanel.test.ts

@@ -60,7 +60,9 @@ test.describe('SelectPanel', () => {
 
     const globals = {
       colorScheme: scenario.theme,
-      featureFlags: {primer_react_select_panel_with_modern_action_list: scenario.modernActionList},
+      featureFlags: {
+        primer_react_select_panel_with_modern_action_list: scenario.modernActionList,
+      },
     }
 
     if (scenario.story.visual !== false) {

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

@@ -92,7 +92,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))
 
@@ -121,6 +121,8 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       else inferredItemRole = 'menuitem'
     } else if (listRole === 'listbox') {
       if (selectionVariant !== undefined && !role) inferredItemRole = 'option'
+    } else if (container && ['SelectPanel', 'FilteredActionList'].includes(container)) {
+      if (selectionVariant !== undefined && !role) inferredItemRole = 'option'
     }
 
     const itemRole = role || inferredItemRole

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,8 @@ 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/FeatureFlags/DefaultFeatureFlags.ts

@@ -8,4 +8,5 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({
   primer_react_segmented_control_tooltip: false,
   primer_react_select_panel_fullscreen_on_narrow: false,
   primer_react_select_panel_order_selected_at_top: false,
+  primer_react_select_panel_remove_active_descendant: false,
 })

packages/react/src/FilteredActionList/FilteredActionList.module.css

@@ -5,6 +5,22 @@
   flex-grow: 1;
 }
 
+.ActionListItem:focus {
+  background: var(--control-transparent-bgColor-selected);
+
+  &::after {
+    @mixin activeIndicatorLine;
+  }
+}
+
+.ActionListItem:where([data-input-focused]):where([data-first-child]) {
+  background: var(--control-transparent-bgColor-selected);
+
+  &::after {
+    @mixin activeIndicatorLine;
+  }
+}
+
 .FullScreenTextInput {
   @media screen and (--viewportRange-narrow) {
     /* Ensures inputs don't zoom on mobile iPhone but are body-font size on iPad */

packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx

@@ -14,18 +14,19 @@ import {useFocusZone} from '../hooks/useFocusZone'
 import {useId} from '../hooks/useId'
 import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
 import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
-import useScrollFlash from '../hooks/useScrollFlash'
 import {VisuallyHidden} from '../VisuallyHidden'
 import type {SxProp} from '../sx'
 import type {FilteredActionListLoadingType} from './FilteredActionListLoaders'
 import {FilteredActionListLoadingTypes, FilteredActionListBodyLoader} from './FilteredActionListLoaders'
 import classes from './FilteredActionList.module.css'
 import Checkbox from '../Checkbox'
 
+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'
+import {useFeatureFlag} from '../FeatureFlags'
 
 const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
 
@@ -75,6 +76,7 @@ export function FilteredActionList({
   message,
   messageText,
   className,
+  selectionVariant,
   announcementsEnabled = true,
   fullScreenOnNarrow,
   onSelectAllChange,
@@ -90,19 +92,46 @@ export function FilteredActionList({
     [onFilterChange, setInternalFilterValue],
   )
 
+  const inputAndListContainerRef = useRef<HTMLDivElement>(null)
+  const listRef = useRef<HTMLUListElement>(null)
+
   const scrollContainerRef = useRef<HTMLDivElement>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
+
+  const usingRemoveActiveDescendant = useFeatureFlag('primer_react_select_panel_remove_active_descendant')
   const [listContainerElement, setListContainerElement] = useState<HTMLUListElement | null>(null)
   const activeDescendantRef = useRef<HTMLElement>()
+
   const listId = useId()
   const inputDescriptionTextId = useId()
+  const [isInputFocused, setIsInputFocused] = useState(false)
 
   const selectAllChecked = items.length > 0 && items.every(item => item.selected)
   const selectAllIndeterminate = !selectAllChecked && items.some(item => item.selected)
 
   const selectAllLabelText = selectAllChecked ? 'Deselect all' : 'Select all'
+  const onInputKeyDown = useCallback(
+    (event: React.KeyboardEvent<HTMLInputElement>) => {
+      if (event.key === 'ArrowDown' || event.key === 'PageDown') {
+        if (listRef.current) {
+          const firstSelectedItem = listRef.current.querySelector('[role="option"]') as HTMLElement | undefined
+          firstSelectedItem?.focus()
+
+          event.preventDefault()
+        }
+      } else if (event.key === 'Enter') {
+        const firstItem = items[0]
+        if (firstItem.onAction) {
+          firstItem.onAction(firstItem, event)
+          event.preventDefault()
+        }
+      }
+    },
+    [items],
+  )
+
   const onInputKeyPress: KeyboardEventHandler = useCallback(
-    event => {
+    (event: React.KeyboardEvent<HTMLInputElement>) => {
       if (event.key === 'Enter' && activeDescendantRef.current) {
         event.preventDefault()
         event.nativeEvent.stopImmediatePropagation()
@@ -115,53 +144,80 @@ export function FilteredActionList({
     [activeDescendantRef],
   )
 
+  // BEGIN: Todo remove when we remove usingRemoveActiveDescendant
   const listContainerRefCallback = useCallback(
     (node: HTMLUListElement | null) => {
       setListContainerElement(node)
       onListContainerRefChanged?.(node)
     },
     [onListContainerRefChanged],
   )
-
   useEffect(() => {
     onInputRefChanged?.(inputRef)
   }, [inputRef, onInputRefChanged])
+  //END: Todo remove when we remove usingRemoveActiveDescendant
 
   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)
+    !usingRemoveActiveDescendant
+      ? {
+          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,
-    ],
+      : undefined,
+    [listContainerElement, usingRemoveActiveDescendant],
   )
 
   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',
       })
     }
-  }, [items])
+  }, [items, inputRef])
+
+  useEffect(() => {
+    if (usingRemoveActiveDescendant) {
+      const inputAndListContainerElement = inputAndListContainerRef.current
+      if (!inputAndListContainerElement) return
+      const list = listRef.current
+      if (!list) return
+
+      // Listen for focus changes within the container
+      const handleFocusIn = (event: FocusEvent) => {
+        if (event.target === inputRef.current || list.contains(event.target as Node)) {
+          setIsInputFocused(inputRef.current && inputRef.current === document.activeElement ? true : false)
+        }
+      }
+
+      inputAndListContainerElement.addEventListener('focusin', handleFocusIn)
+
+      return () => {
+        inputAndListContainerElement.removeEventListener('focusin', handleFocusIn)
+      }
+    }
+  }, [items, inputRef, listContainerElement, usingRemoveActiveDescendant]) // Re-run when items change to update active indicators
 
-  useAnnouncements(items, {current: listContainerElement}, inputRef, announcementsEnabled, loading, messageText)
-  useScrollFlash(scrollContainerRef)
+  useAnnouncements(
+    items,
+    usingRemoveActiveDescendant ? listRef : {current: listContainerElement},
+    inputRef,
+    announcementsEnabled,
+    loading,
+    messageText,
+  )
 
   const handleSelectAllChange = useCallback(
     (e: React.ChangeEvent<HTMLInputElement>) => {
@@ -190,13 +246,13 @@ export function FilteredActionList({
     if (message) {
       return message
     }
-
-    return (
+    let allItemIndex = -1 // to keep track of the index of items across all groups
+    const actionListContent = (
       <ActionList
-        ref={listContainerRefCallback}
+        ref={usingRemoveActiveDescendant ? listRef : listContainerRefCallback}
         showDividers={showItemDividers}
+        selectionVariant={selectionVariant}
         {...listProps}
-        role="listbox"
         id={listId}
         sx={{flexGrow: 1}}
       >
@@ -207,23 +263,62 @@ export function FilteredActionList({
                   <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} />
+                  {getItemListForEachGroup(group.groupId).map(({key: itemKey, ...item}, itemIndex) => {
+                    const key = itemKey ?? item.id?.toString() ?? itemIndex.toString()
+                    allItemIndex += 1
+                    return (
+                      <MappedActionListItem
+                        key={key}
+                        className={clsx(classes.ActionListItem, 'className' in item ? item.className : undefined)}
+                        data-input-focused={isInputFocused ? '' : undefined}
+                        data-first-child={allItemIndex === 0 && itemIndex === 0 ? '' : undefined}
+                        {...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} />
+              return (
+                <MappedActionListItem
+                  key={key}
+                  className={clsx(classes.ActionListItem, 'className' in item ? item.className : undefined)}
+                  data-input-focused={isInputFocused ? '' : undefined}
+                  data-first-child={index === 0 ? '' : undefined}
+                  {...item}
+                  renderItem={listProps.renderItem}
+                />
+              )
             })}
       </ActionList>
     )
+
+    // Use ActionListContainerContext.Provider only for the old behavior (when feature flag is disabled)
+    if (usingRemoveActiveDescendant) {
+      return (
+        <ActionListContainerContext.Provider
+          value={{
+            container: 'FilteredActionList',
+            listRole: 'listbox',
+            selectionAttribute: 'aria-selected',
+            selectionVariant,
+            enableFocusZone: true,
+          }}
+        >
+          {actionListContent}
+        </ActionListContainerContext.Provider>
+      )
+    } else {
+      return actionListContent
+    }
   }
 
   return (
     <Box
+      ref={inputAndListContainerRef}
       display="flex"
       flexDirection="column"
       overflow="hidden"
@@ -240,6 +335,7 @@ export function FilteredActionList({
           value={filterValue}
           onChange={onInputChange}
           onKeyPress={onInputKeyPress}
+          onKeyDown={usingRemoveActiveDescendant ? onInputKeyDown : () => {}}
           placeholder={placeholderText}
           role="combobox"
           aria-expanded="true"