-
Notifications
You must be signed in to change notification settings - Fork 652
perf(SelectPanel): Built-in client-side list virtualization via virtualized prop
#7531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…virtualized` prop Add a `virtualized` boolean prop to SelectPanel/FilteredActionList that enables client-side list virtualization using @tanstack/react-virtual (already a dependency). When enabled, only the visible items plus a small overscan buffer are rendered in the DOM, dramatically improving performance for large lists. - Add `virtualized` prop to FilteredActionListProps with JSDoc - Wire up useVirtualizer with scroll container, dynamic measurement, overscan=10 - Render virtual items with absolute positioning inside sized container - Handle focus zone scroll-to-index for keyboard navigation of virtual items - Thread `virtualized` prop through SelectPanel to FilteredActionList - Add VirtualizedBuiltIn comparison story (side-by-side with timing) - Rename existing consumer-side virtualization story for clarity
🦋 Changeset detectedLatest commit: bf26e4b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds an opt-in virtualized prop to SelectPanel / FilteredActionList to enable client-side list virtualization (via @tanstack/react-virtual) for large datasets, plus Storybook examples to compare performance.
Changes:
- Plumbs a new
virtualizedprop throughSelectPaneltoFilteredActionList. - Implements virtualization in
FilteredActionListusinguseVirtualizerand absolute-positioned list items. - Updates Storybook examples: renames the existing consumer-managed virtualization story and adds a built-in virtualization comparison story.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/react/src/SelectPanel/SelectPanel.tsx | Forwards the new virtualized prop to FilteredActionList. |
| packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx | Renames the existing virtualization story and adds a new side-by-side “built-in virtualization” story. |
| packages/react/src/FilteredActionList/FilteredActionList.tsx | Introduces the virtualized prop and the core @tanstack/react-virtual integration. |
virtualized propvirtualized prop
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/13534 |
francinelucca
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧑🍳 💋. Some non-blocking comments/questions, but otherwise ![]()
| * | ||
| * @default false | ||
| */ | ||
| virtualized?: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO:
- add to SelectPanel.docs.json
| containerRef: {current: listContainerElement}, | ||
| bindKeys: FocusKeys.ArrowVertical | FocusKeys.PageUpDown, | ||
| focusOutBehavior, | ||
| focusOutBehavior: virtualized ? 'stop' : focusOutBehavior, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious why this is needed 👀
| if (groupMetadata?.length) { | ||
| return groupMetadata.map((group, index) => { | ||
| if (index === firstGroupIndex && getItemListForEachGroup(group.groupId).length === 0) { | ||
| firstGroupIndex++ | ||
| } | ||
| 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this mean the virtualized option won't work with groups? if so we should call that out in the prop info
| ? { | ||
| height: virtualizer.getTotalSize(), | ||
| width: '100%', | ||
| position: 'relative' as const, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do the 'height' and 'width' SelectPanel props still work alongside these?
See https://github.com/github/primer/discussions/6407
Overview
Adds a
virtualizedboolean prop toSelectPanel(andFilteredActionList) that enables client-side list virtualization using@tanstack/react-virtual(already a dependency).When enabled, only the visible items plus a small overscan buffer are rendered in the DOM. This is a purely client-side optimization — it does not require server-side pagination or API changes. The consumer can still pass all items at once.
Usage
Performance measurements (1,800 items)
Measured using Chrome DevTools Performance traces and PerformanceObserver API on the
VirtualizedBuiltInstory:Screen.Recording.2026-02-12.at.12.36.52.mov
Open time
[role="option"]elementsFiltering (typing "Item" with panel open)
Changelog
New
virtualizedprop onSelectPanelandFilteredActionList— enables client-side list virtualizationVirtualizedBuiltInstory — side-by-side comparison with open-time measurementChanged
VirtualizedConsumerSidefor clarityRemoved
Rollout strategy
Testing & Reviewing
VirtualizedBuiltInstory in Storybook (Components/SelectPanel/Examples)Implementation details
useVirtualizerfrom@tanstack/react-virtualinsideFilteredActionListscrollContainerReffor the virtualizer scroll elementestimateSize: 49pxwith dynamicmeasureElementfor accurate variable-height itemsoverscan: 10items beyond the viewport for smooth scrollingfocusOutBehavior: "stop"when virtualized to prevent focus wrapping past virtual boundariesscrollToIndexin the focus zone when keyboard focus moves to an item outside the visible rangegroupMetadata) are not virtualized — they are typically small enough not to need itMerge checklist