Skip to content

Commit d0c3991

Browse files
Copilotmattcosta7
andcommitted
Simplify drag optimizations by removing TALL_CONTENT_THRESHOLD
- Remove TALL_CONTENT_THRESHOLD constant - Always apply content-visibility: auto to both pane and content - Use actual offsetHeight for containIntrinsicSize - Simplify setDraggingStyles and removeDraggingStyles to use helper functions - Update tests to remove height threshold checks and verify content-visibility is always applied Co-authored-by: mattcosta7 <[email protected]>
1 parent abd8a2a commit d0c3991

File tree

2 files changed

+37
-116
lines changed

2 files changed

+37
-116
lines changed

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

Lines changed: 28 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -247,91 +247,50 @@ describe('PageLayout', async () => {
247247
)
248248

249249
const pane = container.querySelector<HTMLElement>('[class*="Pane"][data-resizable]')
250+
const content = container.querySelector<HTMLElement>('[class*="PageLayoutContent"]')
250251
const divider = await screen.findByRole('slider')
251252

253+
// Mock offsetHeight for testing
254+
Object.defineProperty(pane, 'offsetHeight', {
255+
configurable: true,
256+
value: 320,
257+
})
258+
Object.defineProperty(content, 'offsetHeight', {
259+
configurable: true,
260+
value: 640,
261+
})
262+
252263
// Before drag - no containment
253264
expect(pane!.style.contain).toBe('')
254265
expect(pane!.style.pointerEvents).toBe('')
266+
expect(pane!.style.contentVisibility).toBe('')
267+
expect(pane!.style.containIntrinsicSize).toBe('')
268+
expect(content!.style.contain).toBe('')
269+
expect(content!.style.pointerEvents).toBe('')
270+
expect(content!.style.contentVisibility).toBe('')
271+
expect(content!.style.containIntrinsicSize).toBe('')
255272

256-
// Start drag - containment is added
273+
// Start drag - containment and content-visibility are added to both pane and content
257274
fireEvent.pointerDown(divider, {clientX: 300, clientY: 200, pointerId: 1})
258275
expect(pane!.style.contain).toBe('layout style paint')
259276
expect(pane!.style.pointerEvents).toBe('none')
277+
expect(pane!.style.contentVisibility).toBe('auto')
278+
expect(pane!.style.containIntrinsicSize).toBe('auto 320px')
279+
expect(content!.style.contain).toBe('layout style paint')
280+
expect(content!.style.pointerEvents).toBe('none')
281+
expect(content!.style.contentVisibility).toBe('auto')
282+
expect(content!.style.containIntrinsicSize).toBe('auto 640px')
260283

261284
// End drag - containment is removed
262285
fireEvent.lostPointerCapture(divider, {pointerId: 1})
263286
expect(pane!.style.contain).toBe('')
264287
expect(pane!.style.pointerEvents).toBe('')
265-
})
266-
267-
it('should apply content-visibility only for tall content during drag', async () => {
268-
const {container} = render(
269-
<PageLayout>
270-
<PageLayout.Pane resizable>
271-
<Placeholder height={320} label="Pane" />
272-
</PageLayout.Pane>
273-
<PageLayout.Content>
274-
<Placeholder height={1200} label="Content" />
275-
</PageLayout.Content>
276-
</PageLayout>,
277-
)
278-
279-
const content = container.querySelector<HTMLElement>('[class*="Content"]')
280-
const divider = await screen.findByRole('slider')
281-
282-
// Mock offsetHeight for tall content (>1000px threshold)
283-
Object.defineProperty(content, 'offsetHeight', {
284-
configurable: true,
285-
value: 1200,
286-
})
287-
288-
// Before drag - no content-visibility
289-
expect(content!.style.contentVisibility).toBe('')
290-
expect(content!.style.containIntrinsicSize).toBe('')
291-
292-
// Start drag - content-visibility is added for tall content
293-
fireEvent.pointerDown(divider, {clientX: 300, clientY: 200, pointerId: 1})
294-
expect(content!.style.contentVisibility).toBe('auto')
295-
expect(content!.style.containIntrinsicSize).toBe('auto 1200px')
296-
297-
// End drag - content-visibility is removed
298-
fireEvent.lostPointerCapture(divider, {pointerId: 1})
299-
expect(content!.style.contentVisibility).toBe('')
300-
expect(content!.style.containIntrinsicSize).toBe('')
301-
})
302-
303-
it('should not apply content-visibility for short content during drag', async () => {
304-
const {container} = render(
305-
<PageLayout>
306-
<PageLayout.Pane resizable>
307-
<Placeholder height={320} label="Pane" />
308-
</PageLayout.Pane>
309-
<PageLayout.Content>
310-
<Placeholder height={640} label="Content" />
311-
</PageLayout.Content>
312-
</PageLayout>,
313-
)
314-
315-
const content = container.querySelector<HTMLElement>('[class*="Content"]')
316-
const divider = await screen.findByRole('slider')
317-
318-
// Mock offsetHeight for short content (<1000px threshold)
319-
Object.defineProperty(content, 'offsetHeight', {
320-
configurable: true,
321-
value: 640,
322-
})
323-
324-
// Start drag
325-
fireEvent.pointerDown(divider, {clientX: 300, clientY: 200, pointerId: 1})
326-
327-
// content-visibility should NOT be applied for short content
288+
expect(pane!.style.contentVisibility).toBe('')
289+
expect(pane!.style.containIntrinsicSize).toBe('')
290+
expect(content!.style.contain).toBe('')
291+
expect(content!.style.pointerEvents).toBe('')
328292
expect(content!.style.contentVisibility).toBe('')
329293
expect(content!.style.containIntrinsicSize).toBe('')
330-
// But basic containment should still be applied
331-
expect(content!.style.contain).toBe('layout style paint')
332-
333-
// End drag
334-
fireEvent.lostPointerCapture(divider, {pointerId: 1})
335294
})
336295
})
337296

Lines changed: 9 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,16 @@
1-
/**
2-
* Height threshold (in pixels) above which content-visibility optimizations are applied.
3-
* Avoids overhead on small content that doesn't benefit from rendering optimizations.
4-
*/
5-
const TALL_CONTENT_THRESHOLD = 1000
6-
71
/**
82
* Apply CSS containment optimizations to isolate an element during resize/drag.
93
* - contain: limits layout/paint recalc to this subtree
10-
* - content-visibility: skip rendering off-screen content (valuable for large DOMs)
11-
* - contain-intrinsic-size: prevents layout thrashing from size estimation when using content-visibility
4+
* - content-visibility: skip rendering off-screen content
5+
* - contain-intrinsic-size: uses actual element height to prevent layout shift
126
* - pointer-events: skip hit-testing large child trees
137
*/
148
export function setContainmentOptimizations(element: HTMLElement | null) {
159
if (!element) return
1610
element.style.contain = 'layout style paint'
11+
element.style.contentVisibility = 'auto'
12+
element.style.containIntrinsicSize = `auto ${element.offsetHeight}px`
1713
element.style.pointerEvents = 'none'
18-
19-
// Only apply content-visibility for tall content to avoid overhead on small elements
20-
const height = element.offsetHeight
21-
if (height > TALL_CONTENT_THRESHOLD) {
22-
element.style.contentVisibility = 'auto'
23-
element.style.containIntrinsicSize = `auto ${height}px`
24-
}
2514
}
2615

2716
/**
@@ -43,46 +32,19 @@ type DraggingStylesParams = {
4332

4433
/** Apply visual feedback and performance optimizations during drag */
4534
export function setDraggingStyles({handle, pane, content}: DraggingStylesParams) {
46-
// Handle visual feedback
4735
handle?.style.setProperty('background-color', 'var(--bgColor-accent-emphasis)')
4836
handle?.style.setProperty('--draggable-handle--drag-opacity', '1')
49-
// Disable transition for instant visual feedback during drag
5037
handle?.style.setProperty('--draggable-handle--transition', 'none')
51-
52-
// Pane: minimal containment (always visible during drag)
53-
if (pane) {
54-
pane.style.contain = 'layout style paint'
55-
pane.style.pointerEvents = 'none'
56-
}
57-
58-
// Content: containment + conditional content-visibility for tall content
59-
if (content) {
60-
content.style.contain = 'layout style paint'
61-
content.style.pointerEvents = 'none'
62-
63-
const height = content.offsetHeight
64-
if (height > TALL_CONTENT_THRESHOLD) {
65-
content.style.contentVisibility = 'auto'
66-
content.style.containIntrinsicSize = `auto ${height}px`
67-
}
68-
}
38+
// No will-change: width - doesn't help layout properties
39+
setContainmentOptimizations(pane)
40+
setContainmentOptimizations(content)
6941
}
7042

7143
/** Remove drag styles and restore normal state */
7244
export function removeDraggingStyles({handle, pane, content}: DraggingStylesParams) {
7345
handle?.style.removeProperty('background-color')
7446
handle?.style.removeProperty('--draggable-handle--drag-opacity')
7547
handle?.style.removeProperty('--draggable-handle--transition')
76-
77-
if (pane) {
78-
pane.style.contain = ''
79-
pane.style.pointerEvents = ''
80-
}
81-
82-
if (content) {
83-
content.style.contain = ''
84-
content.style.pointerEvents = ''
85-
content.style.contentVisibility = ''
86-
content.style.containIntrinsicSize = ''
87-
}
48+
removeContainmentOptimizations(pane)
49+
removeContainmentOptimizations(content)
8850
}

0 commit comments

Comments
 (0)