From 9146dc1c570684e5ea6f46498596ec830129e4d7 Mon Sep 17 00:00:00 2001 From: Patrick A <141967+neogenix@users.noreply.github.com> Date: Fri, 29 May 2026 23:39:27 -0400 Subject: [PATCH] fix(web): persist design files view state across navigation (#2303) * fix(web): persist design-files view state across navigation pageSize, sortKey, sortDir, and kindFilter reset on every navigation because DesignFilesPanel remounts via key={projectId}. Persist them to localStorage under od:design-files:view-state:v1: so each project's view prefs survive tab-switching. - Read persisted state via lazy useState initializers (SSR-safe try/catch) - Write back in a single useEffect keyed on all four values - Scoped per-project so proj-a settings never bleed into proj-b - Schema-guarded: invalid/missing fields fall through to defaults - Red spec: apps/web/tests/components/DesignFilesPanel.view-state-persist.test.tsx * fix(web): address review feedback on view-state persistence - Add typeof window guard in readViewState for explicit SSR safety - Consolidate 4 separate localStorage reads into a single useRef read at mount time; each lazy useState initializer now reads from savedViewState.current instead of re-parsing localStorage independently * fix(web): harden design-files view-state persistence - Validate restored kindFilter values against the current ProjectFileKind union via isProjectFileKind() so stale stored values from a prior schema are dropped silently instead of being cast unchecked. - Introduce DEFAULT_SORT_KEY/SORT_DIR/PAGE_SIZE constants so the useState initialisers and the new validation guard share a single source of truth. - Add viewStateHasMounted ref to skip the first-render write in the persist useEffect. Without this guard every project the user visits accumulates a default-value entry in localStorage on mount, growing stale-key garbage unboundedly and making future field additions silently inject defaults into every existing entry. - Harden kindFilter test: replace the silent early-return-on-missing-trigger with expect(filterTrigger).not.toBeNull() so a render failure surfaces as a real test failure rather than a passing no-op. * test(e2e): design files view state persists across navigation and reload Adds a Playwright UI smoke test in e2e/ui/ that exercises the three key guarantees of the view-state persistence fix: (a) Tab-away / tab-back: navigating to a file tab and returning remounts DesignFilesPanel (conditionally rendered); all four prefs (sortKey, sortDir, pageSize, kindFilter) are restored from localStorage. (b) Hard reload: localStorage survives page.reload(); prefs are intact on the next mount. (c) Per-project key isolation: a second project starts with defaults and does not inherit values from the first project's localStorage entry. The test uses OD_PORT=18011 / OD_WEB_PORT=18012 to avoid port conflicts with the default development ports. Also fixes a race in DesignFilesPanel: the stale-kind cleanup useEffect was running against an empty availableKinds set before the async file list arrived on mount, which cleared a kindFilter correctly restored from localStorage. Guard added: skip the cleanup when availableKinds is empty. Red on origin/main (no persistence logic exists there); green on this branch. * fix(e2e): address code-reviewer feedback on view-state-persist test - Add data-testid='df-page-size-select' to per-page { const val = e.target.value; diff --git a/apps/web/tests/components/DesignFilesPanel.test.tsx b/apps/web/tests/components/DesignFilesPanel.test.tsx index bceda05a4..44fe522fc 100644 --- a/apps/web/tests/components/DesignFilesPanel.test.tsx +++ b/apps/web/tests/components/DesignFilesPanel.test.tsx @@ -1,11 +1,21 @@ // @vitest-environment jsdom import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { DesignFilesPanel } from '../../src/components/DesignFilesPanel'; import type { ProjectFile, ProjectFileKind } from '../../src/types'; +// Stub localStorage so the component's view-state persistence writes to an +// in-memory store. Cleared in beforeEach so no test bleeds state into the next. +const lsStore = new Map(); +vi.stubGlobal('localStorage', { + getItem: (key: string) => lsStore.get(key) ?? null, + setItem: (key: string, value: string) => { lsStore.set(key, value); }, + removeItem: (key: string) => { lsStore.delete(key); }, + clear: () => { lsStore.clear(); }, +}); + function extForKind(kind: ProjectFileKind): string { if (kind === 'html') return 'html'; if (kind === 'image') return 'png'; @@ -80,6 +90,10 @@ function getSelects(container: HTMLElement) { } describe('DesignFilesPanel grouping', () => { + beforeEach(() => { + lsStore.clear(); + }); + afterEach(() => { cleanup(); vi.useRealTimers(); @@ -297,6 +311,14 @@ describe('DesignFilesPanel grouping', () => { }); describe('DesignFilesPanel large-list regression', () => { + beforeEach(() => { + lsStore.clear(); + }); + + afterEach(() => { + cleanup(); + }); + it('renders only the default page size (30) rows with 500 files', () => { const files = generateFiles(500); const { container } = renderPanel(files); diff --git a/apps/web/tests/components/DesignFilesPanel.view-state-persist.test.tsx b/apps/web/tests/components/DesignFilesPanel.view-state-persist.test.tsx new file mode 100644 index 000000000..cab38c390 --- /dev/null +++ b/apps/web/tests/components/DesignFilesPanel.view-state-persist.test.tsx @@ -0,0 +1,191 @@ +// @vitest-environment jsdom +// +// Red spec for bug #3a: view state (pageSize, sortKey, sortDir, kindFilter) +// resets on navigation because the component remounts via key={projectId}. +// These tests must go RED on origin/main and GREEN after the fix. + +import { cleanup, fireEvent, render } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { DesignFilesPanel } from '../../src/components/DesignFilesPanel'; +import type { ProjectFile, ProjectFileKind } from '../../src/types'; + +// Minimal localStorage stub mirroring the pattern in state/config.test.ts +const store = new Map(); +vi.stubGlobal('localStorage', { + getItem: vi.fn((key: string) => store.get(key) ?? null), + setItem: vi.fn((key: string, value: string) => { + store.set(key, value); + }), + removeItem: vi.fn((key: string) => { + store.delete(key); + }), + clear: vi.fn(() => { + store.clear(); + }), +}); + +function file(name: string, kind: ProjectFileKind = 'html', mtime = Date.now()): ProjectFile { + return { path: name, name, type: 'file', size: 1024, mtime, kind, mime: 'text/html' }; +} + +function generateFiles(count: number): ProjectFile[] { + const kinds: ProjectFileKind[] = ['html', 'image', 'sketch', 'text', 'code', 'pdf']; + return Array.from({ length: count }, (_, i) => { + const kind = kinds[i % kinds.length]!; + return file(`file-${i + 1}.html`, kind, Date.now() - i * 60_000); + }); +} + +function renderPanel( + files: ProjectFile[], + projectId = 'proj-a', +) { + return render( + , + ); +} + +function getPerPageSelect(container: HTMLElement): HTMLSelectElement { + // The per-page select is the first select in the panel + return container.querySelector('.df-pagination-start select')!; +} + +function getSortBtn(container: HTMLElement, label: string): HTMLElement { + return Array.from(container.querySelectorAll('.df-th-btn')).find( + (el) => el.textContent?.trim().startsWith(label), + )!; +} + +describe('DesignFilesPanel view-state persistence', () => { + beforeEach(() => { + store.clear(); + vi.mocked(localStorage.getItem).mockClear(); + vi.mocked(localStorage.setItem).mockClear(); + }); + + afterEach(() => { + cleanup(); + }); + + it('restores pageSize from localStorage after remount', () => { + const files = generateFiles(500); + + // First mount: change page size to 60 + const first = renderPanel(files); + const sel = getPerPageSelect(first.container); + fireEvent.change(sel, { target: { value: '60' } }); + first.unmount(); + + // Second mount simulates navigation away and back (key={projectId} causes remount) + const second = renderPanel(files); + const restoredSel = getPerPageSelect(second.container); + expect(restoredSel.value).toBe('60'); + }); + + it('restores sort key from localStorage after remount', () => { + const files = generateFiles(50); + + // First mount: click "Name" header to sort by name + const first = renderPanel(files); + fireEvent.click(getSortBtn(first.container, 'Name')); + first.unmount(); + + // Second mount: "Name" column should show the sort arrow + const second = renderPanel(files); + const nameBtn = getSortBtn(second.container, 'Name'); + expect(nameBtn.textContent).toContain('↑'); + }); + + it('restores sort direction from localStorage after remount', () => { + const files = generateFiles(50); + + // First mount: click "Name" twice to get desc + const first = renderPanel(files); + fireEvent.click(getSortBtn(first.container, 'Name')); + fireEvent.click(getSortBtn(first.container, 'Name')); + first.unmount(); + + // Second mount: Name column should show desc arrow + const second = renderPanel(files); + const nameBtn = getSortBtn(second.container, 'Name'); + expect(nameBtn.textContent).toContain('↓'); + }); + + it('restores kindFilter from localStorage after remount', () => { + // Files with mixed kinds so the filter button appears + const files = [ + file('a.html', 'html'), + file('b.png', 'image'), + file('c.txt', 'text'), + ]; + + // First mount: open the filter popover and check 'HTML page' + const first = renderPanel(files); + const filterTrigger = first.container.querySelector('.df-kind-filter-trigger'); + expect(filterTrigger).not.toBeNull(); + fireEvent.click(filterTrigger!); + const checkboxes = first.container.querySelectorAll( + '.df-kind-filter-list input[type="checkbox"]', + ); + // Check the first checkbox (HTML) + if (checkboxes[0]) fireEvent.click(checkboxes[0]); + first.unmount(); + + // Second mount: filter button should show active state (a kind is selected) + const second = renderPanel(files); + const trigger = second.container.querySelector('.df-kind-filter-trigger'); + expect(trigger?.classList.contains('active')).toBe(true); + }); + + it('does not bleed pageSize from one project into another', () => { + const files = generateFiles(500); + + // Project A: set page size 60 + const first = renderPanel(files, 'proj-a'); + fireEvent.change(getPerPageSelect(first.container), { target: { value: '60' } }); + first.unmount(); + + // Project B: should still have the default (30), not project A's setting + const second = renderPanel(files, 'proj-b'); + expect(getPerPageSelect(second.container).value).toBe('30'); + }); + + it('writes view state to localStorage on pageSize change', () => { + const files = generateFiles(500); + const { container } = renderPanel(files); + + fireEvent.change(getPerPageSelect(container), { target: { value: '45' } }); + + expect(vi.mocked(localStorage.setItem)).toHaveBeenCalledWith( + expect.stringMatching(/od:design-files:view-state/), + expect.any(String), + ); + }); + + it('falls back to default pageSize when stored value is not a supported option', () => { + const files = generateFiles(500); + + // Seed localStorage with an unsupported value (fractional, out-of-set integer) + for (const bad of [0.5, 17, 999, -1, 0]) { + vi.mocked(localStorage.getItem).mockReturnValueOnce(JSON.stringify({ pageSize: bad })); + const { container } = renderPanel(files); + expect(getPerPageSelect(container).value).toBe('30'); + cleanup(); + } + }); +}); diff --git a/e2e/ui/design-files-view-state-persist.test.ts b/e2e/ui/design-files-view-state-persist.test.ts new file mode 100644 index 000000000..16f6692c0 --- /dev/null +++ b/e2e/ui/design-files-view-state-persist.test.ts @@ -0,0 +1,361 @@ +/** + * Verifies that the DesignFilesPanel view state (sortKey, sortDir, pageSize, + * kindFilter) is written to localStorage under the per-project key + * 'od:design-files:view-state:v1:' and is restored correctly + * across three scenarios: + * + * (a) Tab-away / tab-back: navigating to a file tab and returning remounts + * the panel; prefs must survive. + * (b) Hard reload: localStorage persists across page.reload(); prefs must + * survive. + * (c) Project isolation: opening a second project starts with defaults, NOT + * the first project's persisted state. + * + * Each test must PASS on fix/web-design-files-persist-view and FAIL on + * origin/main (where no persistence is implemented). + */ + +import { expect, test } from '@playwright/test'; +import type { Page } from '@playwright/test'; + +// Matches the constant in DesignFilesPanel.tsx +const VIEW_STATE_KEY_PREFIX = 'od:design-files:view-state:v1:'; + +// Config key expected by the web app to skip onboarding +const CONFIG_STORAGE_KEY = 'open-design:config'; + +// Minimal 1x1 PNG, base64-encoded +const TINY_PNG_B64 = + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO5W6McAAAAASUVORK5CYII='; + +// 90 s: each test seeds ~18 files via sequential POST requests; on a cold CI +// runner this alone takes 40-50 s before any assertion. +test.describe.configure({ timeout: 90_000 }); + +// Inject onboarding bypass and mock app-config before each test so the web app +// boots straight into the workspace without prompts. +test.beforeEach(async ({ page }) => { + await page.addInitScript((key) => { + window.localStorage.setItem( + key, + JSON.stringify({ + mode: 'daemon', + apiKey: '', + baseUrl: 'https://api.anthropic.com', + model: 'claude-sonnet-4-5', + agentId: 'mock', + skillId: null, + designSystemId: null, + onboardingCompleted: true, + agentModels: {}, + privacyDecisionAt: 1, + telemetry: { metrics: false, content: false, artifactManifest: false }, + }), + ); + }, CONFIG_STORAGE_KEY); + + await page.route('**/api/app-config', async (route) => { + if (route.request().method() !== 'GET') { + await route.continue(); + return; + } + await route.fulfill({ + json: { + config: { + onboardingCompleted: true, + agentId: 'mock', + skillId: null, + designSystemId: null, + agentModels: {}, + privacyDecisionAt: 1, + telemetry: { metrics: false, content: false, artifactManifest: false }, + }, + }, + }); + }); + + await page.route('**/api/agents', async (route) => { + await route.fulfill({ + json: { + agents: [ + { + id: 'mock', + name: 'Mock Agent', + bin: 'mock-agent', + available: true, + version: 'test', + models: [{ id: 'default', label: 'Default' }], + }, + ], + }, + }); + }); +}); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +async function waitForLoadingToClear(page: Page): Promise { + await page + .getByText('Loading Open Design…') + .waitFor({ state: 'detached', timeout: 15_000 }) + .catch(() => {}); +} + +async function gotoEntryHome(page: Page): Promise { + await page.goto('/', { waitUntil: 'domcontentloaded' }); + await waitForLoadingToClear(page); + const privacyDialog = page + .getByRole('dialog') + .filter({ hasText: 'Help us improve Open Design' }); + if (await privacyDialog.isVisible().catch(() => false)) { + await privacyDialog.getByRole('button', { name: /not now/i }).click(); + await expect(privacyDialog).toHaveCount(0); + } + await expect(page.getByTestId('home-hero')).toBeVisible(); + await expect(page.getByTestId('home-hero-input')).toBeVisible(); +} + +async function createBlankProject(page: Page, name: string): Promise { + await page.getByTestId('entry-nav-new-project').click(); + await expect(page.getByTestId('new-project-modal')).toBeVisible(); + await page.getByTestId('new-project-name').fill(name); + await page.getByTestId('create-project').click(); + await waitForLoadingToClear(page); + await expect(page).toHaveURL(/\/projects\//); + await expect(page.getByTestId('chat-composer')).toBeVisible(); + + const url = new URL(page.url()); + const segments = url.pathname.split('/'); + const projectId = segments[2]; + if (!projectId) throw new Error(`could not extract projectId from ${url.pathname}`); + return projectId; +} + +async function seedFile( + page: Page, + projectId: string, + name: string, + content: string, + encoding?: 'base64', +): Promise { + const res = await page.request.post(`/api/projects/${projectId}/files`, { + data: { name, content, ...(encoding ? { encoding } : {}) }, + timeout: 10_000, + }); + expect(res.ok()).toBeTruthy(); +} + +function seedTextFile(page: Page, projectId: string, name: string): Promise { + return seedFile(page, projectId, name, `# ${name}`); +} + +function seedPngFile(page: Page, projectId: string, name: string): Promise { + return seedFile(page, projectId, name, TINY_PNG_B64, 'base64'); +} + +async function openDesignFilesTab(page: Page): Promise { + await page.getByTestId('design-files-tab').click(); + // The Design Files panel renders a table once files are present; wait for + // the controls row that always appears at the top of the panel. + await expect(page.locator('.df-controls-row')).toBeVisible({ timeout: 10_000 }); +} + +// Wait until the per-page