diff --git a/.changeset/pagelayout-remove-reflow.md b/.changeset/pagelayout-remove-reflow.md new file mode 100644 index 00000000000..25b3fec052f --- /dev/null +++ b/.changeset/pagelayout-remove-reflow.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +**PageLayout**: Eliminate forced reflow (~614ms) on mount by replacing `getComputedStyle` call with a pure JS viewport width check for the `--pane-max-width-diff` CSS variable. diff --git a/packages/react/src/PageLayout/PageLayout.module.css b/packages/react/src/PageLayout/PageLayout.module.css index 5466fce1d67..69aed8353dc 100644 --- a/packages/react/src/PageLayout/PageLayout.module.css +++ b/packages/react/src/PageLayout/PageLayout.module.css @@ -4,6 +4,8 @@ paneMaxWidthDiffBreakpoint: 1280; /* Default value for --pane-max-width-diff below the breakpoint */ paneMaxWidthDiffDefault: 511; + /* Value for --pane-max-width-diff at/above the breakpoint */ + paneMaxWidthDiffWide: 959; } .PageLayoutRoot { diff --git a/packages/react/src/PageLayout/usePaneWidth.test.ts b/packages/react/src/PageLayout/usePaneWidth.test.ts index 7c558522a6a..36b3331aca5 100644 --- a/packages/react/src/PageLayout/usePaneWidth.test.ts +++ b/packages/react/src/PageLayout/usePaneWidth.test.ts @@ -5,7 +5,7 @@ import { isCustomWidthOptions, isPaneWidth, getDefaultPaneWidth, - getPaneMaxWidthDiff, + getMaxWidthDiffFromViewport, updateAriaValues, defaultPaneWidth, DEFAULT_MAX_WIDTH_DIFF, @@ -553,7 +553,7 @@ describe('usePaneWidth', () => { it('should calculate max based on viewport for preset widths', () => { const refs = createMockRefs() - vi.stubGlobal('innerWidth', 1280) + vi.stubGlobal('innerWidth', 1024) const {result} = renderHook(() => usePaneWidth({ @@ -565,8 +565,8 @@ describe('usePaneWidth', () => { }), ) - // viewport (1280) - DEFAULT_MAX_WIDTH_DIFF (511) = 769 - expect(result.current.getMaxPaneWidth()).toBe(769) + // viewport (1024) - DEFAULT_MAX_WIDTH_DIFF (511) = 513 + expect(result.current.getMaxPaneWidth()).toBe(513) }) it('should return minPaneWidth when viewport is too small', () => { @@ -710,10 +710,10 @@ describe('usePaneWidth', () => { }), ) - // Initial --pane-max-width should be set on mount - expect(refs.paneRef.current?.style.getPropertyValue('--pane-max-width')).toBe('769px') + // Initial --pane-max-width should be set on mount (1280 - 959 wide diff = 321) + expect(refs.paneRef.current?.style.getPropertyValue('--pane-max-width')).toBe('321px') - // Shrink viewport + // Shrink viewport (crosses 1280 breakpoint, diff switches to 511) vi.stubGlobal('innerWidth', 1000) // Fire resize - with throttle, first update happens immediately (if THROTTLE_MS passed) @@ -746,10 +746,10 @@ describe('usePaneWidth', () => { }), ) - // Initial ARIA max should be set on mount - expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('769') + // Initial ARIA max should be set on mount (1280 - 959 wide diff = 321) + expect(refs.handleRef.current?.getAttribute('aria-valuemax')).toBe('321') - // Shrink viewport + // Shrink viewport (crosses 1280 breakpoint, diff switches to 511) vi.stubGlobal('innerWidth', 900) // Fire resize - with throttle, update happens via rAF @@ -834,10 +834,10 @@ describe('usePaneWidth', () => { }), ) - // Initial maxPaneWidth state - expect(result.current.maxPaneWidth).toBe(769) + // Initial maxPaneWidth state (1280 - 959 wide diff = 321) + expect(result.current.maxPaneWidth).toBe(321) - // Shrink viewport + // Shrink viewport (crosses 1280 breakpoint, diff switches to 511) vi.stubGlobal('innerWidth', 800) window.dispatchEvent(new Event('resize')) @@ -975,7 +975,8 @@ describe('usePaneWidth', () => { }) describe('on-demand max calculation', () => { - it('should calculate max dynamically based on current viewport', () => { + it('should calculate max dynamically based on current viewport', async () => { + vi.useFakeTimers() vi.stubGlobal('innerWidth', 1280) const refs = createMockRefs() @@ -989,14 +990,21 @@ describe('usePaneWidth', () => { }), ) - // Initial max at 1280px: 1280 - 511 = 769 - expect(result.current.getMaxPaneWidth()).toBe(769) + // Initial max at 1280px: 1280 - 959 (wide diff) = 321 + expect(result.current.getMaxPaneWidth()).toBe(321) - // Viewport changes (no resize event needed) + // Shrink viewport (crosses 1280 breakpoint, diff switches to 511) vi.stubGlobal('innerWidth', 800) + window.dispatchEvent(new Event('resize')) - // getMaxPaneWidth reads window.innerWidth dynamically + await act(async () => { + await vi.runAllTimersAsync() + }) + + // After resize: 800 - 511 = 289 expect(result.current.getMaxPaneWidth()).toBe(289) + + vi.useRealTimers() }) it('should return custom max regardless of viewport for custom widths', () => { @@ -1140,14 +1148,20 @@ describe('helper functions', () => { }) }) - describe('getPaneMaxWidthDiff', () => { - it('should return default when element is null', () => { - expect(getPaneMaxWidthDiff(null)).toBe(DEFAULT_MAX_WIDTH_DIFF) + describe('getMaxWidthDiffFromViewport', () => { + it('should return default value below the breakpoint', () => { + vi.stubGlobal('innerWidth', 1024) + expect(getMaxWidthDiffFromViewport()).toBe(511) + }) + + it('should return wide value at the breakpoint', () => { + vi.stubGlobal('innerWidth', 1280) + expect(getMaxWidthDiffFromViewport()).toBe(959) }) - it('should return default when CSS variable is not set', () => { - const element = document.createElement('div') - expect(getPaneMaxWidthDiff(element)).toBe(DEFAULT_MAX_WIDTH_DIFF) + it('should return wide value above the breakpoint', () => { + vi.stubGlobal('innerWidth', 1920) + expect(getMaxWidthDiffFromViewport()).toBe(959) }) }) diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index b52b99fdd41..49aca96702d 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -62,6 +62,9 @@ export type UsePaneWidthResult = { */ export const DEFAULT_MAX_WIDTH_DIFF = Number(cssExports.paneMaxWidthDiffDefault) +// Value for --pane-max-width-diff at/above the wide breakpoint. +const WIDE_MAX_WIDTH_DIFF = Number(cssExports.paneMaxWidthDiffWide) + // --pane-max-width-diff changes at this breakpoint in PageLayout.module.css. const DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT = Number(cssExports.paneMaxWidthDiffBreakpoint) /** @@ -100,14 +103,13 @@ export const getDefaultPaneWidth = (w: PaneWidthValue): number => { } /** - * 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. + * Derives the --pane-max-width-diff value from viewport width alone. + * Avoids the expensive getComputedStyle call that forces a synchronous layout recalc. + * The CSS only defines two breakpoint-dependent values, so a simple width check is equivalent. */ -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 +export function getMaxWidthDiffFromViewport(): number { + if (!canUseDOM) return DEFAULT_MAX_WIDTH_DIFF + return window.innerWidth >= DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT ? WIDE_MAX_WIDTH_DIFF : DEFAULT_MAX_WIDTH_DIFF } // Helper to update ARIA slider attributes via direct DOM manipulation @@ -315,7 +317,7 @@ export function usePaneWidth({ const syncAll = () => { const currentViewportWidth = window.innerWidth - // Only call getComputedStyle if we crossed the breakpoint (expensive) + // Only update the cached diff value if we crossed the breakpoint const crossedBreakpoint = (lastViewportWidth < DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT && currentViewportWidth >= DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT) || @@ -324,7 +326,7 @@ export function usePaneWidth({ lastViewportWidth = currentViewportWidth if (crossedBreakpoint) { - maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current) + maxWidthDiffRef.current = getMaxWidthDiffFromViewport() } const actualMax = getMaxPaneWidthRef.current() @@ -351,8 +353,10 @@ export function usePaneWidth({ }) } - // Initial calculation on mount - maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current) + // Initial calculation on mount — use viewport-based lookup to avoid + // getComputedStyle which forces a synchronous layout recalc on the + // freshly-committed DOM tree (measured at ~614ms on large pages). + maxWidthDiffRef.current = getMaxWidthDiffFromViewport() const initialMax = getMaxPaneWidthRef.current() setMaxPaneWidth(initialMax) paneRef.current?.style.setProperty('--pane-max-width', `${initialMax}px`)