-
Notifications
You must be signed in to change notification settings - Fork 649
resize-improvemet #7310
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
resize-improvemet #7310
Conversation
|
|
👋 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
This PR optimizes the PageLayout pane resize handling by implementing a two-stage approach: throttled CSS variable updates (16ms) for immediate visual feedback during resize, and debounced synchronization (150ms) of refs, ARIA attributes, and React state when resizing stops. The changes improve performance by reducing expensive operations (getComputedStyle calls, React re-renders) while maintaining visual responsiveness.
Key Changes
- Separated CSS updates from state synchronization for better resize performance
- Added requestAnimationFrame-based throttling for smooth 60fps CSS updates
- Used React 18's startTransition for non-urgent state updates (maxPaneWidth)
- Added comprehensive test coverage for the new throttle/debounce logic
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/react/src/PageLayout/usePaneWidth.ts |
Refactored resize handler to use separate updateCSSOnly (throttled) and syncAll (debounced) functions; added startTransition for maxPaneWidth updates; improved performance by limiting getComputedStyle calls to breakpoint crossings |
packages/react/src/PageLayout/usePaneWidth.test.ts |
Added comprehensive tests for throttled CSS updates, debounced ARIA/state sync, viewport clamping, and cleanup behavior; reorganized existing resize listener tests for clarity |
packages/react/src/PageLayout/PageLayout.tsx |
Added safety clamp to handle edge case where user starts dragging before debounced resize completes |
packages/react/src/PageLayout/PageLayout.module.css |
Added documentation comment explaining that --pane-max-width CSS variable is managed by JavaScript |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/8744 |
09eda58
into
revert-7305-revert-7275-revert-7274-revert-7251-mc/copilot/sub-pr-7248
related to https://github.com/github/pull-requests/issues/21237
Closes #
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist