Remove activedescendant focus pattern from SelectPanel

.changeset/yellow-pears-grin.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Remove activedescendant focus pattern from SelectPanel in favor of regular focus behavior with a roving tab index.

packages/react/src/ActionList/ActionListContainerContext.tsx

@@ -2,6 +2,7 @@
 
 import React from 'react'
 import type {AriaRole} from '../utils/types'
+import type {FocusZoneSettings} from '@primer/behaviors'
 
 type ContextProps = {
   container?: string
@@ -14,6 +15,7 @@ type ContextProps = {
   // eslint-disable-next-line @typescript-eslint/ban-types
   afterSelect?: Function
   enableFocusZone?: boolean
+  focusZoneFocusOutBehavior?: FocusZoneSettings['focusOutBehavior']
   defaultTrailingVisual?: React.ReactElement
 }
 

packages/react/src/ActionList/List.tsx

@@ -41,6 +41,7 @@ export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
       listLabelledBy,
       selectionVariant: containerSelectionVariant, // TODO: Remove after DropdownMenu2 deprecation
       enableFocusZone: enableFocusZoneFromContainer,
+      focusZoneFocusOutBehavior,
     } = React.useContext(ActionListContainerContext)
 
     const ariaLabelledBy = slots.heading ? (slots.heading.props.id ?? headingId) : listLabelledBy
@@ -55,7 +56,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: focusZoneFocusOutBehavior ?? (listRole === 'menu' ? 'wrap' : undefined),
     })
 
     const enabled = useFeatureFlag(actionListCssModulesFlag)

packages/react/src/FilteredActionList/FilteredActionListWithDeprecatedActionList.tsx

@@ -1,7 +1,6 @@
-import type {ScrollIntoViewOptions} from '@primer/behaviors'
-import {scrollIntoView} from '@primer/behaviors'
+import {FocusKeys} from '@primer/behaviors'
 import type {KeyboardEventHandler, RefObject} from 'react'
-import React, {useCallback, useEffect, useRef, useState} from 'react'
+import React, {useCallback, useContext, useEffect, useRef, useState} from 'react'
 import styled from 'styled-components'
 import Box from '../Box'
 import type {TextInputProps} from '../TextInput'
@@ -21,8 +20,7 @@ import {
   FilteredActionListBodyLoader,
   FilteredActionListLoadingTypes,
 } from './FilteredActionListLoaders'
-
-const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
+import {ActionListContainerContext} from '../ActionList/ActionListContainerContext'
 
 export interface FilteredActionListProps
   extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
@@ -62,6 +60,7 @@ export function FilteredActionList({
   announcementsEnabled: _announcementsEnabled = true,
   ...listProps
 }: FilteredActionListProps): JSX.Element {
+  const {container} = useContext(ActionListContainerContext)
   const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
   const onInputChange = useCallback(
     (e: React.ChangeEvent<HTMLInputElement>) => {
@@ -75,21 +74,20 @@ export function FilteredActionList({
   const scrollContainerRef = useRef<HTMLDivElement>(null)
   const [listContainerElement, setListContainerElement] = useState<HTMLDivElement | null>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
-  const activeDescendantRef = useRef<HTMLElement>()
   const listId = useId()
   const inputDescriptionTextId = useId()
   const onInputKeyPress: KeyboardEventHandler = useCallback(
     event => {
-      if (event.key === 'Enter' && activeDescendantRef.current) {
-        event.preventDefault()
-        event.nativeEvent.stopImmediatePropagation()
+      if (event.key === 'Enter' && listContainerElement) {
+        const firstItem = listContainerElement.querySelector('[data-select-panel-item=true]')
 
-        // Forward Enter key press to active descendant so that item gets activated
-        const activeDescendantEvent = new KeyboardEvent(event.type, event.nativeEvent)
-        activeDescendantRef.current.dispatchEvent(activeDescendantEvent)
+        if (firstItem) {
+          const firstItemEvent = new KeyboardEvent(event.type, event.nativeEvent)
+          firstItem.dispatchEvent(firstItemEvent)
+        }
       }
     },
-    [activeDescendantRef],
+    [listContainerElement],
   )
 
   const listContainerRefCallback = useCallback(
@@ -107,35 +105,12 @@ export function FilteredActionList({
   useFocusZone(
     {
       containerRef: {current: listContainerElement},
+      bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | 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,
-    ],
+    [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',
-      })
-    }
-  }, [items])
-
   useScrollFlash(scrollContainerRef)
 
   return (
@@ -171,7 +146,19 @@ export function FilteredActionList({
         {loading && scrollContainerRef.current && loadingType.appearsInBody ? (
           <FilteredActionListBodyLoader loadingType={loadingType} height={scrollContainerRef.current.clientHeight} />
         ) : (
-          <ActionList ref={listContainerRefCallback} items={items} {...listProps} role="listbox" id={listId} />
+          <ActionList
+            ref={listContainerRefCallback}
+            items={items.map(item => {
+              return {
+                ...item,
+                'data-select-panel-item': container === 'SelectPanel',
+                role: 'option',
+              }
+            })}
+            {...listProps}
+            role="listbox"
+            id={listId}
+          />
         )}
       </Box>
     </Box>

packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx

@@ -1,15 +1,12 @@
-import type {ScrollIntoViewOptions} from '@primer/behaviors'
-import {scrollIntoView, FocusKeys} from '@primer/behaviors'
 import type {KeyboardEventHandler} from 'react'
-import React, {useCallback, useEffect, useRef, useState} from 'react'
+import React, {useCallback, useContext, useEffect, useRef} from 'react'
 import styled from 'styled-components'
 import Box from '../Box'
 import type {TextInputProps} from '../TextInput'
 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'
@@ -21,9 +18,7 @@ import {FilteredActionListLoadingTypes, FilteredActionListBodyLoader} from './Fi
 
 import {isValidElementType} from 'react-is'
 import type {RenderItemFn} from '../deprecated/ActionList/List'
-import {useAnnouncements} from './useAnnouncements'
-
-const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
+import {ActionListContainerContext} from '../ActionList/ActionListContainerContext'
 
 export interface FilteredActionListProps
   extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
@@ -39,7 +34,6 @@ export interface FilteredActionListProps
   textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
   inputRef?: React.RefObject<HTMLInputElement>
   className?: string
-  announcementsEnabled?: boolean
 }
 
 const StyledHeader = styled.div`
@@ -53,7 +47,6 @@ export function FilteredActionList({
   filterValue: externalFilterValue,
   loadingType = FilteredActionListLoadingTypes.bodySpinner,
   onFilterChange,
-  onListContainerRefChanged,
   onInputRefChanged,
   items,
   textInputProps,
@@ -62,7 +55,7 @@ export function FilteredActionList({
   groupMetadata,
   showItemDividers,
   className,
-  announcementsEnabled = true,
+  selectionVariant,
   ...listProps
 }: FilteredActionListProps): JSX.Element {
   const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
@@ -77,70 +70,26 @@ export function FilteredActionList({
 
   const scrollContainerRef = useRef<HTMLDivElement>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
-  const [listContainerElement, setListContainerElement] = useState<HTMLUListElement | null>(null)
-  const activeDescendantRef = useRef<HTMLElement>()
   const listId = useId()
   const inputDescriptionTextId = useId()
   const onInputKeyPress: KeyboardEventHandler = useCallback(
     event => {
-      if (event.key === 'Enter' && activeDescendantRef.current) {
-        event.preventDefault()
-        event.nativeEvent.stopImmediatePropagation()
+      if (event.key === 'Enter' && scrollContainerRef.current) {
+        const firstItem = scrollContainerRef.current.querySelector('[data-select-panel-item=true]')
 
-        // Forward Enter key press to active descendant so that item gets activated
-        const activeDescendantEvent = new KeyboardEvent(event.type, event.nativeEvent)
-        activeDescendantRef.current.dispatchEvent(activeDescendantEvent)
+        if (firstItem) {
+          const firstItemEvent = new KeyboardEvent(event.type, event.nativeEvent)
+          firstItem.dispatchEvent(firstItemEvent)
+        }
       }
     },
-    [activeDescendantRef],
-  )
-
-  const listContainerRefCallback = useCallback(
-    (node: HTMLUListElement | null) => {
-      setListContainerElement(node)
-      onListContainerRefChanged?.(node)
-    },
-    [onListContainerRefChanged],
+    [scrollContainerRef],
   )
 
   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',
-      })
-    }
-  }, [items])
-
-  useAnnouncements(items, {current: listContainerElement}, inputRef, announcementsEnabled)
   useScrollFlash(scrollContainerRef)
 
   function getItemListForEachGroup(groupId: string) {
@@ -190,12 +139,12 @@ export function FilteredActionList({
           <FilteredActionListBodyLoader loadingType={loadingType} height={scrollContainerRef.current.clientHeight} />
         ) : (
           <ActionList
-            ref={listContainerRefCallback}
             showDividers={showItemDividers}
             {...listProps}
             role="listbox"
             id={listId}
             sx={{flexGrow: 1}}
+            selectionVariant={selectionVariant}
           >
             {groupMetadata?.length
               ? groupMetadata.map((group, index) => {
@@ -223,9 +172,19 @@ export function FilteredActionList({
 }
 
 function MappedActionListItem(item: ItemInput & {renderItem?: RenderItemFn}) {
+  const {container} = useContext(ActionListContainerContext)
+
   // keep backward compatibility for renderItem
   // escape hatch for custom Item rendering
-  if (typeof item.renderItem === 'function') return item.renderItem(item)
+  if (typeof item.renderItem === 'function') {
+    if (container === 'SelectPanel') {
+      return React.cloneElement(item.renderItem(item), {
+        'data-select-panel-item': true,
+      })
+    }
+
+    return item.renderItem(item)
+  }
 
   const {
     id,
@@ -243,13 +202,13 @@ function MappedActionListItem(item: ItemInput & {renderItem?: RenderItemFn}) {
 
   return (
     <ActionList.Item
-      role="option"
       // @ts-ignore - for now
       onSelect={(e: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>) => {
         if (typeof onAction === 'function')
           onAction(item, e as React.MouseEvent<HTMLDivElement> | React.KeyboardEvent<HTMLDivElement>)
       }}
       data-id={id}
+      data-select-panel-item={container === 'SelectPanel'}
       {...rest}
     >
       {LeadingVisual ? (