Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 29, 2025

The window resize handler was toggling data-dragging containment attributes ~30-60 times per second during continuous resize, applying and removing them on every throttled update.

Changes

Modified usePaneWidth.ts:

  • Added DEBOUNCE_MS = 150 constant for cleanup delay
  • Added debounceId to track debounce timer
  • Removed immediate endResizeOptimizations() calls from throttle logic
  • Debounce cleanup: containment removed 150ms after resize stops
  • Clear debounce timer in cleanup function

Updated usePaneWidth.test.ts:

  • Modified containment attribute test to verify debounced behavior
  • Updated unmount test to confirm immediate cleanup

Behavior

Before:

const handleResize = () => {
  startResizeOptimizations()  // Sets data-dragging="true"
  if (now - lastUpdateTime >= THROTTLE_MS) {
    syncAll()
    endResizeOptimizations()  // Removes immediately
  }
}

After:

  • Apply containment once on first resize event
  • Keep applied during continuous resize
  • Remove once 150ms after resize stops

Changelog

Changed

  • Window resize containment cleanup now debounced (150ms) instead of immediate

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

Test by continuously resizing browser window with PageLayout visible. Containment attributes should remain stable during resize and clean up after stopping.

Merge checklist

Original prompt

Debounce window resize containment cleanup in PageLayout

Problem

In usePaneWidth.ts, the window resize handler currently applies and removes the data-dragging containment attribute on every resize sync:

const handleResize = () => {
  startResizeOptimizations()  // Sets data-dragging="true"
  
  if (now - lastUpdateTime >= THROTTLE_MS) {
    syncAll()
    endResizeOptimizations()  // Removes data-dragging immediately
  } else if (!pendingUpdate) {
    rafId = requestAnimationFrame(() => {
      syncAll()
      endResizeOptimizations()  // Removes data-dragging in rAF
    })
  }
}

During continuous window resize (user dragging browser edge), this causes the attribute to be toggled ~30-60 times per second. While each toggle is cheap (O(1) selector), it's unnecessary overhead.

Solution

Apply containment once when resize starts, remove once after resize stops (debounced):

const THROTTLE_MS = 16
const DEBOUNCE_MS = 150
let debounceId: ReturnType<typeof setTimeout> | null = null

const handleResize = () => {
  // Apply containment on first resize event (stays applied until resize stops)
  startResizeOptimizations()

  const now = Date.now()
  if (now - lastUpdateTime >= THROTTLE_MS) {
    lastUpdateTime = now
    syncAll()
  } else if (!pendingUpdate) {
    pendingUpdate = true
    rafId = requestAnimationFrame(() => {
      pendingUpdate = false
      rafId = null
      lastUpdateTime = Date.now()
      syncAll()
    })
  }

  // Debounce the cleanup — remove containment after resize stops
  if (debounceId !== null) clearTimeout(debounceId)
  debounceId = setTimeout(() => {
    debounceId = null
    endResizeOptimizations()
  }, DEBOUNCE_MS)
}

// Update cleanup to clear debounce timer
return () => {
  if (rafId !== null) cancelAnimationFrame(rafId)
  if (debounceId !== null) clearTimeout(debounceId)
  endResizeOptimizations()
  window.removeEventListener('resize', handleResize)
}

Changes needed

  1. In packages/react/src/PageLayout/usePaneWidth.ts:

    • Add DEBOUNCE_MS = 150 constant
    • Add debounceId variable to track the cleanup timer
    • Remove endResizeOptimizations() calls from inside the throttle logic
    • Add debounced endResizeOptimizations() call at end of handleResize
    • Update cleanup function to clear debounceId
  2. Update tests in packages/react/src/PageLayout/usePaneWidth.test.ts if needed to account for the debounced cleanup behavior

Result

  • Containment is applied once when resize starts
  • Containment is removed once 150ms after resize stops
  • No toggling during continuous resize
  • Cleaner, more correct behavior

This pull request was created from Copilot chat.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@changeset-bot
Copy link

changeset-bot bot commented Dec 29, 2025

⚠️ No Changeset found

Latest commit: 143acfe

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@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 Dec 29, 2025
@github-actions
Copy link
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.

Copilot AI changed the title [WIP] Debounce window resize containment cleanup in PageLayout Debounce window resize containment cleanup in PageLayout Dec 29, 2025
Copilot AI requested a review from mattcosta7 December 29, 2025 19:18
@mattcosta7 mattcosta7 marked this pull request as ready for review December 29, 2025 19:22
@mattcosta7 mattcosta7 requested a review from a team as a code owner December 29, 2025 19:22
@mattcosta7 mattcosta7 requested a review from jonrohan December 29, 2025 19:22
@mattcosta7 mattcosta7 merged commit 5c0533c into second-optimization-pass-for-page-layout Dec 29, 2025
42 of 43 checks passed
@mattcosta7 mattcosta7 deleted the copilot/debounce-window-resize-cleanup branch December 29, 2025 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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