Skip to content

Commit 28e77e6

Browse files
Copilotmattcosta7
andauthored
Simplify resizable API: use persist: false | 'localStorage' | fn (#7395)
Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: mattcosta7 <[email protected]> Co-authored-by: Matthew Costabile <[email protected]>
1 parent 547b87e commit 28e77e6

File tree

4 files changed

+91
-60
lines changed

4 files changed

+91
-60
lines changed

packages/react/src/PageLayout/PageLayout.features.stories.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ export const ResizablePaneWithCustomPersistence: StoryFn = () => {
410410
<PageLayout.Pane
411411
width={widthConfig}
412412
resizable={{
413-
save: width => {
413+
persist: width => {
414414
setWidthConfig(prev => ({...prev, default: `${width}px`}))
415415
localStorage.setItem(key, width.toString())
416416
},
@@ -457,7 +457,7 @@ export const ResizablePaneWithNumberWidth: StoryFn = () => {
457457
<PageLayout.Pane
458458
width={width}
459459
resizable={{
460-
save: newWidth => {
460+
persist: newWidth => {
461461
setWidth(newWidth)
462462
localStorage.setItem(key, newWidth.toString())
463463
},

packages/react/src/PageLayout/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
export * from './PageLayout'
22
export type {
3-
NoPersistConfig,
4-
CustomPersistConfig,
3+
PersistConfig,
4+
PersistFunction,
55
SaveOptions,
66
ResizableConfig,
77
PaneWidth,

packages/react/src/PageLayout/usePaneWidth.test.ts

Lines changed: 60 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ import {
1212
SSR_DEFAULT_MAX_WIDTH,
1313
ARROW_KEY_STEP,
1414
isResizableEnabled,
15-
isNoPersistConfig,
16-
isCustomPersistConfig,
17-
type CustomPersistConfig,
15+
isPersistConfig,
16+
isCustomPersistFunction,
17+
type PersistConfig,
18+
type PersistFunction,
1819
} from './usePaneWidth'
1920

2021
// Mock refs for hook testing
@@ -227,9 +228,29 @@ describe('usePaneWidth', () => {
227228
localStorage.setItem = originalSetItem
228229
})
229230

231+
it('should use localStorage when {persist: "localStorage"} is provided', () => {
232+
const refs = createMockRefs()
233+
const {result} = renderHook(() =>
234+
usePaneWidth({
235+
width: 'medium',
236+
minWidth: 256,
237+
resizable: {persist: 'localStorage'},
238+
widthStorageKey: 'test-explicit-localstorage',
239+
...refs,
240+
}),
241+
)
242+
243+
act(() => {
244+
result.current.saveWidth(450)
245+
})
246+
247+
expect(result.current.currentWidth).toBe(450)
248+
expect(localStorage.getItem('test-explicit-localstorage')).toBe('450')
249+
})
250+
230251
it('should call custom save function with width and options', () => {
231252
const customSave = vi.fn()
232-
const customPersister: CustomPersistConfig = {save: customSave}
253+
const customPersister: PersistConfig = {persist: customSave}
233254
const refs = createMockRefs()
234255

235256
const {result} = renderHook(() =>
@@ -254,7 +275,7 @@ describe('usePaneWidth', () => {
254275

255276
it('should handle async custom save function', async () => {
256277
const customSave = vi.fn().mockResolvedValue(undefined)
257-
const customPersister: CustomPersistConfig = {save: customSave}
278+
const customPersister: PersistConfig = {persist: customSave}
258279
const refs = createMockRefs()
259280

260281
const {result} = renderHook(() =>
@@ -279,7 +300,7 @@ describe('usePaneWidth', () => {
279300
const customSave = vi.fn(() => {
280301
throw new Error('Sync storage error')
281302
})
282-
const customPersister: CustomPersistConfig = {save: customSave}
303+
const customPersister: PersistConfig = {persist: customSave}
283304
const refs = createMockRefs()
284305

285306
const {result} = renderHook(() =>
@@ -303,7 +324,7 @@ describe('usePaneWidth', () => {
303324

304325
it('should handle async rejection from custom save gracefully', async () => {
305326
const customSave = vi.fn().mockRejectedValue(new Error('Async storage error'))
306-
const customPersister: CustomPersistConfig = {save: customSave}
327+
const customPersister: PersistConfig = {persist: customSave}
307328
const refs = createMockRefs()
308329

309330
const {result} = renderHook(() =>
@@ -331,7 +352,7 @@ describe('usePaneWidth', () => {
331352

332353
it('should not read from localStorage when custom save is provided', () => {
333354
localStorage.setItem('test-pane', '500')
334-
const customPersister: CustomPersistConfig = {save: vi.fn()}
355+
const customPersister: PersistConfig = {persist: vi.fn() as PersistFunction}
335356
const refs = createMockRefs()
336357

337358
const {result} = renderHook(() =>
@@ -999,52 +1020,59 @@ describe('type guards', () => {
9991020
expect(isResizableEnabled({persist: false})).toBe(true)
10001021
})
10011022

1002-
it('should return true for {save: fn} (custom persistence)', () => {
1003-
expect(isResizableEnabled({save: () => {}})).toBe(true)
1023+
it('should return true for {persist: "localStorage"}', () => {
1024+
expect(isResizableEnabled({persist: 'localStorage'})).toBe(true)
1025+
})
1026+
1027+
it('should return true for {persist: fn} (custom persistence)', () => {
1028+
expect(isResizableEnabled({persist: () => {}})).toBe(true)
10041029
})
10051030
})
10061031

1007-
describe('isNoPersistConfig', () => {
1032+
describe('isPersistConfig', () => {
10081033
it('should return true for {persist: false}', () => {
1009-
expect(isNoPersistConfig({persist: false})).toBe(true)
1034+
expect(isPersistConfig({persist: false})).toBe(true)
1035+
})
1036+
1037+
it('should return true for {persist: "localStorage"}', () => {
1038+
expect(isPersistConfig({persist: 'localStorage'})).toBe(true)
1039+
})
1040+
1041+
it('should return true for {persist: fn}', () => {
1042+
expect(isPersistConfig({persist: () => {}})).toBe(true)
10101043
})
10111044

10121045
it('should return false for boolean true', () => {
1013-
expect(isNoPersistConfig(true)).toBe(false)
1046+
expect(isPersistConfig(true)).toBe(false)
10141047
})
10151048

10161049
it('should return false for boolean false', () => {
1017-
expect(isNoPersistConfig(false)).toBe(false)
1050+
expect(isPersistConfig(false)).toBe(false)
10181051
})
10191052

1020-
it('should return false for objects without persist: false', () => {
1053+
it('should return false for objects without persist property', () => {
10211054
// @ts-expect-error - testing runtime behavior with arbitrary object
1022-
expect(isNoPersistConfig({other: 'value'})).toBe(false)
1023-
})
1024-
1025-
it('should return false for custom persist config', () => {
1026-
expect(isNoPersistConfig({save: () => {}})).toBe(false)
1055+
expect(isPersistConfig({other: 'value'})).toBe(false)
10271056
})
10281057
})
10291058

1030-
describe('isCustomPersistConfig', () => {
1031-
it('should return true for objects with save function', () => {
1032-
expect(isCustomPersistConfig({save: () => {}})).toBe(true)
1033-
expect(isCustomPersistConfig({save: async () => {}})).toBe(true)
1059+
describe('isCustomPersistFunction', () => {
1060+
it('should return true for function', () => {
1061+
const fn = () => {}
1062+
expect(isCustomPersistFunction(fn)).toBe(true)
10341063
})
10351064

1036-
it('should return false for boolean values', () => {
1037-
expect(isCustomPersistConfig(true)).toBe(false)
1038-
expect(isCustomPersistConfig(false)).toBe(false)
1065+
it('should return true for async function', () => {
1066+
const fn = async () => {}
1067+
expect(isCustomPersistFunction(fn)).toBe(true)
10391068
})
10401069

1041-
it('should return false for {persist: false}', () => {
1042-
expect(isCustomPersistConfig({persist: false})).toBe(false)
1070+
it('should return false for false', () => {
1071+
expect(isCustomPersistFunction(false)).toBe(false)
10431072
})
10441073

1045-
it('should return false for null', () => {
1046-
// @ts-expect-error - testing runtime behavior
1047-
expect(isCustomPersistConfig(null)).toBe(false)
1074+
it('should return false for "localStorage"', () => {
1075+
expect(isCustomPersistFunction('localStorage')).toBe(false)
10481076
})
10491077
})
10501078
})

packages/react/src/PageLayout/usePaneWidth.ts

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,40 +24,43 @@ export type PaneWidth = 'small' | 'medium' | 'large'
2424
export type PaneWidthValue = PaneWidth | number | CustomWidthOptions
2525

2626
/**
27-
* Configuration for resizable without persistence.
28-
* Use this to enable resizing without storing the width anywhere.
27+
* Options passed to custom persist function.
2928
*/
30-
export type NoPersistConfig = {persist: false}
29+
export type SaveOptions = {widthStorageKey: string}
3130

3231
/**
33-
* Options passed to custom save function.
32+
* Custom persist function type.
3433
*/
35-
export type SaveOptions = {widthStorageKey: string}
34+
export type PersistFunction = (width: number, options: SaveOptions) => void | Promise<void>
3635

3736
/**
38-
* Configuration for custom persistence.
39-
* Provide your own save function to persist width to server, IndexedDB, etc.
37+
* Configuration object for resizable pane.
38+
* - `persist: false` - Enable resizing without any persistence
39+
* - `persist: 'localStorage'` - Enable resizing with localStorage persistence
40+
* - `persist: fn` - Enable resizing with custom persistence function
4041
*/
41-
export type CustomPersistConfig = {
42-
save: (width: number, options: SaveOptions) => void | Promise<void>
42+
export type PersistConfig = {
43+
persist: false | 'localStorage' | PersistFunction
4344
}
4445

4546
/**
46-
* Type guard to check if resizable config has a custom save function
47+
* Type guard to check if persist value is a custom function
4748
*/
48-
export const isCustomPersistConfig = (config: ResizableConfig): config is CustomPersistConfig => {
49-
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- config could be null at runtime despite types
50-
return typeof config === 'object' && config !== null && 'save' in config && typeof config.save === 'function'
49+
export const isCustomPersistFunction = (
50+
persist: false | 'localStorage' | PersistFunction,
51+
): persist is PersistFunction => {
52+
return typeof persist === 'function'
5153
}
5254

5355
/**
5456
* Resizable configuration options.
5557
* - `true`: Enable resizing with default localStorage persistence (may cause hydration mismatch)
5658
* - `false`: Disable resizing
5759
* - `{persist: false}`: Enable resizing without any persistence
58-
* - `{save: fn}`: Enable resizing with custom persistence
60+
* - `{persist: 'localStorage'}`: Enable resizing with localStorage persistence
61+
* - `{persist: fn}`: Enable resizing with custom persistence function
5962
*/
60-
export type ResizableConfig = boolean | NoPersistConfig | CustomPersistConfig
63+
export type ResizableConfig = boolean | PersistConfig
6164

6265
export type UsePaneWidthOptions = {
6366
width: PaneWidthValue
@@ -138,18 +141,18 @@ export const getDefaultPaneWidth = (w: PaneWidthValue): number => {
138141
}
139142

140143
/**
141-
* Type guard to check if resizable config is {persist: false}
144+
* Type guard to check if resizable config is a PersistConfig object
142145
*/
143-
export const isNoPersistConfig = (config: ResizableConfig): config is NoPersistConfig => {
146+
export const isPersistConfig = (config: ResizableConfig): config is PersistConfig => {
144147
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- config could be null at runtime despite types
145-
return typeof config === 'object' && config !== null && 'persist' in config && config.persist === false
148+
return typeof config === 'object' && config !== null && 'persist' in config
146149
}
147150

148151
/**
149-
* Check if resizing is enabled (boolean true, {persist: false}, or {save: fn})
152+
* Check if resizing is enabled (boolean true or {persist: ...})
150153
*/
151154
export const isResizableEnabled = (config: ResizableConfig): boolean => {
152-
return config === true || isNoPersistConfig(config) || isCustomPersistConfig(config)
155+
return config === true || isPersistConfig(config)
153156
}
154157

155158
/**
@@ -294,12 +297,12 @@ export function usePaneWidth({
294297

295298
const config = resizableRef.current
296299

297-
// Handle localStorage persistence: resizable === true
298-
if (config === true) {
300+
// Handle localStorage persistence: resizable === true or {persist: 'localStorage'}
301+
if (config === true || (isPersistConfig(config) && config.persist === 'localStorage')) {
299302
localStoragePersister.save(widthStorageKeyRef.current, value)
300-
} else if (isCustomPersistConfig(config)) {
303+
} else if (isPersistConfig(config) && isCustomPersistFunction(config.persist)) {
301304
try {
302-
const result = config.save(value, {widthStorageKey: widthStorageKeyRef.current})
305+
const result = config.persist(value, {widthStorageKey: widthStorageKeyRef.current})
303306
// Handle async rejections silently
304307
if (result instanceof Promise) {
305308
// eslint-disable-next-line github/no-then

0 commit comments

Comments
 (0)