Skip to content

Commit 113191e

Browse files
SelectPanel: Remove aria-activedescendant from SelectPanel (#5801)
Co-authored-by: TylerJDev <[email protected]> Co-authored-by: Marie Lucca <[email protected]> Co-authored-by: francinelucca <[email protected]>
1 parent 687343c commit 113191e

File tree

156 files changed

+166
-192
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

156 files changed

+166
-192
lines changed

.changeset/four-cars-attend.md

Lines changed: 5 additions & 0 deletions

packages/react/src/ActionList/Item.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,12 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
137137
if (selectionVariant === 'single') inferredItemRole = 'menuitemradio'
138138
else if (selectionVariant === 'multiple') inferredItemRole = 'menuitemcheckbox'
139139
else inferredItemRole = 'menuitem'
140-
} else if (container === 'SelectPanel' && listRole === 'listbox') {
140+
} else if (container && ['SelectPanel', 'FilteredActionList'].includes(container) && listRole === 'listbox') {
141141
if (selectionVariant !== undefined) inferredItemRole = 'option'
142142
}
143143

144144
const itemRole = role || inferredItemRole
145-
const menuContext = container === 'ActionMenu' || container === 'SelectPanel'
145+
const menuContext = container === 'ActionMenu' || container === 'SelectPanel' || container === 'FilteredActionList'
146146

147147
if (slots.trailingAction) {
148148
invariant(!menuContext, `ActionList.TrailingAction can not be used within a ${container}.`)

packages/react/src/ActionList/List.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
4141
listLabelledBy,
4242
selectionVariant: containerSelectionVariant, // TODO: Remove after DropdownMenu2 deprecation
4343
enableFocusZone: enableFocusZoneFromContainer,
44+
container,
4445
} = React.useContext(ActionListContainerContext)
4546

4647
const ariaLabelledBy = slots.heading ? (slots.heading.props.id ?? headingId) : listLabelledBy
@@ -55,7 +56,7 @@ export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
5556
disabled: !enableFocusZone,
5657
containerRef: listRef,
5758
bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | FocusKeys.PageUpDown,
58-
focusOutBehavior: listRole === 'menu' ? 'wrap' : undefined,
59+
focusOutBehavior: listRole === 'menu' || container === 'FilteredActionList' ? 'wrap' : undefined,
5960
})
6061

6162
const enabled = useFeatureFlag(actionListCssModulesFlag)

packages/react/src/FilteredActionList/FilteredActionListEntry.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ import {FilteredActionList as WithDeprecatedActionList} from './FilteredActionLi
44
import {FilteredActionList as WithStableActionList} from './FilteredActionListWithModernActionList'
55
import {useFeatureFlag} from '../FeatureFlags'
66

7-
export function FilteredActionList(props: FilteredActionListProps): JSX.Element {
7+
export function FilteredActionList({onListContainerRefChanged, ...props}: FilteredActionListProps) {
88
const enabled = useFeatureFlag('primer_react_select_panel_with_modern_action_list')
99

1010
if (enabled) return <WithStableActionList {...props} />
11-
else return <WithDeprecatedActionList {...props} />
11+
else return <WithDeprecatedActionList onListContainerRefChanged={onListContainerRefChanged} {...props} />
1212
}
1313

1414
FilteredActionList.displayName = 'FilteredActionList'

packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx

Lines changed: 83 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
import type {ScrollIntoViewOptions} from '@primer/behaviors'
2-
import {scrollIntoView, FocusKeys} from '@primer/behaviors'
3-
import type {KeyboardEventHandler} from 'react'
41
import React, {useCallback, useEffect, useRef, useState} from 'react'
52
import styled from 'styled-components'
63
import Box from '../Box'
@@ -9,7 +6,6 @@ import TextInput from '../TextInput'
96
import {get} from '../constants'
107
import {ActionList} from '../ActionList'
118
import type {GroupedListProps, ListPropsBase, ItemInput} from '../SelectPanel/types'
12-
import {useFocusZone} from '../hooks/useFocusZone'
139
import {useId} from '../hooks/useId'
1410
import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
1511
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
@@ -18,13 +14,12 @@ import {VisuallyHidden} from '../VisuallyHidden'
1814
import type {SxProp} from '../sx'
1915
import type {FilteredActionListLoadingType} from './FilteredActionListLoaders'
2016
import {FilteredActionListLoadingTypes, FilteredActionListBodyLoader} from './FilteredActionListLoaders'
17+
import {ActionListContainerContext} from '../ActionList/ActionListContainerContext'
2118

2219
import {isValidElementType} from 'react-is'
2320
import type {RenderItemFn} from '../deprecated/ActionList/List'
2421
import {useAnnouncements} from './useAnnouncements'
2522

26-
const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
27-
2823
export interface FilteredActionListProps
2924
extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
3025
ListPropsBase,
@@ -34,7 +29,6 @@ export interface FilteredActionListProps
3429
placeholderText?: string
3530
filterValue?: string
3631
onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
37-
onListContainerRefChanged?: (ref: HTMLElement | null) => void
3832
onInputRefChanged?: (ref: React.RefObject<HTMLInputElement>) => void
3933
textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
4034
inputRef?: React.RefObject<HTMLInputElement>
@@ -53,7 +47,6 @@ export function FilteredActionList({
5347
filterValue: externalFilterValue,
5448
loadingType = FilteredActionListLoadingTypes.bodySpinner,
5549
onFilterChange,
56-
onListContainerRefChanged,
5750
onInputRefChanged,
5851
items,
5952
textInputProps,
@@ -62,10 +55,14 @@ export function FilteredActionList({
6255
groupMetadata,
6356
showItemDividers,
6457
className,
58+
selectionVariant,
6559
announcementsEnabled = true,
6660
...listProps
6761
}: FilteredActionListProps): JSX.Element {
6862
const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
63+
const [enableAnnouncements, setEnableAnnouncements] = useState(false)
64+
const [selectedItems, setSelectedItems] = useState<(string | number | undefined)[]>([])
65+
6966
const onInputChange = useCallback(
7067
(e: React.ChangeEvent<HTMLInputElement>) => {
7168
const value = e.target.value
@@ -77,70 +74,69 @@ export function FilteredActionList({
7774

7875
const scrollContainerRef = useRef<HTMLDivElement>(null)
7976
const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
80-
const [listContainerElement, setListContainerElement] = useState<HTMLUListElement | null>(null)
81-
const activeDescendantRef = useRef<HTMLElement>()
77+
const listRef = useRef<HTMLUListElement>(null)
8278
const listId = useId()
8379
const inputDescriptionTextId = useId()
84-
const onInputKeyPress: KeyboardEventHandler = useCallback(
85-
event => {
86-
if (event.key === 'Enter' && activeDescendantRef.current) {
87-
event.preventDefault()
88-
event.nativeEvent.stopImmediatePropagation()
8980

90-
// Forward Enter key press to active descendant so that item gets activated
91-
const activeDescendantEvent = new KeyboardEvent(event.type, event.nativeEvent)
92-
activeDescendantRef.current.dispatchEvent(activeDescendantEvent)
93-
}
94-
},
95-
[activeDescendantRef],
96-
)
81+
const keydownListener = useCallback(
82+
(event: React.KeyboardEvent<HTMLDivElement>) => {
83+
if (event.key === 'ArrowDown') {
84+
if (listRef.current) {
85+
const firstSelectedItem = listRef.current.querySelector('[role="option"]') as HTMLElement | undefined
86+
firstSelectedItem?.focus()
9787

98-
const listContainerRefCallback = useCallback(
99-
(node: HTMLUListElement | null) => {
100-
setListContainerElement(node)
101-
onListContainerRefChanged?.(node)
88+
event.preventDefault()
89+
}
90+
} else if (event.key === 'Enter') {
91+
let firstItem
92+
// If there are groups, it's not guaranteed that the first item is the actual first item in the first -
93+
// as groups are rendered in the order of the groupId provided
94+
if (groupMetadata) {
95+
const firstGroup = groupMetadata[0].groupId
96+
firstItem = items.filter(item => item.groupId === firstGroup)[0]
97+
} else {
98+
firstItem = items[0]
99+
}
100+
101+
if (firstItem.onAction) {
102+
firstItem.onAction(firstItem, event)
103+
event.preventDefault()
104+
}
105+
}
102106
},
103-
[onListContainerRefChanged],
107+
[items, groupMetadata],
104108
)
105109

106110
useEffect(() => {
107111
onInputRefChanged?.(inputRef)
108112
}, [inputRef, onInputRefChanged])
109113

110-
useFocusZone(
111-
{
112-
containerRef: {current: listContainerElement},
113-
bindKeys: FocusKeys.ArrowVertical | FocusKeys.PageUpDown,
114-
focusOutBehavior: 'wrap',
115-
focusableElementFilter: element => {
116-
return !(element instanceof HTMLInputElement)
117-
},
118-
activeDescendantFocus: inputRef,
119-
onActiveDescendantChanged: (current, previous, directlyActivated) => {
120-
activeDescendantRef.current = current
114+
useEffect(() => {
115+
if (items.length === 0) {
116+
inputRef.current?.focus()
117+
} else {
118+
const itemIds = items.filter(item => item.selected).map(item => item.id)
119+
const removedItem = selectedItems.find(item => !itemIds.includes(item))
121120

122-
if (current && scrollContainerRef.current && directlyActivated) {
123-
scrollIntoView(current, scrollContainerRef.current, menuScrollMargins)
121+
if (removedItem && document.activeElement !== inputRef.current) {
122+
const list = listRef.current
123+
if (list) {
124+
const firstSelectedItem = list.querySelector('[role="option"]') as HTMLElement
125+
firstSelectedItem.focus()
124126
}
125-
},
126-
},
127-
[
128-
// List container isn't in the DOM while loading. Need to re-bind focus zone when it changes.
129-
listContainerElement,
130-
],
131-
)
127+
}
128+
}
129+
}, [items, inputRef, selectedItems])
132130

133131
useEffect(() => {
134-
// if items changed, we want to instantly move active descendant into view
135-
if (activeDescendantRef.current && scrollContainerRef.current) {
136-
scrollIntoView(activeDescendantRef.current, scrollContainerRef.current, {
137-
...menuScrollMargins,
138-
behavior: 'auto',
139-
})
140-
}
132+
const selectedItemIds = items.filter(item => item.selected).map(item => item.id)
133+
setSelectedItems(selectedItemIds)
141134
}, [items])
142135

143-
useAnnouncements(items, {current: listContainerElement}, inputRef, announcementsEnabled)
136+
useEffect(() => {
137+
setEnableAnnouncements(announcementsEnabled)
138+
}, [announcementsEnabled])
139+
144140
useScrollFlash(scrollContainerRef)
145141

146142
function getItemListForEachGroup(groupId: string) {
@@ -154,6 +150,8 @@ export function FilteredActionList({
154150
return itemsInGroup
155151
}
156152

153+
useAnnouncements(items, listRef, inputRef, enableAnnouncements)
154+
157155
return (
158156
<Box
159157
display="flex"
@@ -171,7 +169,7 @@ export function FilteredActionList({
171169
color="fg.default"
172170
value={filterValue}
173171
onChange={onInputChange}
174-
onKeyPress={onInputKeyPress}
172+
onKeyDown={keydownListener}
175173
placeholder={placeholderText}
176174
role="combobox"
177175
aria-expanded="true"
@@ -189,33 +187,36 @@ export function FilteredActionList({
189187
{loading && scrollContainerRef.current && loadingType.appearsInBody ? (
190188
<FilteredActionListBodyLoader loadingType={loadingType} height={scrollContainerRef.current.clientHeight} />
191189
) : (
192-
<ActionList
193-
ref={listContainerRefCallback}
194-
showDividers={showItemDividers}
195-
{...listProps}
196-
role="listbox"
197-
id={listId}
198-
sx={{flexGrow: 1}}
190+
<ActionListContainerContext.Provider
191+
value={{
192+
container: 'FilteredActionList',
193+
listRole: 'listbox',
194+
selectionAttribute: 'aria-selected',
195+
selectionVariant,
196+
enableFocusZone: true,
197+
}}
199198
>
200-
{groupMetadata?.length
201-
? groupMetadata.map((group, index) => {
202-
return (
203-
<ActionList.Group key={index}>
204-
<ActionList.GroupHeading variant={group.header?.variant ? group.header.variant : undefined}>
205-
{group.header?.title ? group.header.title : `Group ${group.groupId}`}
206-
</ActionList.GroupHeading>
207-
{getItemListForEachGroup(group.groupId).map((item, index) => {
208-
const key = item.key ?? item.id?.toString() ?? index.toString()
209-
return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
210-
})}
211-
</ActionList.Group>
212-
)
213-
})
214-
: items.map((item, index) => {
215-
const key = item.key ?? item.id?.toString() ?? index.toString()
216-
return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
217-
})}
218-
</ActionList>
199+
<ActionList ref={listRef} showDividers={showItemDividers} {...listProps} id={listId} sx={{flexGrow: 1}}>
200+
{groupMetadata?.length
201+
? groupMetadata.map((group, index) => {
202+
return (
203+
<ActionList.Group key={index}>
204+
<ActionList.GroupHeading variant={group.header?.variant ? group.header.variant : undefined}>
205+
{group.header?.title ? group.header.title : `Group ${group.groupId}`}
206+
</ActionList.GroupHeading>
207+
{getItemListForEachGroup(group.groupId).map((item, index) => {
208+
const key = item.key ?? item.id?.toString() ?? index.toString()
209+
return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
210+
})}
211+
</ActionList.Group>
212+
)
213+
})
214+
: items.map((item, index) => {
215+
const key = item.key ?? item.id?.toString() ?? index.toString()
216+
return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
217+
})}
218+
</ActionList>
219+
</ActionListContainerContext.Provider>
219220
)}
220221
</Box>
221222
</Box>

0 commit comments

Comments
 (0)