Skip to content
Closed
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
10 changes: 10 additions & 0 deletions .changeset/perf-filtered-action-list-deferred.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@primer/react': patch
---

perf(FilteredActionList): Use useDeferredValue to improve typing responsiveness with large lists

- Add `useDeferredValue` for items to defer expensive list re-rendering
- Keep text input immediately responsive during typing
- Use deferred items for rendering while maintaining immediate items for user interactions
- SelectPanel automatically inherits these performance improvements
259 changes: 149 additions & 110 deletions packages/react/src/FilteredActionList/FilteredActionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type {ScrollIntoViewOptions} from '@primer/behaviors'
import {scrollIntoView, FocusKeys} from '@primer/behaviors'
import type {KeyboardEventHandler, JSX} from 'react'
import type React from 'react'
import {forwardRef, useCallback, useEffect, useRef, useState} from 'react'
import {forwardRef, memo, useCallback, useDeferredValue, useEffect, useRef, useState} from 'react'
import type {TextInputProps} from '../TextInput'
import TextInput from '../TextInput'
import {ActionList, type ActionListProps} from '../ActionList'
Expand All @@ -24,6 +24,128 @@ import {clsx} from 'clsx'

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

const MappedActionListItem = forwardRef<HTMLLIElement, ItemInput & {renderItem?: RenderItemFn}>((item, ref) => {
// keep backward compatibility for renderItem
// escape hatch for custom Item rendering
if (typeof item.renderItem === 'function') return item.renderItem(item)

const {
id,
description,
descriptionVariant,
text,
trailingVisual: TrailingVisual,
leadingVisual: LeadingVisual,
trailingText,
trailingIcon: TrailingIcon,
onAction,
children,
...rest
} = item

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}
ref={ref}
{...rest}
>
{LeadingVisual ? (
<ActionList.LeadingVisual>
<LeadingVisual />
</ActionList.LeadingVisual>
) : null}
{children}
{text}
{description ? <ActionList.Description variant={descriptionVariant}>{description}</ActionList.Description> : null}
{TrailingVisual ? (
<ActionList.TrailingVisual>
{typeof TrailingVisual !== 'string' && isValidElementType(TrailingVisual) ? (
<TrailingVisual />
) : (
TrailingVisual
)}
</ActionList.TrailingVisual>
) : TrailingIcon || trailingText ? (
<ActionList.TrailingVisual>
{trailingText}
{TrailingIcon && <TrailingIcon />}
</ActionList.TrailingVisual>
) : null}
</ActionList.Item>
)
})

/**
* Memoized component that renders the list items.
* Using React.memo allows React to skip re-rendering when deferredItems hasn't changed yet,
* keeping the input responsive during typing.
*/
interface FilteredActionListItemsProps {
deferredItems: ItemInput[]
groupMetadata?: GroupedListProps['groupMetadata']
getItemListForEachGroup: (groupId: string, itemsList: ItemInput[]) => ItemInput[]
isInputFocused: boolean
renderItem?: RenderItemFn
}

const FilteredActionListItems = memo<FilteredActionListItemsProps>(
({deferredItems, groupMetadata, getItemListForEachGroup, isInputFocused, renderItem}) => {
let firstGroupIndex = 0

return (
<>
{groupMetadata?.length
? groupMetadata.map((group, index) => {
if (index === firstGroupIndex && getItemListForEachGroup(group.groupId, deferredItems).length === 0) {
firstGroupIndex++ // Increment firstGroupIndex if the first group has no items
}
return (
<ActionList.Group key={index}>
<ActionList.GroupHeading variant={group.header?.variant ? group.header.variant : undefined}>
{group.header?.title ? group.header.title : `Group ${group.groupId}`}
</ActionList.GroupHeading>
{getItemListForEachGroup(group.groupId, deferredItems).map(({key: itemKey, ...item}, itemIndex) => {
const key = itemKey ?? item.id?.toString() ?? itemIndex.toString()
return (
<MappedActionListItem
key={key}
className={clsx(classes.ActionListItem, 'className' in item ? item.className : undefined)}
data-input-focused={isInputFocused ? '' : undefined}
data-first-child={index === firstGroupIndex && itemIndex === 0 ? '' : undefined}
{...item}
renderItem={renderItem}
/>
)
})}
</ActionList.Group>
)
})
: deferredItems.map(({key: itemKey, ...item}, index) => {
const key = itemKey ?? item.id?.toString() ?? index.toString()
return (
<MappedActionListItem
key={key}
className={clsx(classes.ActionListItem, 'className' in item ? item.className : undefined)}
data-input-focused={isInputFocused ? '' : undefined}
data-first-child={index === 0 ? '' : undefined}
{...item}
renderItem={renderItem}
/>
)
})}
</>
)
},
)

FilteredActionListItems.displayName = 'FilteredActionListItems'

export interface FilteredActionListProps extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>, ListPropsBase {
loading?: boolean
loadingType?: FilteredActionListLoadingType
Expand Down Expand Up @@ -133,6 +255,7 @@ export function FilteredActionList({
...listProps
}: FilteredActionListProps): JSX.Element {
const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')

const onInputChange = useCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
const value = e.target.value
Expand All @@ -142,6 +265,10 @@ export function FilteredActionList({
[onFilterChange, setInternalFilterValue],
)

// Use deferred value for items to avoid blocking typing during list re-rendering
// This allows the input to remain responsive while the list is being re-rendered with new items
const deferredItems = useDeferredValue(items)

const inputAndListContainerRef = useRef<HTMLDivElement>(null)
const listRef = useRef<HTMLUListElement>(null)

Expand All @@ -163,19 +290,19 @@ export function FilteredActionList({

const selectAllLabelText = selectAllChecked ? 'Deselect all' : 'Select all'

const getItemListForEachGroup = useCallback(
(groupId: string) => {
const itemsInGroup = []
for (const item of items) {
// Look up the group associated with the current item.
if (item.groupId === groupId) {
itemsInGroup.push(item)
}
// Helper function to get items in a specific group
// Takes itemsList as parameter to work with both immediate and deferred items
// Empty dependency array is correct since the function doesn't close over external variables
const getItemListForEachGroup = useCallback((groupId: string, itemsList: ItemInput[]) => {
const itemsInGroup = []
for (const item of itemsList) {
// Look up the group associated with the current item.
if (item.groupId === groupId) {
itemsInGroup.push(item)
}
return itemsInGroup
},
[items],
)
}
return itemsInGroup
}, [])

const onInputKeyDown = useCallback(
(event: React.KeyboardEvent<HTMLInputElement>) => {
Expand All @@ -194,7 +321,7 @@ export function FilteredActionList({
let firstGroupIndex = 0

for (let i = 0; i < groupMetadata.length; i++) {
if (getItemListForEachGroup(groupMetadata[i].groupId).length > 0) {
if (getItemListForEachGroup(groupMetadata[i].groupId, items).length > 0) {
break
} else {
firstGroupIndex++
Expand Down Expand Up @@ -329,7 +456,6 @@ export function FilteredActionList({
if (message) {
return message
}
let firstGroupIndex = 0
const actionListContent = (
<ActionList
ref={usingRovingTabindex ? listRef : listContainerRefCallback}
Expand All @@ -341,45 +467,13 @@ export function FilteredActionList({
id={listId}
className={clsx(classes.ActionList, actionListProps?.className)}
>
{groupMetadata?.length
? groupMetadata.map((group, index) => {
if (index === firstGroupIndex && getItemListForEachGroup(group.groupId).length === 0) {
firstGroupIndex++ // Increment firstGroupIndex if the first group has no items
}
return (
<ActionList.Group key={index}>
<ActionList.GroupHeading variant={group.header?.variant ? group.header.variant : undefined}>
{group.header?.title ? group.header.title : `Group ${group.groupId}`}
</ActionList.GroupHeading>
{getItemListForEachGroup(group.groupId).map(({key: itemKey, ...item}, itemIndex) => {
const key = itemKey ?? item.id?.toString() ?? itemIndex.toString()
return (
<MappedActionListItem
key={key}
className={clsx(classes.ActionListItem, 'className' in item ? item.className : undefined)}
data-input-focused={isInputFocused ? '' : undefined}
data-first-child={index === firstGroupIndex && itemIndex === 0 ? '' : undefined}
{...item}
renderItem={listProps.renderItem}
/>
)
})}
</ActionList.Group>
)
})
: items.map(({key: itemKey, ...item}, index) => {
const key = itemKey ?? item.id?.toString() ?? index.toString()
return (
<MappedActionListItem
key={key}
className={clsx(classes.ActionListItem, 'className' in item ? item.className : undefined)}
data-input-focused={isInputFocused ? '' : undefined}
data-first-child={index === 0 ? '' : undefined}
{...item}
renderItem={listProps.renderItem}
/>
)
})}
<FilteredActionListItems
deferredItems={deferredItems}
groupMetadata={groupMetadata}
getItemListForEachGroup={getItemListForEachGroup}
isInputFocused={isInputFocused}
renderItem={listProps.renderItem}
/>
</ActionList>
)

Expand Down Expand Up @@ -448,66 +542,11 @@ export function FilteredActionList({
)}
{/* @ts-expect-error div needs a non nullable ref */}
<div ref={scrollContainerRef} className={classes.Container}>
{/* eslint-disable-next-line react-hooks/refs -- getBodyContent conditionally accesses scrollContainerRef.current during render for loading indicator height calculation */}
{getBodyContent()}
</div>
</div>
)
}
const MappedActionListItem = forwardRef<HTMLLIElement, ItemInput & {renderItem?: RenderItemFn}>((item, ref) => {
// keep backward compatibility for renderItem
// escape hatch for custom Item rendering
if (typeof item.renderItem === 'function') return item.renderItem(item)

const {
id,
description,
descriptionVariant,
text,
trailingVisual: TrailingVisual,
leadingVisual: LeadingVisual,
trailingText,
trailingIcon: TrailingIcon,
onAction,
children,
...rest
} = item

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}
ref={ref}
{...rest}
>
{LeadingVisual ? (
<ActionList.LeadingVisual>
<LeadingVisual />
</ActionList.LeadingVisual>
) : null}
{children}
{text}
{description ? <ActionList.Description variant={descriptionVariant}>{description}</ActionList.Description> : null}
{TrailingVisual ? (
<ActionList.TrailingVisual>
{typeof TrailingVisual !== 'string' && isValidElementType(TrailingVisual) ? (
<TrailingVisual />
) : (
TrailingVisual
)}
</ActionList.TrailingVisual>
) : TrailingIcon || trailingText ? (
<ActionList.TrailingVisual>
{trailingText}
{TrailingIcon && <TrailingIcon />}
</ActionList.TrailingVisual>
) : null}
</ActionList.Item>
)
})

FilteredActionList.displayName = 'FilteredActionList'
Loading