From 83540ae98845262edc9fca15ded07fc684ebfe45 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Thu, 11 Dec 2025 23:40:45 -0500 Subject: [PATCH 1/7] =?UTF-8?q?Revert=20"Revert=20"Revert=20"Revert=20"Imp?= =?UTF-8?q?rove=20PageLayout=20pane=20drag=20performance=20with=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 335e9e8eedc23de3041656acffb4873cd007f698. --- .changeset/healthy-poets-act.md | 5 - e2e/components/Axe.test.ts | 2 + .../src/PageLayout/PageLayout.module.css | 73 ++- .../PageLayout.performance.stories.tsx | 492 ++++++++++++++++++ packages/react/src/PageLayout/PageLayout.tsx | 438 +++++++++++----- .../__snapshots__/PageLayout.test.tsx.snap | 8 +- 6 files changed, 853 insertions(+), 165 deletions(-) delete mode 100644 .changeset/healthy-poets-act.md create mode 100644 packages/react/src/PageLayout/PageLayout.performance.stories.tsx diff --git a/.changeset/healthy-poets-act.md b/.changeset/healthy-poets-act.md deleted file mode 100644 index d7924f67564..00000000000 --- a/.changeset/healthy-poets-act.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@primer/react": patch ---- - -Revert PR #7275 diff --git a/e2e/components/Axe.test.ts b/e2e/components/Axe.test.ts index 74b17d88f6b..714ac97c6f0 100644 --- a/e2e/components/Axe.test.ts +++ b/e2e/components/Axe.test.ts @@ -14,6 +14,8 @@ const SKIPPED_TESTS = [ 'components-flash-features--with-icon-action-dismiss', // TODO: Remove once color-contrast issues have been resolved 'components-flash-features--with-icon-and-action', // TODO: Remove once color-contrast issues have been resolved 'components-filteredactionlist--default', + 'components-pagelayout-performance-tests--medium-content', + 'components-pagelayout-performance-tests--heavy-content', ] type Component = { diff --git a/packages/react/src/PageLayout/PageLayout.module.css b/packages/react/src/PageLayout/PageLayout.module.css index 5c3041059ea..a90c9e9f7ec 100644 --- a/packages/react/src/PageLayout/PageLayout.module.css +++ b/packages/react/src/PageLayout/PageLayout.module.css @@ -1,15 +1,3 @@ -/* Maintain resize cursor while dragging */ -/* stylelint-disable-next-line selector-no-qualifying-type */ -body[data-page-layout-dragging='true'] { - cursor: col-resize; -} - -/* Disable text selection while dragging */ -/* stylelint-disable-next-line selector-no-qualifying-type */ -body[data-page-layout-dragging='true'] * { - user-select: none; -} - .PageLayoutRoot { /* Region Order */ --region-order-header: 0; @@ -383,6 +371,26 @@ body[data-page-layout-dragging='true'] * { } } +/** + * OPTIMIZATION: Aggressive containment during drag for ContentWrapper + * CSS handles most optimizations automatically via :has() selector + * JavaScript only handles scroll locking (can't be done in CSS) + */ +.PageLayoutContent:has(.DraggableHandle[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%; @@ -409,6 +417,16 @@ body[data-page-layout-dragging='true'] * { } } +/** + * OPTIMIZATION: Freeze content layout during resize drag + * This prevents expensive recalculations of large content areas + * while keeping content visible (just frozen in place) + */ +.PageLayoutContent:has(.DraggableHandle[data-dragging='true']) .Content { + /* Full containment (without size) - isolate from layout recalculations */ + contain: layout style paint; +} + .PaneWrapper { display: flex; width: 100%; @@ -598,6 +616,26 @@ body[data-page-layout-dragging='true'] * { } } +/** + * OPTIMIZATION: Performance enhancements for Pane during drag + * CSS handles all optimizations automatically - JavaScript only locks scroll + */ +.PaneWrapper:has(.DraggableHandle[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 */ @@ -696,12 +734,22 @@ body[data-page-layout-dragging='true'] * { padding: var(--spacing); } +/** + * DraggableHandle - Interactive resize handle + */ .DraggableHandle { 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 + */ + touch-action: none; + user-select: none; } .DraggableHandle:hover { @@ -710,6 +758,7 @@ body[data-page-layout-dragging='true'] * { .DraggableHandle[data-dragging='true'] { background-color: var(--bgColor-accent-emphasis); + cursor: col-resize; } .DraggableHandle[data-dragging='true']:hover { diff --git a/packages/react/src/PageLayout/PageLayout.performance.stories.tsx b/packages/react/src/PageLayout/PageLayout.performance.stories.tsx new file mode 100644 index 00000000000..b1b887999ba --- /dev/null +++ b/packages/react/src/PageLayout/PageLayout.performance.stories.tsx @@ -0,0 +1,492 @@ +import React from 'react' +import type {Meta, StoryObj} from '@storybook/react-vite' +import {PageLayout} from './PageLayout' +import {Button} from '../Button' +import Label from '../Label' +import Heading from '../Heading' + +const meta: Meta = { + title: 'Components/PageLayout/Performance Tests', + component: PageLayout, +} + +export default meta + +type Story = StoryObj + +// ============================================================================ +// Story 1: Baseline - Light Content (~100 elements) +// ============================================================================ + +export const BaselineLight: Story = { + name: '1. Light Content - Baseline (~100 elements)', + render: () => { + return ( + + + Light Content Baseline + + + +
+

Minimal DOM elements to establish baseline.

+

Should be effortless 60 FPS.

+
+ + + +
+

Drag to test - should be instant.

+
+ + + ) + }, +} + +// ============================================================================ +// Story 2: Medium Content - Virtualized Table (~3000 elements) +// ============================================================================ + +export const MediumContent: Story = { + name: '2. Medium Content - Large Table (~3000 elements)', + render: () => { + return ( + + + Medium Content - Large Table + + +
+

Performance Monitor

+
+ DOM Load: ~3,000 elements +
+ Table: 300 rows × 10 cols +
+
+
+ + +
+ {/* Large table with complex cells */} +

Data Table (300 rows × 10 columns)

+
+ + + + {['ID', 'Name', 'Email', 'Role', 'Status', 'Date', 'Count', 'Value', 'Tags', 'Actions'].map( + (header, i) => ( + + ), + )} + + + + {Array.from({length: 300}).map((_, rowIndex) => ( + + + + + + + + + + + + + ))} + +
+ {header} +
#{10000 + rowIndex} + {['Alice', 'Bob', 'Charlie', 'Diana', 'Eve'][rowIndex % 5]}{' '} + {['Smith', 'Jones', 'Davis'][rowIndex % 3]} + + user{rowIndex}@example.com + + {['Admin', 'Editor', 'Viewer', 'Manager'][rowIndex % 4]} + + + + 2024-{String((rowIndex % 12) + 1).padStart(2, '0')}- + {String((rowIndex % 28) + 1).padStart(2, '0')} + + {(rowIndex * 17) % 1000} + + ${((rowIndex * 123.45) % 10000).toFixed(2)} + + + + + + +
+
+
+ + + ) + }, +} + +// ============================================================================ +// Story 3: Heavy Content - Multiple Sections (~5000 elements) +// ============================================================================ + +export const HeavyContent: Story = { + name: '3. Heavy Content - Multiple Sections (~5000 elements)', + render: () => { + return ( + + + Heavy Content - Multiple Sections (~5000 elements) + + + +
+
+ DOM Load: ~5,000 elements +
+ Mix: Cards, tables, lists +
+
+

+ Sections: +

+
    +
  • 200 activity cards (~1000 elem)
    • +
  • 150-row table (~1200 elem)
    • +
  • 200 issue items (~1200 elem)
    • +
  • + Headers, buttons, etc
    • +
+
+ + + +
+ {/* Section 1: Large card grid */} +
+

Activity Feed (200 cards)

+
+ {Array.from({length: 200}).map((_, i) => ( +
+
+ Activity #{i + 1} + {i % 60}m ago +
+
+ User {['Alice', 'Bob', 'Charlie'][i % 3]} performed action on item {i} +
+
+ + +
+
+ ))} +
+
+ + {/* Section 2: Large table */} +
+

Data Table (150 rows × 8 columns)

+ + + + {['ID', 'Name', 'Type', 'Status', 'Date', 'Value', 'Priority', 'Owner'].map((header, i) => ( + + ))} + + + + {Array.from({length: 150}).map((_, i) => ( + + + + + + + + + + + ))} + +
+ {header} +
#{5000 + i}Item {i + 1} + {['Type A', 'Type B', 'Type C', 'Type D'][i % 4]} + + + + Dec {(i % 30) + 1} + ${(i * 50 + 100).toFixed(2)}{['Low', 'Medium', 'High'][i % 3]}user{i % 20}
+
+ + {/* Section 3: List with nested content */} +
+

Issue Tracker (200 items)

+ {Array.from({length: 200}).map((_, i) => ( +
+
+
+ Issue #{i + 1} + +
+ {i % 10}d ago +
+
+ Description for issue {i + 1}: This is some text that describes the issue in detail. +
+
+ 👤 {['alice', 'bob', 'charlie'][i % 3]} + 💬 {i % 15} comments + ⭐ {i % 20} reactions +
+
+ ))} +
+
+ + + ) + }, +} + +export const ResponsiveConstraintsTest: Story = { + render: () => { + const [viewportWidth, setViewportWidth] = React.useState(typeof window !== 'undefined' ? window.innerWidth : 1280) + + React.useEffect(() => { + const handleResize = () => setViewportWidth(window.innerWidth) + // eslint-disable-next-line github/prefer-observers + window.addEventListener('resize', handleResize) + return () => window.removeEventListener('resize', handleResize) + }, []) + + const maxWidthDiff = viewportWidth >= 1280 ? 959 : 511 + const calculatedMaxWidth = Math.max(256, viewportWidth - maxWidthDiff) + + return ( + + + Responsive Constraints Test + + + +
+

Max width: {calculatedMaxWidth}px

+
+ + + +
+

Test responsive max width constraints

+

Resize window and watch max pane width update.

+
+ + + ) + }, +} + +export const KeyboardARIATest: Story = { + name: 'Keyboard & ARIA Test', + render: () => { + const [ariaAttributes, setAriaAttributes] = React.useState({ + valuemin: '—', + valuemax: '—', + valuenow: '—', + valuetext: '—', + }) + + React.useEffect(() => { + if (typeof window === 'undefined') { + return undefined + } + + const ATTRIBUTE_NAMES = ['aria-valuemin', 'aria-valuemax', 'aria-valuenow', 'aria-valuetext'] as const + const attributeFilter = ATTRIBUTE_NAMES.map(attribute => attribute) + let handleElement: HTMLElement | null = null + const mutationObserver = new MutationObserver(() => { + if (!handleElement) return + setAriaAttributes({ + valuemin: handleElement.getAttribute('aria-valuemin') ?? '—', + valuemax: handleElement.getAttribute('aria-valuemax') ?? '—', + valuenow: handleElement.getAttribute('aria-valuenow') ?? '—', + valuetext: handleElement.getAttribute('aria-valuetext') ?? '—', + }) + }) + + const attachObserver = () => { + handleElement = document.querySelector("[role='slider'][aria-label='Draggable pane splitter']") + if (!handleElement) return false + + mutationObserver.observe(handleElement, { + attributes: true, + attributeFilter, + }) + + setAriaAttributes({ + valuemin: handleElement.getAttribute('aria-valuemin') ?? '—', + valuemax: handleElement.getAttribute('aria-valuemax') ?? '—', + valuenow: handleElement.getAttribute('aria-valuenow') ?? '—', + valuetext: handleElement.getAttribute('aria-valuetext') ?? '—', + }) + + return true + } + + const retryInterval = window.setInterval(() => { + if (attachObserver()) { + window.clearInterval(retryInterval) + } + }, 100) + + return () => { + window.clearInterval(retryInterval) + mutationObserver.disconnect() + } + }, []) + + return ( + + + Keyboard & ARIA Test + + + +
+

Use keyboard: ← → ↑ ↓

+
+ + + +
+

Test Instructions

+
    +
  1. Tab to resize handle
    2. +
  2. Use arrow keys to resize
    3. +
  3. Test with screen reader
    4. +
+
+

Live ARIA attributes

+
+
aria-valuemin
+
{ariaAttributes.valuemin}
+
aria-valuemax
+
{ariaAttributes.valuemax}
+
aria-valuenow
+
{ariaAttributes.valuenow}
+
aria-valuetext
+
{ariaAttributes.valuetext}
+
+

+ Values update live when the slider handle changes size via keyboard or pointer interactions. +

+
+
+ + + ) + }, +} diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 29afbe4477a..3aafaf85b22 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -5,13 +5,68 @@ import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' import type {ResponsiveValue} from '../hooks/useResponsiveValue' import {isResponsiveValue} from '../hooks/useResponsiveValue' import {useSlots} from '../hooks/useSlots' -import {canUseDOM} from '../utils/environment' import {useOverflow} from '../hooks/useOverflow' import {warning} from '../utils/warning' import {getResponsiveAttributes} from '../internal/utils/getResponsiveAttributes' import classes from './PageLayout.module.css' import type {FCWithSlotMarker, WithSlotMarker} from '../utils/types' +import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' + +// Module-scoped ResizeObserver subscription for viewport width tracking +let viewportWidthListeners: Set<() => void> | undefined +let viewportWidthObserver: ResizeObserver | undefined + +function subscribeToViewportWidth(callback: () => void) { + if (!viewportWidthListeners) { + viewportWidthListeners = new Set() + viewportWidthObserver = new ResizeObserver(() => { + if (viewportWidthListeners) { + for (const listener of viewportWidthListeners) { + listener() + } + } + }) + viewportWidthObserver.observe(document.documentElement) + } + + viewportWidthListeners.add(callback) + + return () => { + viewportWidthListeners?.delete(callback) + if (viewportWidthListeners?.size === 0) { + viewportWidthObserver?.disconnect() + viewportWidthObserver = undefined + viewportWidthListeners = undefined + } + } +} + +function getViewportWidth() { + return window.innerWidth +} + +function getServerViewportWidth() { + return 0 +} + +/** + * Custom hook that subscribes to viewport width changes using a shared ResizeObserver + */ +function useViewportWidth() { + return React.useSyncExternalStore(subscribeToViewportWidth, getViewportWidth, getServerViewportWidth) +} + +/** + * Gets the --pane-max-width-diff CSS variable value from a pane element. + * This value is set by CSS media queries and controls the max pane width constraint. + * Falls back to 511 (the CSS default) if the value cannot be read. + */ +function getPaneMaxWidthDiff(paneElement: HTMLElement | null): number { + if (!paneElement) return 511 + const value = parseInt(getComputedStyle(paneElement).getPropertyValue('--pane-max-width-diff'), 10) + return value > 0 ? value : 511 +} const REGION_ORDER = { header: 0, @@ -147,16 +202,33 @@ const HorizontalDivider: React.FC> = ({ type DraggableDividerProps = { draggable?: boolean - onDragStart?: () => void - onDrag?: (delta: number, isKeyboard: boolean) => void - onDragEnd?: () => void - onDoubleClick?: () => void + handleRef: React.RefObject + onDrag: (delta: number, isKeyboard: boolean) => void + onDragEnd: () => void + onDoubleClick: () => void +} + +// Helper to update ARIA slider attributes via direct DOM manipulation +// This avoids re-renders when values change during drag or on viewport resize +const updateAriaValues = (handle: HTMLElement | null, values: {current?: number; min?: number; max?: number}) => { + if (!handle) return + if (values.min !== undefined) handle.setAttribute('aria-valuemin', String(values.min)) + if (values.max !== undefined) handle.setAttribute('aria-valuemax', String(values.max)) + if (values.current !== undefined) { + handle.setAttribute('aria-valuenow', String(values.current)) + handle.setAttribute('aria-valuetext', `Pane width ${values.current} pixels`) + } +} + +const DATA_DRAGGING_ATTR = 'data-dragging' +const isDragging = (handle: HTMLElement | null) => { + return handle?.getAttribute(DATA_DRAGGING_ATTR) === 'true' } const VerticalDivider: React.FC> = ({ variant = 'none', draggable = false, - onDragStart, + handleRef, onDrag, onDragEnd, onDoubleClick, @@ -164,100 +236,114 @@ const VerticalDivider: React.FC { - const [isDragging, setIsDragging] = React.useState(false) - const [isKeyboardDrag, setIsKeyboardDrag] = React.useState(false) - const stableOnDrag = React.useRef(onDrag) const stableOnDragEnd = React.useRef(onDragEnd) + React.useEffect(() => { + stableOnDrag.current = onDrag + stableOnDragEnd.current = onDragEnd + }) const {paneRef} = React.useContext(PageLayoutContext) - const [minWidth, setMinWidth] = React.useState(0) - const [maxWidth, setMaxWidth] = React.useState(0) - const [currentWidth, setCurrentWidth] = React.useState(0) - - React.useEffect(() => { - if (paneRef.current !== null) { - const paneStyles = getComputedStyle(paneRef.current as Element) - const maxPaneWidthDiffPixels = paneStyles.getPropertyValue('--pane-max-width-diff') - const minWidthPixels = paneStyles.getPropertyValue('--pane-min-width') - const paneWidth = paneRef.current.getBoundingClientRect().width - const maxPaneWidthDiff = Number(maxPaneWidthDiffPixels.split('px')[0]) - const minPaneWidth = Number(minWidthPixels.split('px')[0]) - const viewportWidth = window.innerWidth - const maxPaneWidth = viewportWidth > maxPaneWidthDiff ? viewportWidth - maxPaneWidthDiff : viewportWidth - setMinWidth(minPaneWidth) - setMaxWidth(maxPaneWidth) - setCurrentWidth(paneWidth || 0) - } - }, [paneRef, isKeyboardDrag, isDragging]) + /** + * 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) + target.setAttribute(DATA_DRAGGING_ATTR, 'true') + }, []) - React.useEffect(() => { - stableOnDrag.current = onDrag - }, [onDrag]) + /** + * Pointer move during drag + * Calls onDrag with movement delta + * Prevents default to avoid unwanted selection behavior + */ + const handlePointerMove = React.useCallback( + (event: React.PointerEvent) => { + if (!isDragging(handleRef.current)) return + event.preventDefault() - React.useEffect(() => { - stableOnDragEnd.current = onDragEnd - }, [onDragEnd]) + if (event.movementX !== 0) { + stableOnDrag.current(event.movementX, false) + } + }, + [handleRef], + ) - React.useEffect(() => { - function handleDrag(event: MouseEvent) { - stableOnDrag.current?.(event.movementX, false) + /** + * 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], + ) - function handleDragEnd(event: MouseEvent) { - setIsDragging(false) - stableOnDragEnd.current?.() - event.preventDefault() - } + /** + * Lost pointer capture ends a drag operation + * Cleans up dragging state + * Calls onDragEnd callback + */ + const handleLostPointerCapture = React.useCallback( + (event: React.PointerEvent) => { + if (!isDragging(handleRef.current)) return + const target = event.currentTarget + target.removeAttribute(DATA_DRAGGING_ATTR) + stableOnDragEnd.current() + }, + [handleRef], + ) - function handleKeyDrag(event: KeyboardEvent) { - let delta = 0 - // https://github.com/github/accessibility/issues/5101#issuecomment-1822870655 - if ((event.key === 'ArrowLeft' || event.key === 'ArrowDown') && currentWidth > minWidth) { - delta = -3 - } else if ((event.key === 'ArrowRight' || event.key === 'ArrowUp') && currentWidth < maxWidth) { - delta = 3 - } else { - return + /** + * Keyboard handling for accessibility + * Arrow keys adjust the pane size in 3px increments + * Prevents default scrolling behavior + * Sets and clears dragging state via data attribute + * Calls onDrag + */ + 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 + + // https://github.com/github/accessibility/issues/5101#issuecomment-1822870655 + const delta = event.key === 'ArrowLeft' || event.key === 'ArrowDown' ? -3 : 3 + + event.currentTarget.setAttribute(DATA_DRAGGING_ATTR, 'true') + stableOnDrag.current(delta, true) } - setCurrentWidth(currentWidth + delta) - stableOnDrag.current?.(delta, true) - event.preventDefault() - } + }, + [paneRef], + ) - function handleKeyDragEnd(event: KeyboardEvent) { - setIsKeyboardDrag(false) - stableOnDragEnd.current?.() + const handleKeyUp = React.useCallback((event: React.KeyboardEvent) => { + if ( + event.key === 'ArrowLeft' || + event.key === 'ArrowRight' || + event.key === 'ArrowUp' || + event.key === 'ArrowDown' + ) { event.preventDefault() + event.currentTarget.removeAttribute(DATA_DRAGGING_ATTR) + stableOnDragEnd.current() } - // TODO: Support touch events - if (isDragging || isKeyboardDrag) { - window.addEventListener('mousemove', handleDrag) - window.addEventListener('keydown', handleKeyDrag) - window.addEventListener('mouseup', handleDragEnd) - window.addEventListener('keyup', handleKeyDragEnd) - const body = document.body as HTMLElement | undefined - body?.setAttribute('data-page-layout-dragging', 'true') - } else { - window.removeEventListener('mousemove', handleDrag) - window.removeEventListener('mouseup', handleDragEnd) - window.removeEventListener('keydown', handleKeyDrag) - window.removeEventListener('keyup', handleKeyDragEnd) - const body = document.body as HTMLElement | undefined - body?.removeAttribute('data-page-layout-dragging') - } - - return () => { - window.removeEventListener('mousemove', handleDrag) - window.removeEventListener('mouseup', handleDragEnd) - window.removeEventListener('keydown', handleKeyDrag) - window.removeEventListener('keyup', handleKeyDragEnd) - const body = document.body as HTMLElement | undefined - body?.removeAttribute('data-page-layout-dragging') - } - }, [isDragging, isKeyboardDrag, currentWidth, minWidth, maxWidth]) + }, []) return (
{draggable ? ( - // Drag handle + // Drag handle - ARIA attributes set via DOM manipulation for performance
{ - if (event.button === 0) { - setIsDragging(true) - onDragStart?.() - } - }} - onKeyDown={(event: React.KeyboardEvent) => { - if ( - event.key === 'ArrowLeft' || - event.key === 'ArrowRight' || - event.key === 'ArrowUp' || - event.key === 'ArrowDown' - ) { - setIsKeyboardDrag(true) - onDragStart?.() - } - }} + onPointerDown={handlePointerDown} + onPointerMove={handlePointerMove} + onPointerUp={handlePointerUp} + onLostPointerCapture={handleLostPointerCapture} + onKeyDown={handleKeyDown} + onKeyUp={handleKeyUp} onDoubleClick={onDoubleClick} /> ) : null} @@ -490,6 +568,15 @@ const isPaneWidth = (width: PaneWidth | CustomWidthOptions): width is PaneWidth return ['small', 'medium', 'large'].includes(width as PaneWidth) } +const getDefaultPaneWidth = (w: PaneWidth | CustomWidthOptions): number => { + if (isPaneWidth(w)) { + return defaultPaneWidth[w] + } else if (isCustomWidthOptions(w)) { + return parseInt(w.default, 10) + } + return 0 +} + export type PageLayoutPaneProps = { position?: keyof typeof panePositions | ResponsiveValue /** @@ -598,40 +685,65 @@ const Pane = React.forwardRef { - if (isPaneWidth(width)) { - return defaultPaneWidth[width] - } else if (isCustomWidthOptions(width)) { - return Number(width.default.split('px')[0]) - } - return 0 - } + // Initial pane width for the first render - only used to set the initial CSS variable. + // After mount, all updates go directly to the DOM via style.setProperty() to avoid re-renders. + const defaultWidth = getDefaultPaneWidth(width) - const [paneWidth, setPaneWidth] = React.useState(() => { - if (!canUseDOM) { - return getDefaultPaneWidth(width) - } + // Track current width during drag - initialized lazily in layout effect + const currentWidthRef = React.useRef(defaultWidth) - let storedWidth + // Track whether we've initialized the width from localStorage + const initializedRef = React.useRef(false) + useIsomorphicLayoutEffect(() => { + // Only initialize once on mount - subsequent updates come from drag operations + if (initializedRef.current || !resizable) return + initializedRef.current = true + // Before paint, check localStorage for a stored width try { - storedWidth = localStorage.getItem(widthStorageKey) - } catch (_error) { - storedWidth = null + const value = localStorage.getItem(widthStorageKey) + if (value !== null && !isNaN(Number(value))) { + const num = Number(value) + currentWidthRef.current = num + paneRef.current?.style.setProperty('--pane-width', `${num}px`) + return + } + } catch { + // localStorage unavailable - set default via DOM } + paneRef.current?.style.setProperty('--pane-width', `${defaultWidth}px`) + }, [widthStorageKey, paneRef, resizable, defaultWidth]) - return storedWidth && !isNaN(Number(storedWidth)) ? Number(storedWidth) : getDefaultPaneWidth(width) - }) + // Subscribe to viewport width changes for responsive max constraint calculation + const viewportWidth = useViewportWidth() - const updatePaneWidth = (width: number) => { - setPaneWidth(width) + // Calculate min width constraint from width configuration + const minPaneWidth = isCustomWidthOptions(width) ? parseInt(width.min, 10) : minWidth - try { - localStorage.setItem(widthStorageKey, width.toString()) - } catch (_error) { - // Ignore errors + // Cache max width constraint - updated when viewport changes (which triggers CSS breakpoint changes) + // This avoids calling getComputedStyle() on every drag frame + const maxPaneWidthRef = React.useRef(minPaneWidth) + React.useEffect(() => { + if (isCustomWidthOptions(width)) { + maxPaneWidthRef.current = parseInt(width.max, 10) + } else { + const maxWidthDiff = getPaneMaxWidthDiff(paneRef.current) + maxPaneWidthRef.current = + viewportWidth > 0 ? Math.max(minPaneWidth, viewportWidth - maxWidthDiff) : minPaneWidth } - } + }, [width, minPaneWidth, viewportWidth, paneRef]) + + // Ref to the drag handle for updating ARIA attributes + const handleRef = React.useRef(null) + + // Update ARIA attributes on mount and when viewport/constraints change + useIsomorphicLayoutEffect(() => { + updateAriaValues(handleRef.current, { + min: minPaneWidth, + max: maxPaneWidthRef.current, + current: currentWidthRef.current!, + }) + }, [minPaneWidth, viewportWidth]) useRefObjectAsForwardedRef(forwardRef, paneRef) @@ -655,6 +767,14 @@ const Pane = React.forwardRef { + try { + localStorage.setItem(widthStorageKey, value.toString()) + } catch { + // Ignore write errors + } + } + return (
@@ -717,25 +837,55 @@ const Pane = React.forwardRef { - // Get the number of pixels the divider was dragged - let deltaWithDirection + const deltaWithDirection = isKeyboard ? delta : position === 'end' ? -delta : delta + const maxWidth = maxPaneWidthRef.current + if (isKeyboard) { - deltaWithDirection = delta + // Clamp keyboard delta to stay within bounds + const newWidth = Math.max(minPaneWidth, Math.min(maxWidth, currentWidthRef.current! + deltaWithDirection)) + if (newWidth !== currentWidthRef.current) { + currentWidthRef.current = newWidth + paneRef.current?.style.setProperty('--pane-width', `${newWidth}px`) + updateAriaValues(handleRef.current, {current: newWidth}) + } } else { - deltaWithDirection = position === 'end' ? -delta : delta + // Apply delta directly via CSS variable for immediate visual feedback + if (paneRef.current) { + const newWidth = currentWidthRef.current! + deltaWithDirection + const clampedWidth = Math.max(minPaneWidth, Math.min(maxWidth, newWidth)) + + // Only update if the clamped width actually changed + // This prevents drift when dragging against min/max constraints + if (clampedWidth !== currentWidthRef.current) { + paneRef.current.style.setProperty('--pane-width', `${clampedWidth}px`) + currentWidthRef.current = clampedWidth + updateAriaValues(handleRef.current, {current: clampedWidth}) + } + } } - updatePaneWidth(paneWidth + deltaWithDirection) }} - // Ensure `paneWidth` state and actual pane width are in sync when the drag ends + // Save final width to localStorage (skip React state update to avoid reconciliation) onDragEnd={() => { - const paneRect = paneRef.current?.getBoundingClientRect() - if (!paneRect) return - updatePaneWidth(paneRect.width) + // For mouse drag: The CSS variable is already set and currentWidthRef is in sync. + // We intentionally skip setPaneWidth() to avoid triggering expensive React + // reconciliation with large DOM trees. The ref is the source of truth for + // subsequent drag operations. + setWidthInLocalStorage(currentWidthRef.current!) }} position={positionProp} // Reset pane width on double click - onDoubleClick={() => updatePaneWidth(getDefaultPaneWidth(width))} + onDoubleClick={() => { + const defaultWidth = getDefaultPaneWidth(width) + // Update CSS variable and ref directly - skip React state to avoid reconciliation + if (paneRef.current) { + paneRef.current.style.setProperty('--pane-width', `${defaultWidth}px`) + currentWidthRef.current = defaultWidth + updateAriaValues(handleRef.current, {current: defaultWidth}) + } + setWidthInLocalStorage(defaultWidth) + }} className={classes.PaneVerticalDivider} style={ { diff --git a/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap b/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap index 5ac82e6ca02..3fd829d183b 100644 --- a/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap +++ b/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap @@ -57,7 +57,7 @@ exports[`PageLayout > renders condensed layout 1`] = ` />
Pane
@@ -149,7 +149,7 @@ exports[`PageLayout > renders default layout 1`] = ` />
Pane
@@ -243,7 +243,7 @@ exports[`PageLayout > renders pane in different position when narrow 1`] = ` />
Pane
@@ -337,7 +337,7 @@ exports[`PageLayout > renders with dividers 1`] = ` />
Pane
From 8b08d1209674745e65576f059358134b13a8eabe Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 12 Dec 2025 09:54:52 -0500 Subject: [PATCH 2/7] Removes a resize subscription and expensive `:has` selectors from pagelayout (#7302) --- .changeset/shiny-buckets-study.md | 5 + .../src/PageLayout/PageLayout.module.css | 23 +- .../PageLayout.performance.stories.tsx | 55 +- .../react/src/PageLayout/PageLayout.test.tsx | 55 ++ packages/react/src/PageLayout/PageLayout.tsx | 481 +++++++--------- .../react/src/PageLayout/usePaneWidth.test.ts | 521 ++++++++++++++++++ packages/react/src/PageLayout/usePaneWidth.ts | 289 ++++++++++ 7 files changed, 1128 insertions(+), 301 deletions(-) create mode 100644 .changeset/shiny-buckets-study.md create mode 100644 packages/react/src/PageLayout/usePaneWidth.test.ts create mode 100644 packages/react/src/PageLayout/usePaneWidth.ts diff --git a/.changeset/shiny-buckets-study.md b/.changeset/shiny-buckets-study.md new file mode 100644 index 00000000000..5888789fad1 --- /dev/null +++ b/.changeset/shiny-buckets-study.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +fix resizeobserver perf regression diff --git a/packages/react/src/PageLayout/PageLayout.module.css b/packages/react/src/PageLayout/PageLayout.module.css index a90c9e9f7ec..dd46fd0fcca 100644 --- a/packages/react/src/PageLayout/PageLayout.module.css +++ b/packages/react/src/PageLayout/PageLayout.module.css @@ -1,3 +1,11 @@ +/* Exported values for JavaScript consumption */ +:export { + /* Breakpoint where --pane-max-width-diff changes (used in usePaneWidth.ts) */ + paneMaxWidthDiffBreakpoint: 1280; + /* Default value for --pane-max-width-diff below the breakpoint */ + paneMaxWidthDiffDefault: 511; +} + .PageLayoutRoot { /* Region Order */ --region-order-header: 0; @@ -19,6 +27,7 @@ --pane-width-small: 100%; --pane-width-medium: 100%; --pane-width-large: 100%; + /* NOTE: This value is exported via :export for use in usePaneWidth.ts */ --pane-max-width-diff: 511px; @media screen and (min-width: 768px) { @@ -33,6 +42,7 @@ --pane-width-large: 320px; } + /* NOTE: This breakpoint value is exported via :export for use in usePaneWidth.ts */ @media screen and (min-width: 1280px) { --pane-max-width-diff: 959px; } @@ -373,10 +383,10 @@ /** * OPTIMIZATION: Aggressive containment during drag for ContentWrapper - * CSS handles most optimizations automatically via :has() selector - * JavaScript only handles scroll locking (can't be done in CSS) + * data-dragging is set on PageLayoutContent by JavaScript + * This avoids expensive :has() selectors */ -.PageLayoutContent:has(.DraggableHandle[data-dragging='true']) .ContentWrapper { +.PageLayoutContent[data-dragging='true'] .ContentWrapper { /* Add paint containment during drag - safe since user can't interact */ contain: layout style paint; @@ -422,7 +432,7 @@ * This prevents expensive recalculations of large content areas * while keeping content visible (just frozen in place) */ -.PageLayoutContent:has(.DraggableHandle[data-dragging='true']) .Content { +.PageLayoutContent[data-dragging='true'] .Content { /* Full containment (without size) - isolate from layout recalculations */ contain: layout style paint; } @@ -618,9 +628,10 @@ /** * OPTIMIZATION: Performance enhancements for Pane during drag - * CSS handles all optimizations automatically - JavaScript only locks scroll + * data-dragging is set on PageLayoutContent by JavaScript + * This avoids expensive :has() selectors */ -.PaneWrapper:has(.DraggableHandle[data-dragging='true']) .Pane { +.PageLayoutContent[data-dragging='true'] .Pane { /* Full containment - isolate from layout recalculations */ contain: layout style paint; diff --git a/packages/react/src/PageLayout/PageLayout.performance.stories.tsx b/packages/react/src/PageLayout/PageLayout.performance.stories.tsx index b1b887999ba..ad7f81d6046 100644 --- a/packages/react/src/PageLayout/PageLayout.performance.stories.tsx +++ b/packages/react/src/PageLayout/PageLayout.performance.stories.tsx @@ -1,9 +1,11 @@ -import React from 'react' +import React, {useState} from 'react' import type {Meta, StoryObj} from '@storybook/react-vite' import {PageLayout} from './PageLayout' import {Button} from '../Button' import Label from '../Label' import Heading from '../Heading' +import Autocomplete from '../Autocomplete' +import FormControl from '../FormControl' const meta: Meta = { title: 'Components/PageLayout/Performance Tests', @@ -14,6 +16,47 @@ export default meta type Story = StoryObj +// Autocomplete suggestions data +const suggestions = [ + {id: '1', text: 'JavaScript'}, + {id: '2', text: 'TypeScript'}, + {id: '3', text: 'React'}, + {id: '4', text: 'Vue'}, + {id: '5', text: 'Angular'}, + {id: '6', text: 'Svelte'}, + {id: '7', text: 'Node.js'}, + {id: '8', text: 'Python'}, + {id: '9', text: 'Ruby'}, + {id: '10', text: 'Go'}, +] + +// Reusable stateful autocomplete search component +function SearchInput() { + const [filterValue, setFilterValue] = useState('') + const filteredItems = suggestions.filter(item => item.text.toLowerCase().includes(filterValue.toLowerCase())) + + return ( + + Search + + setFilterValue(e.target.value)} + placeholder="Search items..." + /> + + + + + + ) +} + // ============================================================================ // Story 1: Baseline - Light Content (~100 elements) // ============================================================================ @@ -29,7 +72,8 @@ export const BaselineLight: Story = {
-

Minimal DOM elements to establish baseline.

+ +

Minimal DOM elements to establish baseline.

Should be effortless 60 FPS.

 @@ -58,7 +102,6 @@ export const MediumContent: Story = {
-

Performance Monitor

+ {/* Large table with complex cells */} -

Data Table (300 rows × 10 columns)

+

Data Table (300 rows × 10 columns)

+ {/* Section 1: Large card grid */} -
+

Activity Feed (200 cards)

{Array.from({length: 200}).map((_, i) => ( diff --git a/packages/react/src/PageLayout/PageLayout.test.tsx b/packages/react/src/PageLayout/PageLayout.test.tsx index 370f6e6ca33..8220f425c4e 100644 --- a/packages/react/src/PageLayout/PageLayout.test.tsx +++ b/packages/react/src/PageLayout/PageLayout.test.tsx @@ -174,6 +174,61 @@ describe('PageLayout', async () => { const finalWidth = (pane as HTMLElement).style.getPropertyValue('--pane-width') expect(finalWidth).not.toEqual(initialWidth) }) + + it('should set data-dragging attribute during pointer drag', async () => { + const {container} = render( + + + + + + + + , + ) + + const content = container.querySelector('[class*="PageLayoutContent"]') + const divider = await screen.findByRole('slider') + + // Before drag - no data-dragging attribute + expect(content).not.toHaveAttribute('data-dragging') + + // Start drag + fireEvent.pointerDown(divider, {clientX: 300, clientY: 200, pointerId: 1}) + expect(content).toHaveAttribute('data-dragging', 'true') + + // End drag - pointer capture lost ends the drag and removes attribute + fireEvent.lostPointerCapture(divider, {pointerId: 1}) + expect(content).not.toHaveAttribute('data-dragging') + }) + + it('should set data-dragging attribute during keyboard resize', async () => { + const {container} = render( + + + + + + + + , + ) + + const content = container.querySelector('[class*="PageLayoutContent"]') + const divider = await screen.findByRole('slider') + + // Before interaction - no data-dragging attribute + expect(content).not.toHaveAttribute('data-dragging') + + // Start keyboard resize (focus first) + fireEvent.focus(divider) + fireEvent.keyDown(divider, {key: 'ArrowRight'}) + expect(content).toHaveAttribute('data-dragging', 'true') + + // End keyboard resize - removes attribute + fireEvent.keyUp(divider, {key: 'ArrowRight'}) + expect(content).not.toHaveAttribute('data-dragging') + }) }) describe('PageLayout.Content', () => { diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 3aafaf85b22..49d2f9cfb8c 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -11,62 +11,15 @@ import {getResponsiveAttributes} from '../internal/utils/getResponsiveAttributes import classes from './PageLayout.module.css' import type {FCWithSlotMarker, WithSlotMarker} from '../utils/types' -import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' - -// Module-scoped ResizeObserver subscription for viewport width tracking -let viewportWidthListeners: Set<() => void> | undefined -let viewportWidthObserver: ResizeObserver | undefined - -function subscribeToViewportWidth(callback: () => void) { - if (!viewportWidthListeners) { - viewportWidthListeners = new Set() - viewportWidthObserver = new ResizeObserver(() => { - if (viewportWidthListeners) { - for (const listener of viewportWidthListeners) { - listener() - } - } - }) - viewportWidthObserver.observe(document.documentElement) - } - - viewportWidthListeners.add(callback) - - return () => { - viewportWidthListeners?.delete(callback) - if (viewportWidthListeners?.size === 0) { - viewportWidthObserver?.disconnect() - viewportWidthObserver = undefined - viewportWidthListeners = undefined - } - } -} - -function getViewportWidth() { - return window.innerWidth -} - -function getServerViewportWidth() { - return 0 -} - -/** - * Custom hook that subscribes to viewport width changes using a shared ResizeObserver - */ -function useViewportWidth() { - return React.useSyncExternalStore(subscribeToViewportWidth, getViewportWidth, getServerViewportWidth) -} - -/** - * Gets the --pane-max-width-diff CSS variable value from a pane element. - * This value is set by CSS media queries and controls the max pane width constraint. - * Falls back to 511 (the CSS default) if the value cannot be read. - */ -function getPaneMaxWidthDiff(paneElement: HTMLElement | null): number { - if (!paneElement) return 511 - const value = parseInt(getComputedStyle(paneElement).getPropertyValue('--pane-max-width-diff'), 10) - return value > 0 ? value : 511 -} +import { + usePaneWidth, + updateAriaValues, + isCustomWidthOptions, + isPaneWidth, + ARROW_KEY_STEP, + type CustomWidthOptions, + type PaneWidth, +} from './usePaneWidth' const REGION_ORDER = { header: 0, @@ -88,11 +41,13 @@ const PageLayoutContext = React.createContext<{ rowGap: keyof typeof SPACING_MAP columnGap: keyof typeof SPACING_MAP paneRef: React.RefObject + contentRef: React.RefObject }>({ padding: 'normal', rowGap: 'normal', columnGap: 'normal', paneRef: {current: null}, + contentRef: {current: null}, }) // ---------------------------------------------------------------------------- @@ -132,6 +87,7 @@ const Root: React.FC> = ({ _slotsConfig: slotsConfig, }) => { const paneRef = useRef(null) + const contentRef = useRef(null) const [slots, rest] = useSlots(children, slotsConfig ?? {header: Header, footer: Footer}) @@ -141,8 +97,9 @@ const Root: React.FC> = ({ rowGap, columnGap, paneRef, + contentRef, } - }, [padding, rowGap, columnGap, paneRef]) + }, [padding, rowGap, columnGap, paneRef, contentRef]) return ( @@ -157,7 +114,9 @@ const Root: React.FC> = ({ >
{slots.header} -
{rest}
+
+ {rest} +
{slots.footer}
@@ -200,24 +159,44 @@ const HorizontalDivider: React.FC> = ({ ) } -type DraggableDividerProps = { +type VerticalDividerProps = DividerProps & { draggable?: boolean +} + +const VerticalDivider: React.FC> = ({ + variant = 'none', + position, + className, + style, + children, +}) => { + return ( +
+ {children} +
+ ) +} + +type DragHandleProps = { + /** Ref for imperative ARIA updates during drag */ handleRef: React.RefObject + /** Called with movement delta on each drag tick */ onDrag: (delta: number, isKeyboard: boolean) => void + /** Called when drag operation completes */ onDragEnd: () => void - onDoubleClick: () => void -} - -// Helper to update ARIA slider attributes via direct DOM manipulation -// This avoids re-renders when values change during drag or on viewport resize -const updateAriaValues = (handle: HTMLElement | null, values: {current?: number; min?: number; max?: number}) => { - if (!handle) return - if (values.min !== undefined) handle.setAttribute('aria-valuemin', String(values.min)) - if (values.max !== undefined) handle.setAttribute('aria-valuemax', String(values.max)) - if (values.current !== undefined) { - handle.setAttribute('aria-valuenow', String(values.current)) - handle.setAttribute('aria-valuetext', `Pane width ${values.current} pixels`) - } + /** Reset width on double-click */ + onDoubleClick?: React.MouseEventHandler + /** ARIA slider min value */ + 'aria-valuemin'?: number + /** ARIA slider max value */ + 'aria-valuemax'?: number + /** ARIA slider current value */ + 'aria-valuenow'?: number } const DATA_DRAGGING_ATTR = 'data-dragging' @@ -225,16 +204,19 @@ const isDragging = (handle: HTMLElement | null) => { return handle?.getAttribute(DATA_DRAGGING_ATTR) === 'true' } -const VerticalDivider: React.FC> = ({ - variant = 'none', - draggable = false, +/** + * 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 = ({ handleRef, onDrag, onDragEnd, onDoubleClick, - position, - className, - style, + 'aria-valuemin': ariaValueMin, + 'aria-valuemax': ariaValueMax, + 'aria-valuenow': ariaValueNow, }) => { const stableOnDrag = React.useRef(onDrag) const stableOnDragEnd = React.useRef(onDragEnd) @@ -243,20 +225,38 @@ const VerticalDivider: React.FC { + 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], + ) /** * 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) - target.setAttribute(DATA_DRAGGING_ATTR, 'true') - }, []) + const handlePointerDown = React.useCallback( + (event: React.PointerEvent) => { + if (event.button !== 0) return + event.preventDefault() + const target = event.currentTarget + target.setPointerCapture(event.pointerId) + setDragging(true) + }, + [setDragging], + ) /** * Pointer move during drag @@ -293,15 +293,11 @@ const VerticalDivider: React.FC) => { - if (!isDragging(handleRef.current)) return - const target = event.currentTarget - target.removeAttribute(DATA_DRAGGING_ATTR) - stableOnDragEnd.current() - }, - [handleRef], - ) + const handleLostPointerCapture = React.useCallback(() => { + if (!isDragging(handleRef.current)) return + setDragging(false) + stableOnDragEnd.current() + }, [handleRef, setDragging]) /** * Keyboard handling for accessibility @@ -323,60 +319,50 @@ const VerticalDivider: React.FC) => { - if ( - event.key === 'ArrowLeft' || - event.key === 'ArrowRight' || - event.key === 'ArrowUp' || - event.key === 'ArrowDown' - ) { - event.preventDefault() - event.currentTarget.removeAttribute(DATA_DRAGGING_ATTR) - stableOnDragEnd.current() - } - }, []) + 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() + } + }, + [setDragging], + ) return (
- {draggable ? ( - // Drag handle - ARIA attributes set via DOM manipulation for performance -
- ) : null} -
+ ref={handleRef} + className={classes.DraggableHandle} + role="slider" + aria-label="Draggable pane splitter" + aria-valuemin={ariaValueMin} + aria-valuemax={ariaValueMax} + aria-valuenow={ariaValueNow} + aria-valuetext={ariaValueNow !== undefined ? `Pane width ${ariaValueNow} pixels` : undefined} + tabIndex={0} + onPointerDown={handlePointerDown} + onPointerMove={handlePointerMove} + onPointerUp={handlePointerUp} + onLostPointerCapture={handleLostPointerCapture} + onKeyDown={handleKeyDown} + onKeyUp={handleKeyUp} + onDoubleClick={onDoubleClick} + /> ) } @@ -549,34 +535,6 @@ Content.displayName = 'PageLayout.Content' // ---------------------------------------------------------------------------- // PageLayout.Pane -type Measurement = `${number}px` - -type CustomWidthOptions = { - min: Measurement - default: Measurement - max: Measurement -} - -type PaneWidth = keyof typeof paneWidths - -const isCustomWidthOptions = (width: PaneWidth | CustomWidthOptions): width is CustomWidthOptions => { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - return (width as CustomWidthOptions).default !== undefined -} - -const isPaneWidth = (width: PaneWidth | CustomWidthOptions): width is PaneWidth => { - return ['small', 'medium', 'large'].includes(width as PaneWidth) -} - -const getDefaultPaneWidth = (w: PaneWidth | CustomWidthOptions): number => { - if (isPaneWidth(w)) { - return defaultPaneWidth[w] - } else if (isCustomWidthOptions(w)) { - return parseInt(w.default, 10) - } - return 0 -} - export type PageLayoutPaneProps = { position?: keyof typeof panePositions | ResponsiveValue /** @@ -631,15 +589,6 @@ const panePositions = { end: REGION_ORDER.paneEnd, } -// eslint-disable-next-line @typescript-eslint/no-unused-vars -const paneWidths = { - small: ['100%', null, '240px', '256px'], - medium: ['100%', null, '256px', '296px'], - large: ['100%', null, '256px', '320px'], -} - -const defaultPaneWidth = {small: 256, medium: 296, large: 320} - const overflowProps = {tabIndex: 0, role: 'region'} const Pane = React.forwardRef>( @@ -685,65 +634,18 @@ const Pane = React.forwardRef { - // Only initialize once on mount - subsequent updates come from drag operations - if (initializedRef.current || !resizable) return - initializedRef.current = true - // Before paint, check localStorage for a stored width - try { - const value = localStorage.getItem(widthStorageKey) - if (value !== null && !isNaN(Number(value))) { - const num = Number(value) - currentWidthRef.current = num - paneRef.current?.style.setProperty('--pane-width', `${num}px`) - return - } - } catch { - // localStorage unavailable - set default via DOM - } - paneRef.current?.style.setProperty('--pane-width', `${defaultWidth}px`) - }, [widthStorageKey, paneRef, resizable, defaultWidth]) - - // Subscribe to viewport width changes for responsive max constraint calculation - const viewportWidth = useViewportWidth() - - // Calculate min width constraint from width configuration - const minPaneWidth = isCustomWidthOptions(width) ? parseInt(width.min, 10) : minWidth - - // Cache max width constraint - updated when viewport changes (which triggers CSS breakpoint changes) - // This avoids calling getComputedStyle() on every drag frame - const maxPaneWidthRef = React.useRef(minPaneWidth) - React.useEffect(() => { - if (isCustomWidthOptions(width)) { - maxPaneWidthRef.current = parseInt(width.max, 10) - } else { - const maxWidthDiff = getPaneMaxWidthDiff(paneRef.current) - maxPaneWidthRef.current = - viewportWidth > 0 ? Math.max(minPaneWidth, viewportWidth - maxWidthDiff) : minPaneWidth - } - }, [width, minPaneWidth, viewportWidth, paneRef]) - // Ref to the drag handle for updating ARIA attributes const handleRef = React.useRef(null) - // Update ARIA attributes on mount and when viewport/constraints change - useIsomorphicLayoutEffect(() => { - updateAriaValues(handleRef.current, { - min: minPaneWidth, - max: maxPaneWidthRef.current, - current: currentWidthRef.current!, + const {currentWidth, currentWidthRef, minPaneWidth, maxPaneWidth, getMaxPaneWidth, saveWidth, getDefaultWidth} = + usePaneWidth({ + width, + minWidth, + resizable, + widthStorageKey, + paneRef, + handleRef, }) - }, [minPaneWidth, viewportWidth]) useRefObjectAsForwardedRef(forwardRef, paneRef) @@ -767,14 +669,6 @@ const Pane = React.forwardRef { - try { - localStorage.setItem(widthStorageKey, value.toString()) - } catch { - // Ignore write errors - } - } - return (
@@ -837,62 +733,67 @@ const Pane = React.forwardRef { - const deltaWithDirection = isKeyboard ? delta : position === 'end' ? -delta : delta - const maxWidth = maxPaneWidthRef.current - - if (isKeyboard) { - // Clamp keyboard delta to stay within bounds - const newWidth = Math.max(minPaneWidth, Math.min(maxWidth, currentWidthRef.current! + deltaWithDirection)) - if (newWidth !== currentWidthRef.current) { - currentWidthRef.current = newWidth - paneRef.current?.style.setProperty('--pane-width', `${newWidth}px`) - updateAriaValues(handleRef.current, {current: newWidth}) - } - } else { - // Apply delta directly via CSS variable for immediate visual feedback - if (paneRef.current) { - const newWidth = currentWidthRef.current! + deltaWithDirection - const clampedWidth = Math.max(minPaneWidth, Math.min(maxWidth, newWidth)) - - // Only update if the clamped width actually changed - // This prevents drift when dragging against min/max constraints - if (clampedWidth !== currentWidthRef.current) { - paneRef.current.style.setProperty('--pane-width', `${clampedWidth}px`) - currentWidthRef.current = clampedWidth - updateAriaValues(handleRef.current, {current: clampedWidth}) - } - } - } - }} - // Save final width to localStorage (skip React state update to avoid reconciliation) - onDragEnd={() => { - // For mouse drag: The CSS variable is already set and currentWidthRef is in sync. - // We intentionally skip setPaneWidth() to avoid triggering expensive React - // reconciliation with large DOM trees. The ref is the source of truth for - // subsequent drag operations. - setWidthInLocalStorage(currentWidthRef.current!) - }} position={positionProp} - // Reset pane width on double click - onDoubleClick={() => { - const defaultWidth = getDefaultPaneWidth(width) - // Update CSS variable and ref directly - skip React state to avoid reconciliation - if (paneRef.current) { - paneRef.current.style.setProperty('--pane-width', `${defaultWidth}px`) - currentWidthRef.current = defaultWidth - updateAriaValues(handleRef.current, {current: defaultWidth}) - } - setWidthInLocalStorage(defaultWidth) - }} className={classes.PaneVerticalDivider} style={ { '--spacing': `var(--spacing-${columnGap})`, } as React.CSSProperties } - /> + > + {resizable ? ( + { + const deltaWithDirection = isKeyboard ? delta : position === 'end' ? -delta : delta + const maxWidth = getMaxPaneWidth() + + if (isKeyboard) { + // Clamp keyboard delta to stay within bounds + const newWidth = Math.max( + minPaneWidth, + Math.min(maxWidth, currentWidthRef.current! + deltaWithDirection), + ) + if (newWidth !== currentWidthRef.current) { + currentWidthRef.current = newWidth + paneRef.current?.style.setProperty('--pane-width', `${newWidth}px`) + updateAriaValues(handleRef.current, {current: newWidth, max: maxWidth}) + } + } else { + // Apply delta directly via CSS variable for immediate visual feedback + if (paneRef.current) { + const newWidth = currentWidthRef.current! + deltaWithDirection + const clampedWidth = Math.max(minPaneWidth, Math.min(maxWidth, newWidth)) + + // Only update if the clamped width actually changed + // This prevents drift when dragging against min/max constraints + if (clampedWidth !== currentWidthRef.current) { + paneRef.current.style.setProperty('--pane-width', `${clampedWidth}px`) + currentWidthRef.current = clampedWidth + updateAriaValues(handleRef.current, {current: clampedWidth, max: maxWidth}) + } + } + } + }} + onDragEnd={() => { + // Sync React state so parent re-renders use the correct width + saveWidth(currentWidthRef.current!) + }} + onDoubleClick={() => { + const resetWidth = getDefaultWidth() + if (paneRef.current) { + paneRef.current.style.setProperty('--pane-width', `${resetWidth}px`) + currentWidthRef.current = resetWidth + updateAriaValues(handleRef.current, {current: resetWidth}) + } + saveWidth(resetWidth) + }} + /> + ) : null} +
) }, diff --git a/packages/react/src/PageLayout/usePaneWidth.test.ts b/packages/react/src/PageLayout/usePaneWidth.test.ts new file mode 100644 index 00000000000..4543fdfffb7 --- /dev/null +++ b/packages/react/src/PageLayout/usePaneWidth.test.ts @@ -0,0 +1,521 @@ +import {describe, it, expect, vi, beforeEach, afterEach} from 'vitest' +import {renderHook, act} from '@testing-library/react' +import { + usePaneWidth, + isCustomWidthOptions, + isPaneWidth, + getDefaultPaneWidth, + getPaneMaxWidthDiff, + updateAriaValues, + defaultPaneWidth, + DEFAULT_MAX_WIDTH_DIFF, + SSR_DEFAULT_MAX_WIDTH, + ARROW_KEY_STEP, +} from './usePaneWidth' + +// Mock refs for hook testing +const createMockRefs = () => ({ + paneRef: {current: document.createElement('div')} as React.RefObject, + handleRef: {current: document.createElement('div')} as React.RefObject, +}) + +describe('usePaneWidth', () => { + beforeEach(() => { + localStorage.clear() + vi.stubGlobal('innerWidth', 1280) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + describe('initialization', () => { + it('should initialize with default width for preset size', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) + }) + + it('should initialize with custom width default', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: {min: '200px', default: '350px', max: '500px'}, + minWidth: 256, + resizable: true, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + expect(result.current.currentWidth).toBe(350) + }) + + it('should restore width from localStorage on mount', () => { + localStorage.setItem('test-pane', '400') + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + expect(result.current.currentWidth).toBe(400) + }) + + it('should not restore from localStorage when not resizable', () => { + localStorage.setItem('test-pane', '400') + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: false, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + // Should use default, not localStorage value + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) + }) + + it('should ignore invalid localStorage values', () => { + localStorage.setItem('test-pane', 'invalid') + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) + }) + + it('should ignore zero or negative localStorage values', () => { + localStorage.setItem('test-pane', '0') + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) + }) + }) + + describe('saveWidth', () => { + it('should update state and localStorage', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-save', + ...refs, + }), + ) + + act(() => { + result.current.saveWidth(450) + }) + + expect(result.current.currentWidth).toBe(450) + expect(result.current.currentWidthRef.current).toBe(450) + expect(localStorage.getItem('test-save')).toBe('450') + }) + + it('should handle localStorage write errors gracefully', () => { + const refs = createMockRefs() + + // Mock localStorage.setItem to throw + const originalSetItem = localStorage.setItem + localStorage.setItem = vi.fn(() => { + throw new Error('QuotaExceeded') + }) + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-save', + ...refs, + }), + ) + + // Should not throw + act(() => { + result.current.saveWidth(450) + }) + + // State should still update + expect(result.current.currentWidth).toBe(450) + + localStorage.setItem = originalSetItem + }) + }) + + describe('minPaneWidth', () => { + it('should use minWidth prop for preset widths', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 200, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(result.current.minPaneWidth).toBe(200) + }) + + it('should use width.min for custom widths', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: {min: '150px', default: '300px', max: '500px'}, + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(result.current.minPaneWidth).toBe(150) + }) + }) + + describe('maxPaneWidth', () => { + it('should use SSR default initially for preset widths', () => { + const refs = createMockRefs() + // We need to test the initial state before effects run + // Since we're in browser environment, the effect runs immediately + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: false, // Disable to prevent effect from updating + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(result.current.maxPaneWidth).toBe(SSR_DEFAULT_MAX_WIDTH) + }) + + it('should use custom max for custom widths', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: {min: '150px', default: '300px', max: '500px'}, + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(result.current.maxPaneWidth).toBe(500) + }) + }) + + describe('getMaxPaneWidth', () => { + it('should return custom max for custom widths regardless of viewport', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: {min: '150px', default: '300px', max: '400px'}, + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(result.current.getMaxPaneWidth()).toBe(400) + + // Even if viewport changes, custom max is fixed + vi.stubGlobal('innerWidth', 500) + expect(result.current.getMaxPaneWidth()).toBe(400) + }) + + it('should calculate max based on viewport for preset widths', () => { + const refs = createMockRefs() + vi.stubGlobal('innerWidth', 1280) + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + // viewport (1280) - DEFAULT_MAX_WIDTH_DIFF (511) = 769 + expect(result.current.getMaxPaneWidth()).toBe(769) + }) + + it('should return minPaneWidth when viewport is too small', () => { + const refs = createMockRefs() + vi.stubGlobal('innerWidth', 300) // Very small viewport + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + // 300 - 511 = -211, so Math.max(256, -211) = 256 + expect(result.current.getMaxPaneWidth()).toBe(256) + }) + }) + + describe('getDefaultWidth', () => { + it('should return default for preset width', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: 'large', + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(result.current.getDefaultWidth()).toBe(defaultPaneWidth.large) + }) + + it('should return custom default for custom widths', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: {min: '150px', default: '275px', max: '500px'}, + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(result.current.getDefaultWidth()).toBe(275) + }) + }) + + describe('resize listener', () => { + it('should not add resize listener when not resizable', () => { + const addEventListenerSpy = vi.spyOn(window, 'addEventListener') + const refs = createMockRefs() + + renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: false, + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(addEventListenerSpy).not.toHaveBeenCalledWith('resize', expect.any(Function)) + addEventListenerSpy.mockRestore() + }) + + it('should not add resize listener for custom widths (max is fixed)', () => { + const addEventListenerSpy = vi.spyOn(window, 'addEventListener') + const refs = createMockRefs() + + renderHook(() => + usePaneWidth({ + width: {min: '150px', default: '300px', max: '500px'}, + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(addEventListenerSpy).not.toHaveBeenCalledWith('resize', expect.any(Function)) + addEventListenerSpy.mockRestore() + }) + + it('should add resize listener for preset widths when resizable', () => { + const addEventListenerSpy = vi.spyOn(window, 'addEventListener') + const refs = createMockRefs() + + renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + expect(addEventListenerSpy).toHaveBeenCalledWith('resize', expect.any(Function)) + addEventListenerSpy.mockRestore() + }) + + it('should cleanup resize listener on unmount', () => { + const removeEventListenerSpy = vi.spyOn(window, 'removeEventListener') + const refs = createMockRefs() + + const {unmount} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test', + ...refs, + }), + ) + + unmount() + + expect(removeEventListenerSpy).toHaveBeenCalledWith('resize', expect.any(Function)) + removeEventListenerSpy.mockRestore() + }) + }) +}) + +describe('helper functions', () => { + describe('isCustomWidthOptions', () => { + it('should return true for custom width objects', () => { + expect(isCustomWidthOptions({min: '100px', default: '200px', max: '300px'})).toBe(true) + }) + + it('should return false for preset width strings', () => { + expect(isCustomWidthOptions('small')).toBe(false) + expect(isCustomWidthOptions('medium')).toBe(false) + expect(isCustomWidthOptions('large')).toBe(false) + }) + }) + + describe('isPaneWidth', () => { + it('should return true for valid preset widths', () => { + expect(isPaneWidth('small')).toBe(true) + expect(isPaneWidth('medium')).toBe(true) + expect(isPaneWidth('large')).toBe(true) + }) + + it('should return false for custom width objects', () => { + expect(isPaneWidth({min: '100px', default: '200px', max: '300px'})).toBe(false) + }) + }) + + describe('getDefaultPaneWidth', () => { + it('should return correct default for preset widths', () => { + expect(getDefaultPaneWidth('small')).toBe(defaultPaneWidth.small) + expect(getDefaultPaneWidth('medium')).toBe(defaultPaneWidth.medium) + expect(getDefaultPaneWidth('large')).toBe(defaultPaneWidth.large) + }) + + it('should parse custom width default', () => { + expect(getDefaultPaneWidth({min: '100px', default: '250px', max: '400px'})).toBe(250) + }) + }) + + describe('getPaneMaxWidthDiff', () => { + it('should return default when element is null', () => { + expect(getPaneMaxWidthDiff(null)).toBe(DEFAULT_MAX_WIDTH_DIFF) + }) + + it('should return default when CSS variable is not set', () => { + const element = document.createElement('div') + expect(getPaneMaxWidthDiff(element)).toBe(DEFAULT_MAX_WIDTH_DIFF) + }) + }) + + describe('updateAriaValues', () => { + it('should set ARIA attributes on element', () => { + const handle = document.createElement('div') + + updateAriaValues(handle, {min: 100, max: 500, current: 300}) + + expect(handle.getAttribute('aria-valuemin')).toBe('100') + expect(handle.getAttribute('aria-valuemax')).toBe('500') + expect(handle.getAttribute('aria-valuenow')).toBe('300') + expect(handle.getAttribute('aria-valuetext')).toBe('Pane width 300 pixels') + }) + + it('should handle null element gracefully', () => { + // Should not throw + expect(() => updateAriaValues(null, {min: 100, max: 500, current: 300})).not.toThrow() + }) + + it('should only update provided values', () => { + const handle = document.createElement('div') + handle.setAttribute('aria-valuemin', '50') + handle.setAttribute('aria-valuemax', '600') + + updateAriaValues(handle, {current: 300}) + + // Original values unchanged + expect(handle.getAttribute('aria-valuemin')).toBe('50') + expect(handle.getAttribute('aria-valuemax')).toBe('600') + // Updated value + expect(handle.getAttribute('aria-valuenow')).toBe('300') + }) + }) +}) + +describe('constants', () => { + it('should export expected constants', () => { + expect(DEFAULT_MAX_WIDTH_DIFF).toBe(511) + expect(SSR_DEFAULT_MAX_WIDTH).toBe(600) + expect(ARROW_KEY_STEP).toBe(3) + expect(defaultPaneWidth).toEqual({small: 256, medium: 296, large: 320}) + }) + + /** + * This test documents the CSS/JS coupling. + * The CSS variable --pane-max-width-diff changes at a breakpoint: + * - Below breakpoint: 511px (DEFAULT_MAX_WIDTH_DIFF) + * - At/above breakpoint: 959px + * + * The breakpoint value is exported from PageLayout.module.css via :export + * and imported into usePaneWidth.ts, so they stay in sync automatically. + */ + it('should have DEFAULT_MAX_WIDTH_DIFF matching CSS value below breakpoint', () => { + // This constant must match --pane-max-width-diff in PageLayout.module.css + // for viewports below the breakpoint. + expect(DEFAULT_MAX_WIDTH_DIFF).toBe(511) + }) +}) diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts new file mode 100644 index 00000000000..120403e68ea --- /dev/null +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -0,0 +1,289 @@ +import React from 'react' +import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' +import cssExports from './PageLayout.module.css' + +// ---------------------------------------------------------------------------- +// Types + +type Measurement = `${number}px` + +export type CustomWidthOptions = { + min: Measurement + default: Measurement + max: Measurement +} + +export type PaneWidth = 'small' | 'medium' | 'large' + +export type UsePaneWidthOptions = { + width: PaneWidth | CustomWidthOptions + minWidth: number + resizable: boolean + widthStorageKey: string + paneRef: React.RefObject + handleRef: React.RefObject +} + +export type UsePaneWidthResult = { + /** Current width for React state (used in ARIA attributes) */ + currentWidth: number + /** Mutable ref tracking width during drag operations */ + currentWidthRef: React.MutableRefObject + /** Minimum allowed pane width */ + minPaneWidth: number + /** Maximum allowed pane width (updates on viewport resize) */ + maxPaneWidth: number + /** Calculate current max width constraint */ + getMaxPaneWidth: () => number + /** Persist width to localStorage and sync React state */ + saveWidth: (value: number) => void + /** Reset to default width */ + getDefaultWidth: () => number +} + +// ---------------------------------------------------------------------------- +// Constants + +/** + * Default value for --pane-max-width-diff CSS variable. + * Imported from CSS to ensure JS fallback matches the CSS default. + */ +export const DEFAULT_MAX_WIDTH_DIFF = Number(cssExports.paneMaxWidthDiffDefault) + +// --pane-max-width-diff changes at this breakpoint in PageLayout.module.css. +const DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT = Number(cssExports.paneMaxWidthDiffBreakpoint) +/** + * Default max pane width for SSR when viewport is unknown. + * Updated to actual value in layout effect before paint. + */ +export const SSR_DEFAULT_MAX_WIDTH = 600 + +/** + * Pixel increment for keyboard arrow key resizing. + * @see https://github.com/github/accessibility/issues/5101#issuecomment-1822870655 + */ +export const ARROW_KEY_STEP = 3 + +/** Default widths for preset size options */ +export const defaultPaneWidth: Record = {small: 256, medium: 296, large: 320} + +// ---------------------------------------------------------------------------- +// Helper functions + +export const isCustomWidthOptions = (width: PaneWidth | CustomWidthOptions): width is CustomWidthOptions => { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + return (width as CustomWidthOptions).default !== undefined +} + +export const isPaneWidth = (width: PaneWidth | CustomWidthOptions): width is PaneWidth => { + return ['small', 'medium', 'large'].includes(width as PaneWidth) +} + +export const getDefaultPaneWidth = (w: PaneWidth | CustomWidthOptions): number => { + if (isPaneWidth(w)) { + return defaultPaneWidth[w] + } else if (isCustomWidthOptions(w)) { + return parseInt(w.default, 10) + } + return 0 +} + +/** + * Gets the --pane-max-width-diff CSS variable value from a pane element. + * This value is set by CSS media queries and controls the max pane width constraint. + * Note: This calls getComputedStyle which forces layout - cache the result when possible. + */ +export function getPaneMaxWidthDiff(paneElement: HTMLElement | null): number { + if (!paneElement) return DEFAULT_MAX_WIDTH_DIFF + const value = parseInt(getComputedStyle(paneElement).getPropertyValue('--pane-max-width-diff'), 10) + return value > 0 ? value : DEFAULT_MAX_WIDTH_DIFF +} + +// Helper to update ARIA slider attributes via direct DOM manipulation +// This avoids re-renders when values change during drag or on viewport resize +export const updateAriaValues = ( + handle: HTMLElement | null, + values: {current?: number; min?: number; max?: number}, +) => { + if (!handle) return + if (values.min !== undefined) handle.setAttribute('aria-valuemin', String(values.min)) + if (values.max !== undefined) handle.setAttribute('aria-valuemax', String(values.max)) + if (values.current !== undefined) { + handle.setAttribute('aria-valuenow', String(values.current)) + handle.setAttribute('aria-valuetext', `Pane width ${values.current} pixels`) + } +} + +// ---------------------------------------------------------------------------- +// Hook + +/** + * Manages pane width state with localStorage persistence and viewport constraints. + * Handles initialization from storage, clamping on viewport resize, and provides + * functions to save and reset width. + */ +export function usePaneWidth({ + width, + minWidth, + resizable, + widthStorageKey, + paneRef, + handleRef, +}: UsePaneWidthOptions): UsePaneWidthResult { + // Derive constraints from width configuration + const isCustomWidth = isCustomWidthOptions(width) + const minPaneWidth = isCustomWidth ? parseInt(width.min, 10) : minWidth + const customMaxWidth = isCustomWidth ? parseInt(width.max, 10) : null + + // Cache the CSS variable value to avoid getComputedStyle during drag (causes layout thrashing) + // Updated on mount and resize when breakpoints might change + const maxWidthDiffRef = React.useRef(DEFAULT_MAX_WIDTH_DIFF) + + // Calculate max width constraint - for custom widths this is fixed, otherwise viewport-dependent + const getMaxPaneWidth = React.useCallback(() => { + if (customMaxWidth !== null) return customMaxWidth + const viewportWidth = window.innerWidth + return viewportWidth > 0 ? Math.max(minPaneWidth, viewportWidth - maxWidthDiffRef.current) : minPaneWidth + }, [customMaxWidth, minPaneWidth]) + + // --- State --- + // Current width for React renders (ARIA attributes). Updates go through saveWidth() or clamp on resize. + const [currentWidth, setCurrentWidth] = React.useState(() => getDefaultPaneWidth(width)) + // Mutable ref for drag operations - avoids re-renders on every pixel move + const currentWidthRef = React.useRef(currentWidth) + // Max width for ARIA - SSR uses custom max or a sensible default, updated on mount + const [maxPaneWidth, setMaxPaneWidth] = React.useState(() => customMaxWidth ?? SSR_DEFAULT_MAX_WIDTH) + + // --- Callbacks --- + const getDefaultWidth = React.useCallback(() => getDefaultPaneWidth(width), [width]) + + const saveWidth = React.useCallback( + (value: number) => { + currentWidthRef.current = value + setCurrentWidth(value) + try { + localStorage.setItem(widthStorageKey, value.toString()) + } catch { + // Ignore write errors (private browsing, quota exceeded, etc.) + } + }, + [widthStorageKey], + ) + + // --- Effects --- + // Track whether we've initialized the width from localStorage + const initializedRef = React.useRef(false) + + // Initialize from localStorage on mount (before paint to avoid CLS) + useIsomorphicLayoutEffect(() => { + if (initializedRef.current || !resizable) return + initializedRef.current = true + + try { + const stored = localStorage.getItem(widthStorageKey) + if (stored !== null) { + const parsed = Number(stored) + if (!isNaN(parsed) && parsed > 0) { + currentWidthRef.current = parsed + paneRef.current?.style.setProperty('--pane-width', `${parsed}px`) + setCurrentWidth(parsed) + } + } + } catch { + // localStorage unavailable - keep default + } + }, [widthStorageKey, paneRef, resizable]) + + // Stable ref to getMaxPaneWidth for use in resize handler without re-subscribing + const getMaxPaneWidthRef = React.useRef(getMaxPaneWidth) + useIsomorphicLayoutEffect(() => { + getMaxPaneWidthRef.current = getMaxPaneWidth + }) + + // Update max pane width on mount and window resize for accurate ARIA values + useIsomorphicLayoutEffect(() => { + if (!resizable) return + + let lastViewportWidth = window.innerWidth + + const updateMax = ({forceRecalcCss = false}: {forceRecalcCss?: boolean} = {}) => { + // Track last viewport width to detect breakpoint crossings. + const currentViewportWidth = window.innerWidth + const crossedBreakpoint = + (lastViewportWidth < DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT && + currentViewportWidth >= DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT) || + (lastViewportWidth >= DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT && + currentViewportWidth < DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT) + lastViewportWidth = currentViewportWidth + + // Only call getComputedStyle if we crossed the breakpoint (expensive operation) + if (forceRecalcCss || crossedBreakpoint) { + maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current) + } + + const actualMax = getMaxPaneWidthRef.current() + setMaxPaneWidth(actualMax) + + // Clamp current width if it exceeds new max (viewport shrunk) + if (currentWidthRef.current > actualMax) { + currentWidthRef.current = actualMax + paneRef.current?.style.setProperty('--pane-width', `${actualMax}px`) + setCurrentWidth(actualMax) + } + + updateAriaValues(handleRef.current, {min: minPaneWidth, max: actualMax, current: currentWidthRef.current}) + } + + // Initial calculation - force CSS recalc to get correct value + updateMax({forceRecalcCss: true}) + + // For custom widths, max is fixed - no need to listen to resize + if (customMaxWidth !== null) return + + // Throttle resize with trailing edge: + // - Execute at most once per THROTTLE_MS during rapid resizing + // - Guarantee final event executes when resize stops + const THROTTLE_MS = 100 + let timeoutId: ReturnType | null = null + let lastExecution = 0 + + const handleResize = () => { + const now = Date.now() + const timeSinceLastExecution = now - lastExecution + + if (timeoutId !== null) { + clearTimeout(timeoutId) + timeoutId = null + } + + if (timeSinceLastExecution >= THROTTLE_MS) { + lastExecution = now + updateMax() + } else { + // Schedule trailing edge execution + timeoutId = setTimeout(() => { + lastExecution = Date.now() + updateMax() + timeoutId = null + }, THROTTLE_MS - timeSinceLastExecution) + } + } + + // 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 (timeoutId !== null) clearTimeout(timeoutId) + window.removeEventListener('resize', handleResize) + } + }, [resizable, customMaxWidth, minPaneWidth, paneRef, handleRef]) + + return { + currentWidth, + currentWidthRef, + minPaneWidth, + maxPaneWidth, + getMaxPaneWidth, + saveWidth, + getDefaultWidth, + } +} From 0fb5ee5d5d67a3f68eb3d094b9ea565395735e11 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 12 Dec 2025 10:04:14 -0500 Subject: [PATCH 3/7] Update shiny-buckets-study.md --- .changeset/shiny-buckets-study.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/shiny-buckets-study.md b/.changeset/shiny-buckets-study.md index 5888789fad1..55c239e77f8 100644 --- a/.changeset/shiny-buckets-study.md +++ b/.changeset/shiny-buckets-study.md @@ -2,4 +2,4 @@ '@primer/react': patch --- -fix resizeobserver perf regression +reapplies PageLayout resizable enhancements without INP drop from expensive selectors From 09eda584c8098e79ddd977adedb153b7d4961004 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 12 Dec 2025 16:07:08 -0500 Subject: [PATCH 4/7] resize-improvemet (#7310) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../src/PageLayout/PageLayout.module.css | 4 + packages/react/src/PageLayout/PageLayout.tsx | 6 + .../react/src/PageLayout/usePaneWidth.test.ts | 274 +++++++++++++++++- packages/react/src/PageLayout/usePaneWidth.ts | 98 ++++--- 4 files changed, 336 insertions(+), 46 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.module.css b/packages/react/src/PageLayout/PageLayout.module.css index dd46fd0fcca..079d34f6cff 100644 --- a/packages/react/src/PageLayout/PageLayout.module.css +++ b/packages/react/src/PageLayout/PageLayout.module.css @@ -621,6 +621,10 @@ width: 100%; @media screen and (min-width: 768px) { + /* + * --pane-max-width is set by JS on mount and updated on resize (debounced). + * JS calculates viewport - margin to avoid scrollbar discrepancy with 100vw. + */ width: clamp(var(--pane-min-width), var(--pane-width), var(--pane-max-width)); } } diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 49d2f9cfb8c..1e003a038ee 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -751,6 +751,12 @@ const Pane = React.forwardRef maxWidth) { + currentWidthRef.current = maxWidth + } + if (isKeyboard) { // Clamp keyboard delta to stay within bounds const newWidth = Math.max( diff --git a/packages/react/src/PageLayout/usePaneWidth.test.ts b/packages/react/src/PageLayout/usePaneWidth.test.ts index 4543fdfffb7..a529d0b7367 100644 --- a/packages/react/src/PageLayout/usePaneWidth.test.ts +++ b/packages/react/src/PageLayout/usePaneWidth.test.ts @@ -339,7 +339,7 @@ describe('usePaneWidth', () => { }) describe('resize listener', () => { - it('should not add resize listener when not resizable', () => { + it('should add debounced resize listener for preset widths', () => { const addEventListenerSpy = vi.spyOn(window, 'addEventListener') const refs = createMockRefs() @@ -347,36 +347,75 @@ describe('usePaneWidth', () => { usePaneWidth({ width: 'medium', minWidth: 256, - resizable: false, + resizable: true, widthStorageKey: 'test', ...refs, }), ) - expect(addEventListenerSpy).not.toHaveBeenCalledWith('resize', expect.any(Function)) + // Adds resize listener for throttled CSS updates and debounced state sync + expect(addEventListenerSpy).toHaveBeenCalledWith('resize', expect.any(Function)) addEventListenerSpy.mockRestore() }) - it('should not add resize listener for custom widths (max is fixed)', () => { + it('should not add resize listener for custom widths (fixed max)', () => { const addEventListenerSpy = vi.spyOn(window, 'addEventListener') const refs = createMockRefs() renderHook(() => usePaneWidth({ - width: {min: '150px', default: '300px', max: '500px'}, + width: {min: '150px', default: '300px', max: '400px'}, minWidth: 256, resizable: true, - widthStorageKey: 'test', + widthStorageKey: 'test-custom', ...refs, }), ) - expect(addEventListenerSpy).not.toHaveBeenCalledWith('resize', expect.any(Function)) + // Custom widths have fixed max - no need for resize listener + expect(addEventListenerSpy).not.toHaveBeenCalledWith('resize', expect.any(Function), expect.anything()) addEventListenerSpy.mockRestore() }) - it('should add resize listener for preset widths when resizable', () => { - const addEventListenerSpy = vi.spyOn(window, 'addEventListener') + it('should clamp ref when viewport shrinks', async () => { + vi.useFakeTimers() + vi.stubGlobal('innerWidth', 1280) + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-clamp', + ...refs, + }), + ) + + // Set current width to max + const initialMax = result.current.getMaxPaneWidth() // 1280 - 511 = 769 + result.current.currentWidthRef.current = initialMax + + // Shrink viewport + vi.stubGlobal('innerWidth', 800) + + // Wrap resize + debounce in act() since it triggers startTransition state update + await act(async () => { + window.dispatchEvent(new Event('resize')) + await vi.advanceTimersByTimeAsync(150) + }) + + // getMaxPaneWidth now returns 800 - 511 = 289 + expect(result.current.getMaxPaneWidth()).toBe(289) + // ref should be clamped after resize handler fires + expect(result.current.currentWidthRef.current).toBe(289) + + vi.useRealTimers() + }) + + it('should update CSS variable immediately via throttle', async () => { + vi.useFakeTimers() + vi.stubGlobal('innerWidth', 1280) const refs = createMockRefs() renderHook(() => @@ -384,17 +423,161 @@ describe('usePaneWidth', () => { width: 'medium', minWidth: 256, resizable: true, - widthStorageKey: 'test', + widthStorageKey: 'test-css-throttle', ...refs, }), ) - expect(addEventListenerSpy).toHaveBeenCalledWith('resize', expect.any(Function)) - addEventListenerSpy.mockRestore() + // Initial --pane-max-width should be set on mount + expect(refs.paneRef.current?.style.getPropertyValue('--pane-max-width')).toBe('769px') + + // Shrink viewport + vi.stubGlobal('innerWidth', 1000) + + // Fire resize - CSS should update immediately (throttled at 16ms) + window.dispatchEvent(new Event('resize')) + + // CSS variable should be updated immediately: 1000 - 511 = 489 + expect(refs.paneRef.current?.style.getPropertyValue('--pane-max-width')).toBe('489px') + + vi.useRealTimers() + }) + + it('should update ARIA attributes after debounce', async () => { + vi.useFakeTimers() + vi.stubGlobal('innerWidth', 1280) + const refs = createMockRefs() + + renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-aria-debounce', + ...refs, + }), + ) + + // Initial ARIA max should be set on mount + expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('769') + + // Shrink viewport + vi.stubGlobal('innerWidth', 900) + + // Fire resize but don't wait for debounce + window.dispatchEvent(new Event('resize')) + await vi.advanceTimersByTimeAsync(50) + + // ARIA should NOT be updated yet + expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('769') + + // Wait for debounce + await act(async () => { + await vi.advanceTimersByTimeAsync(100) + }) + + // ARIA should now be updated: 900 - 511 = 389 + expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('389') + + vi.useRealTimers() + }) + + it('should throttle CSS updates and debounce full sync on rapid resize', async () => { + vi.useFakeTimers() + vi.stubGlobal('innerWidth', 1280) + const refs = createMockRefs() + + const setPropertySpy = vi.spyOn(refs.paneRef.current!.style, 'setProperty') + + renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-throttle-debounce', + ...refs, + }), + ) + + // Clear mount calls + setPropertySpy.mockClear() + + // Fire resize - first one updates CSS immediately + 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() + + // Fire more resize events rapidly (within throttle window) + 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() + + // But ARIA should not be updated yet (debounced) + expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('769') // Still initial + + // Wait for debounce to complete + await act(async () => { + await vi.advanceTimersByTimeAsync(150) + }) + + // Now ARIA and refs are synced + expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('389') // 900 - 511 + + vi.useRealTimers() + }) + + it('should update React state via startTransition after debounce', async () => { + vi.useFakeTimers() + vi.stubGlobal('innerWidth', 1280) + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-state-transition', + ...refs, + }), + ) + + // Initial maxPaneWidth state + expect(result.current.maxPaneWidth).toBe(769) + + // Shrink viewport + vi.stubGlobal('innerWidth', 800) + window.dispatchEvent(new Event('resize')) + + // Before debounce completes, state unchanged + await vi.advanceTimersByTimeAsync(50) + expect(result.current.maxPaneWidth).toBe(769) + + // After debounce, state updated via startTransition + await act(async () => { + await vi.advanceTimersByTimeAsync(100) + }) + + // State now reflects new max: 800 - 511 = 289 + expect(result.current.maxPaneWidth).toBe(289) + + vi.useRealTimers() }) it('should cleanup resize listener on unmount', () => { const removeEventListenerSpy = vi.spyOn(window, 'removeEventListener') + const cancelAnimationFrameSpy = vi.spyOn(window, 'cancelAnimationFrame') const refs = createMockRefs() const {unmount} = renderHook(() => @@ -410,7 +593,74 @@ describe('usePaneWidth', () => { unmount() expect(removeEventListenerSpy).toHaveBeenCalledWith('resize', expect.any(Function)) + // cancelAnimationFrame called with null is fine (no pending RAF) removeEventListenerSpy.mockRestore() + cancelAnimationFrameSpy.mockRestore() + }) + + it('should not add resize listener when not resizable', () => { + const addEventListenerSpy = vi.spyOn(window, 'addEventListener') + const refs = createMockRefs() + + renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: false, + widthStorageKey: 'test-not-resizable', + ...refs, + }), + ) + + expect(addEventListenerSpy).not.toHaveBeenCalledWith('resize', expect.any(Function)) + addEventListenerSpy.mockRestore() + }) + }) + + describe('on-demand max calculation', () => { + it('should calculate max dynamically based on current viewport', () => { + vi.stubGlobal('innerWidth', 1280) + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-dynamic', + ...refs, + }), + ) + + // Initial max at 1280px: 1280 - 511 = 769 + expect(result.current.getMaxPaneWidth()).toBe(769) + + // Viewport changes (no resize event needed) + vi.stubGlobal('innerWidth', 800) + + // getMaxPaneWidth reads window.innerWidth dynamically + expect(result.current.getMaxPaneWidth()).toBe(289) + }) + + it('should return custom max regardless of viewport for custom widths', () => { + vi.stubGlobal('innerWidth', 1280) + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: {min: '150px', default: '300px', max: '400px'}, + minWidth: 256, + resizable: true, + widthStorageKey: 'test-custom', + ...refs, + }), + ) + + expect(result.current.getMaxPaneWidth()).toBe(400) + + // Viewport changes don't affect custom max + vi.stubGlobal('innerWidth', 500) + expect(result.current.getMaxPaneWidth()).toBe(400) }) }) }) diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index 120403e68ea..76b9519b396 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -1,4 +1,4 @@ -import React from 'react' +import React, {startTransition} from 'react' import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' import cssExports from './PageLayout.module.css' @@ -200,15 +200,26 @@ export function usePaneWidth({ getMaxPaneWidthRef.current = getMaxPaneWidth }) - // Update max pane width on mount and window resize for accurate ARIA values + // 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 useIsomorphicLayoutEffect(() => { if (!resizable) return let lastViewportWidth = window.innerWidth - const updateMax = ({forceRecalcCss = false}: {forceRecalcCss?: boolean} = {}) => { - // Track last viewport width to detect breakpoint crossings. + // 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 + + // Only call getComputedStyle if we crossed the breakpoint (expensive) const crossedBreakpoint = (lastViewportWidth < DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT && currentViewportWidth >= DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT) || @@ -216,63 +227,82 @@ export function usePaneWidth({ currentViewportWidth < DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT) lastViewportWidth = currentViewportWidth - // Only call getComputedStyle if we crossed the breakpoint (expensive operation) - if (forceRecalcCss || crossedBreakpoint) { + if (crossedBreakpoint) { maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current) } const actualMax = getMaxPaneWidthRef.current() - setMaxPaneWidth(actualMax) - // Clamp current width if it exceeds new max (viewport shrunk) - if (currentWidthRef.current > actualMax) { + // Update CSS variable for visual clamping (may already be set by throttled update) + paneRef.current?.style.setProperty('--pane-max-width', `${actualMax}px`) + + // Track if we clamped current width + const wasClamped = currentWidthRef.current > actualMax + if (wasClamped) { currentWidthRef.current = actualMax paneRef.current?.style.setProperty('--pane-width', `${actualMax}px`) - setCurrentWidth(actualMax) } - updateAriaValues(handleRef.current, {min: minPaneWidth, max: actualMax, current: currentWidthRef.current}) + // Update ARIA via DOM - cheap, no React re-render + updateAriaValues(handleRef.current, {max: actualMax, current: currentWidthRef.current}) + + // Defer state updates so parent re-renders see accurate values + startTransition(() => { + setMaxPaneWidth(actualMax) + if (wasClamped) { + setCurrentWidth(actualMax) + } + }) } - // Initial calculation - force CSS recalc to get correct value - updateMax({forceRecalcCss: true}) + // Initial calculation on mount + maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current) + const initialMax = getMaxPaneWidthRef.current() + setMaxPaneWidth(initialMax) + paneRef.current?.style.setProperty('--pane-max-width', `${initialMax}px`) + updateAriaValues(handleRef.current, {min: minPaneWidth, max: initialMax, current: currentWidthRef.current}) // For custom widths, max is fixed - no need to listen to resize if (customMaxWidth !== null) return - // Throttle resize with trailing edge: - // - Execute at most once per THROTTLE_MS during rapid resizing - // - Guarantee final event executes when resize stops - const THROTTLE_MS = 100 - let timeoutId: ReturnType | null = null - let lastExecution = 0 + // Throttle CSS updates (16ms ≈ 60fps), debounce full sync (150ms) + const THROTTLE_MS = 16 + const DEBOUNCE_MS = 150 + let rafId: number | null = null + let debounceId: ReturnType | null = null + let lastThrottleTime = 0 const handleResize = () => { const now = Date.now() - const timeSinceLastExecution = now - lastExecution - if (timeoutId !== null) { - clearTimeout(timeoutId) - timeoutId = null + // 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() + }) } - if (timeSinceLastExecution >= THROTTLE_MS) { - lastExecution = now - updateMax() - } else { - // Schedule trailing edge execution - timeoutId = setTimeout(() => { - lastExecution = Date.now() - updateMax() - timeoutId = null - }, THROTTLE_MS - timeSinceLastExecution) + // Debounced full sync (refs, ARIA, state) when resize stops + if (debounceId !== null) { + clearTimeout(debounceId) } + debounceId = setTimeout(() => { + debounceId = null + syncAll() + }, DEBOUNCE_MS) } // 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 (timeoutId !== null) clearTimeout(timeoutId) + if (rafId !== null) cancelAnimationFrame(rafId) + if (debounceId !== null) clearTimeout(debounceId) window.removeEventListener('resize', handleResize) } }, [resizable, customMaxWidth, minPaneWidth, paneRef, handleRef]) From 5817e161b8d3544f5deefd1d1b1aa385671187c9 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 12 Dec 2025 21:26:06 +0000 Subject: [PATCH 5/7] initialize from localStorage, suppressing hydration error --- packages/react/src/PageLayout/PageLayout.tsx | 4 ++ packages/react/src/PageLayout/usePaneWidth.ts | 52 ++++++++++--------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 1e003a038ee..b5d1858cbc9 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -697,6 +697,10 @@ const Pane = React.forwardRef
getDefaultPaneWidth(width)) + // + // NOTE: We read from localStorage during initial state to avoid a visible resize flicker + // when the stored width differs from the default. This causes a React hydration mismatch + // (server renders default width, client renders stored width), but we handle this with + // suppressHydrationWarning on the Pane element. The mismatch only affects the --pane-width + // CSS variable, not DOM structure or children. + const [currentWidth, setCurrentWidth] = React.useState(() => { + const defaultWidth = getDefaultPaneWidth(width) + + if (!resizable || !canUseDOM) { + return defaultWidth + } + + try { + const storedWidth = localStorage.getItem(widthStorageKey) + if (storedWidth !== null) { + const parsed = Number(storedWidth) + if (!isNaN(parsed) && parsed > 0) { + return parsed + } + } + } catch { + // localStorage unavailable - keep default + } + + return defaultWidth + }) // Mutable ref for drag operations - avoids re-renders on every pixel move const currentWidthRef = React.useRef(currentWidth) // Max width for ARIA - SSR uses custom max or a sensible default, updated on mount @@ -171,29 +198,6 @@ export function usePaneWidth({ ) // --- Effects --- - // Track whether we've initialized the width from localStorage - const initializedRef = React.useRef(false) - - // Initialize from localStorage on mount (before paint to avoid CLS) - useIsomorphicLayoutEffect(() => { - if (initializedRef.current || !resizable) return - initializedRef.current = true - - try { - const stored = localStorage.getItem(widthStorageKey) - if (stored !== null) { - const parsed = Number(stored) - if (!isNaN(parsed) && parsed > 0) { - currentWidthRef.current = parsed - paneRef.current?.style.setProperty('--pane-width', `${parsed}px`) - setCurrentWidth(parsed) - } - } - } catch { - // localStorage unavailable - keep default - } - }, [widthStorageKey, paneRef, resizable]) - // Stable ref to getMaxPaneWidth for use in resize handler without re-subscribing const getMaxPaneWidthRef = React.useRef(getMaxPaneWidth) useIsomorphicLayoutEffect(() => { From d0dd7d02e873b9ee2ca8a507171426cc74ae4652 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 12 Dec 2025 22:16:10 +0000 Subject: [PATCH 6/7] abckout snapshot --- packages/react/src/PageLayout/PageLayout.tsx | 4 +--- .../src/PageLayout/__snapshots__/PageLayout.test.tsx.snap | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index b5d1858cbc9..ba7bfdd0686 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -713,9 +713,7 @@ const Pane = React.forwardRef diff --git a/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap b/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap index 3fd829d183b..5ac82e6ca02 100644 --- a/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap +++ b/packages/react/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap @@ -57,7 +57,7 @@ exports[`PageLayout > renders condensed layout 1`] = ` />
Pane
@@ -149,7 +149,7 @@ exports[`PageLayout > renders default layout 1`] = ` />
Pane
@@ -243,7 +243,7 @@ exports[`PageLayout > renders pane in different position when narrow 1`] = ` />
Pane
@@ -337,7 +337,7 @@ exports[`PageLayout > renders with dividers 1`] = ` />
Pane
From ae89c0d16a03412b5dba151815f96fff83c77734 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 12 Dec 2025 23:22:06 +0000 Subject: [PATCH 7/7] keep cursor closer to edge --- packages/react/src/PageLayout/PageLayout.tsx | 70 ++++++++++++-------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index ba7bfdd0686..901db1c5120 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -185,8 +185,10 @@ const VerticalDivider: React.FC> = type DragHandleProps = { /** Ref for imperative ARIA updates during drag */ handleRef: React.RefObject - /** Called with movement delta on each drag tick */ - onDrag: (delta: number, isKeyboard: boolean) => void + /** Called once when drag starts with initial cursor X position */ + onDragStart: (clientX: number) => void + /** Called on each drag tick with cursor position (pointer) or delta (keyboard) */ + onDrag: (value: number, isKeyboard: boolean) => void /** Called when drag operation completes */ onDragEnd: () => void /** Reset width on double-click */ @@ -211,6 +213,7 @@ const isDragging = (handle: HTMLElement | null) => { */ const DragHandle: React.FC = ({ handleRef, + onDragStart, onDrag, onDragEnd, onDoubleClick, @@ -218,9 +221,11 @@ const DragHandle: React.FC = ({ 'aria-valuemax': ariaValueMax, 'aria-valuenow': ariaValueNow, }) => { + const stableOnDragStart = React.useRef(onDragStart) const stableOnDrag = React.useRef(onDrag) const stableOnDragEnd = React.useRef(onDragEnd) React.useEffect(() => { + stableOnDragStart.current = onDragStart stableOnDrag.current = onDrag stableOnDragEnd.current = onDragEnd }) @@ -253,6 +258,7 @@ const DragHandle: React.FC = ({ event.preventDefault() const target = event.currentTarget target.setPointerCapture(event.pointerId) + stableOnDragStart.current(event.clientX) setDragging(true) }, [setDragging], @@ -260,7 +266,8 @@ const DragHandle: React.FC = ({ /** * Pointer move during drag - * Calls onDrag with movement delta + * Calls onDrag with absolute cursor X position + * Using absolute position avoids drift from accumulated deltas * Prevents default to avoid unwanted selection behavior */ const handlePointerMove = React.useCallback( @@ -268,9 +275,7 @@ const DragHandle: React.FC = ({ if (!isDragging(handleRef.current)) return event.preventDefault() - if (event.movementX !== 0) { - stableOnDrag.current(event.movementX, false) - } + stableOnDrag.current(event.clientX, false) }, [handleRef], ) @@ -637,6 +642,13 @@ const Pane = React.forwardRef(null) + // Cache drag start values to calculate relative delta during drag + // This approach is immune to layout shifts (scrollbars appearing/disappearing) + const dragStartClientXRef = React.useRef(0) + const dragStartWidthRef = React.useRef(0) + // Cache max width at drag start - won't change during a drag operation + const dragMaxWidthRef = React.useRef(0) + const {currentWidth, currentWidthRef, minPaneWidth, maxPaneWidth, getMaxPaneWidth, saveWidth, getDefaultWidth} = usePaneWidth({ width, @@ -749,39 +761,45 @@ const Pane = React.forwardRef { - const deltaWithDirection = isKeyboard ? delta : position === 'end' ? -delta : delta - const maxWidth = getMaxPaneWidth() - - // Safety clamp: if user starts dragging before debounced resize fires, - // sync ref to actual max. Rare edge case but prevents confusing behavior. - if (currentWidthRef.current! > maxWidth) { - currentWidthRef.current = maxWidth - } + onDragStart={clientX => { + // Cache cursor position and pane width at drag start + // Using relative delta (current - start) is immune to layout shifts + // (e.g., scrollbars appearing/disappearing during drag) + dragStartClientXRef.current = clientX + dragStartWidthRef.current = paneRef.current?.getBoundingClientRect().width ?? currentWidthRef.current! + // Cache max width - won't change during drag + dragMaxWidthRef.current = getMaxPaneWidth() + }} + onDrag={(value, isKeyboard) => { + // Use cached max width for pointer drag, fresh value for keyboard (less frequent) + const maxWidth = isKeyboard ? getMaxPaneWidth() : dragMaxWidthRef.current if (isKeyboard) { - // Clamp keyboard delta to stay within bounds - const newWidth = Math.max( - minPaneWidth, - Math.min(maxWidth, currentWidthRef.current! + deltaWithDirection), - ) + // Keyboard: value is a delta (e.g., +3 or -3) + const delta = value + const newWidth = Math.max(minPaneWidth, Math.min(maxWidth, currentWidthRef.current! + delta)) if (newWidth !== currentWidthRef.current) { currentWidthRef.current = newWidth paneRef.current?.style.setProperty('--pane-width', `${newWidth}px`) updateAriaValues(handleRef.current, {current: newWidth, max: maxWidth}) } } else { - // Apply delta directly via CSS variable for immediate visual feedback + // Pointer: value is clientX - calculate width using relative delta from drag start + // This approach is immune to layout shifts during drag if (paneRef.current) { - const newWidth = currentWidthRef.current! + deltaWithDirection + const deltaX = value - dragStartClientXRef.current + // For position='end': cursor moving left (negative delta) increases width + // For position='start': cursor moving right (positive delta) increases width + const directedDelta = position === 'end' ? -deltaX : deltaX + const newWidth = dragStartWidthRef.current + directedDelta + const clampedWidth = Math.max(minPaneWidth, Math.min(maxWidth, newWidth)) - // Only update if the clamped width actually changed - // This prevents drift when dragging against min/max constraints - if (clampedWidth !== currentWidthRef.current) { + // Only update if width actually changed + if (Math.round(clampedWidth) !== Math.round(currentWidthRef.current!)) { paneRef.current.style.setProperty('--pane-width', `${clampedWidth}px`) currentWidthRef.current = clampedWidth - updateAriaValues(handleRef.current, {current: clampedWidth, max: maxWidth}) + updateAriaValues(handleRef.current, {current: Math.round(clampedWidth), max: maxWidth}) } } }