perf(TreeView): defer scrollIntoView to coalesce reflows during rapid navigation#7545
perf(TreeView): defer scrollIntoView to coalesce reflows during rapid navigation#7545hectahertz wants to merge 2 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: a0368de 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 |
github-actions
bot
added
the
integration-tests: recommended
|
👋 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.
Pull request overview
This PR improves TreeView keyboard-navigation performance by deferring scrollIntoView work to requestAnimationFrame, so multiple rapid focus changes coalesce into a single scroll/reflow per frame.
Changes:
- Added a
scrollElementIntoViewhelper onRootContextthat schedules (and cancels)scrollIntoViewviarequestAnimationFrame. - Updated TreeView item
onFocushandling to use the deferred scroll helper instead of callingscrollIntoViewsynchronously. - Added a patch changeset documenting the perf change.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/react/src/TreeView/TreeView.tsx | Defers and coalesces focus-driven scrolling via rAF and exposes the helper through RootContext. |
| .changeset/treeview-defer-scroll.md | Adds a patch changeset entry describing the TreeView perf improvement. |
| pendingScrollRef.current = null | ||
| element.scrollIntoView({block: 'nearest', inline: 'nearest'}) | ||
| }) | ||
| }, []) | ||
|
|
There was a problem hiding this comment.
scrollElementIntoView schedules a rAF callback but there’s no unmount cleanup. If the TreeView unmounts (or items are removed) before the frame fires, the callback can still run and call scrollIntoView on a detached element. Consider adding a useEffect cleanup that cancels any pending animation frame (and clears the ref), and optionally guarding the callback with if (!element.isConnected) return before calling scrollIntoView.
| pendingScrollRef.current = null | |
| element.scrollIntoView({block: 'nearest', inline: 'nearest'}) | |
| }) | |
| }, []) | |
| pendingScrollRef.current = null | |
| // Guard against calling scrollIntoView on a detached element | |
| if (!('isConnected' in element) || !(element as Element & {isConnected: boolean}).isConnected) { | |
| return | |
| } | |
| element.scrollIntoView({block: 'nearest', inline: 'nearest'}) | |
| }) | |
| }, []) | |
| useEffect(() => { | |
| return () => { | |
| if (pendingScrollRef.current !== null) { | |
| cancelAnimationFrame(pendingScrollRef.current) | |
| pendingScrollRef.current = null | |
| } | |
| } | |
| }, []) |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/14144 |
|
Friendly ping for review: @jonrohan @siddharthkp @TylerJDev 🙏 |
jonrohan
left a comment
jonrohan
left a comment
There was a problem hiding this comment.
Seems ok to me, but don't feel like I know a ton about react animation frames.
Closes #
Defers
scrollIntoViewcalls in TreeView'sonFocushandler usingrequestAnimationFrame, so rapid keyboard navigation (held arrow key) coalesces multiple scroll requests into a single reflow per frame instead of one per keystroke.Previously, every focus change called
scrollIntoViewsynchronously, forcing the browser to compute layout for the entire DOM. With large trees (26K items, 289K DOM nodes), this cost ~3.7ms per call, adding up to 164ms of blocked main thread time across 50 rapid keypresses.Each
onFocusnow schedules a scroll via rAF. If a new focus event fires before the frame callback runs, the previous one is cancelled. Only the final focused element's scroll actually executes. Exposed toItemcomponents throughRootContext.This preserves the existing behavior for single-focus scenarios (tab into tree, click, single arrow press). The scroll still happens, just one frame later.
Benchmarks (26,000 items, 289K DOM nodes)
scrollIntoView cost per call:
50 rapid focus changes (held key simulation):
Changelog
New
N/A
Changed
scrollIntoViewin TreeView itemonFocusis now deferred viarequestAnimationFrame, coalescing multiple scroll requests during rapid keyboard navigation into a single reflow per frameRemoved
N/A
Rollout strategy
Testing & Reviewing
Merge checklist