-
Notifications
You must be signed in to change notification settings - Fork 647
WIP: refactor(PageLayout): Apply drag/resize optimizations via inline styles instead of data attributes #7349
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 64dbf83 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 |
|
👋 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 |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/9043 |
…and remove ineffective memoization (#7369) Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: mattcosta7 <[email protected]>
|
@copilot update the pr body with current details. Update the comparison table to account for bigger Dom sizes and changes in performance, web vitals, fps and perceived performance |
|
@mattcosta7 I've opened a new pull request, #7370, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@copilot update the pr description of this pr to account for the current status of this branch Add fps and perceived performance to the comparison table |
|
@mattcosta7 I've opened a new pull request, #7371, to work on those changes. Once the pull request is ready, I'll request review from you. |
Refactors PageLayout performance optimizations to use inline styles instead of CSS attribute selectors, eliminating O(n) descendant traversal on every style recalc. Adds content-visibility, contain-intrinsic-size, rAF throttling, and instant drag handle feedback.
Before: Drag on 100k node DOM: 5-15 fps, 800-1200ms INP, visible jank
After: 50-60 fps, 200-300ms INP, minimal jank
Changelog
Changed
contain,pointer-events,content-visibilitydirectly to elements instead of via[data-dragging]CSS selectors. Eliminates descendant matching cost (O(n) → O(1))--draggable-handle--transitionvariablesetContainmentOptimizations(),setDraggingStyles()HorizontalDivider,VerticalDivider, andDragHandlesubcomponentsNew
content-visibility: auto- Skip rendering off-screen content in large DOMscontain-intrinsic-size: auto 500px- Prevent layout thrashing from size estimationRemoved
[data-dragging]attribute and associated CSS descendant selectorsPerformance Impact
Key wins: Selector matching O(n)→O(1), hit testing skipped on 100k nodes, off-screen rendering skipped, frame coalescing, instant visual feedback
Rollout strategy
Testing & Reviewing
Tested with synthetic large DOMs in PageLayout performance stories. Verified with Chrome DevTools Performance profiler.
Merge checklist