From df64c00420137be3ee8d11280da7ce4b6a886cf6 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Wed, 17 Dec 2025 01:23:28 +0000 Subject: [PATCH 01/30] more page layout optimizing --- .../src/PageLayout/PageLayout.module.css | 76 +---- .../react/src/PageLayout/PageLayout.test.tsx | 57 +++- packages/react/src/PageLayout/PageLayout.tsx | 314 ++++++++++-------- .../react/src/PageLayout/usePaneWidth.test.ts | 171 ++++++++-- packages/react/src/PageLayout/usePaneWidth.ts | 84 +++-- 5 files changed, 438 insertions(+), 264 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.module.css b/packages/react/src/PageLayout/PageLayout.module.css index 079d34f6cff..eb3d2a1e053 100644 --- a/packages/react/src/PageLayout/PageLayout.module.css +++ b/packages/react/src/PageLayout/PageLayout.module.css @@ -381,26 +381,6 @@ } } -/** - * OPTIMIZATION: Aggressive containment during drag for ContentWrapper - * data-dragging is set on PageLayoutContent by JavaScript - * This avoids expensive :has() selectors - */ -.PageLayoutContent[data-dragging='true'] .ContentWrapper { - /* Add paint containment during drag - safe since user can't interact */ - contain: layout style paint; - - /* Disable interactions */ - pointer-events: none; - - /* Disable transitions to prevent expensive recalculations */ - transition: none; - - /* Force compositor layer for hardware acceleration */ - will-change: width; - transform: translateZ(0); -} - .Content { width: 100%; @@ -427,16 +407,6 @@ } } -/** - * OPTIMIZATION: Freeze content layout during resize drag - * This prevents expensive recalculations of large content areas - * while keeping content visible (just frozen in place) - */ -.PageLayoutContent[data-dragging='true'] .Content { - /* Full containment (without size) - isolate from layout recalculations */ - contain: layout style paint; -} - .PaneWrapper { display: flex; width: 100%; @@ -630,27 +600,6 @@ } } -/** - * OPTIMIZATION: Performance enhancements for Pane during drag - * data-dragging is set on PageLayoutContent by JavaScript - * This avoids expensive :has() selectors - */ -.PageLayoutContent[data-dragging='true'] .Pane { - /* Full containment - isolate from layout recalculations */ - contain: layout style paint; - - /* Disable interactions during drag */ - pointer-events: none; - - /* Disable transitions during drag */ - transition: none; - - /* Force hardware acceleration */ - will-change: width, transform; - transform: translateZ(0); - backface-visibility: hidden; -} - .PaneHorizontalDivider { &:where([data-position='start']) { /* stylelint-disable-next-line primer/spacing */ @@ -756,26 +705,23 @@ position: absolute; inset: 0 -2px; cursor: col-resize; - background-color: transparent; - transition-delay: 0.1s; - /** - * OPTIMIZATION: Prevent touch scrolling and text selection during drag - * This is done in CSS because it needs to be set before any pointer events - */ + /* Prevent touch scrolling and text selection during drag */ touch-action: none; user-select: none; } -.DraggableHandle:hover { +.DraggableHandle::before { + content: ''; + position: absolute; + inset: 0; background-color: var(--bgColor-neutral-muted); + opacity: var(--drag-opacity, 0); + transition: opacity 150ms ease; /* compositor-friendly */ + border-radius: inherit; /* optional if you need rounded corners */ } -.DraggableHandle[data-dragging='true'] { - background-color: var(--bgColor-accent-emphasis); - cursor: col-resize; -} - -.DraggableHandle[data-dragging='true']:hover { - background-color: var(--bgColor-accent-emphasis); +/* Hover effect */ +.DraggableHandle:hover::before { + opacity: 1; } diff --git a/packages/react/src/PageLayout/PageLayout.test.tsx b/packages/react/src/PageLayout/PageLayout.test.tsx index 36a0d6098de..e14209208bc 100644 --- a/packages/react/src/PageLayout/PageLayout.test.tsx +++ b/packages/react/src/PageLayout/PageLayout.test.tsx @@ -175,7 +175,7 @@ describe('PageLayout', async () => { expect(finalWidth).not.toEqual(initialWidth) }) - it('should set data-dragging attribute during pointer drag', async () => { + it('should set optimization styles during pointer drag', async () => { const {container} = render( @@ -187,22 +187,22 @@ describe('PageLayout', async () => { , ) - const content = container.querySelector('[class*="PageLayoutContent"]') + const content = container.querySelector('[class*="PageLayoutContent"]') as HTMLElement const divider = await screen.findByRole('slider') - // Before drag - no data-dragging attribute - expect(content).not.toHaveAttribute('data-dragging') + // Before drag - no contain property + expect(content.style.getPropertyValue('contain')).toBe('') - // Start drag + // Start drag - optimization properties are set fireEvent.pointerDown(divider, {clientX: 300, clientY: 200, pointerId: 1}) - expect(content).toHaveAttribute('data-dragging', 'true') + expect(content.style.getPropertyValue('contain')).toBe('layout style paint') - // End drag - pointer capture lost ends the drag and removes attribute + // End drag - pointer capture lost ends the drag and removes optimization properties fireEvent.lostPointerCapture(divider, {pointerId: 1}) - expect(content).not.toHaveAttribute('data-dragging') + expect(content.style.getPropertyValue('contain')).toBe('') }) - it('should set data-dragging attribute during keyboard resize', async () => { + it('should set optimization styles during keyboard resize', async () => { const {container} = render( @@ -214,20 +214,47 @@ describe('PageLayout', async () => { , ) - const content = container.querySelector('[class*="PageLayoutContent"]') + const content = container.querySelector('[class*="PageLayoutContent"]') as HTMLElement const divider = await screen.findByRole('slider') - // Before interaction - no data-dragging attribute - expect(content).not.toHaveAttribute('data-dragging') + // Before interaction - no contain property + expect(content.style.getPropertyValue('contain')).toBe('') // Start keyboard resize (focus first) fireEvent.focus(divider) fireEvent.keyDown(divider, {key: 'ArrowRight'}) - expect(content).toHaveAttribute('data-dragging', 'true') + expect(content.style.getPropertyValue('contain')).toBe('layout style paint') - // End keyboard resize - removes attribute + // End keyboard resize - removes optimization properties fireEvent.keyUp(divider, {key: 'ArrowRight'}) - expect(content).not.toHaveAttribute('data-dragging') + expect(content.style.getPropertyValue('contain')).toBe('') + }) + + it('should add will-change during drag for optimized updates', async () => { + const {container} = render( + + + + + + + + , + ) + + const pane = container.querySelector('[class*="Pane"][data-resizable]') as HTMLElement + const divider = await screen.findByRole('slider') + + // Before drag - no will-change + expect(pane.style.willChange).toBe('') + + // Start drag - will-change is added + fireEvent.pointerDown(divider, {clientX: 300, clientY: 200, pointerId: 1}) + expect(pane.style.willChange).toBe('width') + + // End drag - will-change is removed + fireEvent.lostPointerCapture(divider, {pointerId: 1}) + expect(pane.style.willChange).toBe('') }) }) diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 901db1c5120..1cee5973ddc 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -1,4 +1,4 @@ -import React, {useRef} from 'react' +import React, {memo, useRef} from 'react' import {clsx} from 'clsx' import {useId} from '../hooks/useId' import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' @@ -29,6 +29,9 @@ const REGION_ORDER = { footer: 4, } +const ARROW_KEYS = new Set(['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown']) +const SHRINK_KEYS = new Set(['ArrowLeft', 'ArrowDown']) + // eslint-disable-next-line @typescript-eslint/no-unused-vars const SPACING_MAP = { none: 0, @@ -103,15 +106,7 @@ const Root: React.FC> = ({ return ( -
+
{slots.header}
@@ -119,11 +114,32 @@ const Root: React.FC> = ({
{slots.footer}
-
+ ) } +const RootWrapper = memo(function RootWrapper({ + style, + padding, + children, + className, +}: React.PropsWithChildren>) { + return ( +
+ {children} +
+ ) +}) + Root.displayName = 'PageLayout' // ---------------------------------------------------------------------------- @@ -136,51 +152,48 @@ type DividerProps = { position?: keyof typeof panePositions | ResponsiveValue } -const HorizontalDivider: React.FC> = ({ - variant = 'none', - className, - position, - style, -}) => { - const {padding} = React.useContext(PageLayoutContext) +const HorizontalDivider = memo>( + ({variant = 'none', className, position, style}) => { + const {padding} = React.useContext(PageLayoutContext) - return ( -
- ) -} + return ( +
+ ) + }, +) + +HorizontalDivider.displayName = 'HorizontalDivider' type VerticalDividerProps = DividerProps & { draggable?: boolean } -const VerticalDivider: React.FC> = ({ - variant = 'none', - position, - className, - style, - children, -}) => { - return ( -
- {children} -
- ) -} +const VerticalDivider = memo>( + ({variant = 'none', position, className, style, children}) => { + return ( +
+ {children} +
+ ) + }, +) + +VerticalDivider.displayName = 'VerticalDivider' type DragHandleProps = { /** Ref for imperative ARIA updates during drag */ @@ -201,17 +214,25 @@ type DragHandleProps = { 'aria-valuenow'?: number } -const DATA_DRAGGING_ATTR = 'data-dragging' -const isDragging = (handle: HTMLElement | null) => { - return handle?.getAttribute(DATA_DRAGGING_ATTR) === 'true' +function setDraggingStyleOptimizations(element: HTMLElement | null) { + if (!element) return + element.style.pointerEvents = 'none' + element.style.contain = 'layout style paint' + element.style.contentVisibility = 'auto' } +function removeDraggingStylesOptimizations(element: HTMLElement | null) { + if (!element) return + element.style.pointerEvents = '' + element.style.contain = '' + element.style.contentVisibility = '' +} /** * DragHandle - handles all pointer and keyboard interactions for resizing * ARIA values are set in JSX for SSR accessibility, * then updated via DOM manipulation during drag for performance */ -const DragHandle: React.FC = ({ +const DragHandle = memo(function DragHandle({ handleRef, onDragStart, onDrag, @@ -220,7 +241,7 @@ const DragHandle: React.FC = ({ 'aria-valuemin': ariaValueMin, 'aria-valuemax': ariaValueMax, 'aria-valuenow': ariaValueNow, -}) => { +}) { const stableOnDragStart = React.useRef(onDragStart) const stableOnDrag = React.useRef(onDrag) const stableOnDragEnd = React.useRef(onDragEnd) @@ -232,77 +253,116 @@ const DragHandle: React.FC = ({ const {paneRef, contentRef} = React.useContext(PageLayoutContext) - // Helper to set/remove dragging attribute on content wrapper - // This avoids expensive :has() selectors - CSS uses simple descendant selectors instead - const setDragging = React.useCallback( - (dragging: boolean) => { - if (dragging) { - handleRef.current?.setAttribute(DATA_DRAGGING_ATTR, 'true') - contentRef.current?.setAttribute(DATA_DRAGGING_ATTR, 'true') - } else { - handleRef.current?.removeAttribute(DATA_DRAGGING_ATTR) - contentRef.current?.removeAttribute(DATA_DRAGGING_ATTR) - } - }, - [handleRef, contentRef], - ) + // Dragging state as a ref - cheaper than reading from DOM style + const isDraggingRef = React.useRef(false) + + // Helper to set/remove dragging optimization styles + // All properties set as inline styles ONLY during drag for zero overhead at rest + const startDragging = React.useCallback(() => { + if (isDraggingRef.current) return + // Handle: visual feedback + handleRef.current?.style.setProperty('background-color', 'var(--bgColor-accent-emphasis)') + handleRef.current?.style.setProperty('--drag-opacity', '1') + // Pane: hint browser to optimize for width changes + paneRef.current?.style.setProperty('will-change', 'width') + // Content & Pane: isolate from children during drag + // contain: limits layout/paint recalc to this subtree + // content-visibility: skip rendering off-screen content (valuable for large DOMs) + // pointer-events: skip hit-testing large child trees + setDraggingStyleOptimizations(contentRef.current) + setDraggingStyleOptimizations(paneRef.current) + isDraggingRef.current = true + }, [handleRef, contentRef, paneRef]) + + const endDragging = React.useCallback(() => { + if (!isDraggingRef.current) return + const handle = handleRef.current + const content = contentRef.current + const pane = paneRef.current + + handle?.style.removeProperty('background-color') + handle?.style.removeProperty('--drag-opacity') + pane?.style.removeProperty('will-change') + removeDraggingStylesOptimizations(content) + removeDraggingStylesOptimizations(pane) + isDraggingRef.current = false + }, [handleRef, contentRef, paneRef]) /** * Pointer down starts a drag operation * Capture the pointer to continue receiving events outside the handle area - * Set a data attribute to indicate dragging state */ const handlePointerDown = React.useCallback( (event: React.PointerEvent) => { if (event.button !== 0) return event.preventDefault() const target = event.currentTarget - target.setPointerCapture(event.pointerId) + // Try to capture pointer - may fail in test environments or if pointer is already released + try { + target.setPointerCapture(event.pointerId) + } catch { + // Ignore - pointer capture is a nice-to-have for dragging outside the element + } stableOnDragStart.current(event.clientX) - setDragging(true) + startDragging() }, - [setDragging], + [startDragging], ) + // Simple rAF throttle - one update per frame, latest position wins + const rafIdRef = React.useRef(null) + const pendingClientXRef = React.useRef(null) + /** * Pointer move during drag * Calls onDrag with absolute cursor X position - * Using absolute position avoids drift from accumulated deltas - * Prevents default to avoid unwanted selection behavior + * Uses rAF to coalesce updates to one per frame */ - const handlePointerMove = React.useCallback( - (event: React.PointerEvent) => { - if (!isDragging(handleRef.current)) return - event.preventDefault() - - stableOnDrag.current(event.clientX, false) - }, - [handleRef], - ) + const handlePointerMove = React.useCallback((event: React.PointerEvent) => { + if (!isDraggingRef.current) return + event.preventDefault() + + // Store latest position - only the final position before rAF fires matters + pendingClientXRef.current = event.clientX + + // Schedule update if not already scheduled + if (rafIdRef.current === null) { + rafIdRef.current = requestAnimationFrame(() => { + rafIdRef.current = null + if (pendingClientXRef.current !== null) { + stableOnDrag.current(pendingClientXRef.current, false) + pendingClientXRef.current = null + } + }) + } + }, []) /** * Pointer up ends a drag operation * Prevents default to avoid unwanted selection behavior */ - const handlePointerUp = React.useCallback( - (event: React.PointerEvent) => { - if (!isDragging(handleRef.current)) return - event.preventDefault() - // Cleanup will happen in onLostPointerCapture - }, - [handleRef], - ) + const handlePointerUp = React.useCallback((event: React.PointerEvent) => { + if (!isDraggingRef.current) return + event.preventDefault() + // Cleanup will happen in onLostPointerCapture + }, []) /** * Lost pointer capture ends a drag operation - * Cleans up dragging state + * Cleans up dragging state and cancels any pending rAF * Calls onDragEnd callback */ const handleLostPointerCapture = React.useCallback(() => { - if (!isDragging(handleRef.current)) return - setDragging(false) + if (!isDraggingRef.current) return + // Cancel any pending rAF to prevent stale updates + if (rafIdRef.current !== null) { + cancelAnimationFrame(rafIdRef.current) + rafIdRef.current = null + pendingClientXRef.current = null + } + endDragging() stableOnDragEnd.current() - }, [handleRef, setDragging]) + }, [endDragging]) /** * Keyboard handling for accessibility @@ -313,40 +373,29 @@ const DragHandle: React.FC = ({ */ const handleKeyDown = React.useCallback( (event: React.KeyboardEvent) => { - if ( - event.key === 'ArrowLeft' || - event.key === 'ArrowRight' || - event.key === 'ArrowUp' || - event.key === 'ArrowDown' - ) { - event.preventDefault() - - if (!paneRef.current) return + if (!ARROW_KEYS.has(event.key)) return + event.preventDefault() - // https://github.com/github/accessibility/issues/5101#issuecomment-1822870655 - const delta = event.key === 'ArrowLeft' || event.key === 'ArrowDown' ? -ARROW_KEY_STEP : ARROW_KEY_STEP + // https://github.com/github/accessibility/issues/5101#issuecomment-1822870655 + const delta = SHRINK_KEYS.has(event.key) ? -ARROW_KEY_STEP : ARROW_KEY_STEP - setDragging(true) - stableOnDrag.current(delta, true) + // Only set dragging on first keydown (not repeats) + if (!isDraggingRef.current) { + startDragging() } + stableOnDrag.current(delta, true) }, - [paneRef, setDragging], + [startDragging], ) const handleKeyUp = React.useCallback( (event: React.KeyboardEvent) => { - if ( - event.key === 'ArrowLeft' || - event.key === 'ArrowRight' || - event.key === 'ArrowUp' || - event.key === 'ArrowDown' - ) { - event.preventDefault() - setDragging(false) - stableOnDragEnd.current() - } + if (!ARROW_KEYS.has(event.key)) return + event.preventDefault() + endDragging() + stableOnDragEnd.current() }, - [setDragging], + [endDragging], ) return ( @@ -369,7 +418,7 @@ const DragHandle: React.FC = ({ onDoubleClick={onDoubleClick} /> ) -} +}) // ---------------------------------------------------------------------------- // PageLayout.Header @@ -407,7 +456,7 @@ export type PageLayoutHeaderProps = { style?: React.CSSProperties } -const Header: FCWithSlotMarker> = ({ +const Header: FCWithSlotMarker> = memo(function Header({ 'aria-label': label, 'aria-labelledby': labelledBy, padding = 'none', @@ -417,7 +466,7 @@ const Header: FCWithSlotMarker> = children, style, className, -}) => { +}) { // Combine divider and dividerWhenNarrow for backwards compatibility const dividerProp = !isResponsiveValue(divider) && dividerWhenNarrow !== 'inherit' @@ -460,7 +509,7 @@ const Header: FCWithSlotMarker> = /> ) -} +}) Header.displayName = 'PageLayout.Header' @@ -499,7 +548,7 @@ const contentWidths = { xlarge: '1280px', } -const Content: FCWithSlotMarker> = ({ +const Content: FCWithSlotMarker> = memo(function Content({ as = 'main', 'aria-label': label, 'aria-labelledby': labelledBy, @@ -509,7 +558,7 @@ const Content: FCWithSlotMarker> children, className, style, -}) => { +}) { const Component = as return ( @@ -533,7 +582,7 @@ const Content: FCWithSlotMarker>
) -} +}) Content.displayName = 'PageLayout.Content' @@ -637,7 +686,7 @@ const Pane = React.forwardRef(null) @@ -657,6 +706,7 @@ const Pane = React.forwardRef> = ({ +const Footer: FCWithSlotMarker> = memo(function Footer({ 'aria-label': label, 'aria-labelledby': labelledBy, padding = 'none', @@ -872,7 +922,7 @@ const Footer: FCWithSlotMarker> = children, className, style, -}) => { +}) { // Combine divider and dividerWhenNarrow for backwards compatibility const dividerProp = !isResponsiveValue(divider) && dividerWhenNarrow !== 'inherit' @@ -915,7 +965,7 @@ const Footer: FCWithSlotMarker> =
) -} +}) Footer.displayName = 'PageLayout.Footer' diff --git a/packages/react/src/PageLayout/usePaneWidth.test.ts b/packages/react/src/PageLayout/usePaneWidth.test.ts index a529d0b7367..78ed49ecf16 100644 --- a/packages/react/src/PageLayout/usePaneWidth.test.ts +++ b/packages/react/src/PageLayout/usePaneWidth.test.ts @@ -17,6 +17,7 @@ import { const createMockRefs = () => ({ paneRef: {current: document.createElement('div')} as React.RefObject, handleRef: {current: document.createElement('div')} as React.RefObject, + contentRef: {current: document.createElement('div')} as React.RefObject, }) describe('usePaneWidth', () => { @@ -413,7 +414,7 @@ describe('usePaneWidth', () => { vi.useRealTimers() }) - it('should update CSS variable immediately via throttle', async () => { + it('should debounce CSS variable update (no throttle)', async () => { vi.useFakeTimers() vi.stubGlobal('innerWidth', 1280) const refs = createMockRefs() @@ -423,7 +424,7 @@ describe('usePaneWidth', () => { width: 'medium', minWidth: 256, resizable: true, - widthStorageKey: 'test-css-throttle', + widthStorageKey: 'test-css-debounce', ...refs, }), ) @@ -434,10 +435,18 @@ describe('usePaneWidth', () => { // Shrink viewport vi.stubGlobal('innerWidth', 1000) - // Fire resize - CSS should update immediately (throttled at 16ms) + // Fire resize - CSS should NOT update immediately (debounce only, no throttle) window.dispatchEvent(new Event('resize')) - // CSS variable should be updated immediately: 1000 - 511 = 489 + // CSS variable should still be old value (debounced) + expect(refs.paneRef.current?.style.getPropertyValue('--pane-max-width')).toBe('769px') + + // Wait for debounce + await act(async () => { + await vi.advanceTimersByTimeAsync(150) + }) + + // CSS variable should now be updated: 1000 - 511 = 489 expect(refs.paneRef.current?.style.getPropertyValue('--pane-max-width')).toBe('489px') vi.useRealTimers() @@ -482,7 +491,7 @@ describe('usePaneWidth', () => { vi.useRealTimers() }) - it('should throttle CSS updates and debounce full sync on rapid resize', async () => { + it('should debounce full sync on rapid resize (no throttle)', async () => { vi.useFakeTimers() vi.stubGlobal('innerWidth', 1280) const refs = createMockRefs() @@ -494,7 +503,7 @@ describe('usePaneWidth', () => { width: 'medium', minWidth: 256, resizable: true, - widthStorageKey: 'test-throttle-debounce', + widthStorageKey: 'test-debounce-only', ...refs, }), ) @@ -502,29 +511,24 @@ describe('usePaneWidth', () => { // Clear mount calls setPropertySpy.mockClear() - // Fire resize - first one updates CSS immediately + // Fire resize events rapidly vi.stubGlobal('innerWidth', 1100) window.dispatchEvent(new Event('resize')) - // CSS should update immediately (first call, throttle allows) - expect(setPropertySpy).toHaveBeenCalledWith('--pane-max-width', '589px') // 1100 - 511 - - setPropertySpy.mockClear() + // CSS should NOT update immediately (debounce only, no throttle) + expect(setPropertySpy).not.toHaveBeenCalledWith('--pane-max-width', '589px') - // Fire more resize events rapidly (within throttle window) + // Fire more resize events for (let i = 0; i < 3; i++) { vi.stubGlobal('innerWidth', 1000 - i * 50) window.dispatchEvent(new Event('resize')) } - // Throttle limits calls - may have scheduled RAF but not executed yet - // Advance past throttle window to let RAF execute - await vi.advanceTimersByTimeAsync(20) - - // Should have at least one more CSS update from RAF - expect(setPropertySpy).toHaveBeenCalled() + // Advance a bit but not past debounce - no CSS updates yet + await vi.advanceTimersByTimeAsync(50) + expect(setPropertySpy).not.toHaveBeenCalledWith('--pane-max-width', expect.any(String)) - // But ARIA should not be updated yet (debounced) + // ARIA should not be updated yet (debounced) expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('769') // Still initial // Wait for debounce to complete @@ -532,8 +536,9 @@ describe('usePaneWidth', () => { await vi.advanceTimersByTimeAsync(150) }) - // Now ARIA and refs are synced - expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('389') // 900 - 511 + // Now CSS, ARIA and refs are synced with final viewport value (900) + expect(setPropertySpy).toHaveBeenCalledWith('--pane-max-width', '389px') // 900 - 511 + expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('389') vi.useRealTimers() }) @@ -615,6 +620,130 @@ describe('usePaneWidth', () => { expect(addEventListenerSpy).not.toHaveBeenCalledWith('resize', expect.any(Function)) addEventListenerSpy.mockRestore() }) + + it('should apply containment styles during resize', async () => { + vi.useFakeTimers() + vi.stubGlobal('innerWidth', 1280) + const refs = createMockRefs() + + renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-containment', + ...refs, + }), + ) + + // Initially no containment + expect(refs.paneRef.current?.style.contain).toBe('') + expect(refs.contentRef.current?.style.contain).toBe('') + + // Fire resize + vi.stubGlobal('innerWidth', 1000) + window.dispatchEvent(new Event('resize')) + + // Containment should be applied immediately + expect(refs.paneRef.current?.style.contain).toBe('layout style paint') + expect(refs.paneRef.current?.style.contentVisibility).toBe('auto') + expect(refs.contentRef.current?.style.contain).toBe('layout style paint') + expect(refs.contentRef.current?.style.contentVisibility).toBe('auto') + + // Wait for debounce to complete + await act(async () => { + await vi.advanceTimersByTimeAsync(150) + }) + + // Containment should be removed after debounce + expect(refs.paneRef.current?.style.contain).toBe('') + expect(refs.paneRef.current?.style.contentVisibility).toBe('') + expect(refs.contentRef.current?.style.contain).toBe('') + expect(refs.contentRef.current?.style.contentVisibility).toBe('') + + vi.useRealTimers() + }) + + it('should skip resize handling while dragging', async () => { + vi.useFakeTimers() + vi.stubGlobal('innerWidth', 1280) + const refs = createMockRefs() + + renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-skip-while-dragging', + ...refs, + }), + ) + + // Simulate drag start by setting background-color (as DragHandle does) + refs.handleRef.current!.style.backgroundColor = 'var(--bgColor-accent-emphasis)' + + // Fire resize + vi.stubGlobal('innerWidth', 1000) + window.dispatchEvent(new Event('resize')) + + // Containment should NOT be applied (resize skipped while dragging) + expect(refs.paneRef.current?.style.contain).toBe('') + + // Wait past debounce + await act(async () => { + await vi.advanceTimersByTimeAsync(200) + }) + + // CSS variable should still be old value (resize was skipped) + expect(refs.paneRef.current?.style.getPropertyValue('--pane-max-width')).toBe('769px') + + // Clear drag state + refs.handleRef.current!.style.backgroundColor = '' + + // Now resize should work + window.dispatchEvent(new Event('resize')) + + await act(async () => { + await vi.advanceTimersByTimeAsync(150) + }) + + // Now CSS variable should be updated + expect(refs.paneRef.current?.style.getPropertyValue('--pane-max-width')).toBe('489px') // 1000 - 511 + + vi.useRealTimers() + }) + + it('should cleanup containment styles on unmount during resize', async () => { + vi.useFakeTimers() + vi.stubGlobal('innerWidth', 1280) + const refs = createMockRefs() + + const {unmount} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-cleanup-containment', + ...refs, + }), + ) + + // Fire resize to apply containment + vi.stubGlobal('innerWidth', 1000) + window.dispatchEvent(new Event('resize')) + + // Verify containment is applied + expect(refs.paneRef.current?.style.contain).toBe('layout style paint') + + // Unmount before debounce completes + unmount() + + // Containment should be cleaned up + expect(refs.paneRef.current?.style.contain).toBe('') + expect(refs.contentRef.current?.style.contain).toBe('') + + vi.useRealTimers() + }) }) describe('on-demand max calculation', () => { diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index 2449901b0a0..afb8f7cafdc 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -23,6 +23,7 @@ export type UsePaneWidthOptions = { widthStorageKey: string paneRef: React.RefObject handleRef: React.RefObject + contentRef: React.RefObject } export type UsePaneWidthResult = { @@ -76,8 +77,9 @@ export const isCustomWidthOptions = (width: PaneWidth | CustomWidthOptions): wid return (width as CustomWidthOptions).default !== undefined } +const PANE_WIDTHS = new Set(['small', 'medium', 'large']) export const isPaneWidth = (width: PaneWidth | CustomWidthOptions): width is PaneWidth => { - return ['small', 'medium', 'large'].includes(width as PaneWidth) + return PANE_WIDTHS.has(width as PaneWidth) } export const getDefaultPaneWidth = (w: PaneWidth | CustomWidthOptions): number => { @@ -130,6 +132,7 @@ export function usePaneWidth({ widthStorageKey, paneRef, handleRef, + contentRef, }: UsePaneWidthOptions): UsePaneWidthResult { // Derive constraints from width configuration const isCustomWidth = isCustomWidthOptions(width) @@ -187,7 +190,10 @@ export function usePaneWidth({ const saveWidth = React.useCallback( (value: number) => { currentWidthRef.current = value - setCurrentWidth(value) + // Visual update already done via inline styles - React state sync is non-urgent + startTransition(() => { + setCurrentWidth(value) + }) try { localStorage.setItem(widthStorageKey, value.toString()) } catch { @@ -205,20 +211,12 @@ export function usePaneWidth({ }) // Update CSS variable, refs, and ARIA on mount and window resize. - // Strategy: - // 1. Throttled (16ms): Update --pane-max-width CSS variable for immediate visual clamp - // 2. Debounced (150ms): Sync refs, ARIA, and React state when resize stops + // Strategy: Only sync when resize stops (debounced) to avoid layout thrashing on large DOMs useIsomorphicLayoutEffect(() => { if (!resizable) return let lastViewportWidth = window.innerWidth - // Quick CSS-only update for immediate visual feedback (throttled) - const updateCSSOnly = () => { - const actualMax = getMaxPaneWidthRef.current() - paneRef.current?.style.setProperty('--pane-max-width', `${actualMax}px`) - } - // Full sync of refs, ARIA, and state (debounced, runs when resize stops) const syncAll = () => { const currentViewportWidth = window.innerWidth @@ -269,28 +267,51 @@ export function usePaneWidth({ // For custom widths, max is fixed - no need to listen to resize if (customMaxWidth !== null) return - // Throttle CSS updates (16ms ≈ 60fps), debounce full sync (150ms) - const THROTTLE_MS = 16 + // Only sync when resize stops (debounced) - no updates during continuous resize + // CSS handles visual clamping naturally via viewport-relative max values + // Updating --pane-max-width during resize causes expensive layout recalcs on large DOMs const DEBOUNCE_MS = 150 - let rafId: number | null = null let debounceId: ReturnType | null = null - let lastThrottleTime = 0 + let isResizing = false + + // Apply containment during resize to reduce layout thrashing on large DOMs + const startResizeOptimizations = () => { + if (isResizing) return + isResizing = true + const pane = paneRef.current + const content = contentRef.current + if (pane) { + pane.style.contain = 'layout style paint' + pane.style.contentVisibility = 'auto' + } + if (content) { + content.style.contain = 'layout style paint' + content.style.contentVisibility = 'auto' + } + } - const handleResize = () => { - const now = Date.now() - - // Throttled CSS update for immediate visual feedback - if (now - lastThrottleTime >= THROTTLE_MS) { - lastThrottleTime = now - updateCSSOnly() - } else if (rafId === null) { - // Schedule next frame if we're within throttle window - rafId = requestAnimationFrame(() => { - rafId = null - lastThrottleTime = Date.now() - updateCSSOnly() - }) + const endResizeOptimizations = () => { + if (!isResizing) return + isResizing = false + const pane = paneRef.current + const content = contentRef.current + if (pane) { + pane.style.contain = '' + pane.style.contentVisibility = '' } + if (content) { + content.style.contain = '' + content.style.contentVisibility = '' + } + } + + const handleResize = () => { + // Skip resize handling while dragging - dragging sets contain style via DragHandle + // Check for background-color which is set on drag start + if (handleRef.current?.style.backgroundColor) return + + // Apply containment at start of resize + startResizeOptimizations() // Debounced full sync (refs, ARIA, state) when resize stops if (debounceId !== null) { @@ -298,6 +319,7 @@ export function usePaneWidth({ } debounceId = setTimeout(() => { debounceId = null + endResizeOptimizations() syncAll() }, DEBOUNCE_MS) } @@ -305,11 +327,11 @@ export function usePaneWidth({ // eslint-disable-next-line github/prefer-observers -- Uses window resize events instead of ResizeObserver to avoid INP issues. ResizeObserver on document.documentElement fires on any content change (typing, etc), while window resize only fires on actual viewport changes. window.addEventListener('resize', handleResize) return () => { - if (rafId !== null) cancelAnimationFrame(rafId) if (debounceId !== null) clearTimeout(debounceId) + endResizeOptimizations() window.removeEventListener('resize', handleResize) } - }, [resizable, customMaxWidth, minPaneWidth, paneRef, handleRef]) + }, [resizable, customMaxWidth, minPaneWidth, paneRef, handleRef, contentRef]) return { currentWidth, From a1fafe2475f7f45409d32f8c647a9c23ddae80df Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Wed, 17 Dec 2025 01:45:11 +0000 Subject: [PATCH 02/30] organization --- .../src/PageLayout/PageLayout.module.css | 2 +- packages/react/src/PageLayout/PageLayout.tsx | 53 +++----- .../__snapshots__/PageLayout.test.tsx.snap | 120 +++++++++--------- packages/react/src/PageLayout/paneUtils.ts | 46 +++++++ packages/react/src/PageLayout/usePaneWidth.ts | 26 +--- 5 files changed, 128 insertions(+), 119 deletions(-) create mode 100644 packages/react/src/PageLayout/paneUtils.ts diff --git a/packages/react/src/PageLayout/PageLayout.module.css b/packages/react/src/PageLayout/PageLayout.module.css index eb3d2a1e053..4a7388568b1 100644 --- a/packages/react/src/PageLayout/PageLayout.module.css +++ b/packages/react/src/PageLayout/PageLayout.module.css @@ -716,7 +716,7 @@ position: absolute; inset: 0; background-color: var(--bgColor-neutral-muted); - opacity: var(--drag-opacity, 0); + opacity: var(--draggable-handle--drag-opacity, 0); transition: opacity 150ms ease; /* compositor-friendly */ border-radius: inherit; /* optional if you need rounded corners */ } diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 1cee5973ddc..5c471be53d7 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -20,6 +20,7 @@ import { type CustomWidthOptions, type PaneWidth, } from './usePaneWidth' +import {setDraggingStyles, removeDraggingStyles} from './paneUtils' const REGION_ORDER = { header: 0, @@ -214,19 +215,6 @@ type DragHandleProps = { 'aria-valuenow'?: number } -function setDraggingStyleOptimizations(element: HTMLElement | null) { - if (!element) return - element.style.pointerEvents = 'none' - element.style.contain = 'layout style paint' - element.style.contentVisibility = 'auto' -} - -function removeDraggingStylesOptimizations(element: HTMLElement | null) { - if (!element) return - element.style.pointerEvents = '' - element.style.contain = '' - element.style.contentVisibility = '' -} /** * DragHandle - handles all pointer and keyboard interactions for resizing * ARIA values are set in JSX for SSR accessibility, @@ -256,35 +244,25 @@ const DragHandle = memo(function DragHandle({ // Dragging state as a ref - cheaper than reading from DOM style const isDraggingRef = React.useRef(false) - // Helper to set/remove dragging optimization styles - // All properties set as inline styles ONLY during drag for zero overhead at rest + // Set inline styles for drag optimizations - zero overhead at rest const startDragging = React.useCallback(() => { if (isDraggingRef.current) return - // Handle: visual feedback - handleRef.current?.style.setProperty('background-color', 'var(--bgColor-accent-emphasis)') - handleRef.current?.style.setProperty('--drag-opacity', '1') - // Pane: hint browser to optimize for width changes - paneRef.current?.style.setProperty('will-change', 'width') - // Content & Pane: isolate from children during drag - // contain: limits layout/paint recalc to this subtree - // content-visibility: skip rendering off-screen content (valuable for large DOMs) - // pointer-events: skip hit-testing large child trees - setDraggingStyleOptimizations(contentRef.current) - setDraggingStyleOptimizations(paneRef.current) + setDraggingStyles({ + handle: handleRef.current, + pane: paneRef.current, + content: contentRef.current, + }) isDraggingRef.current = true }, [handleRef, contentRef, paneRef]) const endDragging = React.useCallback(() => { if (!isDraggingRef.current) return - const handle = handleRef.current - const content = contentRef.current - const pane = paneRef.current - - handle?.style.removeProperty('background-color') - handle?.style.removeProperty('--drag-opacity') - pane?.style.removeProperty('will-change') - removeDraggingStylesOptimizations(content) - removeDraggingStylesOptimizations(pane) + removeDraggingStyles({ + handle: handleRef.current, + pane: paneRef.current, + content: contentRef.current, + }) + isDraggingRef.current = false }, [handleRef, contentRef, paneRef]) @@ -338,13 +316,12 @@ const DragHandle = memo(function DragHandle({ }, []) /** - * Pointer up ends a drag operation - * Prevents default to avoid unwanted selection behavior + * Pointer up - cleanup is handled by onLostPointerCapture event + * which fires when pointer capture is released (including on pointerup) */ const handlePointerUp = React.useCallback((event: React.PointerEvent) => { if (!isDraggingRef.current) return event.preventDefault() - // Cleanup will happen in onLostPointerCapture }, []) /** diff --git a/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap b/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap index 0f60be9ba4a..8cdf4be50eb 100644 --- a/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap +++ b/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap @@ -3,39 +3,39 @@ exports[`PageLayout > renders condensed layout 1`] = `
Header
@@ -43,26 +43,26 @@ exports[`PageLayout > renders condensed layout 1`] = `
Pane
renders condensed layout 1`] = `