fix(web): persist design files view state across navigation#2303
fix(web): persist design files view state across navigation#2303neogenix wants to merge 8 commits into
Conversation
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:<projectId> 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
- 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
- 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.
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.
- Add data-testid='df-page-size-select' to per-page <select> in DesignFilesPanel (W2: decouple test from i18n string 'Show') - Add StrictMode comment to viewStateHasMounted guard explaining the dev-mode double-write behaviour (W1: document the invariant) - Switch nav-away from dblclick to single-click + Open button, matching the pattern used in app-design-files.test.ts (W4) - Raise timeout from 60s to 90s for cold CI runners (W3) - Unify seedTextFile/seedPngFile into shared seedFile helper (N3) - Add home-hero-input assertion in gotoEntryHome (N2) - Switch waitForPageSizeSelect to use data-testid (W2)
lefarcen
added
size/L
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5432470a65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function isPageSize(v: unknown): v is number | 'all' { | ||
| if (v === 'all') return true; | ||
| return typeof v === 'number' && Number.isFinite(v) && v > 0; |
There was a problem hiding this comment.
Tighten isPageSize validation before restoring from localStorage: it currently accepts any positive finite number, so a corrupted value like 0.5 is treated as valid. That makes effectivePageSize fractional, and sortedFiles.slice(...) can yield an empty page even when files exist; with small projects (sortedFiles.length <= 15), list controls stay hidden, so users have no in-UI way to recover. Since this path is meant to sanitize persisted state, it should only accept supported integer sizes (or at least positive integers).
Useful? React with 👍 / 👎.
| function isPageSize(v: unknown): v is number | 'all' { | ||
| if (v === 'all') return true; | ||
| return typeof v === 'number' && Number.isFinite(v) && v > 0; |
There was a problem hiding this comment.
The new restore path still accepts any positive finite pageSize, but the UI only supports 15, 30, 45, 60, or "all". That means a stale or malformed localStorage value like 0.5, 17, or 999 now gets restored into state instead of falling back to the default. In the 0.5 case, effectivePageSize becomes fractional, sortedFiles.slice(...) produces an empty first page, and projects with <= 15 files never render the page-size controls, so the user has no in-UI way to recover. Since this code is explicitly validating persisted state before reuse, it should reject unsupported values here. Please validate against the same discrete option set the <select> renders (for example a shared PAGE_SIZE_OPTIONS constant) and add a regression test that an invalid stored pageSize falls back to 30.
…e regression test
| sortDir, | ||
| pageSize, | ||
| kindFilter: Array.from(kindFilter), | ||
| }); |
There was a problem hiding this comment.
Persisting every preference change under the real project key here makes the existing apps/web/tests/components/DesignFilesPanel.test.tsx suite stateful across cases, because its renderPanel() helper hardcodes projectId="test-project" and never clears od:design-files:view-state:v1:test-project between tests. On this head, pnpm --filter @open-design/web exec vitest run -c vitest.config.ts tests/components/DesignFilesPanel.test.tsx now fails 4 cases: after shows 60 rows when page size is changed to 60, the later defaults-based assertions start from the restored pageSize=60 instead of 30 (navigates pages... sees 60 rows, updates page info... sees 1–60 of 500, etc.). Because this PR introduces that persisted state, the existing regression suite now goes red at HEAD. Please isolate those tests from this storage key—for example by clearing it in afterEach, or by giving each case a unique projectId—so the pre-existing large-list assertions stay deterministic.
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
@neogenix I re-checked the current head persistence logic and the added regression coverage for page size, sort, filter restoration, and per-project isolation. The earlier page-size validation and test-isolation regressions are addressed on this head, and the changed ranges look consistent. Nice cleanup closing the loop on this fix.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.3 participants
Why
The design-files page resets its view state on every navigation away
and back. The cause:
DesignFilesPanelis mounted withkey={projectId}in
FileWorkspace.tsx, which forces a full React remount whenever theuser leaves the Design Files tab. Every
useStatevalue re-initializesto its default, so:
A user who set the page to 50 items, sorted by name, filtered to HTML
artifacts, then clicked into a file's preview tab loses all three the
moment they navigate back. This is a regression against every other
tabular UI in the product (which preserve their state).
What users will see
pageSize,sortKey,sortDir, andkindFilternow persist acrossnavigation and across hard reloads. Settings are scoped per-project, so
project A's preferences do not bleed into project B — each gets its own
fresh defaults until the user changes something.
Storage key format:
od:design-files:view-state:v1:<projectId>. Thev1segment is in place so a future field-shape change can roll thekey forward without crashing on stale data.
Surface area
No new keys, no new settings, no new CLI surface. The persisted shape is
local-only and never leaves the browser.
Screenshots
Behavior change only — no new UI element. Reproduction: open a project,
go to Design Files, change items-per-page to 50, switch to another tab
and back. On
main, page size is back to 30; on this branch, it'sstill 50.
Bug fix verification
apps/web/tests/components/DesignFilesPanel.view-state-persist.test.tsx(6 cases): each of the four persisted fields survives a remount;
per-project key isolation; legacy/missing storage falls through to
defaults.
origin/main(no persistence), green here.e2e/ui/design-files-view-state-persist.test.ts(3 scenarios): (a) prefs survive navigating away to a file tab and
back, (b) prefs survive a hard browser reload, (c) project B's view
state is independent of project A's (the safety net the 10-pass
reviewer asked for explicitly).
main, 3 green here (browser-verified in Chromium, 14.6s).The 10-pass review surfaced and resolved on this branch:
kindFilterrestore previously cast a stored valueblindly with
kf as ProjectFileKind[]. A malformed localStoragevalue could inject an unknown kind that no UI then knew how to
render. Fixed with a
VALID_KIND_SETmembership check that dropsunknown values to defaults.
defaults, so every project visit stamped localStorage with defaults
even when the user hadn't touched anything. Fixed with a
viewStateHasMountedref so writes only happen on genuineuser-driven changes.
readViewStatelacked an explicittypeof windowguard. Added one to mirror the SSR-safety pattern other components
in the codebase use.
was missing. Replaced with an explicit
expect(...).not.toBeNull().Validation
pnpm guard(clean)pnpm typecheck(clean)pnpm --filter @open-design/web test DesignFilesPanel.view-state-persist— 6/6 pass
pnpm exec playwright test -c playwright.config.ts design-files-view-state-persist— 3/3 pass (browser-verified in Chromium, 14.6s)
Adjacent issues (out of scope)
od:design-files:view-state:v1:<projectId>keys after aproject is deleted: the localStorage entry is never cleaned up. Low
severity (it's a few bytes per dead project), but worth a follow-up
that hooks the project-deletion handler.
gap, not introduced by this PR.
aria-liveannouncement when persisted state restores with anon-default sort/filter on remount: minor a11y polish for a future
pass.
Coordination note
There is an open upstream PR #1995 ("Follow up #1990: cover kind filter
pagination") that touches
DesignFilesPanel.tsxheavily (+176/-23).The diffs do not overlap on the same lines — this PR adds
state-persistence plumbing, #1995 adds test-coverage plumbing — but a
textual rebase will be needed depending on which lands first. PR #2302
("feat(web): shift-click range selection on design files") also lives
on the same file in a different region and is functionally independent.