Skip to content

Commit fe58f67

Browse files
Chore/revert select panel active descendant changes (#5825)
1 parent 91b52f7 commit fe58f67

File tree

185 files changed

+366
-843
lines changed

Some content is hidden

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

185 files changed

+366
-843
lines changed

.changeset/four-cars-attend.md

Lines changed: 0 additions & 5 deletions
This file was deleted.

.changeset/new-mangos-fold.md

Lines changed: 0 additions & 5 deletions
This file was deleted.

e2e/components/SelectPanel.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const scenarios = matrix({
2020
name: 'With Placeholder for Search Input',
2121
},
2222
{id: 'components-selectpanel-examples--above-tall-body', name: 'Above Tall Body'},
23-
{id: 'components-selectpanel-examples--height-variations-and-scroll', name: 'Height Variations and Scroll'},
23+
{id: 'components-selectpanel-examples--height-variantions-and-scroll', name: 'Height Variantions and Scroll'},
2424
{
2525
id: 'components-selectpanel-examples--height-initial-with-overflowing-items-story',
2626
name: 'Height Initial with Overflowing Items',

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', 'FilteredActionList'].includes(container) && listRole === 'listbox') {
140+
} else if (container === 'SelectPanel' && listRole === 'listbox') {
141141
if (selectionVariant !== undefined) inferredItemRole = 'option'
142142
}
143143

144144
const itemRole = role || inferredItemRole
145-
const menuContext = container === 'ActionMenu' || container === 'SelectPanel' || container === 'FilteredActionList'
145+
const menuContext = container === 'ActionMenu' || container === 'SelectPanel'
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: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
4141
listLabelledBy,
4242
selectionVariant: containerSelectionVariant, // TODO: Remove after DropdownMenu2 deprecation
4343
enableFocusZone: enableFocusZoneFromContainer,
44-
container,
4544
} = React.useContext(ActionListContainerContext)
4645

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

6261
const enabled = useFeatureFlag(actionListCssModulesFlag)

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

Lines changed: 0 additions & 6 deletions
This file was deleted.

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

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

1414
FilteredActionList.displayName = 'FilteredActionList'

packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx

Lines changed: 89 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import type {ScrollIntoViewOptions} from '@primer/behaviors'
2+
import {scrollIntoView, FocusKeys} from '@primer/behaviors'
3+
import type {KeyboardEventHandler} from 'react'
14
import React, {useCallback, useEffect, useRef, useState} from 'react'
25
import styled from 'styled-components'
36
import Box from '../Box'
@@ -6,6 +9,7 @@ import TextInput from '../TextInput'
69
import {get} from '../constants'
710
import {ActionList} from '../ActionList'
811
import type {GroupedListProps, ListPropsBase, ItemInput} from '../SelectPanel/types'
12+
import {useFocusZone} from '../hooks/useFocusZone'
913
import {useId} from '../hooks/useId'
1014
import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
1115
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
@@ -14,13 +18,13 @@ import {VisuallyHidden} from '../VisuallyHidden'
1418
import type {SxProp} from '../sx'
1519
import type {FilteredActionListLoadingType} from './FilteredActionListLoaders'
1620
import {FilteredActionListLoadingTypes, FilteredActionListBodyLoader} from './FilteredActionListLoaders'
17-
import classes from './FilteredActionList.module.css'
18-
import {ActionListContainerContext} from '../ActionList/ActionListContainerContext'
1921

2022
import {isValidElementType} from 'react-is'
2123
import type {RenderItemFn} from '../deprecated/ActionList/List'
2224
import {useAnnouncements} from './useAnnouncements'
2325

26+
const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
27+
2428
export interface FilteredActionListProps
2529
extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
2630
ListPropsBase,
@@ -30,10 +34,10 @@ export interface FilteredActionListProps
3034
placeholderText?: string
3135
filterValue?: string
3236
onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
37+
onListContainerRefChanged?: (ref: HTMLElement | null) => void
3338
onInputRefChanged?: (ref: React.RefObject<HTMLInputElement>) => void
3439
textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
3540
inputRef?: React.RefObject<HTMLInputElement>
36-
message?: React.ReactNode
3741
className?: string
3842
announcementsEnabled?: boolean
3943
}
@@ -49,23 +53,19 @@ export function FilteredActionList({
4953
filterValue: externalFilterValue,
5054
loadingType = FilteredActionListLoadingTypes.bodySpinner,
5155
onFilterChange,
56+
onListContainerRefChanged,
5257
onInputRefChanged,
5358
items,
5459
textInputProps,
5560
inputRef: providedInputRef,
5661
sx,
5762
groupMetadata,
5863
showItemDividers,
59-
message,
6064
className,
61-
selectionVariant,
6265
announcementsEnabled = true,
6366
...listProps
6467
}: FilteredActionListProps): JSX.Element {
6568
const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
66-
const [enableAnnouncements, setEnableAnnouncements] = useState(false)
67-
const [selectedItems, setSelectedItems] = useState<(string | number | undefined)[]>([])
68-
6969
const onInputChange = useCallback(
7070
(e: React.ChangeEvent<HTMLInputElement>) => {
7171
const value = e.target.value
@@ -77,69 +77,70 @@ export function FilteredActionList({
7777

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

84-
const keydownListener = useCallback(
85-
(event: React.KeyboardEvent<HTMLDivElement>) => {
86-
if (event.key === 'ArrowDown') {
87-
if (listRef.current) {
88-
const firstSelectedItem = listRef.current.querySelector('[role="option"]') as HTMLElement | undefined
89-
firstSelectedItem?.focus()
90-
91-
event.preventDefault()
92-
}
93-
} else if (event.key === 'Enter') {
94-
let firstItem
95-
// If there are groups, it's not guaranteed that the first item is the actual first item in the first -
96-
// as groups are rendered in the order of the groupId provided
97-
if (groupMetadata) {
98-
const firstGroup = groupMetadata[0].groupId
99-
firstItem = items.filter(item => item.groupId === firstGroup)[0]
100-
} else {
101-
firstItem = items[0]
102-
}
103-
104-
if (firstItem.onAction) {
105-
firstItem.onAction(firstItem, event)
106-
event.preventDefault()
107-
}
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)
10893
}
10994
},
110-
[items, groupMetadata],
95+
[activeDescendantRef],
96+
)
97+
98+
const listContainerRefCallback = useCallback(
99+
(node: HTMLUListElement | null) => {
100+
setListContainerElement(node)
101+
onListContainerRefChanged?.(node)
102+
},
103+
[onListContainerRefChanged],
111104
)
112105

113106
useEffect(() => {
114107
onInputRefChanged?.(inputRef)
115108
}, [inputRef, onInputRefChanged])
116109

117-
useEffect(() => {
118-
if (items.length === 0) {
119-
inputRef.current?.focus()
120-
} else {
121-
const itemIds = items.filter(item => item.selected).map(item => item.id)
122-
const removedItem = selectedItems.find(item => !itemIds.includes(item))
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
123121

124-
if (removedItem && document.activeElement !== inputRef.current) {
125-
const list = listRef.current
126-
if (list) {
127-
const firstSelectedItem = list.querySelector('[role="option"]') as HTMLElement
128-
firstSelectedItem.focus()
122+
if (current && scrollContainerRef.current && directlyActivated) {
123+
scrollIntoView(current, scrollContainerRef.current, menuScrollMargins)
129124
}
130-
}
131-
}
132-
}, [items, inputRef, selectedItems])
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+
)
133132

134133
useEffect(() => {
135-
const selectedItemIds = items.filter(item => item.selected).map(item => item.id)
136-
setSelectedItems(selectedItemIds)
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+
}
137141
}, [items])
138142

139-
useEffect(() => {
140-
setEnableAnnouncements(announcementsEnabled)
141-
}, [announcementsEnabled])
142-
143+
useAnnouncements(items, {current: listContainerElement}, inputRef, announcementsEnabled)
143144
useScrollFlash(scrollContainerRef)
144145

145146
function getItemListForEachGroup(groupId: string) {
@@ -153,49 +154,6 @@ export function FilteredActionList({
153154
return itemsInGroup
154155
}
155156

156-
function getBodyContent() {
157-
if (loading && scrollContainerRef.current && loadingType.appearsInBody) {
158-
return <FilteredActionListBodyLoader loadingType={loadingType} height={scrollContainerRef.current.clientHeight} />
159-
}
160-
if (message) {
161-
return message
162-
}
163-
164-
return (
165-
<ActionListContainerContext.Provider
166-
value={{
167-
container: 'FilteredActionList',
168-
listRole: 'listbox',
169-
selectionAttribute: 'aria-selected',
170-
selectionVariant,
171-
enableFocusZone: true,
172-
}}
173-
>
174-
<ActionList ref={listRef} showDividers={showItemDividers} {...listProps} id={listId} sx={{flexGrow: 1}}>
175-
{groupMetadata?.length
176-
? groupMetadata.map((group, index) => {
177-
return (
178-
<ActionList.Group key={index}>
179-
<ActionList.GroupHeading variant={group.header?.variant ? group.header.variant : undefined}>
180-
{group.header?.title ? group.header.title : `Group ${group.groupId}`}
181-
</ActionList.GroupHeading>
182-
{getItemListForEachGroup(group.groupId).map((item, index) => {
183-
const key = item.key ?? item.id?.toString() ?? index.toString()
184-
return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
185-
})}
186-
</ActionList.Group>
187-
)
188-
})
189-
: items.map((item, index) => {
190-
const key = item.key ?? item.id?.toString() ?? index.toString()
191-
return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
192-
})}
193-
</ActionList>
194-
</ActionListContainerContext.Provider>
195-
)
196-
}
197-
useAnnouncements(items, listRef, inputRef, enableAnnouncements)
198-
199157
return (
200158
<Box
201159
display="flex"
@@ -213,7 +171,7 @@ export function FilteredActionList({
213171
color="fg.default"
214172
value={filterValue}
215173
onChange={onInputChange}
216-
onKeyDown={keydownListener}
174+
onKeyPress={onInputKeyPress}
217175
placeholder={placeholderText}
218176
role="combobox"
219177
aria-expanded="true"
@@ -227,9 +185,39 @@ export function FilteredActionList({
227185
/>
228186
</StyledHeader>
229187
<VisuallyHidden id={inputDescriptionTextId}>Items will be filtered as you type</VisuallyHidden>
230-
<div ref={scrollContainerRef} className={classes.Container}>
231-
{getBodyContent()}
232-
</div>
188+
<Box ref={scrollContainerRef} overflow="auto" display="flex" flexGrow={1}>
189+
{loading && scrollContainerRef.current && loadingType.appearsInBody ? (
190+
<FilteredActionListBodyLoader loadingType={loadingType} height={scrollContainerRef.current.clientHeight} />
191+
) : (
192+
<ActionList
193+
ref={listContainerRefCallback}
194+
showDividers={showItemDividers}
195+
{...listProps}
196+
role="listbox"
197+
id={listId}
198+
sx={{flexGrow: 1}}
199+
>
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+
)}
220+
</Box>
233221
</Box>
234222
)
235223
}

0 commit comments

Comments
 (0)