From 0aeea89b1b85f788a84a019e4fa073dd4cb9d9d0 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Tue, 16 Dec 2025 15:08:41 +0000 Subject: [PATCH 01/10] accept a persister --- packages/react/src/PageLayout/PageLayout.tsx | 20 +- packages/react/src/PageLayout/index.ts | 1 + .../react/src/PageLayout/usePaneWidth.test.ts | 247 ++++++++++++++++++ packages/react/src/PageLayout/usePaneWidth.ts | 169 +++++++++--- 4 files changed, 400 insertions(+), 37 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 901db1c5120..3954007ef5b 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -19,6 +19,7 @@ import { ARROW_KEY_STEP, type CustomWidthOptions, type PaneWidth, + type ResizableConfig, } from './usePaneWidth' const REGION_ORDER = { @@ -561,7 +562,13 @@ export type PageLayoutPaneProps = { 'aria-label'?: string width?: PaneWidth | CustomWidthOptions minWidth?: number - resizable?: boolean + /** + * Enable resizable pane behavior. + * - `true`: Enable with default localStorage persistence + * - `false`: Disable resizing + * - `WidthPersister`: Enable with custom storage (e.g., sessionStorage, server-side) + */ + resizable?: ResizableConfig widthStorageKey?: string padding?: keyof typeof SPACING_MAP divider?: 'none' | 'line' | ResponsiveValue<'none' | 'line', 'none' | 'line' | 'filled'> @@ -709,10 +716,11 @@ const Pane = React.forwardRef
{ expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) }) + + it('should use default width when custom WidthPersister is provided', () => { + const customPersister: WidthPersister = { + save: vi.fn(), + } + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: customPersister, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + // Custom persisters use default width - consumer provides initial via width prop + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) + }) + + it('should not read from localStorage when custom WidthPersister is provided', () => { + localStorage.setItem('test-pane', '500') + const customPersister: WidthPersister = { + save: vi.fn(), + } + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: customPersister, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + // Should use default, not localStorage value + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) + }) + + it('should not read from localStorage when empty object is provided', () => { + localStorage.setItem('test-pane', '500') + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: {}, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + // Should use default, not localStorage value + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) + }) + + it('should not save to any storage when empty object is provided', () => { + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: {}, + widthStorageKey: 'test-pane', + ...refs, + }), + ) + + act(() => { + result.current.saveWidth(450) + }) + + // Width state should update + expect(result.current.currentWidth).toBe(450) + // But localStorage should not be written + expect(localStorage.getItem('test-pane')).toBeNull() + }) }) describe('saveWidth', () => { @@ -181,6 +267,112 @@ describe('usePaneWidth', () => { localStorage.setItem = originalSetItem }) + + it('should call custom persister save method', () => { + const customPersister: WidthPersister = { + save: vi.fn(), + } + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: customPersister, + widthStorageKey: 'test-custom-save', + ...refs, + }), + ) + + act(() => { + result.current.saveWidth(450) + }) + + expect(result.current.currentWidth).toBe(450) + expect(customPersister.save).toHaveBeenCalledWith(450) + // Should NOT write to localStorage + expect(localStorage.getItem('test-custom-save')).toBeNull() + }) + + it('should handle async custom persister save', async () => { + const customPersister: WidthPersister = { + save: vi.fn().mockResolvedValue(undefined), + } + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: customPersister, + widthStorageKey: 'test-async-save', + ...refs, + }), + ) + + act(() => { + result.current.saveWidth(350) + }) + + expect(result.current.currentWidth).toBe(350) + expect(customPersister.save).toHaveBeenCalledWith(350) + }) + + it('should handle sync errors from custom persister gracefully', () => { + const customPersister: WidthPersister = { + save: vi.fn(() => { + throw new Error('Sync storage error') + }), + } + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: customPersister, + widthStorageKey: 'test-sync-error', + ...refs, + }), + ) + + // Should not throw - state should still update + act(() => { + result.current.saveWidth(450) + }) + + expect(result.current.currentWidth).toBe(450) + expect(customPersister.save).toHaveBeenCalledWith(450) + }) + + it('should handle async rejection from custom persister gracefully', async () => { + const customPersister: WidthPersister = { + save: vi.fn().mockRejectedValue(new Error('Async storage error')), + } + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: customPersister, + widthStorageKey: 'test-async-error', + ...refs, + }), + ) + + // Should not throw - state should still update + act(() => { + result.current.saveWidth(450) + }) + + // Wait for promise rejection to be handled + await vi.waitFor(() => { + expect(customPersister.save).toHaveBeenCalledWith(450) + }) + + expect(result.current.currentWidth).toBe(450) + }) }) describe('minPaneWidth', () => { @@ -769,3 +961,58 @@ describe('constants', () => { expect(DEFAULT_MAX_WIDTH_DIFF).toBe(511) }) }) + +describe('type guards', () => { + describe('isWidthPersister', () => { + it('should return true for objects with save method', () => { + expect(isWidthPersister({save: () => {}})).toBe(true) + expect(isWidthPersister({save: async () => {}})).toBe(true) + }) + + it('should return false for boolean values', () => { + expect(isWidthPersister(true)).toBe(false) + expect(isWidthPersister(false)).toBe(false) + }) + + it('should return false for null', () => { + // @ts-expect-error - testing runtime behavior + expect(isWidthPersister(null)).toBe(false) + }) + }) + + describe('isResizableEnabled', () => { + it('should return true for boolean true', () => { + expect(isResizableEnabled(true)).toBe(true) + }) + + it('should return false for boolean false', () => { + expect(isResizableEnabled(false)).toBe(false) + }) + + it('should return true for WidthPersister objects', () => { + expect(isResizableEnabled({save: () => {}})).toBe(true) + }) + + it('should return true for empty object (resizable without persistence)', () => { + expect(isResizableEnabled({})).toBe(true) + }) + }) + + describe('isResizableWithoutPersistence', () => { + it('should return true for empty object', () => { + expect(isResizableWithoutPersistence({})).toBe(true) + }) + + it('should return false for boolean true', () => { + expect(isResizableWithoutPersistence(true)).toBe(false) + }) + + it('should return false for boolean false', () => { + expect(isResizableWithoutPersistence(false)).toBe(false) + }) + + it('should return false for WidthPersister', () => { + expect(isResizableWithoutPersistence({save: () => {}})).toBe(false) + }) + }) +}) diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index 2449901b0a0..2bf67c9320d 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -1,5 +1,4 @@ import React, {startTransition} from 'react' -import {canUseDOM} from '../utils/environment' import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' import cssExports from './PageLayout.module.css' @@ -16,10 +15,30 @@ export type CustomWidthOptions = { export type PaneWidth = 'small' | 'medium' | 'large' +/** + * Interface for persisting pane width to storage. + * Allows consumers to provide custom storage implementations (e.g., sessionStorage, IndexedDB, server-side). + * + * For custom persisters, the initial width should be passed via the `width` prop. + */ +export interface WidthPersister { + /** Save the width value to storage. Can be async for server-side persistence. */ + save?: (value: number) => void | Promise +} + +/** + * Resizable configuration options. + * - `true`: Enable resizing with default localStorage persistence (may cause hydration mismatch) + * - `false`: Disable resizing + * - `{}`: Enable resizing without persistence (no hydration issues) + * - `WidthPersister`: Enable resizing with custom storage implementation (no hydration issues) + */ +export type ResizableConfig = boolean | WidthPersister | Record + export type UsePaneWidthOptions = { width: PaneWidth | CustomWidthOptions minWidth: number - resizable: boolean + resizable: ResizableConfig widthStorageKey: string paneRef: React.RefObject handleRef: React.RefObject @@ -89,6 +108,63 @@ export const getDefaultPaneWidth = (w: PaneWidth | CustomWidthOptions): number = return 0 } +/** + * Type guard to check if resizable config is a WidthPersister (has save method) + */ +export const isWidthPersister = (config: ResizableConfig): config is WidthPersister => { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- config could be null at runtime despite types + return typeof config === 'object' && config !== null && 'save' in config +} + +/** + * Type guard to check if resizable config is an empty object (resizable without persistence) + */ +export const isResizableWithoutPersistence = (config: ResizableConfig): config is Record => { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- config could be null at runtime despite types + return typeof config === 'object' && config !== null && !('save' in config) +} + +/** + * Check if resizing is enabled (boolean true, empty object, or persister object) + */ +export const isResizableEnabled = (config: ResizableConfig): boolean => { + return config === true || typeof config === 'object' +} + +/** + * Internal interface for localStorage persister (includes get for reading stored value) + */ +interface InternalLocalStoragePersister extends WidthPersister { + get: () => number | null +} + +/** + * Creates a default localStorage-based persister (internal use only) + */ +const createLocalStoragePersister = (storageKey: string): InternalLocalStoragePersister => ({ + get: () => { + try { + const storedWidth = localStorage.getItem(storageKey) + if (storedWidth !== null) { + const parsed = Number(storedWidth) + if (!isNaN(parsed) && parsed > 0) { + return parsed + } + } + } catch { + // localStorage unavailable + } + return null + }, + save: (value: number) => { + try { + localStorage.setItem(storageKey, value.toString()) + } catch { + // Ignore write errors (private browsing, quota exceeded, etc.) + } + }, +}) + /** * 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. @@ -119,9 +195,14 @@ export const updateAriaValues = ( // Hook /** - * Manages pane width state with localStorage persistence and viewport constraints. + * Manages pane width state with storage persistence and viewport constraints. * Handles initialization from storage, clamping on viewport resize, and provides * functions to save and reset width. + * + * Storage behavior: + * - When `resizable` is `true`: Uses localStorage with the provided `widthStorageKey` + * - When `resizable` is a `WidthPersister`: Uses the custom persister's save method + * (consumer provides initial width via `width` prop) */ export function usePaneWidth({ width, @@ -136,6 +217,22 @@ export function usePaneWidth({ const minPaneWidth = isCustomWidth ? parseInt(width.min, 10) : minWidth const customMaxWidth = isCustomWidth ? parseInt(width.max, 10) : null + // Create persister - either use provided one or default to localStorage + // For custom persisters, we only use save(). For localStorage, we also use get(). + const localStoragePersister = React.useMemo(() => { + if (resizable !== true) return null + return createLocalStoragePersister(widthStorageKey) + }, [resizable, widthStorageKey]) + + // The persister used for saving - either custom or localStorage + const persister = React.useMemo(() => { + if (!isResizableEnabled(resizable)) return null + if (isWidthPersister(resizable)) return resizable + return localStoragePersister + }, [resizable, localStoragePersister]) + + const resizableRef = React.useRef(resizable) + // 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) @@ -149,31 +246,19 @@ export function usePaneWidth({ // --- State --- // Current width for React renders (ARIA attributes). Updates go through saveWidth() or clamp on resize. - // - // 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. + // For resizable=true (localStorage), we read synchronously in initializer to avoid flash on CSR. + // This causes hydration mismatch (server renders default, client reads localStorage) which is + // suppressed via suppressHydrationWarning on the pane element. + // For other resizable configs ({} or WidthPersister), consumer provides initial via `width` prop. 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 - } + // Only try localStorage for default persister (resizable === true) + if (resizable === true && localStoragePersister) { + const storedValue = localStoragePersister.get() + if (storedValue !== null && storedValue > 0) { + return storedValue } - } catch { - // localStorage unavailable - keep default } - return defaultWidth }) // Mutable ref for drag operations - avoids re-renders on every pixel move @@ -188,16 +273,38 @@ export function usePaneWidth({ (value: number) => { currentWidthRef.current = value setCurrentWidth(value) - try { - localStorage.setItem(widthStorageKey, value.toString()) - } catch { - // Ignore write errors (private browsing, quota exceeded, etc.) + // Persist to storage (async is fine - fire and forget) + // Wrapped in try-catch to prevent consumer errors from breaking the component + if (persister?.save) { + try { + const result = persister.save(value) + // Handle async rejections silently + if (result instanceof Promise) { + // eslint-disable-next-line github/no-then + result.catch(() => { + // Ignore - consumer should handle their own errors + }) + } + } catch { + // Ignore sync errors - consumer should handle their own errors + } } }, - [widthStorageKey], + [persister], ) // --- Effects --- + // Keep ref in sync with state (ref is used during drag to avoid re-renders). + // CSS variable is set via inline style in PageLayout.tsx, so no need to duplicate here. + // TODO: Consider reading localStorage here instead of useState initializer. + // This would avoid hydration mismatch (and need for suppressHydrationWarning) at the + // cost of one extra render cycle. Since useLayoutEffect runs before paint, user won't + // see the flash. Would also simplify the code by removing the special-case SSR logic. + useIsomorphicLayoutEffect(() => { + if (!isResizableEnabled(resizableRef.current)) return + currentWidthRef.current = currentWidth + }, [currentWidth]) + // Stable ref to getMaxPaneWidth for use in resize handler without re-subscribing const getMaxPaneWidthRef = React.useRef(getMaxPaneWidth) useIsomorphicLayoutEffect(() => { @@ -209,7 +316,7 @@ export function usePaneWidth({ // 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 + if (!isResizableEnabled(resizableRef.current)) return let lastViewportWidth = window.innerWidth @@ -309,7 +416,7 @@ export function usePaneWidth({ if (debounceId !== null) clearTimeout(debounceId) window.removeEventListener('resize', handleResize) } - }, [resizable, customMaxWidth, minPaneWidth, paneRef, handleRef]) + }, [customMaxWidth, minPaneWidth, paneRef, handleRef]) return { currentWidth, From d40a873baf3b6067eac9a58fbe2838fd0052fea3 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Tue, 16 Dec 2025 15:29:45 +0000 Subject: [PATCH 02/10] feat(PageLayout): add onWidthChange callback and controlled mode support - Add onWidthChange prop to PageLayout.Pane for tracking width changes - Support controlled mode by syncing internal state from width prop changes - onWidthChange fires at drag end and double-click reset (not during drag) - Both onWidthChange and persister.save fire when both are provided - Add error handling for onWidthChange callback errors - Add comprehensive tests for new functionality --- packages/react/src/PageLayout/PageLayout.tsx | 9 + .../react/src/PageLayout/usePaneWidth.test.ts | 173 ++++++++++++++++++ packages/react/src/PageLayout/usePaneWidth.ts | 39 ++++ 3 files changed, 221 insertions(+) diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 3954007ef5b..7702d529d51 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -566,10 +566,17 @@ export type PageLayoutPaneProps = { * Enable resizable pane behavior. * - `true`: Enable with default localStorage persistence * - `false`: Disable resizing + * - `{}`: Enable without persistence (no hydration issues) * - `WidthPersister`: Enable with custom storage (e.g., sessionStorage, server-side) */ resizable?: ResizableConfig widthStorageKey?: string + /** + * Callback fired when the pane width changes (on drag end or reset). + * Use this for controlled mode or to sync width to external state/storage. + * Fires in addition to any persistence configured via `resizable`. + */ + onWidthChange?: (width: number) => void padding?: keyof typeof SPACING_MAP divider?: 'none' | 'line' | ResponsiveValue<'none' | 'line', 'none' | 'line' | 'filled'> /** @@ -615,6 +622,7 @@ const Pane = React.forwardRef { }) }) + describe('onWidthChange', () => { + it('should call onWidthChange when saveWidth is called', () => { + const onWidthChange = vi.fn() + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-callback', + onWidthChange, + ...refs, + }), + ) + + act(() => { + result.current.saveWidth(450) + }) + + expect(onWidthChange).toHaveBeenCalledWith(450) + expect(result.current.currentWidth).toBe(450) + }) + + it('should call both onWidthChange and persister.save', () => { + const onWidthChange = vi.fn() + const customPersister: WidthPersister = { + save: vi.fn(), + } + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: customPersister, + widthStorageKey: 'test-both', + onWidthChange, + ...refs, + }), + ) + + act(() => { + result.current.saveWidth(400) + }) + + expect(onWidthChange).toHaveBeenCalledWith(400) + expect(customPersister.save).toHaveBeenCalledWith(400) + }) + + it('should call onWidthChange without persister when resizable={}', () => { + const onWidthChange = vi.fn() + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: {}, + widthStorageKey: 'test-no-persist', + onWidthChange, + ...refs, + }), + ) + + act(() => { + result.current.saveWidth(350) + }) + + expect(onWidthChange).toHaveBeenCalledWith(350) + expect(localStorage.getItem('test-no-persist')).toBeNull() + }) + + it('should handle errors in onWidthChange gracefully', () => { + const onWidthChange = vi.fn(() => { + throw new Error('Callback error') + }) + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, + widthStorageKey: 'test-error', + onWidthChange, + ...refs, + }), + ) + + // Should not throw + act(() => { + result.current.saveWidth(450) + }) + + expect(onWidthChange).toHaveBeenCalledWith(450) + expect(result.current.currentWidth).toBe(450) + }) + }) + + describe('width prop sync (controlled mode)', () => { + it('should sync internal state when width prop changes', () => { + const refs = createMockRefs() + + const {result, rerender} = renderHook( + ({width}: {width: 'small' | 'medium' | 'large'}) => + usePaneWidth({ + width, + minWidth: 256, + resizable: true, + widthStorageKey: 'test-sync', + ...refs, + }), + {initialProps: {width: 'medium' as 'small' | 'medium' | 'large'}}, + ) + + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) + + // Change width prop + rerender({width: 'large'}) + + expect(result.current.currentWidth).toBe(defaultPaneWidth.large) + }) + + it('should sync when width changes to custom width', () => { + const refs = createMockRefs() + type WidthType = 'medium' | {min: `${number}px`; default: `${number}px`; max: `${number}px`} + + const {result, rerender} = renderHook( + ({width}: {width: WidthType}) => + usePaneWidth({ + width, + minWidth: 256, + resizable: true, + widthStorageKey: 'test-sync-custom', + ...refs, + }), + {initialProps: {width: 'medium' as WidthType}}, + ) + + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) + + // Change to custom width + rerender({width: {min: '200px', default: '400px', max: '600px'}}) + + expect(result.current.currentWidth).toBe(400) + }) + + it('should not fire onWidthChange when width prop changes externally', () => { + const onWidthChange = vi.fn() + const refs = createMockRefs() + + const {rerender} = renderHook( + ({width}: {width: 'small' | 'medium' | 'large'}) => + usePaneWidth({ + width, + minWidth: 256, + resizable: true, + widthStorageKey: 'test-no-callback-sync', + onWidthChange, + ...refs, + }), + {initialProps: {width: 'medium' as 'small' | 'medium' | 'large'}}, + ) + + // Change width prop externally + rerender({width: 'large'}) + + // Should not fire - this is external sync, not user action + expect(onWidthChange).not.toHaveBeenCalled() + }) + }) + describe('minPaneWidth', () => { it('should use minWidth prop for preset widths', () => { const refs = createMockRefs() diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index 2bf67c9320d..605b629f87e 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -42,6 +42,11 @@ export type UsePaneWidthOptions = { widthStorageKey: string paneRef: React.RefObject handleRef: React.RefObject + /** + * Callback fired when the pane width changes (on drag end or reset). + * Use for controlled mode or to sync width to external state/storage. + */ + onWidthChange?: (width: number) => void } export type UsePaneWidthResult = { @@ -211,6 +216,7 @@ export function usePaneWidth({ widthStorageKey, paneRef, handleRef, + onWidthChange, }: UsePaneWidthOptions): UsePaneWidthResult { // Derive constraints from width configuration const isCustomWidth = isCustomWidthOptions(width) @@ -269,10 +275,26 @@ export function usePaneWidth({ // --- Callbacks --- const getDefaultWidth = React.useCallback(() => getDefaultPaneWidth(width), [width]) + // Stable ref for onWidthChange to avoid re-creating saveWidth on every render + const onWidthChangeRef = React.useRef(onWidthChange) + useIsomorphicLayoutEffect(() => { + onWidthChangeRef.current = onWidthChange + }) + const saveWidth = React.useCallback( (value: number) => { currentWidthRef.current = value setCurrentWidth(value) + + // Fire callback if provided (for controlled mode / external sync) + if (onWidthChangeRef.current) { + try { + onWidthChangeRef.current(value) + } catch { + // Ignore consumer errors + } + } + // Persist to storage (async is fine - fire and forget) // Wrapped in try-catch to prevent consumer errors from breaking the component if (persister?.save) { @@ -305,6 +327,23 @@ export function usePaneWidth({ currentWidthRef.current = currentWidth }, [currentWidth]) + // Sync internal state from width prop changes (for controlled mode). + // This allows consumers to change width externally and have the pane respond. + const prevWidthPropRef = React.useRef(width) + useIsomorphicLayoutEffect(() => { + const newDefault = getDefaultPaneWidth(width) + const oldDefault = getDefaultPaneWidth(prevWidthPropRef.current) + + // If width prop changed, sync internal state (but don't fire onWidthChange - this is external sync) + if (newDefault !== oldDefault) { + currentWidthRef.current = newDefault + setCurrentWidth(newDefault) + paneRef.current?.style.setProperty('--pane-width', `${newDefault}px`) + updateAriaValues(handleRef.current, {current: newDefault}) + } + prevWidthPropRef.current = width + }, [width, paneRef, handleRef]) + // Stable ref to getMaxPaneWidth for use in resize handler without re-subscribing const getMaxPaneWidthRef = React.useRef(getMaxPaneWidth) useIsomorphicLayoutEffect(() => { From 23b2b1aa495e636dcc2d1331361b0e18ee3b1e33 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Tue, 16 Dec 2025 16:03:36 +0000 Subject: [PATCH 03/10] refactor(PageLayout): use {persist: false} instead of empty object - Replace Record with explicit NoPersistConfig type - {persist: false} is more self-documenting than {} - Make save required on WidthPersister (cleaner type) - Rename isResizableWithoutPersistence to isNoPersistConfig - Export NoPersistConfig type - Update tests for new API --- packages/react/src/PageLayout/PageLayout.tsx | 2 +- packages/react/src/PageLayout/index.ts | 2 +- .../react/src/PageLayout/usePaneWidth.test.ts | 30 +++++++++---------- packages/react/src/PageLayout/usePaneWidth.ts | 22 +++++++++----- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 7702d529d51..75237c0fccc 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -566,7 +566,7 @@ export type PageLayoutPaneProps = { * Enable resizable pane behavior. * - `true`: Enable with default localStorage persistence * - `false`: Disable resizing - * - `{}`: Enable without persistence (no hydration issues) + * - `{persist: false}`: Enable without persistence (no hydration issues) * - `WidthPersister`: Enable with custom storage (e.g., sessionStorage, server-side) */ resizable?: ResizableConfig diff --git a/packages/react/src/PageLayout/index.ts b/packages/react/src/PageLayout/index.ts index 39da560d044..0fa4440a4ab 100644 --- a/packages/react/src/PageLayout/index.ts +++ b/packages/react/src/PageLayout/index.ts @@ -1,2 +1,2 @@ export * from './PageLayout' -export type {WidthPersister, ResizableConfig} from './usePaneWidth' +export type {WidthPersister, NoPersistConfig, ResizableConfig} from './usePaneWidth' diff --git a/packages/react/src/PageLayout/usePaneWidth.test.ts b/packages/react/src/PageLayout/usePaneWidth.test.ts index 0e8cbb672bc..00807b3729c 100644 --- a/packages/react/src/PageLayout/usePaneWidth.test.ts +++ b/packages/react/src/PageLayout/usePaneWidth.test.ts @@ -13,7 +13,7 @@ import { ARROW_KEY_STEP, isWidthPersister, isResizableEnabled, - isResizableWithoutPersistence, + isNoPersistConfig, type WidthPersister, } from './usePaneWidth' @@ -174,7 +174,7 @@ describe('usePaneWidth', () => { expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) }) - it('should not read from localStorage when empty object is provided', () => { + it('should not read from localStorage when {persist: false} is provided', () => { localStorage.setItem('test-pane', '500') const refs = createMockRefs() @@ -182,7 +182,7 @@ describe('usePaneWidth', () => { usePaneWidth({ width: 'medium', minWidth: 256, - resizable: {}, + resizable: {persist: false}, widthStorageKey: 'test-pane', ...refs, }), @@ -192,14 +192,14 @@ describe('usePaneWidth', () => { expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) }) - it('should not save to any storage when empty object is provided', () => { + it('should not save to any storage when {persist: false} is provided', () => { const refs = createMockRefs() const {result} = renderHook(() => usePaneWidth({ width: 'medium', minWidth: 256, - resizable: {}, + resizable: {persist: false}, widthStorageKey: 'test-pane', ...refs, }), @@ -425,7 +425,7 @@ describe('usePaneWidth', () => { expect(customPersister.save).toHaveBeenCalledWith(400) }) - it('should call onWidthChange without persister when resizable={}', () => { + it('should call onWidthChange without persister when resizable={persist: false}', () => { const onWidthChange = vi.fn() const refs = createMockRefs() @@ -433,7 +433,7 @@ describe('usePaneWidth', () => { usePaneWidth({ width: 'medium', minWidth: 256, - resizable: {}, + resizable: {persist: false}, widthStorageKey: 'test-no-persist', onWidthChange, ...refs, @@ -1166,26 +1166,26 @@ describe('type guards', () => { expect(isResizableEnabled({save: () => {}})).toBe(true) }) - it('should return true for empty object (resizable without persistence)', () => { - expect(isResizableEnabled({})).toBe(true) + it('should return true for {persist: false} (resizable without persistence)', () => { + expect(isResizableEnabled({persist: false})).toBe(true) }) }) - describe('isResizableWithoutPersistence', () => { - it('should return true for empty object', () => { - expect(isResizableWithoutPersistence({})).toBe(true) + describe('isNoPersistConfig', () => { + it('should return true for {persist: false}', () => { + expect(isNoPersistConfig({persist: false})).toBe(true) }) it('should return false for boolean true', () => { - expect(isResizableWithoutPersistence(true)).toBe(false) + expect(isNoPersistConfig(true)).toBe(false) }) it('should return false for boolean false', () => { - expect(isResizableWithoutPersistence(false)).toBe(false) + expect(isNoPersistConfig(false)).toBe(false) }) it('should return false for WidthPersister', () => { - expect(isResizableWithoutPersistence({save: () => {}})).toBe(false) + expect(isNoPersistConfig({save: () => {}})).toBe(false) }) }) }) diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index 605b629f87e..da5231d992d 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -23,17 +23,23 @@ export type PaneWidth = 'small' | 'medium' | 'large' */ export interface WidthPersister { /** Save the width value to storage. Can be async for server-side persistence. */ - save?: (value: number) => void | Promise + save: (value: number) => void | Promise } +/** + * Configuration for resizable without persistence. + * Use this to enable resizing without storing the width anywhere. + */ +export type NoPersistConfig = {persist: false} + /** * Resizable configuration options. * - `true`: Enable resizing with default localStorage persistence (may cause hydration mismatch) * - `false`: Disable resizing - * - `{}`: Enable resizing without persistence (no hydration issues) + * - `{persist: false}`: Enable resizing without persistence (no hydration issues) * - `WidthPersister`: Enable resizing with custom storage implementation (no hydration issues) */ -export type ResizableConfig = boolean | WidthPersister | Record +export type ResizableConfig = boolean | WidthPersister | NoPersistConfig export type UsePaneWidthOptions = { width: PaneWidth | CustomWidthOptions @@ -122,15 +128,15 @@ export const isWidthPersister = (config: ResizableConfig): config is WidthPersis } /** - * Type guard to check if resizable config is an empty object (resizable without persistence) + * Type guard to check if resizable config is {persist: false} (resizable without persistence) */ -export const isResizableWithoutPersistence = (config: ResizableConfig): config is Record => { +export const isNoPersistConfig = (config: ResizableConfig): config is NoPersistConfig => { // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- config could be null at runtime despite types - return typeof config === 'object' && config !== null && !('save' in config) + return typeof config === 'object' && config !== null && 'persist' in config && config.persist === false } /** - * Check if resizing is enabled (boolean true, empty object, or persister object) + * Check if resizing is enabled (boolean true, {persist: false}, or persister object) */ export const isResizableEnabled = (config: ResizableConfig): boolean => { return config === true || typeof config === 'object' @@ -297,7 +303,7 @@ export function usePaneWidth({ // Persist to storage (async is fine - fire and forget) // Wrapped in try-catch to prevent consumer errors from breaking the component - if (persister?.save) { + if (persister) { try { const result = persister.save(value) // Handle async rejections silently From fdfa3082afbce3cdbd990d35fd7b000abad9ba81 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Tue, 16 Dec 2025 16:52:53 +0000 Subject: [PATCH 04/10] basic update for persisters on resizable --- packages/react/src/PageLayout/PageLayout.tsx | 8 - packages/react/src/PageLayout/index.ts | 2 +- .../react/src/PageLayout/usePaneWidth.test.ts | 261 ++++-------------- packages/react/src/PageLayout/usePaneWidth.ts | 164 +++++------ 4 files changed, 133 insertions(+), 302 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 75237c0fccc..5a26f5559ef 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -571,12 +571,6 @@ export type PageLayoutPaneProps = { */ resizable?: ResizableConfig widthStorageKey?: string - /** - * Callback fired when the pane width changes (on drag end or reset). - * Use this for controlled mode or to sync width to external state/storage. - * Fires in addition to any persistence configured via `resizable`. - */ - onWidthChange?: (width: number) => void padding?: keyof typeof SPACING_MAP divider?: 'none' | 'line' | ResponsiveValue<'none' | 'line', 'none' | 'line' | 'filled'> /** @@ -622,7 +616,6 @@ const Pane = React.forwardRef { expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) }) - it('should use default width when custom WidthPersister is provided', () => { - const customPersister: WidthPersister = { - save: vi.fn(), - } - const refs = createMockRefs() - - const {result} = renderHook(() => - usePaneWidth({ - width: 'medium', - minWidth: 256, - resizable: customPersister, - widthStorageKey: 'test-pane', - ...refs, - }), - ) - - // Custom persisters use default width - consumer provides initial via width prop - expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) - }) - - it('should not read from localStorage when custom WidthPersister is provided', () => { - localStorage.setItem('test-pane', '500') - const customPersister: WidthPersister = { - save: vi.fn(), - } - const refs = createMockRefs() - - const {result} = renderHook(() => - usePaneWidth({ - width: 'medium', - minWidth: 256, - resizable: customPersister, - widthStorageKey: 'test-pane', - ...refs, - }), - ) - - // Should use default, not localStorage value - expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) - }) - it('should not read from localStorage when {persist: false} is provided', () => { localStorage.setItem('test-pane', '500') const refs = createMockRefs() @@ -268,10 +227,9 @@ describe('usePaneWidth', () => { localStorage.setItem = originalSetItem }) - it('should call custom persister save method', () => { - const customPersister: WidthPersister = { - save: vi.fn(), - } + it('should call custom save function with width and options', () => { + const customSave = vi.fn() + const customPersister: CustomPersistConfig = {save: customSave} const refs = createMockRefs() const {result} = renderHook(() => @@ -279,7 +237,7 @@ describe('usePaneWidth', () => { width: 'medium', minWidth: 256, resizable: customPersister, - widthStorageKey: 'test-custom-save', + widthStorageKey: 'my-custom-key', ...refs, }), ) @@ -289,15 +247,14 @@ describe('usePaneWidth', () => { }) expect(result.current.currentWidth).toBe(450) - expect(customPersister.save).toHaveBeenCalledWith(450) + expect(customSave).toHaveBeenCalledWith(450, {widthStorageKey: 'my-custom-key'}) // Should NOT write to localStorage - expect(localStorage.getItem('test-custom-save')).toBeNull() + expect(localStorage.getItem('my-custom-key')).toBeNull() }) - it('should handle async custom persister save', async () => { - const customPersister: WidthPersister = { - save: vi.fn().mockResolvedValue(undefined), - } + it('should handle async custom save function', async () => { + const customSave = vi.fn().mockResolvedValue(undefined) + const customPersister: CustomPersistConfig = {save: customSave} const refs = createMockRefs() const {result} = renderHook(() => @@ -305,7 +262,7 @@ describe('usePaneWidth', () => { width: 'medium', minWidth: 256, resizable: customPersister, - widthStorageKey: 'test-async-save', + widthStorageKey: 'test-async', ...refs, }), ) @@ -315,15 +272,14 @@ describe('usePaneWidth', () => { }) expect(result.current.currentWidth).toBe(350) - expect(customPersister.save).toHaveBeenCalledWith(350) + expect(customSave).toHaveBeenCalledWith(350, {widthStorageKey: 'test-async'}) }) - it('should handle sync errors from custom persister gracefully', () => { - const customPersister: WidthPersister = { - save: vi.fn(() => { - throw new Error('Sync storage error') - }), - } + it('should handle sync errors from custom save gracefully', () => { + const customSave = vi.fn(() => { + throw new Error('Sync storage error') + }) + const customPersister: CustomPersistConfig = {save: customSave} const refs = createMockRefs() const {result} = renderHook(() => @@ -342,13 +298,12 @@ describe('usePaneWidth', () => { }) expect(result.current.currentWidth).toBe(450) - expect(customPersister.save).toHaveBeenCalledWith(450) + expect(customSave).toHaveBeenCalledWith(450, {widthStorageKey: 'test-sync-error'}) }) - it('should handle async rejection from custom persister gracefully', async () => { - const customPersister: WidthPersister = { - save: vi.fn().mockRejectedValue(new Error('Async storage error')), - } + it('should handle async rejection from custom save gracefully', async () => { + const customSave = vi.fn().mockRejectedValue(new Error('Async storage error')) + const customPersister: CustomPersistConfig = {save: customSave} const refs = createMockRefs() const {result} = renderHook(() => @@ -368,42 +323,15 @@ describe('usePaneWidth', () => { // Wait for promise rejection to be handled await vi.waitFor(() => { - expect(customPersister.save).toHaveBeenCalledWith(450) + expect(customSave).toHaveBeenCalledWith(450, {widthStorageKey: 'test-async-error'}) }) expect(result.current.currentWidth).toBe(450) }) - }) - - describe('onWidthChange', () => { - it('should call onWidthChange when saveWidth is called', () => { - const onWidthChange = vi.fn() - const refs = createMockRefs() - const {result} = renderHook(() => - usePaneWidth({ - width: 'medium', - minWidth: 256, - resizable: true, - widthStorageKey: 'test-callback', - onWidthChange, - ...refs, - }), - ) - - act(() => { - result.current.saveWidth(450) - }) - - expect(onWidthChange).toHaveBeenCalledWith(450) - expect(result.current.currentWidth).toBe(450) - }) - - it('should call both onWidthChange and persister.save', () => { - const onWidthChange = vi.fn() - const customPersister: WidthPersister = { - save: vi.fn(), - } + it('should not read from localStorage when custom save is provided', () => { + localStorage.setItem('test-pane', '500') + const customPersister: CustomPersistConfig = {save: vi.fn()} const refs = createMockRefs() const {result} = renderHook(() => @@ -411,67 +339,13 @@ describe('usePaneWidth', () => { width: 'medium', minWidth: 256, resizable: customPersister, - widthStorageKey: 'test-both', - onWidthChange, - ...refs, - }), - ) - - act(() => { - result.current.saveWidth(400) - }) - - expect(onWidthChange).toHaveBeenCalledWith(400) - expect(customPersister.save).toHaveBeenCalledWith(400) - }) - - it('should call onWidthChange without persister when resizable={persist: false}', () => { - const onWidthChange = vi.fn() - const refs = createMockRefs() - - const {result} = renderHook(() => - usePaneWidth({ - width: 'medium', - minWidth: 256, - resizable: {persist: false}, - widthStorageKey: 'test-no-persist', - onWidthChange, - ...refs, - }), - ) - - act(() => { - result.current.saveWidth(350) - }) - - expect(onWidthChange).toHaveBeenCalledWith(350) - expect(localStorage.getItem('test-no-persist')).toBeNull() - }) - - it('should handle errors in onWidthChange gracefully', () => { - const onWidthChange = vi.fn(() => { - throw new Error('Callback error') - }) - const refs = createMockRefs() - - const {result} = renderHook(() => - usePaneWidth({ - width: 'medium', - minWidth: 256, - resizable: true, - widthStorageKey: 'test-error', - onWidthChange, + widthStorageKey: 'test-pane', ...refs, }), ) - // Should not throw - act(() => { - result.current.saveWidth(450) - }) - - expect(onWidthChange).toHaveBeenCalledWith(450) - expect(result.current.currentWidth).toBe(450) + // Should use default, not localStorage value + expect(result.current.currentWidth).toBe(defaultPaneWidth.medium) }) }) @@ -522,30 +396,6 @@ describe('usePaneWidth', () => { expect(result.current.currentWidth).toBe(400) }) - - it('should not fire onWidthChange when width prop changes externally', () => { - const onWidthChange = vi.fn() - const refs = createMockRefs() - - const {rerender} = renderHook( - ({width}: {width: 'small' | 'medium' | 'large'}) => - usePaneWidth({ - width, - minWidth: 256, - resizable: true, - widthStorageKey: 'test-no-callback-sync', - onWidthChange, - ...refs, - }), - {initialProps: {width: 'medium' as 'small' | 'medium' | 'large'}}, - ) - - // Change width prop externally - rerender({width: 'large'}) - - // Should not fire - this is external sync, not user action - expect(onWidthChange).not.toHaveBeenCalled() - }) }) describe('minPaneWidth', () => { @@ -1136,23 +986,6 @@ describe('constants', () => { }) describe('type guards', () => { - describe('isWidthPersister', () => { - it('should return true for objects with save method', () => { - expect(isWidthPersister({save: () => {}})).toBe(true) - expect(isWidthPersister({save: async () => {}})).toBe(true) - }) - - it('should return false for boolean values', () => { - expect(isWidthPersister(true)).toBe(false) - expect(isWidthPersister(false)).toBe(false) - }) - - it('should return false for null', () => { - // @ts-expect-error - testing runtime behavior - expect(isWidthPersister(null)).toBe(false) - }) - }) - describe('isResizableEnabled', () => { it('should return true for boolean true', () => { expect(isResizableEnabled(true)).toBe(true) @@ -1162,13 +995,13 @@ describe('type guards', () => { expect(isResizableEnabled(false)).toBe(false) }) - it('should return true for WidthPersister objects', () => { - expect(isResizableEnabled({save: () => {}})).toBe(true) - }) - it('should return true for {persist: false} (resizable without persistence)', () => { expect(isResizableEnabled({persist: false})).toBe(true) }) + + it('should return true for {save: fn} (custom persistence)', () => { + expect(isResizableEnabled({save: () => {}})).toBe(true) + }) }) describe('isNoPersistConfig', () => { @@ -1184,8 +1017,34 @@ describe('type guards', () => { expect(isNoPersistConfig(false)).toBe(false) }) - it('should return false for WidthPersister', () => { + it('should return false for objects without persist: false', () => { + // @ts-expect-error - testing runtime behavior with arbitrary object + expect(isNoPersistConfig({other: 'value'})).toBe(false) + }) + + it('should return false for custom persist config', () => { expect(isNoPersistConfig({save: () => {}})).toBe(false) }) }) + + describe('isCustomPersistConfig', () => { + it('should return true for objects with save function', () => { + expect(isCustomPersistConfig({save: () => {}})).toBe(true) + expect(isCustomPersistConfig({save: async () => {}})).toBe(true) + }) + + it('should return false for boolean values', () => { + expect(isCustomPersistConfig(true)).toBe(false) + expect(isCustomPersistConfig(false)).toBe(false) + }) + + it('should return false for {persist: false}', () => { + expect(isCustomPersistConfig({persist: false})).toBe(false) + }) + + it('should return false for null', () => { + // @ts-expect-error - testing runtime behavior + expect(isCustomPersistConfig(null)).toBe(false) + }) + }) }) diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index da5231d992d..3147b1caafd 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -16,30 +16,40 @@ export type CustomWidthOptions = { export type PaneWidth = 'small' | 'medium' | 'large' /** - * Interface for persisting pane width to storage. - * Allows consumers to provide custom storage implementations (e.g., sessionStorage, IndexedDB, server-side). - * - * For custom persisters, the initial width should be passed via the `width` prop. + * Configuration for resizable without persistence. + * Use this to enable resizing without storing the width anywhere. + */ +export type NoPersistConfig = {persist: false} + +/** + * Options passed to custom save function. + */ +export type SaveOptions = {widthStorageKey: string} + +/** + * Configuration for custom persistence. + * Provide your own save function to persist width to server, IndexedDB, etc. */ -export interface WidthPersister { - /** Save the width value to storage. Can be async for server-side persistence. */ - save: (value: number) => void | Promise +export type CustomPersistConfig = { + save: (width: number, options: SaveOptions) => void | Promise } /** - * Configuration for resizable without persistence. - * Use this to enable resizing without storing the width anywhere. + * Type guard to check if resizable config has a custom save function */ -export type NoPersistConfig = {persist: false} +export const isCustomPersistConfig = (config: ResizableConfig): config is CustomPersistConfig => { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- config could be null at runtime despite types + return typeof config === 'object' && config !== null && 'save' in config && typeof config.save === 'function' +} /** * Resizable configuration options. * - `true`: Enable resizing with default localStorage persistence (may cause hydration mismatch) * - `false`: Disable resizing - * - `{persist: false}`: Enable resizing without persistence (no hydration issues) - * - `WidthPersister`: Enable resizing with custom storage implementation (no hydration issues) + * - `{persist: false}`: Enable resizing without any persistence + * - `{save: fn}`: Enable resizing with custom persistence */ -export type ResizableConfig = boolean | WidthPersister | NoPersistConfig +export type ResizableConfig = boolean | NoPersistConfig | CustomPersistConfig export type UsePaneWidthOptions = { width: PaneWidth | CustomWidthOptions @@ -48,11 +58,6 @@ export type UsePaneWidthOptions = { widthStorageKey: string paneRef: React.RefObject handleRef: React.RefObject - /** - * Callback fired when the pane width changes (on drag end or reset). - * Use for controlled mode or to sync width to external state/storage. - */ - onWidthChange?: (width: number) => void } export type UsePaneWidthResult = { @@ -120,15 +125,7 @@ export const getDefaultPaneWidth = (w: PaneWidth | CustomWidthOptions): number = } /** - * Type guard to check if resizable config is a WidthPersister (has save method) - */ -export const isWidthPersister = (config: ResizableConfig): config is WidthPersister => { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- config could be null at runtime despite types - return typeof config === 'object' && config !== null && 'save' in config -} - -/** - * Type guard to check if resizable config is {persist: false} (resizable without persistence) + * Type guard to check if resizable config is {persist: false} */ export const isNoPersistConfig = (config: ResizableConfig): config is NoPersistConfig => { // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- config could be null at runtime despite types @@ -136,17 +133,18 @@ export const isNoPersistConfig = (config: ResizableConfig): config is NoPersistC } /** - * Check if resizing is enabled (boolean true, {persist: false}, or persister object) + * Check if resizing is enabled (boolean true, {persist: false}, or {save: fn}) */ export const isResizableEnabled = (config: ResizableConfig): boolean => { - return config === true || typeof config === 'object' + return config === true || isNoPersistConfig(config) || isCustomPersistConfig(config) } /** - * Internal interface for localStorage persister (includes get for reading stored value) + * Internal interface for localStorage persister */ -interface InternalLocalStoragePersister extends WidthPersister { +interface InternalLocalStoragePersister { get: () => number | null + save: (value: number) => void } /** @@ -212,8 +210,8 @@ export const updateAriaValues = ( * * Storage behavior: * - When `resizable` is `true`: Uses localStorage with the provided `widthStorageKey` - * - When `resizable` is a `WidthPersister`: Uses the custom persister's save method - * (consumer provides initial width via `width` prop) + * - When `resizable` is `{persist: false}`: Resizable without any persistence + * - When `resizable` is `{save: fn}`: Resizable with custom persistence */ export function usePaneWidth({ width, @@ -222,27 +220,18 @@ export function usePaneWidth({ widthStorageKey, paneRef, handleRef, - onWidthChange, }: 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 - // Create persister - either use provided one or default to localStorage - // For custom persisters, we only use save(). For localStorage, we also use get(). - const localStoragePersister = React.useMemo(() => { + // Create localStorage persister - only used when resizable === true + const persister = React.useMemo(() => { if (resizable !== true) return null return createLocalStoragePersister(widthStorageKey) }, [resizable, widthStorageKey]) - // The persister used for saving - either custom or localStorage - const persister = React.useMemo(() => { - if (!isResizableEnabled(resizable)) return null - if (isWidthPersister(resizable)) return resizable - return localStoragePersister - }, [resizable, localStoragePersister]) - const resizableRef = React.useRef(resizable) // Cache the CSS variable value to avoid getComputedStyle during drag (causes layout thrashing) @@ -256,35 +245,54 @@ export function usePaneWidth({ return viewportWidth > 0 ? Math.max(minPaneWidth, viewportWidth - maxWidthDiffRef.current) : minPaneWidth }, [customMaxWidth, minPaneWidth]) + const defaultWidth = getDefaultPaneWidth(width) // --- State --- // Current width for React renders (ARIA attributes). Updates go through saveWidth() or clamp on resize. // For resizable=true (localStorage), we read synchronously in initializer to avoid flash on CSR. // This causes hydration mismatch (server renders default, client reads localStorage) which is // suppressed via suppressHydrationWarning on the pane element. - // For other resizable configs ({} or WidthPersister), consumer provides initial via `width` prop. + // For other resizable configs ({persist: false}), consumer provides initial via `width` prop. const [currentWidth, setCurrentWidth] = React.useState(() => { - const defaultWidth = getDefaultPaneWidth(width) // Only try localStorage for default persister (resizable === true) - if (resizable === true && localStoragePersister) { - const storedValue = localStoragePersister.get() + if (persister) { + const storedValue = persister.get() if (storedValue !== null && storedValue > 0) { return storedValue } } return defaultWidth }) + + // Inline state sync when width prop changes (avoids effect) + // See: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes + const [prevDefaultWidth, setPrevDefaultWidth] = React.useState(defaultWidth) + if (defaultWidth !== prevDefaultWidth) { + setPrevDefaultWidth(defaultWidth) + setCurrentWidth(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 const [maxPaneWidth, setMaxPaneWidth] = React.useState(() => customMaxWidth ?? SSR_DEFAULT_MAX_WIDTH) + // Keep currentWidthRef in sync with state (ref is used during drag to avoid re-renders) + useIsomorphicLayoutEffect(() => { + currentWidthRef.current = currentWidth + }, [currentWidth]) + // --- Callbacks --- const getDefaultWidth = React.useCallback(() => getDefaultPaneWidth(width), [width]) - // Stable ref for onWidthChange to avoid re-creating saveWidth on every render - const onWidthChangeRef = React.useRef(onWidthChange) + // Refs for accessing current values in callbacks without re-creating them + const resizableConfigRef = React.useRef(resizable) + useIsomorphicLayoutEffect(() => { + resizableConfigRef.current = resizable + }) + + const widthStorageKeyRef = React.useRef(widthStorageKey) useIsomorphicLayoutEffect(() => { - onWidthChangeRef.current = onWidthChange + widthStorageKeyRef.current = widthStorageKey }) const saveWidth = React.useCallback( @@ -292,20 +300,12 @@ export function usePaneWidth({ currentWidthRef.current = value setCurrentWidth(value) - // Fire callback if provided (for controlled mode / external sync) - if (onWidthChangeRef.current) { - try { - onWidthChangeRef.current(value) - } catch { - // Ignore consumer errors - } - } + const config = resizableConfigRef.current - // Persist to storage (async is fine - fire and forget) - // Wrapped in try-catch to prevent consumer errors from breaking the component - if (persister) { + // Handle custom persistence: {save: fn} + if (isCustomPersistConfig(config)) { try { - const result = persister.save(value) + const result = config.save(value, {widthStorageKey: widthStorageKeyRef.current}) // Handle async rejections silently if (result instanceof Promise) { // eslint-disable-next-line github/no-then @@ -314,42 +314,22 @@ export function usePaneWidth({ }) } } catch { - // Ignore sync errors - consumer should handle their own errors + // Ignore sync errors } + return } + + // Handle localStorage persistence: resizable === true + if (persister) { + persister.save(value) + } + + // {persist: false} - no persistence }, [persister], ) // --- Effects --- - // Keep ref in sync with state (ref is used during drag to avoid re-renders). - // CSS variable is set via inline style in PageLayout.tsx, so no need to duplicate here. - // TODO: Consider reading localStorage here instead of useState initializer. - // This would avoid hydration mismatch (and need for suppressHydrationWarning) at the - // cost of one extra render cycle. Since useLayoutEffect runs before paint, user won't - // see the flash. Would also simplify the code by removing the special-case SSR logic. - useIsomorphicLayoutEffect(() => { - if (!isResizableEnabled(resizableRef.current)) return - currentWidthRef.current = currentWidth - }, [currentWidth]) - - // Sync internal state from width prop changes (for controlled mode). - // This allows consumers to change width externally and have the pane respond. - const prevWidthPropRef = React.useRef(width) - useIsomorphicLayoutEffect(() => { - const newDefault = getDefaultPaneWidth(width) - const oldDefault = getDefaultPaneWidth(prevWidthPropRef.current) - - // If width prop changed, sync internal state (but don't fire onWidthChange - this is external sync) - if (newDefault !== oldDefault) { - currentWidthRef.current = newDefault - setCurrentWidth(newDefault) - paneRef.current?.style.setProperty('--pane-width', `${newDefault}px`) - updateAriaValues(handleRef.current, {current: newDefault}) - } - prevWidthPropRef.current = width - }, [width, paneRef, handleRef]) - // Stable ref to getMaxPaneWidth for use in resize handler without re-subscribing const getMaxPaneWidthRef = React.useRef(getMaxPaneWidth) useIsomorphicLayoutEffect(() => { From 6d51d8c65cf90bc78170fa9a4c088313aa64857f Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Tue, 16 Dec 2025 17:42:42 +0000 Subject: [PATCH 05/10] update api --- .../PageLayout.features.stories.tsx | 60 ++++++++ packages/react/src/PageLayout/usePaneWidth.ts | 132 +++++++----------- 2 files changed, 108 insertions(+), 84 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.features.stories.tsx b/packages/react/src/PageLayout/PageLayout.features.stories.tsx index aef5c9346b0..e90b99eda6f 100644 --- a/packages/react/src/PageLayout/PageLayout.features.stories.tsx +++ b/packages/react/src/PageLayout/PageLayout.features.stories.tsx @@ -1,4 +1,5 @@ import type {Meta, StoryFn} from '@storybook/react-vite' +import React from 'react' import {PageLayout} from './PageLayout' import {Placeholder} from '../Placeholder' import {BranchName, Heading, Link, StateLabel, Text} from '..' @@ -358,3 +359,62 @@ export const WithCustomPaneHeading: StoryFn = () => ( ) + +export const ResizablePaneWithoutPersistence: StoryFn = () => ( + + + + + + + + + + + + + + +) +ResizablePaneWithoutPersistence.storyName = 'Resizable pane without persistence' + +export const ResizablePaneWithCustomPersistence: StoryFn = () => { + const [savedWidth, setSavedWidth] = React.useState(null) + + return ( +
+
+ + Last saved width: {savedWidth !== null ? `${savedWidth}px` : 'Not saved yet'} + + + (Resize the pane to see the custom save function in action) + +
+ + + + + { + // eslint-disable-next-line no-console + console.log(`Custom save: ${width}px with key "${widthStorageKey}"`) + setSavedWidth(width) + }, + }} + aria-label="Side pane" + > + + + + + + + + + +
+ ) +} +ResizablePaneWithCustomPersistence.storyName = 'Resizable pane with custom persistence' diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index 3147b1caafd..d80effc2d22 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -139,41 +139,6 @@ export const isResizableEnabled = (config: ResizableConfig): boolean => { return config === true || isNoPersistConfig(config) || isCustomPersistConfig(config) } -/** - * Internal interface for localStorage persister - */ -interface InternalLocalStoragePersister { - get: () => number | null - save: (value: number) => void -} - -/** - * Creates a default localStorage-based persister (internal use only) - */ -const createLocalStoragePersister = (storageKey: string): InternalLocalStoragePersister => ({ - get: () => { - try { - const storedWidth = localStorage.getItem(storageKey) - if (storedWidth !== null) { - const parsed = Number(storedWidth) - if (!isNaN(parsed) && parsed > 0) { - return parsed - } - } - } catch { - // localStorage unavailable - } - return null - }, - save: (value: number) => { - try { - localStorage.setItem(storageKey, value.toString()) - } catch { - // Ignore write errors (private browsing, quota exceeded, etc.) - } - }, -}) - /** * 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. @@ -226,14 +191,15 @@ export function usePaneWidth({ const minPaneWidth = isCustomWidth ? parseInt(width.min, 10) : minWidth const customMaxWidth = isCustomWidth ? parseInt(width.max, 10) : null - // Create localStorage persister - only used when resizable === true - const persister = React.useMemo(() => { - if (resizable !== true) return null - return createLocalStoragePersister(widthStorageKey) - }, [resizable, widthStorageKey]) - + // Refs for stable callbacks - updated in layout effect below + const widthStorageKeyRef = React.useRef(widthStorageKey) const resizableRef = React.useRef(resizable) + // Keep refs in sync with props for stable callbacks + useIsomorphicLayoutEffect(() => { + resizableRef.current = resizable + widthStorageKeyRef.current = widthStorageKey + }) // 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) @@ -254,10 +220,18 @@ export function usePaneWidth({ // For other resizable configs ({persist: false}), consumer provides initial via `width` prop. const [currentWidth, setCurrentWidth] = React.useState(() => { // Only try localStorage for default persister (resizable === true) - if (persister) { - const storedValue = persister.get() - if (storedValue !== null && storedValue > 0) { - return storedValue + // Read directly here instead of via persister to satisfy react-hooks/refs lint rule + if (resizable === true) { + try { + const storedWidth = localStorage.getItem(widthStorageKey) + if (storedWidth !== null) { + const parsed = Number(storedWidth) + if (!isNaN(parsed) && parsed > 0) { + return parsed + } + } + } catch { + // localStorage unavailable } } return defaultWidth @@ -284,50 +258,40 @@ export function usePaneWidth({ // --- Callbacks --- const getDefaultWidth = React.useCallback(() => getDefaultPaneWidth(width), [width]) - // Refs for accessing current values in callbacks without re-creating them - const resizableConfigRef = React.useRef(resizable) - useIsomorphicLayoutEffect(() => { - resizableConfigRef.current = resizable - }) - - const widthStorageKeyRef = React.useRef(widthStorageKey) - useIsomorphicLayoutEffect(() => { - widthStorageKeyRef.current = widthStorageKey - }) - - const saveWidth = React.useCallback( - (value: number) => { - currentWidthRef.current = value - setCurrentWidth(value) - - const config = resizableConfigRef.current - - // Handle custom persistence: {save: fn} - if (isCustomPersistConfig(config)) { - try { - const result = config.save(value, {widthStorageKey: widthStorageKeyRef.current}) - // Handle async rejections silently - if (result instanceof Promise) { - // eslint-disable-next-line github/no-then - result.catch(() => { - // Ignore - consumer should handle their own errors - }) - } - } catch { - // Ignore sync errors + const saveWidth = React.useCallback((value: number) => { + currentWidthRef.current = value + setCurrentWidth(value) + + const config = resizableRef.current + + // Handle custom persistence: {save: fn} + if (isCustomPersistConfig(config)) { + try { + const result = config.save(value, {widthStorageKey: widthStorageKeyRef.current}) + // Handle async rejections silently + if (result instanceof Promise) { + // eslint-disable-next-line github/no-then + result.catch(() => { + // Ignore - consumer should handle their own errors + }) } - return + } catch { + // Ignore sync errors } + return + } - // Handle localStorage persistence: resizable === true - if (persister) { - persister.save(value) + // Handle localStorage persistence: resizable === true + if (resizableRef.current === true) { + try { + localStorage.setItem(widthStorageKeyRef.current, value.toString()) + } catch { + // Ignore write errors (private browsing, quota exceeded, etc.) } + } - // {persist: false} - no persistence - }, - [persister], - ) + // {persist: false} - no persistence + }, []) // --- Effects --- // Stable ref to getMaxPaneWidth for use in resize handler without re-subscribing From 6f82d54d940789d605da0dedb030c07289ef3295 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Tue, 16 Dec 2025 17:46:51 +0000 Subject: [PATCH 06/10] Add changeset for PageLayout resizable persistence --- .../pagelayout-resizable-persistence.md | 35 +++++++++++ packages/react/src/PageLayout/PageLayout.tsx | 2 +- packages/react/src/PageLayout/usePaneWidth.ts | 62 ++++++++++--------- 3 files changed, 70 insertions(+), 29 deletions(-) create mode 100644 .changeset/pagelayout-resizable-persistence.md diff --git a/.changeset/pagelayout-resizable-persistence.md b/.changeset/pagelayout-resizable-persistence.md new file mode 100644 index 00000000000..ffa991de540 --- /dev/null +++ b/.changeset/pagelayout-resizable-persistence.md @@ -0,0 +1,35 @@ +--- +'@primer/react': minor +--- + +Add custom persistence options to PageLayout.Pane's `resizable` prop + +The `resizable` prop now accepts additional configuration options: + +- `true` - Enable resizing with default localStorage persistence (existing behavior) +- `false` - Disable resizing (existing behavior) +- `{persist: false}` - Enable resizing without any persistence (avoids hydration mismatches) +- `{save: fn}` - Enable resizing with custom persistence (e.g., server-side, IndexedDB) + +**New types exported:** +- `NoPersistConfig` - Type for `{persist: false}` configuration +- `CustomPersistConfig` - Type for `{save: fn}` configuration +- `SaveOptions` - Options passed to custom save function (`{widthStorageKey: string}`) +- `ResizableConfig` - Union type for all resizable configurations + +**Example usage:** + +```tsx +// No persistence - useful for SSR to avoid hydration mismatches + + +// Custom persistence - save to your own storage + { + // Save to server, IndexedDB, sessionStorage, etc. + myStorage.set(widthStorageKey, width) + } + }} +/> +``` diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 5a26f5559ef..e856054de00 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -567,7 +567,7 @@ export type PageLayoutPaneProps = { * - `true`: Enable with default localStorage persistence * - `false`: Disable resizing * - `{persist: false}`: Enable without persistence (no hydration issues) - * - `WidthPersister`: Enable with custom storage (e.g., sessionStorage, server-side) + * - `{save: fn}`: Enable with custom persistence (e.g., server-side, IndexedDB) */ resizable?: ResizableConfig widthStorageKey?: string diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index d80effc2d22..ed1a4ef5014 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -1,4 +1,4 @@ -import React, {startTransition} from 'react' +import React, {startTransition, useMemo} from 'react' import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' import cssExports from './PageLayout.module.css' @@ -107,8 +107,7 @@ export const defaultPaneWidth: Record = {small: 256, medium: // 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 + return typeof width !== 'string' } export const isPaneWidth = (width: PaneWidth | CustomWidthOptions): width is PaneWidth => { @@ -165,6 +164,30 @@ export const updateAriaValues = ( } } +const localStoragePersister = { + save: (key: string, width: number) => { + try { + localStorage.setItem(key, width.toString()) + } catch { + // Ignore write errors (private browsing, quota exceeded, etc.) + } + }, + get: (key: string): number | null => { + try { + const storedWidth = localStorage.getItem(key) + if (storedWidth !== null) { + const parsed = Number(storedWidth) + if (!isNaN(parsed) && parsed > 0) { + return parsed + } + } + } catch { + // localStorage unavailable + } + return null + }, +} + // ---------------------------------------------------------------------------- // Hook @@ -211,7 +234,7 @@ export function usePaneWidth({ return viewportWidth > 0 ? Math.max(minPaneWidth, viewportWidth - maxWidthDiffRef.current) : minPaneWidth }, [customMaxWidth, minPaneWidth]) - const defaultWidth = getDefaultPaneWidth(width) + const defaultWidth = useMemo(() => getDefaultPaneWidth(width), [width]) // --- State --- // Current width for React renders (ARIA attributes). Updates go through saveWidth() or clamp on resize. // For resizable=true (localStorage), we read synchronously in initializer to avoid flash on CSR. @@ -222,16 +245,9 @@ export function usePaneWidth({ // Only try localStorage for default persister (resizable === true) // Read directly here instead of via persister to satisfy react-hooks/refs lint rule if (resizable === true) { - try { - const storedWidth = localStorage.getItem(widthStorageKey) - if (storedWidth !== null) { - const parsed = Number(storedWidth) - if (!isNaN(parsed) && parsed > 0) { - return parsed - } - } - } catch { - // localStorage unavailable + const storedWidth = localStoragePersister.get(widthStorageKey) + if (storedWidth !== null) { + return storedWidth } } return defaultWidth @@ -264,8 +280,10 @@ export function usePaneWidth({ const config = resizableRef.current - // Handle custom persistence: {save: fn} - if (isCustomPersistConfig(config)) { + // Handle localStorage persistence: resizable === true + if (config === true) { + localStoragePersister.save(widthStorageKeyRef.current, value) + } else if (isCustomPersistConfig(config)) { try { const result = config.save(value, {widthStorageKey: widthStorageKeyRef.current}) // Handle async rejections silently @@ -278,19 +296,7 @@ export function usePaneWidth({ } catch { // Ignore sync errors } - return - } - - // Handle localStorage persistence: resizable === true - if (resizableRef.current === true) { - try { - localStorage.setItem(widthStorageKeyRef.current, value.toString()) - } catch { - // Ignore write errors (private browsing, quota exceeded, etc.) - } } - - // {persist: false} - no persistence }, []) // --- Effects --- From db2ee1fd17f0e15320d829277935bfa5585f8076 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Tue, 16 Dec 2025 17:58:33 +0000 Subject: [PATCH 07/10] fix: use theme-aware colors in custom persistence story --- .../PageLayout.features.stories.tsx | 59 ++++++++----------- 1 file changed, 26 insertions(+), 33 deletions(-) diff --git a/packages/react/src/PageLayout/PageLayout.features.stories.tsx b/packages/react/src/PageLayout/PageLayout.features.stories.tsx index e90b99eda6f..a52a25824c8 100644 --- a/packages/react/src/PageLayout/PageLayout.features.stories.tsx +++ b/packages/react/src/PageLayout/PageLayout.features.stories.tsx @@ -382,39 +382,32 @@ export const ResizablePaneWithCustomPersistence: StoryFn = () => { const [savedWidth, setSavedWidth] = React.useState(null) return ( -
-
- - Last saved width: {savedWidth !== null ? `${savedWidth}px` : 'Not saved yet'} - - - (Resize the pane to see the custom save function in action) - -
- - - - - { - // eslint-disable-next-line no-console - console.log(`Custom save: ${width}px with key "${widthStorageKey}"`) - setSavedWidth(width) - }, - }} - aria-label="Side pane" - > - - - - - - - - - -
+ + + + + { + // eslint-disable-next-line no-console + console.log(`Custom save: ${width}px with key "${widthStorageKey}"`) + setSavedWidth(width) + }, + }} + aria-label="Side pane" + > + + + + + + + + + ) } ResizablePaneWithCustomPersistence.storyName = 'Resizable pane with custom persistence' From 9691b593c067d9058a84d20a554849646017ff5e Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Tue, 16 Dec 2025 18:37:23 +0000 Subject: [PATCH 08/10] feat(PageLayout): add number support to width prop - Add PaneWidthValue type (PaneWidth | number | CustomWidthOptions) - When width is a number, use minWidth prop and viewport-based max - Export PaneWidthValue type and isNumericWidth helper - Add 'Resizable pane with number width' story - Update changeset documentation --- .../pagelayout-resizable-persistence.md | 40 +++++- .../PageLayout.features.stories.tsx | 85 +++++++++++-- packages/react/src/PageLayout/PageLayout.tsx | 18 ++- .../__snapshots__/PageLayout.test.tsx.snap | 120 +++++++++--------- packages/react/src/PageLayout/index.ts | 10 +- packages/react/src/PageLayout/usePaneWidth.ts | 26 +++- 6 files changed, 214 insertions(+), 85 deletions(-) diff --git a/.changeset/pagelayout-resizable-persistence.md b/.changeset/pagelayout-resizable-persistence.md index ffa991de540..2e8cb1f4e83 100644 --- a/.changeset/pagelayout-resizable-persistence.md +++ b/.changeset/pagelayout-resizable-persistence.md @@ -2,7 +2,7 @@ '@primer/react': minor --- -Add custom persistence options to PageLayout.Pane's `resizable` prop +Add custom persistence options to PageLayout.Pane's `resizable` prop and number support for `width` The `resizable` prop now accepts additional configuration options: @@ -11,11 +11,24 @@ The `resizable` prop now accepts additional configuration options: - `{persist: false}` - Enable resizing without any persistence (avoids hydration mismatches) - `{save: fn}` - Enable resizing with custom persistence (e.g., server-side, IndexedDB) +The `width` prop now accepts numbers in addition to named sizes and custom objects: + +- `'small' | 'medium' | 'large'` - Preset width names (existing behavior) +- `number` - Explicit pixel width (uses `minWidth` prop and viewport-based max) **NEW** +- `{min, default, max}` - Custom width configuration (existing behavior) + **New types exported:** + - `NoPersistConfig` - Type for `{persist: false}` configuration -- `CustomPersistConfig` - Type for `{save: fn}` configuration +- `CustomPersistConfig` - Type for `{save: fn}` configuration - `SaveOptions` - Options passed to custom save function (`{widthStorageKey: string}`) - `ResizableConfig` - Union type for all resizable configurations +- `PaneWidth` - Type for preset width names (`'small' | 'medium' | 'large'`) +- `PaneWidthValue` - Union type for all width values (`PaneWidth | number | CustomWidthOptions`) + +**New values exported:** + +- `defaultPaneWidth` - Record of preset width values (`{small: 256, medium: 296, large: 320}`) **Example usage:** @@ -24,12 +37,31 @@ The `resizable` prop now accepts additional configuration options: // Custom persistence - save to your own storage - { // Save to server, IndexedDB, sessionStorage, etc. myStorage.set(widthStorageKey, width) } - }} + }} /> + +// Number width - uses minWidth prop and viewport-based max constraints +const [savedWidth, setSavedWidth] = useState(defaultPaneWidth.medium) + setSavedWidth(width) + }} +/> + +// Using defaultPaneWidth for custom width configurations +import {defaultPaneWidth} from '@primer/react' + +const widthConfig = { + min: '256px', + default: `${defaultPaneWidth.medium}px`, + max: '600px' +} + ``` diff --git a/packages/react/src/PageLayout/PageLayout.features.stories.tsx b/packages/react/src/PageLayout/PageLayout.features.stories.tsx index a52a25824c8..03a6d297e4d 100644 --- a/packages/react/src/PageLayout/PageLayout.features.stories.tsx +++ b/packages/react/src/PageLayout/PageLayout.features.stories.tsx @@ -2,9 +2,11 @@ import type {Meta, StoryFn} from '@storybook/react-vite' import React from 'react' import {PageLayout} from './PageLayout' import {Placeholder} from '../Placeholder' -import {BranchName, Heading, Link, StateLabel, Text} from '..' +import {BranchName, Heading, Link, StateLabel, Text, useIsomorphicLayoutEffect} from '..' import TabNav from '../TabNav' import classes from './PageLayout.features.stories.module.css' +import type {CustomWidthOptions} from './usePaneWidth' +import {defaultPaneWidth} from './usePaneWidth' export default { title: 'Components/PageLayout/Features', @@ -379,27 +381,43 @@ export const ResizablePaneWithoutPersistence: StoryFn = () => ( ResizablePaneWithoutPersistence.storyName = 'Resizable pane without persistence' export const ResizablePaneWithCustomPersistence: StoryFn = () => { - const [savedWidth, setSavedWidth] = React.useState(null) + const key = 'page-layout-features-stories-custom-persistence-pane-width' + // Read initial width from localStorage (CSR only), falling back to medium preset + const getInitialWidth = (): CustomWidthOptions => { + const base: CustomWidthOptions = {min: '256px', default: `${defaultPaneWidth.medium}px`, max: '600px'} + if (typeof window !== 'undefined') { + const storedWidth = localStorage.getItem(key) + if (storedWidth !== null) { + const parsed = parseFloat(storedWidth) + if (!isNaN(parsed) && parsed > 0) { + return {...base, default: `${parsed}px`} + } + } + } + return base + } + + const [widthConfig, setWidthConfig] = React.useState(getInitialWidth) + useIsomorphicLayoutEffect(() => { + setWidthConfig(getInitialWidth()) + }, []) return ( { - // eslint-disable-next-line no-console - console.log(`Custom save: ${width}px with key "${widthStorageKey}"`) - setSavedWidth(width) + save: width => { + setWidthConfig(prev => ({...prev, default: `${width}px`})) + localStorage.setItem(key, width.toString()) }, }} aria-label="Side pane" > - + @@ -411,3 +429,50 @@ export const ResizablePaneWithCustomPersistence: StoryFn = () => { ) } ResizablePaneWithCustomPersistence.storyName = 'Resizable pane with custom persistence' + +export const ResizablePaneWithNumberWidth: StoryFn = () => { + const key = 'page-layout-features-stories-number-width' + + // Read initial width from localStorage (CSR only), falling back to medium preset + const getInitialWidth = (): number => { + if (typeof window !== 'undefined') { + const storedWidth = localStorage.getItem(key) + if (storedWidth !== null) { + const parsed = parseInt(storedWidth, 10) + if (!isNaN(parsed) && parsed > 0) { + return parsed + } + } + } + return defaultPaneWidth.medium + } + + const [width, setWidth] = React.useState(getInitialWidth) + + return ( + + + + + { + setWidth(newWidth) + localStorage.setItem(key, newWidth.toString()) + }, + }} + aria-label="Side pane" + > + + + + + + + + + + ) +} +ResizablePaneWithNumberWidth.storyName = 'Resizable pane with number width' diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index e856054de00..231e885e09a 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -16,9 +16,9 @@ import { updateAriaValues, isCustomWidthOptions, isPaneWidth, + isNumericWidth, ARROW_KEY_STEP, - type CustomWidthOptions, - type PaneWidth, + type PaneWidthValue, type ResizableConfig, } from './usePaneWidth' @@ -560,7 +560,13 @@ export type PageLayoutPaneProps = { positionWhenNarrow?: 'inherit' | keyof typeof panePositions 'aria-labelledby'?: string 'aria-label'?: string - width?: PaneWidth | CustomWidthOptions + /** + * The width of the pane. + * - Named sizes: `'small'` | `'medium'` | `'large'` + * - Number: explicit pixel width (uses `minWidth` prop and viewport-based max) + * - Custom object: `{min: string, default: string, max: string}` + */ + width?: PaneWidthValue minWidth?: number /** * Enable resizable pane behavior. @@ -732,7 +738,11 @@ const Pane = React.forwardRef renders condensed layout 1`] = `
Header
@@ -43,26 +43,26 @@ exports[`PageLayout > renders condensed layout 1`] = `
Pane
renders condensed layout 1`] = `