Build ErsatzTV Image / Build & test (.NET) (pull_request) Failing after 3m15s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 11s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 9m29s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
148 lines
9.2 KiB
Markdown
148 lines
9.2 KiB
Markdown
# 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 `ScreenRoute` object reference**
|
|
for the base path and every sub-path under it. `App`'s state update is
|
|
`setActiveRoute(routeFromLocation())`; React's `useState` setter bails via `Object.is` when the new
|
|
value is reference-equal to the old one — so navigating from `/app/blocks` to `/app/blocks/42` (or
|
|
between `/app/blocks/42` and `/app/blocks/17`) **never re-invokes `ScreenContent`** at the `App`
|
|
level. See the comment block directly above `PlayoutsRouteScreen` in `App.tsx` (~line 3540) for the
|
|
canonical explanation, and its implementation (`useState(() => window.location.pathname)` +
|
|
`useEffect` with a `popstate` 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`.
|
|
|
|
## 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).
|
|
|
|
## 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/`:
|
|
```bash
|
|
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.
|