ersatztv/docs/spa-conventions.md

SPA conventions — "Add a screen" playbook

Purpose: a precise playbook for adding a new screen (or sub-path editor) to the ChicoryTV React SPA (web/), for an agent with no prior context in this repo. Update this doc in the same PR that changes any convention below.

Companion to api-conventions.md (the API surface the SPA talks to) and docs/contributing.md (general repo conventions).

1. Stack & layout

Vite + React + TypeScript, builds to ErsatzTV/wwwroot/app (see web/vite.config.ts: base: '/app/', build.outDir: '../ErsatzTV/wwwroot/app'), served by the ASP.NET host at /app.

• Routes + nav: web/src/App.tsx — one big route table of ScreenRoute objects (path, label, title, kicker, icon, etc.) plus an allowSubPaths?: boolean flag.
• Screens: web/src/screens/*.tsx, one file per top-level screen, generally with a colocated *.test.tsx.
• API clients: web/src/api/<domain>.ts (see §4).
• Styling: web/src/shell.css (+ web/src/components/components.css) — utility classes with a ctv- prefix (~690 occurrences across those two files). Reuse an existing ctv-* class before inventing a new one.

2. CRITICAL: sub-path screens must own their own pathname state

If a route sets allowSubPaths: true (e.g. so /app/blocks/{id} works under the /app/blocks nav entry), the screen component itself must track window.location.pathname and listen for popstate — do not rely on App.tsx re-rendering ScreenContent when the sub-path changes.

Why: App.tsx's routeFromLocation() matches an allowSubPaths route by prefix (pathname.startsWith(\${route.path}/`)) and returns the **same ScreenRouteobject reference** for the base path and every sub-path under it.App's state update is setActiveRoute(routeFromLocation()); React's useStatesetter bails viaObject.iswhen the new value is reference-equal to the old one — so navigating from/app/blocksto/app/blocks/42(or between/app/blocks/42and/app/blocks/17) **never re-invokes ScreenContent** at the Applevel. See the comment block directly abovePlayoutsRouteScreeninApp.tsx (~line 3540) for the canonical explanation, and its implementation (useState(() => window.location.pathname)+useEffectwith apopstate` listener local to the wrapper component) for the fix.

Exemplars of screens that already do this correctly: BlocksScreen.tsx, TemplatesScreen.tsx, DecosScreen.tsx, DecoTemplatesScreen.tsx, and the PlayoutsRouteScreen wrapper in App.tsx (which owns two sibling sub-paths, /playouts/{id}/alternate-schedules and /playouts/{id}/templates, dispatching internally via parsePlayoutSubRoute).

3. Data loading pattern

Reference implementation: web/src/screens/LogsScreen.tsx. Structure to copy for any screen that fetches from the API:

• A discriminated-union state type covering loading/success/error, e.g. type LogsState = { status: 'loading'; ... } | { status: 'success'; ... } | { status: 'error'; ... }.
• A seqRef (monotonically incremented request counter) + activeRef (mount-tracking boolean, flipped in a mount/unmount useEffect) pair — guards against a stale, slower request overwriting a newer one's result, and against setting state after unmount.
• The actual fetch lives in a useCallback (load), called from a separate useEffect(() => { load(); }, [load]).
• Lint rule — react-hooks "no set-state-in-effect": never call setState synchronously in the body of a useEffect. State transitions happen only inside event handlers or promise .then()/.catch() callbacks (as in LogsScreen's load). This is enforced by eslint-plugin-react-hooks in web/eslint.config.js — a synchronous setState in an effect body will fail npm run lint.

3a. "Keep results visible during refetch" ⇒ gate mutations + show a refreshing cue

Some grid screens deliberately keep the previous successful result set rendered while a refetch is in flight (no full-screen loading state on a query/kind/page change), so the grid doesn't flash empty. SearchScreen.tsx and MediaBrowseScreen.tsx do this. If such a screen also carries mutation surfaces (per-card Add-to menu, Select/select-mode, a selection action bar, "Add all", "Save as smart collection"), those surfaces would otherwise stay live over a stale result set — an add/select action then targets the about-to-be-replaced items, or (worse) a query-wide "Add all" bulk request resolves against the previous query. This was issue #221 (adversarial-reviewer#18).

Convention — when a screen keeps stale results visible during a refetch:

• Key the success state to the request params that produced it. Store the identifying params on the status: 'success' variant (SearchScreen: the query; MediaBrowseScreen: a kind|query|page key), set in the seq-guarded .then. Derive const refreshing = state.status === 'success' && state.<key> !== <current params>; in render. Prefer this over a synchronously-set refreshing flag: setting state synchronously from the load path trips the react-hooks "no set-state-in-effect" rule (§3). Invariant, not a guarantee: the derivation is only self-correcting when every value the current params can take will actually trigger a fetch. If load() early-returns for some param value (e.g. SearchScreen's blank-query guard), state never updates for that value and a stale status: 'success' variant lingers — so the comparison must exclude params that suppress fetching, or gate the whole flag on the same condition that gates the fetch (SearchScreen: const refreshing = hasQuery && state.status === 'success' && state.query !== query.trim(); — fixed post-review in #222 after the naive derivation got stuck true once the query was cleared to empty, see PR discussion for #221).
• While refreshing: show a visible cue (a role="status" "Refreshing…" row with <Spinner> plus the .ctv-media-grid-dim opacity class on the grid) and disable every mutation surface — per-card Add-to menu (withhold the actions node), select toggle + in-grid selection (const canSelect = selectMode && !refreshing; gates onToggleSelect), the selection action bar, "Add all", "Save as smart collection". Card navigation (onOpen) may stay live — but only outside select mode: while selectMode && refreshing, MediaPosterCard falls back to onOpen whenever onToggleSelect is undefined, so both props must be withheld together or a mid-select click navigates away instead of no-op'ing. The select-mode toggle itself should only be disabled while refreshing when entering select mode (refreshing && !selectMode) — exiting only clears selection, not a mutation, so it must stay available.
• Bind async bulk completions to their request params, not just mount. A whole-query/whole-set request (e.g. getSearchAllItems) must, on resolve, check that its snapshotted params are still current (compare against a ref that always holds the committed value — SearchScreen reuses lastQueryRef) and discard otherwise. Checking only activeRef (mounted) is insufficient.

4. API client modules

One file per domain in web/src/api/, e.g. logs.ts, blocks.ts, playouts.ts. Pattern (see web/src/api/logs.ts):

• Re-export the generated response/DTO types from ./generated/v1: export type LogEntry = components['schemas']['LogEntryResponseModel'];
• A typed params interface for the endpoint's query string (e.g. GetLogsParams).
• The fetch function builds a URLSearchParams from only the params that are set, then calls the shared request<T>(url) helper from ./client.
• An error-message helper (e.g. messageFromLogsError) that narrows unknown → ApiError (from ./client) → a human string, with a fallback message — screens use this instead of stringifying errors themselves.
• web/src/api/index.ts re-exports everything so screens import from '../api', not from the individual domain file directly.

Trust the generated key casing — it mirrors the runtime. Since #198 the OpenAPI spec is generated to match the runtime Newtonsoft serializer exactly (a schema transformer runs the same contract resolver; see api-conventions.md §5a), so the generated types carry the real wire keys — including Newtonsoft's acronym quirks like ffmpegProfileId (channel FFmpeg-profile id) and ffmpegProfile (channel FFmpeg-profile display name). Do not hand-cast responses to "fix" a key or dual-read a spec-cased vs runtime-cased key (the old PlaybackTroubleshootingScreen #198 escape hatch that read data.channel.fFmpegProfileId has been removed — read ffmpegProfileId straight off the typed response). When you mock an API response in a test, use the generated (runtime) casing.

5. Artwork rendering

Render item.artwork / item.poster (or whatever the DTO field is named) directly as an <img src> — since PR #181, API responses already return rooted, directly-usable URLs (see api-conventions.md §4). Do not client-side-prefix artwork paths (no /artwork/posters/ string building in SPA code) — if you see that pattern, it's stale/wrong.

5b. HLS video preview

Screens that preview an ErsatzTV HLS stream use the reusable HlsPlayer component (web/src/media/HlsPlayer.tsx, introduced with #145). Pass it a src (the .m3u8 URL, or null for idle) and — when the manifest GET itself starts a server-side session (e.g. troubleshooting playback.m3u8) — a playToken you increment per play, so a repeat play with an identical URL still tears down and re-attaches (an unchanged src alone is a state no-op that never issues a new request); it attaches hls.js when Media Source Extensions are available and falls back to native HLS (video.canPlayType('application/vnd.apple.mpegurl'), i.e. Safari) otherwise, and tears down the hls.js instance on src change and unmount. Its config mirrors the legacy _Host.cshtml previewChannel (liveDurationInfinity: true + an unbounded manifest maxTimeToFirstByteMs) because the troubleshooting playback.m3u8 endpoint blocks until segments exist before it 302s to the live manifest. In tests, mock hls.js wholesale (vi.mock('hls.js', …) with a class exposing static isSupported(), static Events, and loadSource/attachMedia/on/destroy) so jsdom never touches a real MediaSource; assert the manifest URL via the mocked loadSource spy (see PlaybackTroubleshootingScreen.test.tsx).

5c. Media "Add to…" affordances

Screens that let the user add media items to a collection/playlist/schedule use the shared layer in web/src/media/addTo/ — AddToMenu (popover for a MediaPosterCard actions slot or a detail page's action row) and the AddToCollectionDialog / AddToPlaylistDialog / AddToScheduleDialog / SaveAsSmartCollectionDialog it drives. Do not build screen-local target pickers. Multi-select on grid screens is an explicit "Select" toggle (see docs/decisions.md 2026-07-10 for the rationale and the accepted deviations from Blazor).

5d. Client-local preferences: localStorage, namespaced ctv-* keys

Per-browser UI preferences (theme, an auth token, a screen's remembered page size) live in window.localStorage under a namespaced ctv- key, not a round-trip through the API — the established pattern is designSystem.ts's getStoredDesignSystemTheme/applyDesignSystemTheme (ctv-theme): a small getStorage() helper that returns window.localStorage wrapped in a try/catch (so a disabled/unavailable storage API degrades to the default instead of throwing), a getter that validates the stored value against the known option set before trusting it, and a setter that writes straight through. LogsScreen.tsx's page-size persistence (ctv-logs-page-size, #213) follows the same shape. Reserve this for state that's genuinely local to the browser/user session — if a preference needs to be shared across devices or is really server/business state (e.g. Blazor's ConfigElement-backed settings), it belongs behind an API endpoint instead; see docs/decisions.md 2026-07-11 for the specific reasoning on logs page-size.

6. Tests

• vitest, colocated *.test.ts / *.test.tsx next to the source file.
• Every screen with meaningful logic gets a screen test; every API client module gets a param-mapping / URL-building test (e.g. logs.test.ts next to logs.ts).
• web/src/App.test.tsx covers navigation + the route table, including regressions like the sub-path bug in §2 (see the tests around PlayoutsRouteScreen, ~line 1682+, that click into /app/playouts/{id}/... sub-paths and assert the correct sub-screen rendered).
• Nav-label test-selector care: getByRole('link'/'button', { name: /Regex/ }) matches by substring by default — a loose regex can match more than one nav item. Verified example: the System nav button is matched with an anchored regex (name: /^System/) rather than a bare /System/, specifically to avoid ambiguous matches against other labels that start with or contain "System". Anchor (^/$) or use exact strings in getByRole name matchers whenever a new label could be a substring of (or share a substring with) an existing one — check App.tsx's nav label: list for collisions before picking a new label.

7. Verification gate — run before every commit touching web/

From web/:

npm test          # vitest
npm run lint      # eslint .
npm run build     # tsc -b && vite build

Also run npm run check:api if you touched anything OpenAPI-relevant (see api-conventions.md §5) — it regenerates src/api/generated/v1.d.ts and fails the build if it's out of sync with what's committed.

8. Unsaved-changes navigation guard

Screens with a draft / explicit-Save model (edits accumulate locally, one Save flushes them — e.g. the schedules editor, web/src/screens/SchedulesScreen.tsx) must guard against losing the draft to navigation. The shared module web/src/navigationGuard.ts is the seam:

• The screen registers a guard on mount: registerNavigationGuard(() => !dirtyRef.current || window.confirm(...)), returning the unregister fn from the mount useEffect for cleanup. Only one guard is active at a time (the mounted screen); a stale unregister only clears its own guard.
• App.tsx's navigate handler calls canLeaveCurrentScreen() before pushState — a false return aborts the in-app sidebar/nav click. This covers React-router-less in-app navigation only.
• The screen also guards the three paths the module can't see — a schedule switch and any in-screen action that would replace the draft use their own window.confirm(...), and a beforeunload listener (installed while dirty) covers full-page unloads (reload / close tab / external link).

Keep the guard predicate reading a ref (dirtyRef), not the dirty state value, so canLeaveCurrentScreen() sees the current dirtiness synchronously at click time.

9. Review checklist — temporal semantics

• For every effect / timer / async completion, ask: when does it fire (mount, dependency change, unmount, StrictMode double-invoke) and which render/request does it still own? A debounce timer fires on mount too (§3, the lastQueryRef no-change guard exists precisely for that); a .then can resolve after the params it was launched for have moved on (§3a, the refreshing gate and the Add-all query binding exist for that). A guard that only checks "still mounted" (activeRef) does not answer "still current".
• For any "make X consistent with Y" change, re-validate the exemplar Y's temporal behavior before copying it. #221 came from copying a fetch model that keeps stale results visible onto screens that had gained mutation surfaces — the exemplar was safe read-only, the copy was not. Copying a pattern copies its assumptions; confirm they still hold in the new context.