Skip to content
Open
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/fruity-groups-brush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Added callback prop onActiveDescendantChanged to FilterActionList'
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in changeset message: "FilterActionList" should be "FilteredActionList" (missing 'ed').

Suggested change
Added callback prop onActiveDescendantChanged to FilterActionList'
Added callback prop onActiveDescendantChanged to FilteredActionList

Copilot uses AI. Check for mistakes.
18 changes: 9 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 16 additions & 2 deletions packages/react/src/FilteredActionList/FilteredActionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,18 @@ export interface FilteredActionListProps extends Partial<Omit<GroupedListProps,
* @default 'wrap'
*/
focusOutBehavior?: 'stop' | 'wrap'
/**
* Callback function that is called whenever the active descendant changes.
*
* @param newActiveDescendant - The new active descendant element.
* @param previousActiveDescendant - The previous active descendant element.
* @param directlyActivated - Whether the active descendant was directly activated (e.g., by a keyboard event).
*/
onActiveDescendantChanged?: (
newActiveDescendant: HTMLElement | undefined,
previousActiveDescendant: HTMLElement | undefined,
directlyActivated: boolean,
) => void
/**
* Private API for use internally only. Adds the ability to switch between
* `active-descendant` and roving tabindex.
Expand Down Expand Up @@ -106,6 +118,7 @@ export function FilteredActionList({
actionListProps,
focusOutBehavior = 'wrap',
_PrivateFocusManagement = 'active-descendant',
onActiveDescendantChanged,
...listProps
}: FilteredActionListProps): JSX.Element {
const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
Expand Down Expand Up @@ -228,14 +241,15 @@ export function FilteredActionList({
activeDescendantFocus: inputRef,
onActiveDescendantChanged: (current, previous, directlyActivated) => {
activeDescendantRef.current = current

if (current && scrollContainerRef.current && directlyActivated) {
scrollIntoView(current, scrollContainerRef.current, menuScrollMargins)
}

onActiveDescendantChanged?.(current, previous, directlyActivated)
},
}
: undefined,
[listContainerElement, usingRovingTabindex],
[listContainerElement, usingRovingTabindex, onActiveDescendantChanged],
)

useEffect(() => {
Expand Down
93 changes: 48 additions & 45 deletions packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useState, useMemo, useRef, useEffect} from 'react'
import React, {useState, useMemo, useRef, useEffect, useCallback} from 'react'
import type {Meta} from '@storybook/react-vite'
import {Button} from '../Button'
import type {ItemInput} from '../FilteredActionList'
Expand Down Expand Up @@ -601,6 +601,7 @@ export const Virtualized = () => {
const [renderSubset, setRenderSubset] = useState(true)

const [filter, setFilter] = useState('')
// const filterUpdatedRef = useRef(false)
const [scrollContainer, setScrollContainer] = useState<HTMLDivElement | null>(null)
const filteredItems = lotsOfItems.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))

Expand All @@ -627,54 +628,14 @@ export const Virtualized = () => {
count: filteredItems.length,
getScrollElement: () => scrollContainer ?? null,
estimateSize: () => DEFAULT_VIRTUAL_ITEM_HEIGHT,
overscan: 10,
overscan: 5,
enabled: renderSubset,
getItemKey: index => filteredItems[index].id,
measureElement: el => {
return (el as HTMLElement).scrollHeight
},
})

const virtualizedContainerStyle = useMemo(
() =>
renderSubset
? {
height: virtualizer.getTotalSize(),
width: '100%',
position: 'relative' as const,
}
: undefined,
[renderSubset, virtualizer],
)

const virtualizedItems = useMemo(
() =>
renderSubset
? virtualizer.getVirtualItems().map((virtualItem: VirtualItem) => {
const item = filteredItems[virtualItem.index]

return {
...item,
key: virtualItem.index,
'data-index': virtualItem.index,
ref: (node: Element | null) => {
if (node && node.getAttribute('data-index')) {
virtualizer.measureElement(node)
}
},
style: {
position: 'absolute',
top: 0,
left: 0,
width: '100%',
height: `${virtualItem.size}px`,
transform: `translateY(${virtualItem.start}px)`,
},
}
})
: filteredItems,
[renderSubset, virtualizer, filteredItems],
)

return (
<form>
<FormControl>
Expand Down Expand Up @@ -709,7 +670,32 @@ export const Virtualized = () => {
)}
open={open}
onOpenChange={onOpenChange}
items={virtualizedItems}
items={
renderSubset
? virtualizer.getVirtualItems().map((virtualItem: VirtualItem) => {
const item = filteredItems[virtualItem.index]

return {
...item,
key: item.id,
'data-index': virtualItem.index,
ref: (node: Element | null) => {
if (node && node.getAttribute('data-index')) {
virtualizer.measureElement(node)
}
},
style: {
position: 'absolute',
top: 0,
left: 0,
width: '100%',
height: `${virtualItem.size}px`,
transform: `translateY(${virtualItem.start}px)`,
},
}
})
: filteredItems
}
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
Expand All @@ -719,10 +705,27 @@ export const Virtualized = () => {
overlayProps={{
id: 'select-labels-panel-dialog',
}}
onActiveDescendantChanged={useCallback(
(newActivedescendant: HTMLElement | undefined) => {
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameter name has inconsistent casing: newActivedescendant should use camelCase as newActiveDescendant (capital 'D' in 'Descendant') to match the parameter names in the callback signature and TypeScript naming conventions.

Copilot generated this review using guidance from repository custom instructions.
const index = newActivedescendant?.getAttribute('data-index')
const range = virtualizer.range
if (newActivedescendant === undefined) return
Comment on lines +710 to +712
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant check: The condition checks newActivedescendant === undefined on line 718 and returns early, making the null-safe optional chaining on line 720 (newActivedescendant?.getAttribute) unnecessary. After the early return, newActivedescendant is guaranteed to be defined. Consider using newActivedescendant.getAttribute('data-index') directly on line 720 for clearer logic.

Suggested change
const index = newActivedescendant?.getAttribute('data-index')
const range = virtualizer.range
if (newActivedescendant === undefined) return
if (newActivedescendant === undefined) return
const index = newActivedescendant.getAttribute('data-index')
const range = virtualizer.range

Copilot uses AI. Check for mistakes.
if (index && range && (Number(index) < range.startIndex || Number(index) >= range.endIndex)) {
virtualizer.scrollToIndex(Number(newActivedescendant.getAttribute('data-index')), {align: 'auto'})
}
},
[virtualizer],
)}
focusOutBehavior="stop"
scrollContainerRef={node => setScrollContainer(node)}
actionListProps={{
style: virtualizedContainerStyle,
style: renderSubset
? {
height: virtualizer.getTotalSize(),
width: '100%',
position: 'relative' as const,
}
: undefined,
}}
/>
</FormControl>
Expand Down
13 changes: 13 additions & 0 deletions packages/react/src/SelectPanel/SelectPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,19 @@ for (const usingRemoveActiveDescendant of [false, true]) {
expect(trigger).toHaveAttribute('aria-expanded', 'false')
})

it('should call onActiveDescendantChanged when using keyboard while focusing on an item', async () => {
const user = userEvent.setup()
// jest function
const onActiveDescendantChanged = vi.fn()

render(<BasicSelectPanel onActiveDescendantChanged={onActiveDescendantChanged} />)

await user.click(screen.getByText('Select items'))

await user.type(document.activeElement!, '{ArrowDown}')
expect(onActiveDescendantChanged).toHaveBeenCalled()
})

it('should open the select panel when activating the trigger', async () => {
const user = userEvent.setup()

Expand Down
Loading