Skip to content

Commit 713d5a5

Browse files
mattcosta7CopilotCopilot
authored
refactor(PageLayout): drag/resize performance with inline styles and O(1) CSS selectors (#7349)
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 06c8320 commit 713d5a5

File tree

7 files changed

+447
-288
lines changed

7 files changed

+447
-288
lines changed
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@primer/react": patch
3+
---
4+
5+
PageLayout: Optimize drag/resize performance with inline styles and new optimizations
6+
7+
**Refactored:**
8+
- Use direct attribute selectors (`.Pane[data-dragging='true']`) instead of descendant selectors for CSS containment (O(1) vs O(n) selector matching)
9+
- Extract optimization utilities to `paneUtils.ts`
10+
- Apply drag handle visual feedback via inline styles and CSS variables
11+
12+
**Added:**
13+
- `content-visibility: auto` during drag/resize to skip off-screen content rendering
14+
- rAF throttle for drag updates (one update per frame, latest position wins)
15+
- Containment during window resize (parity with drag)
16+
17+
These changes improve style recalculation performance on large DOMs (100k+ nodes) by eliminating descendant selector traversal.

packages/react/src/PageLayout/PageLayout.module.css

Lines changed: 23 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -381,26 +381,6 @@
381381
}
382382
}
383383

384-
/**
385-
* OPTIMIZATION: Aggressive containment during drag for ContentWrapper
386-
* data-dragging is set on PageLayoutContent by JavaScript
387-
* This avoids expensive :has() selectors
388-
*/
389-
.PageLayoutContent[data-dragging='true'] .ContentWrapper {
390-
/* Add paint containment during drag - safe since user can't interact */
391-
contain: layout style paint;
392-
393-
/* Disable interactions */
394-
pointer-events: none;
395-
396-
/* Disable transitions to prevent expensive recalculations */
397-
transition: none;
398-
399-
/* Force compositor layer for hardware acceleration */
400-
will-change: width;
401-
transform: translateZ(0);
402-
}
403-
404384
.Content {
405385
width: 100%;
406386

@@ -427,16 +407,6 @@
427407
}
428408
}
429409

430-
/**
431-
* OPTIMIZATION: Freeze content layout during resize drag
432-
* This prevents expensive recalculations of large content areas
433-
* while keeping content visible (just frozen in place)
434-
*/
435-
.PageLayoutContent[data-dragging='true'] .Content {
436-
/* Full containment (without size) - isolate from layout recalculations */
437-
contain: layout style paint;
438-
}
439-
440410
.PaneWrapper {
441411
display: flex;
442412
width: 100%;
@@ -630,27 +600,6 @@
630600
}
631601
}
632602

633-
/**
634-
* OPTIMIZATION: Performance enhancements for Pane during drag
635-
* data-dragging is set on PageLayoutContent by JavaScript
636-
* This avoids expensive :has() selectors
637-
*/
638-
.PageLayoutContent[data-dragging='true'] .Pane {
639-
/* Full containment - isolate from layout recalculations */
640-
contain: layout style paint;
641-
642-
/* Disable interactions during drag */
643-
pointer-events: none;
644-
645-
/* Disable transitions during drag */
646-
transition: none;
647-
648-
/* Force hardware acceleration */
649-
will-change: width, transform;
650-
transform: translateZ(0);
651-
backface-visibility: hidden;
652-
}
653-
654603
.PaneHorizontalDivider {
655604
&:where([data-position='start']) {
656605
/* stylelint-disable-next-line primer/spacing */
@@ -756,26 +705,36 @@
756705
position: absolute;
757706
inset: 0 -2px;
758707
cursor: col-resize;
759-
background-color: transparent;
760-
transition-delay: 0.1s;
761708

762-
/**
763-
* OPTIMIZATION: Prevent touch scrolling and text selection during drag
764-
* This is done in CSS because it needs to be set before any pointer events
765-
*/
709+
/* Prevent touch scrolling and text selection during drag */
766710
touch-action: none;
767711
user-select: none;
768712
}
769713

770-
.DraggableHandle:hover {
771-
background-color: var(--bgColor-neutral-muted);
714+
.DraggableHandle::before {
715+
content: '';
716+
position: absolute;
717+
inset: 0;
718+
/* stylelint-disable-next-line primer/colors */
719+
background-color: var(--draggable-handle--bg-color, var(--bgColor-neutral-muted));
720+
opacity: var(--draggable-handle--drag-opacity, 0);
721+
transition: var(--draggable-handle--transition, opacity 150ms ease); /* compositor-friendly, disabled during drag */
722+
border-radius: inherit; /* optional if you need rounded corners */
772723
}
773724

774-
.DraggableHandle[data-dragging='true'] {
775-
background-color: var(--bgColor-accent-emphasis);
776-
cursor: col-resize;
725+
/* Hover effect */
726+
.DraggableHandle:hover::before {
727+
opacity: 1;
777728
}
778729

779-
.DraggableHandle[data-dragging='true']:hover {
780-
background-color: var(--bgColor-accent-emphasis);
730+
/**
731+
* OPTIMIZATION: CSS containment during drag/resize
732+
* Direct attribute selectors are O(1) - only the attributed element is invalidated
733+
* (Unlike descendant selectors which require O(n) traversal)
734+
*/
735+
.Pane[data-dragging='true'],
736+
.ContentWrapper[data-dragging='true'] {
737+
contain: layout style paint;
738+
pointer-events: none;
739+
content-visibility: auto;
781740
}

packages/react/src/PageLayout/PageLayout.test.tsx

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,8 @@ describe('PageLayout', async () => {
165165
)
166166

167167
const placeholder = await screen.findByText('Pane')
168-
const pane = placeholder.parentNode
169-
const initialWidth = (pane as HTMLElement).style.getPropertyValue('--pane-width')
168+
const pane = placeholder.parentNode as HTMLElement | null
169+
const initialWidth = pane?.style.getPropertyValue('--pane-width')
170170
const divider = await screen.findByRole('slider')
171171

172172
// Moving divider should resize pane.
@@ -176,11 +176,11 @@ describe('PageLayout', async () => {
176176
fireEvent.keyDown(divider, {key: 'ArrowRight'})
177177
fireEvent.keyDown(divider, {key: 'ArrowRight'})
178178

179-
const finalWidth = (pane as HTMLElement).style.getPropertyValue('--pane-width')
179+
const finalWidth = pane?.style.getPropertyValue('--pane-width')
180180
expect(finalWidth).not.toEqual(initialWidth)
181181
})
182182

183-
it('should set data-dragging attribute during pointer drag', async () => {
183+
it('should set optimization styles during pointer drag', async () => {
184184
const {container} = render(
185185
<PageLayout>
186186
<PageLayout.Pane resizable>
@@ -192,22 +192,21 @@ describe('PageLayout', async () => {
192192
</PageLayout>,
193193
)
194194

195-
const content = container.querySelector('[class*="PageLayoutContent"]')
195+
const contentWrapper = container.querySelector<HTMLElement>('[class*="ContentWrapper"]')
196196
const divider = await screen.findByRole('slider')
197197

198198
// Before drag - no data-dragging attribute
199-
expect(content).not.toHaveAttribute('data-dragging')
199+
expect(contentWrapper).not.toHaveAttribute('data-dragging')
200200

201-
// Start drag
201+
// Start drag - optimization attribute is set
202202
fireEvent.pointerDown(divider, {clientX: 300, clientY: 200, pointerId: 1})
203-
expect(content).toHaveAttribute('data-dragging', 'true')
204-
205-
// End drag - pointer capture lost ends the drag and removes attribute
203+
expect(contentWrapper).toHaveAttribute('data-dragging', 'true')
204+
// End drag - pointer capture lost ends the drag and removes optimization attribute
206205
fireEvent.lostPointerCapture(divider, {pointerId: 1})
207-
expect(content).not.toHaveAttribute('data-dragging')
206+
expect(contentWrapper).not.toHaveAttribute('data-dragging')
208207
})
209208

210-
it('should set data-dragging attribute during keyboard resize', async () => {
209+
it('should set optimization styles during keyboard resize', async () => {
211210
const {container} = render(
212211
<PageLayout>
213212
<PageLayout.Pane resizable>
@@ -219,20 +218,46 @@ describe('PageLayout', async () => {
219218
</PageLayout>,
220219
)
221220

222-
const content = container.querySelector('[class*="PageLayoutContent"]')
221+
const contentWrapper = container.querySelector<HTMLElement>('[class*="ContentWrapper"]')
223222
const divider = await screen.findByRole('slider')
224223

225224
// Before interaction - no data-dragging attribute
226-
expect(content).not.toHaveAttribute('data-dragging')
225+
expect(contentWrapper).not.toHaveAttribute('data-dragging')
227226

228227
// Start keyboard resize (focus first)
229228
fireEvent.focus(divider)
230229
fireEvent.keyDown(divider, {key: 'ArrowRight'})
231-
expect(content).toHaveAttribute('data-dragging', 'true')
230+
expect(contentWrapper).toHaveAttribute('data-dragging', 'true')
232231

233-
// End keyboard resize - removes attribute
232+
// End keyboard resize - removes optimization attribute
234233
fireEvent.keyUp(divider, {key: 'ArrowRight'})
235-
expect(content).not.toHaveAttribute('data-dragging')
234+
expect(contentWrapper).not.toHaveAttribute('data-dragging')
235+
})
236+
237+
it('should not add will-change during drag', async () => {
238+
const {container} = render(
239+
<PageLayout>
240+
<PageLayout.Pane resizable>
241+
<Placeholder height={320} label="Pane" />
242+
</PageLayout.Pane>
243+
<PageLayout.Content>
244+
<Placeholder height={640} label="Content" />
245+
</PageLayout.Content>
246+
</PageLayout>,
247+
)
248+
249+
const pane = container.querySelector<HTMLElement>('[class*="Pane"][data-resizable]')
250+
const divider = await screen.findByRole('slider')
251+
252+
// Before drag - no will-change
253+
expect(pane!.style.willChange).toBe('')
254+
255+
// Start drag - will-change should still not be set (removed optimization)
256+
fireEvent.pointerDown(divider, {clientX: 300, clientY: 200, pointerId: 1})
257+
expect(pane!.style.willChange).toBe('')
258+
// End drag - will-change remains unset
259+
fireEvent.lostPointerCapture(divider, {pointerId: 1})
260+
expect(pane!.style.willChange).toBe('')
236261
})
237262
})
238263

0 commit comments

Comments
 (0)