Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pagelayout-remove-reflow.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions packages/react/src/PageLayout/PageLayout.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
paneMaxWidthDiffDefault: 511;
/* Default value for --sidebar-max-width-diff (constant across all viewports) */
sidebarMaxWidthDiffDefault: 256;
/* Value for --pane-max-width-diff at/above the breakpoint */
paneMaxWidthDiffWide: 959;
}

.PageLayoutRoot {
Expand Down
67 changes: 36 additions & 31 deletions packages/react/src/PageLayout/usePaneWidth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
isCustomWidthOptions,
isPaneWidth,
getDefaultPaneWidth,
getPaneMaxWidthDiff,
getMaxWidthDiffFromViewport,
updateAriaValues,
defaultPaneWidth,
DEFAULT_MAX_WIDTH_DIFF,
Expand Down Expand Up @@ -554,7 +554,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({
Expand All @@ -566,8 +566,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', () => {
Expand Down Expand Up @@ -711,10 +711,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)
Expand Down Expand Up @@ -747,10 +747,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
Expand Down Expand Up @@ -835,10 +835,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'))

Expand Down Expand Up @@ -976,7 +976,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()

Expand All @@ -990,14 +991,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', () => {
Expand Down Expand Up @@ -1141,23 +1149,20 @@ describe('helper functions', () => {
})
})

describe('getPaneMaxWidthDiff', () => {
it('should return default pane diff 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 default sidebar diff when element is null and isSidebar is true', () => {
expect(getPaneMaxWidthDiff(null, true)).toBe(DEFAULT_SIDEBAR_MAX_WIDTH_DIFF)
})

it('should return default pane diff when CSS variable is not set', () => {
const element = document.createElement('div')
expect(getPaneMaxWidthDiff(element)).toBe(DEFAULT_MAX_WIDTH_DIFF)
it('should return wide value at the breakpoint', () => {
vi.stubGlobal('innerWidth', 1280)
expect(getMaxWidthDiffFromViewport()).toBe(959)
})

it('should return default sidebar diff when CSS variable is not set and isSidebar is true', () => {
const element = document.createElement('div')
expect(getPaneMaxWidthDiff(element, true)).toBe(DEFAULT_SIDEBAR_MAX_WIDTH_DIFF)
it('should return wide value above the breakpoint', () => {
vi.stubGlobal('innerWidth', 1920)
expect(getMaxWidthDiffFromViewport()).toBe(959)
})
})

Expand Down
30 changes: 16 additions & 14 deletions packages/react/src/PageLayout/usePaneWidth.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, {startTransition, useMemo} from 'react'
import {canUseDOM} from '../utils/environment'
import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect'
import cssExports from './PageLayout.module.css'

Expand Down Expand Up @@ -75,6 +76,9 @@ export const DEFAULT_MAX_WIDTH_DIFF = Number(cssExports.paneMaxWidthDiffDefault)
*/
export const DEFAULT_SIDEBAR_MAX_WIDTH_DIFF = Number(cssExports.sidebarMaxWidthDiffDefault)

// 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)
/**
Expand Down Expand Up @@ -113,17 +117,13 @@ export const getDefaultPaneWidth = (w: PaneWidthValue): number => {
}

/**
* Gets the max-width-diff CSS variable value from a pane element.
* For sidebars, reads --sidebar-max-width-diff (constant across viewports).
* For panes, reads --pane-max-width-diff (changes at 1280px breakpoint).
* 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, isSidebar = false): number {
const defaultValue = isSidebar ? DEFAULT_SIDEBAR_MAX_WIDTH_DIFF : DEFAULT_MAX_WIDTH_DIFF
const cssVar = isSidebar ? '--sidebar-max-width-diff' : '--pane-max-width-diff'
if (!paneElement) return defaultValue
const value = parseInt(getComputedStyle(paneElement).getPropertyValue(cssVar), 10)
return value > 0 ? value : defaultValue
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
Expand Down Expand Up @@ -338,7 +338,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) ||
Expand All @@ -347,7 +347,7 @@ export function usePaneWidth({
lastViewportWidth = currentViewportWidth

if (crossedBreakpoint) {
maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current, constrainToViewport)
maxWidthDiffRef.current = getMaxWidthDiffFromViewport()
}

const actualMax = getMaxPaneWidthRef.current()
Expand All @@ -374,8 +374,10 @@ export function usePaneWidth({
})
}

// Initial calculation on mount
maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current, constrainToViewport)
// 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`)
Expand Down
Loading