Skip to content

perf(PageLayout): eliminate per-frame React renders during window resize#7847

Draft
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-pane-width-update
Draft

perf(PageLayout): eliminate per-frame React renders during window resize#7847
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-pane-width-update

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

During fast window resizing, usePaneWidth called startTransition(setState) on every throttled tick (~60 fps), stacking render requests that caused the pane DOM to visually "catch up" after the gesture ended.

What changed

packages/react/src/PageLayout/usePaneWidth.ts

  • Split syncAll() into two functions:
    • syncDom() — mutates CSS custom properties (--pane-max-width, --pane-width), clamps currentWidthRef, and updates ARIA attributes via direct DOM manipulation. Zero React state changes.
    • commitToReact() — called exactly once at gesture end; flushes pending values into React state via startTransition only if they actually changed.
  • Replaced the Date.now() throttle + rAF-fallback pattern (which could fire syncAll twice per frame on bursty input) with a single requestAnimationFrame coalesce: if a rAF is already pending, new resize events are dropped until it fires.
  • The debounce callback now runs syncDom() (guaranteed final-frame sync) → commitToReact() (the one React commit for the whole gesture) → endResizeOptimizations().
  • Added pendingMaxRef / pendingClampedRef to carry values across frames into the commit.
handleResize:
  startResizeOptimizations()          // synchronous, sets data-dragging
  if (rafId === null) {
    rafId = rAF(() => { syncDom() })  // DOM only, coalesced per frame
  }
  resetDebounce(() => {
    endResizeOptimizations()
    syncDom()          // guarantee final frame
    commitToReact()    // single startTransition for the whole gesture
  }, 150)

packages/react/src/PageLayout/usePaneWidth.test.ts

  • Updated test names/comments to reflect rAF-coalescing semantics.
  • Added a test that asserts startTransition is called zero times while resize events are firing (pre-debounce) and exactly once when the gesture ends.

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

The key behavioral invariants are unchanged — --pane-max-width/--pane-width CSS variables and ARIA attributes continue to track the viewport in real time; only the React state commit is deferred. The new test exercises the zero-renders-during-gesture / one-commit-at-end contract directly.

Custom widths with constrainToViewport === false still bail out of the resize listener early (untouched). The drag-handle path is untouched.

Merge checklist

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 18, 2026

🦋 Changeset detected

Latest commit: 71cb606

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

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

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

Copilot AI and others added 2 commits May 18, 2026 02:54
…PaneWidth

Split syncAll() into syncDom() (DOM/refs/ARIA, called each rAF frame) and
commitToReact() (single startTransition at gesture end). Replace the
Date.now() throttle + rAF fallback with a single rAF coalesce — rAF already
guarantees at most one call per frame, and the throttle was the source of
double-fire on bursty input. The debounce callback now also calls syncDom()
for a guaranteed final frame and commitToReact() for the one React commit
per gesture.

Add pendingMaxRef / pendingClampedRef to carry values across frames into the
commit. Update test descriptions and add a new assertion that verifies zero
startTransition calls during resize frames and exactly one at gesture end.

Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
…TimeAsync(149/1) split

Use 149ms advance (flushes rAF, misses debounce) + 1ms advance (fires debounce)
to clearly prove zero startTransition calls during resize and exactly one at end.
Removes the nonexistent vi.runAllImmediateAsync optional-chaining guard.

Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix lag in PageLayout pane updates during quick window resizing perf(PageLayout): eliminate per-frame React renders during window resize May 18, 2026
Copilot AI requested a review from mattcosta7 May 18, 2026 02:56
@github-actions github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label May 22, 2026
@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 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. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

@github-actions github-actions Bot requested a deployment to storybook-preview-7847 May 22, 2026 13:28 Abandoned
@mattcosta7 mattcosta7 added the Canary Release Apply this label when you want CI to create a canary release of the current PR label May 22, 2026
@primer-integration
Copy link
Copy Markdown

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

@primer-integration
Copy link
Copy Markdown

Integration test results from github/github-ui:

Passed  CI   Passed
Passed  VRT   Passed
Passed  Projects   Passed

All checks passed!

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

Labels

Canary Release Apply this label when you want CI to create a canary release of the current PR integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants