Skip to content

perf(TreeView): defer scrollIntoView to coalesce reflows during rapid navigation - #7545

Merged
hectahertz merged 5 commits into
mainfrom
hectahertz/perf-treeview-defer-scroll
Mar 12, 2026
Merged

perf(TreeView): defer scrollIntoView to coalesce reflows during rapid navigation#7545
hectahertz merged 5 commits into
mainfrom
hectahertz/perf-treeview-defer-scroll

Conversation

@hectahertz

@hectahertzhectahertz commented Feb 14, 2026

Copy link
Copy Markdown
Contributor

Closes #

Defers scrollIntoView calls in TreeView's onFocus handler using requestAnimationFrame, 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 scrollIntoView synchronously, 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 onFocus now 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 to Item components through RootContext.

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:

ApproachAvgMax
Sync (old)3.72ms4.80ms
rAF schedule (new)0.01ms0.10ms

50 rapid focus changes (held key simulation):

MetricSync (old)rAF coalesced (new)
Total time164ms0.1ms
Per-call cost3.28ms0.002ms
Actual scrollIntoView calls501 (deferred)
Reflows forced501

Changelog

New

N/A

Changed

  • scrollIntoView in TreeView item onFocus is now deferred via requestAnimationFrame, coalescing multiple scroll requests during rapid keyboard navigation into a single reflow per frame

Removed

N/A

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

  • All 51 existing TreeView tests pass unchanged
  • To manually verify: open the StressTest story, expand several directories, and hold ArrowDown. Focus should scroll smoothly without jank.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Storybook)
  • Changes are SSR compatible
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge
  • (GitHub staff only) Integration tests pass at github/github

@changeset-bot

changeset-botBot commented Feb 14, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8ed9ae7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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-actionsgithub-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Feb 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

👋 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 integration-tests: skipped manually label to skip these checks.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 scrollElementIntoView helper on RootContext that schedules (and cancels) scrollIntoView via requestAnimationFrame.
  • Updated TreeView item onFocus handling to use the deferred scroll helper instead of calling scrollIntoView synchronously.
  • 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.

FileDescription
packages/react/src/TreeView/TreeView.tsxDefers and coalesces focus-driven scrolling via rAF and exposes the helper through RootContext.
.changeset/treeview-defer-scroll.mdAdds a patch changeset entry describing the TreeView perf improvement.

Comment threadpackages/react/src/TreeView/TreeView.tsx
@hectahertz

Copy link
Copy Markdown
ContributorAuthor

Friendly ping for review: @jonrohan@siddharthkp@TylerJDev 🙏

@jonrohanjonrohan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems ok to me, but don't feel like I know a ton about react animation frames.

@github-actions
github-actionsBottemporarily deployed to storybook-preview-7545 March 12, 2026 17:06 Inactive
@hectahertzhectahertz added the integration-tests: skipped manually Changes in this PR do not require an integration test label Mar 12, 2026
@hectahertz
hectahertz added this pull request to the merge queueMar 12, 2026
Merged via the queue into main with commit 17ec46fMar 12, 2026
55 of 56 checks passed
@hectahertz
hectahertz deleted the hectahertz/perf-treeview-defer-scroll branch March 12, 2026 18:17
@primerprimerBot mentioned this pull request Mar 12, 2026
@primer-integration

Copy link
Copy Markdown

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/15933

@primer-integration

Copy link
Copy Markdown

Integration test results from github/github-ui:

Passed CI  Passed
Waiting VRT   Waiting
Passed Projects  Passed

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommendedThis change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpmintegration-tests: skipped manuallyChanges in this PR do not require an integration test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@hectahertz@jonrohan@siddharthkp