Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 3, 2026

Refines the PageLayout.Pane resizable persistence API with clearer configuration types and simplified custom persistence functions based on design review feedback.

Changes

Type Structure

  • Split PersistConfig into three distinct types:
    • NoPersistConfig: {persist: false} - SSR-safe, no width control
    • LocalStoragePersistConfig: {persist: 'localStorage', widthStorageKey: string, width: number | undefined} - explicit storage key in config
    • CustomPersistConfig: {persist: fn, width: number | undefined} - consumer manages storage
  • width is required (can be undefined) for persistence configs to ensure intentional state management
  • Added type guards: isNoPersistConfig, isLocalStoragePersistConfig, isCustomPersistConfig

Custom Persist Functions

  • Simplified signature: (width: number) => void | Promise<void>
  • No longer receives widthStorageKey - consumers manage their own storage keys
  • SaveOptions type is now empty

Deprecations

  • widthStorageKey prop marked @deprecated - use config object instead
  • resizable={true} implicitly deprecated - use explicit config
  • Backwards compatibility maintained

Migration Examples

Before (deprecated):

<PageLayout.Pane resizable={true} widthStorageKey="my-pane" />

<PageLayout.Pane
  resizable={{
    persist: (width, {widthStorageKey}) => storage.set(widthStorageKey, width)
  }}
  widthStorageKey="my-key"
/>

After:

<PageLayout.Pane 
  resizable={{
    persist: 'localStorage',
    widthStorageKey: 'my-pane',
    width: undefined
  }} 
/>

<PageLayout.Pane
  resizable={{
    persist: (width) => storage.set('my-key', width),
    width: currentWidth
  }}
/>

SSR-safe (no persistence):

<PageLayout.Pane resizable={{persist: false}} />

Changelog

New

  • NoPersistConfig, LocalStoragePersistConfig, CustomPersistConfig types
  • Type guards for each config type
  • Migration examples story in Storybook

Changed

  • PersistFunction signature: no longer receives options parameter
  • widthStorageKey moved into LocalStoragePersistConfig from prop-only
  • width now required (can be undefined) for persistence configs
  • SaveOptions is now an empty object

Removed

  • SaveOptions from public exports (internal only)

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Minor release with deprecation warnings. Breaking changes deferred to next major version. Backwards compatibility maintained via fallback to widthStorageKey prop when resizable={true}.

Testing & Reviewing

  • 86 unit tests covering all new types and migration paths
  • Type guards validated for all config variants
  • Backwards compatibility with resizable={true} tested
  • Storybook examples demonstrate migration patterns

Merge checklist

Original prompt

Summary

Refine the PageLayout.Pane resizable persistence API based on design review feedback. This builds on the existing work in the make-primer-react-pagelayout-accept-persister branch.

API Changes

New Type Definitions

Update the types in packages/react/src/PageLayout/usePaneWidth.ts:

/**
 * Options passed to custom persist function.
 * Note: widthStorageKey is no longer passed - custom persisters manage their own storage.
 */
export type SaveOptions = {}

/**
 * Custom persist function type.
 * Receives the width value; consumer manages their own storage key/mechanism.
 */
export type PersistFunction = (width: number) => void | Promise<void>

/**
 * No persistence - SSR safe, uses default width from width prop.
 * width is not allowed here - always uses default.
 */
export type NoPersistConfig = {
  persist: false
}

/**
 * localStorage persistence - requires explicit storage key.
 * width is REQUIRED (can be undefined to use default).
 */
export type LocalStoragePersistConfig = {
  persist: 'localStorage'
  /** Storage key for localStorage. Required to avoid key collisions. */
  widthStorageKey: string
  /** Current controlled width value. Required - use undefined for "use default". */
  width: number | undefined
}

/**
 * Custom persistence - consumer handles storage.
 * width is REQUIRED (can be undefined to use default).
 */
export type CustomPersistConfig = {
  persist: PersistFunction
  /** Current controlled width value. Required - use undefined for "use default". */
  width: number | undefined
}

export type PersistConfig = NoPersistConfig | LocalStoragePersistConfig | CustomPersistConfig

/**
 * Resizable configuration options.
 * - `true`: Enable resizing with default localStorage persistence (DEPRECATED - use {persist: 'localStorage', widthStorageKey, width} instead)
 * - `false`: Disable resizing
 * - `{persist: false}`: Enable resizing without persistence (SSR-safe)
 * - `{persist: 'localStorage', widthStorageKey, width}`: Enable with localStorage, width required
 * - `{persist: fn, width}`: Enable with custom persistence, width required
 */
export type ResizableConfig = boolean | PersistConfig

Usage Examples

// Disable resizing
resizable={false}

// No persistence - SSR safe, simple
resizable={{persist: false}}

// localStorage - widthStorageKey required, width required (undefined = use default)
resizable={{persist: 'localStorage', widthStorageKey: 'my-pane-width', width: undefined}}
resizable={{persist: 'localStorage', widthStorageKey: 'my-pane-width', width: currentWidth}}

// Custom persistence - width required (undefined = use default)
resizable={{persist: (width) => saveToServer(width), width: undefined}}
resizable={{persist: (width) => setCurrentWidth(width), width: currentWidth}}

// DEPRECATED: Legacy boolean true still works with widthStorageKey prop
<PageLayout.Pane resizable={true} widthStorageKey="my-pane" />

Deprecation Strategy

  1. Deprecate widthStorageKey prop on PageLayout.Pane:

    • Add @deprecated JSDoc annotation
    • Implementation should prefer resizable.widthStorageKey (for localStorage config), falling back to the prop for backwards compatibility with resizable={true}
  2. resizable={true} is implicitly deprecated:

    • Still works for backwards compatibility
    • Uses widthStorageKey prop (with default fallback)
    • Document migration path to {persist: 'localStorage', widthStorageKey: '...', width: undefined}

Implementation Changes

  1. Update usePaneWidth.ts:

    • Update type definitions as shown above
    • Add type guards: isLocalStoragePersistConfig, isCustomPersistConfig, isNoPersistConfig
    • Update saveWidth to not pass widthStorageKey to custom persist functions
    • Handle widthStorageKey from config for localStorage persist
    • For resizable === true, fall back to widthStorageKey from hook options (prop)
  2. Update PageLayout.tsx:

    • Add @deprecated to widthStorageKey prop JSDoc
    • Pass widthStorageKey prop to hook for backwards compatibility
  3. Update PageLayout/index.ts:

    • Export new types: NoPersistConfig, LocalStoragePersistConfig, CustomPersistConfig
    • Remove SaveOptions export (no longer needed externally)
  4. Update .changeset/pagelayout-resizable-persistence.md:

    • Document the refined API
    • Include migration examples
    • Note deprecations
  5. Update usePaneWidth.test.ts:

    • Add tests for new type guards
    • Add tests for localStorage config with widthStorageKey
    • Add tests for custom persist without widthStorageKey in callback
    • Add tests for backwards compatibility with resizable={true} + widthStorageKey prop
  6. Update PageLayout.features.stories.tsx:

    • Update stories to use new API
    • Show migration examples

Rationale

  1. width required for non-false persist: When consumers opt into persistence (localStorage or custom), they should be intentional abo...

This pull request was created from Copilot chat.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@changeset-bot
Copy link

changeset-bot bot commented Jan 3, 2026

⚠️ No Changeset found

Latest commit: 820ba2a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes changesets to release 1 package
Name Type
@primer/react Minor

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copilot AI changed the title [WIP] Refine PageLayout.Pane resizable persistence API based on feedback Refine PageLayout.Pane resizable persistence API Jan 3, 2026
Copilot AI requested a review from mattcosta7 January 3, 2026 23:33
@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Jan 6, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2026

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the integration-tests: skipped manually label to skip these checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants