Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/rotten-goats-stare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

FilteredActionList: Remove `usingRemoveActiveDescendant` feature flag, add private prop
47 changes: 33 additions & 14 deletions packages/react/src/FilteredActionList/FilteredActionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {ActionListContainerContext} from '../ActionList/ActionListContainerConte
import {isValidElementType} from 'react-is'
import {useAnnouncements} from './useAnnouncements'
import {clsx} from 'clsx'
import {useFeatureFlag} from '../FeatureFlags'

const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}

Expand All @@ -45,6 +44,26 @@ export interface FilteredActionListProps extends Partial<Omit<GroupedListProps,
announcementsEnabled?: boolean
fullScreenOnNarrow?: boolean
onSelectAllChange?: (checked: boolean) => void
/**
* Private API for use internally only. Adds the ability to switch between
* `active-descendant` and roving tabindex.
*
* By default, FilteredActionList uses `aria-activedescendant` to manage focus.
*
* Roving tabindex is an alternative focus management method that moves
* focus to the list items themselves instead of keeping focus on the input.
*
* Improper usage can lead to inaccessible experiences, so this prop should be used with caution.
*
* For usage, refer to the documentation:
*
* WAI-ARIA `aria-activedescendant`: https://www.w3.org/TR/wai-aria-1.2/#aria-activedescendant
*
* Roving Tabindex: https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#kbd_roving_tabindex
*
* @default 'active-descendant'
*/
_PrivateFocusManagement?: 'roving-tabindex' | 'active-descendant'
}

export function FilteredActionList({
Expand All @@ -67,6 +86,7 @@ export function FilteredActionList({
announcementsEnabled = true,
fullScreenOnNarrow,
onSelectAllChange,
_PrivateFocusManagement = 'active-descendant',
...listProps
}: FilteredActionListProps): JSX.Element {
const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
Expand All @@ -85,7 +105,7 @@ export function FilteredActionList({
const scrollContainerRef = useRef<HTMLDivElement>(null)
const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)

const usingRemoveActiveDescendant = useFeatureFlag('primer_react_select_panel_remove_active_descendant')
const usingRovingTabindex = _PrivateFocusManagement === 'roving-tabindex'
const [listContainerElement, setListContainerElement] = useState<HTMLUListElement | null>(null)
const activeDescendantRef = useRef<HTMLElement>()

Expand Down Expand Up @@ -164,7 +184,6 @@ export function FilteredActionList({
[activeDescendantRef],
)

// BEGIN: Todo remove when we remove usingRemoveActiveDescendant
const listContainerRefCallback = useCallback(
(node: HTMLUListElement | null) => {
setListContainerElement(node)
Expand All @@ -175,10 +194,9 @@ export function FilteredActionList({
useEffect(() => {
onInputRefChanged?.(inputRef)
}, [inputRef, onInputRefChanged])
//END: Todo remove when we remove usingRemoveActiveDescendant

useFocusZone(
!usingRemoveActiveDescendant
!usingRovingTabindex
? {
containerRef: {current: listContainerElement},
bindKeys: FocusKeys.ArrowVertical | FocusKeys.PageUpDown,
Expand All @@ -196,7 +214,7 @@ export function FilteredActionList({
},
}
: undefined,
[listContainerElement, usingRemoveActiveDescendant],
[listContainerElement, usingRovingTabindex],
)

useEffect(() => {
Expand All @@ -209,7 +227,7 @@ export function FilteredActionList({
}, [items, inputRef])

useEffect(() => {
if (usingRemoveActiveDescendant) {
if (usingRovingTabindex) {
const inputAndListContainerElement = inputAndListContainerRef.current
if (!inputAndListContainerElement) return
const list = listRef.current
Expand All @@ -228,21 +246,22 @@ export function FilteredActionList({
inputAndListContainerElement.removeEventListener('focusin', handleFocusIn)
}
}
}, [items, inputRef, listContainerElement, usingRemoveActiveDescendant]) // Re-run when items change to update active indicators
}, [items, inputRef, listContainerElement, usingRovingTabindex]) // Re-run when items change to update active indicators

useEffect(() => {
if (usingRemoveActiveDescendant && !loading) {
if (usingRovingTabindex && !loading) {
setIsInputFocused(inputRef.current && inputRef.current === document.activeElement ? true : false)
}
}, [loading, inputRef, usingRemoveActiveDescendant])
}, [loading, inputRef, usingRovingTabindex])

useAnnouncements(
items,
usingRemoveActiveDescendant ? listRef : {current: listContainerElement},
usingRovingTabindex ? listRef : {current: listContainerElement},
inputRef,
announcementsEnabled,
loading,
messageText,
_PrivateFocusManagement,
)
useScrollFlash(scrollContainerRef)

Expand All @@ -265,7 +284,7 @@ export function FilteredActionList({
let firstGroupIndex = 0
const actionListContent = (
<ActionList
ref={usingRemoveActiveDescendant ? listRef : listContainerRefCallback}
ref={usingRovingTabindex ? listRef : listContainerRefCallback}
showDividers={showItemDividers}
selectionVariant={selectionVariant}
{...listProps}
Expand Down Expand Up @@ -316,7 +335,7 @@ export function FilteredActionList({
)

// Use ActionListContainerContext.Provider only for the old behavior (when feature flag is disabled)
if (usingRemoveActiveDescendant) {
if (usingRovingTabindex) {
return (
<ActionListContainerContext.Provider
value={{
Expand Down Expand Up @@ -348,7 +367,7 @@ export function FilteredActionList({
value={filterValue}
onChange={onInputChange}
onKeyPress={onInputKeyPress}
onKeyDown={usingRemoveActiveDescendant ? onInputKeyDown : () => {}}
onKeyDown={usingRovingTabindex ? onInputKeyDown : () => {}}
placeholder={placeholderText}
role="combobox"
aria-expanded="true"
Expand Down
15 changes: 7 additions & 8 deletions packages/react/src/FilteredActionList/useAnnouncements.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {announce as liveRegionAnnounce} from '@primer/live-region-element'
import {useCallback, useEffect, useRef} from 'react'
import type {FilteredActionListProps} from './index'
import type {ItemInput} from '../SelectPanel'
import {useFeatureFlag} from '../FeatureFlags'

// we add a delay so that it does not interrupt default screen reader announcement and queues after it
const delayMs = 500
Expand All @@ -19,7 +18,6 @@ const useFirstRender = () => {
return firstRender.current
}

//TODO remove this when we remove usingRemoveActiveDescendant
const getItemWithActiveDescendant = (
listRef: React.RefObject<HTMLUListElement>,
items: FilteredActionListProps['items'],
Expand All @@ -39,7 +37,6 @@ const getItemWithActiveDescendant = (

return {index, text, selected}
}
//TODO remove this when we remove usingRemoveActiveDescendant

export const useAnnouncements = (
items: FilteredActionListProps['items'],
Expand All @@ -48,8 +45,10 @@ export const useAnnouncements = (
enabled: boolean = true,
loading: boolean = false,
message?: {title: string; description: string},
focusManagement?: 'active-descendant' | 'roving-tabindex',
) => {
const usingRemoveActiveDescendant = useFeatureFlag('primer_react_select_panel_remove_active_descendant')
const usingRovingTabindex = focusManagement === 'roving-tabindex'

const liveRegion = document.querySelector('live-region')

// Notify user of the number of items available
Expand All @@ -67,7 +66,7 @@ export const useAnnouncements = (
useEffect(
function announceInitialFocus() {
const focusHandler = () => {
if (usingRemoveActiveDescendant) {
if (usingRovingTabindex) {
const announcementText = `${items.length} item${items.length > 1 ? 's' : ''} available, ${selectedItems} selected.`
announce(announcementText, {
delayMs,
Expand Down Expand Up @@ -98,7 +97,7 @@ export const useAnnouncements = (
inputElement?.addEventListener('focus', focusHandler)
return () => inputElement?.removeEventListener('focus', focusHandler)
},
[listContainerRef, inputRef, items, liveRegion, announce, usingRemoveActiveDescendant, selectedItems],
[listContainerRef, inputRef, items, liveRegion, announce, usingRovingTabindex, selectedItems],
)

const isFirstRender = useFirstRender()
Expand All @@ -113,7 +112,7 @@ export const useAnnouncements = (
return
}

if (usingRemoveActiveDescendant) {
if (usingRovingTabindex) {
const announcementText = `${items.length} item${items.length > 1 ? 's' : ''} available, ${selectedItems} selected.`

announce(announcementText, {
Expand Down Expand Up @@ -147,7 +146,7 @@ export const useAnnouncements = (
items,
listContainerRef,
liveRegion,
usingRemoveActiveDescendant,
usingRovingTabindex,
message?.title,
message?.description,
loading,
Expand Down
Loading
Loading