From b5599b294592b0dbc3f6c150ad1c332f26012cb9 Mon Sep 17 00:00:00 2001 From: lyzno1 <92089059+lyzno1@users.noreply.github.com> Date: Tue, 22 Jul 2025 08:11:01 +0800 Subject: [PATCH] fix: prevent panel width localStorage pollution during viewport compression (#22745) (#22747) --- .../components/workflow-panel/index.spec.tsx | 178 ++++++++++++++++++ .../_base/components/workflow-panel/index.tsx | 16 +- .../panel/debug-and-preview/index.spec.tsx | 145 ++++++++++++++ .../panel/debug-and-preview/index.tsx | 9 +- 4 files changed, 340 insertions(+), 8 deletions(-) create mode 100644 web/app/components/workflow/nodes/_base/components/workflow-panel/index.spec.tsx create mode 100644 web/app/components/workflow/panel/debug-and-preview/index.spec.tsx diff --git a/web/app/components/workflow/nodes/_base/components/workflow-panel/index.spec.tsx b/web/app/components/workflow/nodes/_base/components/workflow-panel/index.spec.tsx new file mode 100644 index 0000000000..5c6ffb7a52 --- /dev/null +++ b/web/app/components/workflow/nodes/_base/components/workflow-panel/index.spec.tsx @@ -0,0 +1,178 @@ +/** + * Workflow Panel Width Persistence Tests + * Tests for GitHub issue #22745: Panel width persistence bug fix + */ + +import '@testing-library/jest-dom' + +type PanelWidthSource = 'user' | 'system' + +// Mock localStorage for testing +const createMockLocalStorage = () => { + const storage: Record = {} + return { + getItem: jest.fn((key: string) => storage[key] || null), + setItem: jest.fn((key: string, value: string) => { + storage[key] = value + }), + removeItem: jest.fn((key: string) => { + delete storage[key] + }), + clear: jest.fn(() => { + Object.keys(storage).forEach(key => delete storage[key]) + }), + get storage() { return { ...storage } }, + } +} + +// Core panel width logic extracted from the component +const createPanelWidthManager = (storageKey: string) => { + return { + updateWidth: (width: number, source: PanelWidthSource = 'user') => { + const newValue = Math.max(400, Math.min(width, 800)) + if (source === 'user') + localStorage.setItem(storageKey, `${newValue}`) + + return newValue + }, + getStoredWidth: () => { + const stored = localStorage.getItem(storageKey) + return stored ? Number.parseFloat(stored) : 400 + }, + } +} + +describe('Workflow Panel Width Persistence', () => { + let mockLocalStorage: ReturnType + + beforeEach(() => { + mockLocalStorage = createMockLocalStorage() + Object.defineProperty(globalThis, 'localStorage', { + value: mockLocalStorage, + writable: true, + }) + }) + + afterEach(() => { + jest.clearAllMocks() + }) + + describe('Node Panel Width Management', () => { + const storageKey = 'workflow-node-panel-width' + + it('should save user resize to localStorage', () => { + const manager = createPanelWidthManager(storageKey) + + const result = manager.updateWidth(500, 'user') + + expect(result).toBe(500) + expect(localStorage.setItem).toHaveBeenCalledWith(storageKey, '500') + }) + + it('should not save system compression to localStorage', () => { + const manager = createPanelWidthManager(storageKey) + + const result = manager.updateWidth(200, 'system') + + expect(result).toBe(400) // Respects minimum width + expect(localStorage.setItem).not.toHaveBeenCalled() + }) + + it('should enforce minimum width of 400px', () => { + const manager = createPanelWidthManager(storageKey) + + // User tries to set below minimum + const userResult = manager.updateWidth(300, 'user') + expect(userResult).toBe(400) + expect(localStorage.setItem).toHaveBeenCalledWith(storageKey, '400') + + // System compression below minimum + const systemResult = manager.updateWidth(150, 'system') + expect(systemResult).toBe(400) + expect(localStorage.setItem).toHaveBeenCalledTimes(1) // Only user call + }) + + it('should preserve user preferences during system compression', () => { + localStorage.setItem(storageKey, '600') + const manager = createPanelWidthManager(storageKey) + + // System compresses panel + manager.updateWidth(200, 'system') + + // User preference should remain unchanged + expect(localStorage.getItem(storageKey)).toBe('600') + }) + }) + + describe('Bug Scenario Reproduction', () => { + it('should reproduce original bug behavior (for comparison)', () => { + const storageKey = 'workflow-node-panel-width' + + // Original buggy behavior - always saves regardless of source + const buggyUpdate = (width: number) => { + localStorage.setItem(storageKey, `${width}`) + return Math.max(400, width) + } + + localStorage.setItem(storageKey, '500') // User preference + buggyUpdate(200) // System compression pollutes localStorage + + expect(localStorage.getItem(storageKey)).toBe('200') // Bug: corrupted state + }) + + it('should verify fix prevents localStorage pollution', () => { + const storageKey = 'workflow-node-panel-width' + const manager = createPanelWidthManager(storageKey) + + localStorage.setItem(storageKey, '500') // User preference + manager.updateWidth(200, 'system') // System compression + + expect(localStorage.getItem(storageKey)).toBe('500') // Fix: preserved state + }) + }) + + describe('Edge Cases', () => { + it('should handle multiple rapid operations correctly', () => { + const manager = createPanelWidthManager('workflow-node-panel-width') + + // Rapid system adjustments + manager.updateWidth(300, 'system') + manager.updateWidth(250, 'system') + manager.updateWidth(180, 'system') + + // Single user adjustment + manager.updateWidth(550, 'user') + + expect(localStorage.setItem).toHaveBeenCalledTimes(1) + expect(localStorage.setItem).toHaveBeenCalledWith('workflow-node-panel-width', '550') + }) + + it('should handle corrupted localStorage gracefully', () => { + localStorage.setItem('workflow-node-panel-width', '150') // Below minimum + const manager = createPanelWidthManager('workflow-node-panel-width') + + const storedWidth = manager.getStoredWidth() + expect(storedWidth).toBe(150) // Returns raw value + + // User can correct the preference + const correctedWidth = manager.updateWidth(500, 'user') + expect(correctedWidth).toBe(500) + expect(localStorage.getItem('workflow-node-panel-width')).toBe('500') + }) + }) + + describe('TypeScript Type Safety', () => { + it('should enforce source parameter type', () => { + const manager = createPanelWidthManager('workflow-node-panel-width') + + // Valid source values + manager.updateWidth(500, 'user') + manager.updateWidth(500, 'system') + + // Default to 'user' + manager.updateWidth(500) + + expect(localStorage.setItem).toHaveBeenCalledTimes(2) // user + default + }) + }) +}) diff --git a/web/app/components/workflow/nodes/_base/components/workflow-panel/index.tsx b/web/app/components/workflow/nodes/_base/components/workflow-panel/index.tsx index d50b99e213..93fab83172 100644 --- a/web/app/components/workflow/nodes/_base/components/workflow-panel/index.tsx +++ b/web/app/components/workflow/nodes/_base/components/workflow-panel/index.tsx @@ -99,15 +99,18 @@ const BasePanel: FC = ({ return Math.max(available, 400) }, [workflowCanvasWidth, otherPanelWidth]) - const updateNodePanelWidth = useCallback((width: number) => { + const updateNodePanelWidth = useCallback((width: number, source: 'user' | 'system' = 'user') => { // Ensure the width is within the min and max range const newValue = Math.max(400, Math.min(width, maxNodePanelWidth)) - localStorage.setItem('workflow-node-panel-width', `${newValue}`) + + if (source === 'user') + localStorage.setItem('workflow-node-panel-width', `${newValue}`) + setNodePanelWidth(newValue) }, [maxNodePanelWidth, setNodePanelWidth]) const handleResize = useCallback((width: number) => { - updateNodePanelWidth(width) + updateNodePanelWidth(width, 'user') }, [updateNodePanelWidth]) const { @@ -121,7 +124,10 @@ const BasePanel: FC = ({ onResize: debounce(handleResize), }) - const debounceUpdate = debounce(updateNodePanelWidth) + const debounceUpdate = debounce((width: number) => { + updateNodePanelWidth(width, 'system') + }) + useEffect(() => { if (!workflowCanvasWidth) return @@ -132,7 +138,7 @@ const BasePanel: FC = ({ const target = Math.max(workflowCanvasWidth - otherPanelWidth - reservedCanvasWidth, 400) debounceUpdate(target) } - }, [nodePanelWidth, otherPanelWidth, workflowCanvasWidth, updateNodePanelWidth]) + }, [nodePanelWidth, otherPanelWidth, workflowCanvasWidth, debounceUpdate]) const { handleNodeSelect } = useNodesInteractions() const { nodesReadOnly } = useNodesReadOnly() diff --git a/web/app/components/workflow/panel/debug-and-preview/index.spec.tsx b/web/app/components/workflow/panel/debug-and-preview/index.spec.tsx new file mode 100644 index 0000000000..1ac70d1ab3 --- /dev/null +++ b/web/app/components/workflow/panel/debug-and-preview/index.spec.tsx @@ -0,0 +1,145 @@ +/** + * Debug and Preview Panel Width Persistence Tests + * Tests for GitHub issue #22745: Panel width persistence bug fix + */ + +import '@testing-library/jest-dom' + +type PanelWidthSource = 'user' | 'system' + +// Mock localStorage for testing +const createMockLocalStorage = () => { + const storage: Record = {} + return { + getItem: jest.fn((key: string) => storage[key] || null), + setItem: jest.fn((key: string, value: string) => { + storage[key] = value + }), + removeItem: jest.fn((key: string) => { + delete storage[key] + }), + clear: jest.fn(() => { + Object.keys(storage).forEach(key => delete storage[key]) + }), + get storage() { return { ...storage } }, + } +} + +// Preview panel width logic +const createPreviewPanelManager = () => { + const storageKey = 'debug-and-preview-panel-width' + + return { + updateWidth: (width: number, source: PanelWidthSource = 'user') => { + const newValue = Math.max(400, Math.min(width, 800)) + if (source === 'user') + localStorage.setItem(storageKey, `${newValue}`) + + return newValue + }, + getStoredWidth: () => { + const stored = localStorage.getItem(storageKey) + return stored ? Number.parseFloat(stored) : 400 + }, + } +} + +describe('Debug and Preview Panel Width Persistence', () => { + let mockLocalStorage: ReturnType + + beforeEach(() => { + mockLocalStorage = createMockLocalStorage() + Object.defineProperty(globalThis, 'localStorage', { + value: mockLocalStorage, + writable: true, + }) + }) + + afterEach(() => { + jest.clearAllMocks() + }) + + describe('Preview Panel Width Management', () => { + it('should save user resize to localStorage', () => { + const manager = createPreviewPanelManager() + + const result = manager.updateWidth(450, 'user') + + expect(result).toBe(450) + expect(localStorage.setItem).toHaveBeenCalledWith('debug-and-preview-panel-width', '450') + }) + + it('should not save system compression to localStorage', () => { + const manager = createPreviewPanelManager() + + const result = manager.updateWidth(300, 'system') + + expect(result).toBe(400) // Respects minimum width + expect(localStorage.setItem).not.toHaveBeenCalled() + }) + + it('should behave identically to Node Panel', () => { + const manager = createPreviewPanelManager() + + // Both user and system operations should behave consistently + manager.updateWidth(500, 'user') + expect(localStorage.setItem).toHaveBeenCalledWith('debug-and-preview-panel-width', '500') + + manager.updateWidth(200, 'system') + expect(localStorage.getItem('debug-and-preview-panel-width')).toBe('500') + }) + }) + + describe('Dual Panel Scenario', () => { + it('should maintain independence from Node Panel', () => { + localStorage.setItem('workflow-node-panel-width', '600') + localStorage.setItem('debug-and-preview-panel-width', '450') + + const manager = createPreviewPanelManager() + + // System compresses preview panel + manager.updateWidth(200, 'system') + + // Only preview panel storage key should be unaffected + expect(localStorage.getItem('debug-and-preview-panel-width')).toBe('450') + expect(localStorage.getItem('workflow-node-panel-width')).toBe('600') + }) + + it('should handle F12 scenario consistently', () => { + const manager = createPreviewPanelManager() + + // User sets preference + manager.updateWidth(500, 'user') + expect(localStorage.getItem('debug-and-preview-panel-width')).toBe('500') + + // F12 opens causing viewport compression + manager.updateWidth(180, 'system') + + // User preference preserved + expect(localStorage.getItem('debug-and-preview-panel-width')).toBe('500') + }) + }) + + describe('Consistency with Node Panel', () => { + it('should enforce same minimum width rules', () => { + const manager = createPreviewPanelManager() + + // Same 400px minimum as Node Panel + const result = manager.updateWidth(300, 'user') + expect(result).toBe(400) + expect(localStorage.setItem).toHaveBeenCalledWith('debug-and-preview-panel-width', '400') + }) + + it('should use same source parameter pattern', () => { + const manager = createPreviewPanelManager() + + // Default to 'user' when source not specified + manager.updateWidth(500) + expect(localStorage.setItem).toHaveBeenCalledWith('debug-and-preview-panel-width', '500') + + // Explicit 'system' source + manager.updateWidth(300, 'system') + expect(localStorage.setItem).toHaveBeenCalledTimes(1) // Only user call + }) + }) +}) diff --git a/web/app/components/workflow/panel/debug-and-preview/index.tsx b/web/app/components/workflow/panel/debug-and-preview/index.tsx index ff09f48625..baf4c21dcd 100644 --- a/web/app/components/workflow/panel/debug-and-preview/index.tsx +++ b/web/app/components/workflow/panel/debug-and-preview/index.tsx @@ -53,8 +53,9 @@ const DebugAndPreview = () => { const nodePanelWidth = useStore(s => s.nodePanelWidth) const panelWidth = useStore(s => s.previewPanelWidth) const setPanelWidth = useStore(s => s.setPreviewPanelWidth) - const handleResize = useCallback((width: number) => { - localStorage.setItem('debug-and-preview-panel-width', `${width}`) + const handleResize = useCallback((width: number, source: 'user' | 'system' = 'user') => { + if (source === 'user') + localStorage.setItem('debug-and-preview-panel-width', `${width}`) setPanelWidth(width) }, [setPanelWidth]) const maxPanelWidth = useMemo(() => { @@ -74,7 +75,9 @@ const DebugAndPreview = () => { triggerDirection: 'left', minWidth: 400, maxWidth: maxPanelWidth, - onResize: debounce(handleResize), + onResize: debounce((width: number) => { + handleResize(width, 'user') + }), }) return (