Two missing-base-reset defects in the ChicoryTV SPA shell, both confirmed by rendering the real app (not source-reading): - #373: the browser-default `body { margin: 8px }` was never reset, so it framed every full-viewport layout — the app shell and the shell-less `.ctv-auth-page` boot pages (login/setup/checking/error), both `min-height:100vh` — with a light border on all four edges. Fix: `html, body { margin: 0 }` + paint the app surface on `body` so any residual gap/overscroll stays dark (matches the design-system reset in forms.card.html / chicorytv-admin templates). Verified body margin 0 and no edge border on the boot page and the shell, warm theme, 1280px and 900px. - #377: the SPA ships no global `box-sizing` reset (default content-box), so `.ctv-nav-item { width:100%; padding:0 10px }` overflowed `.ctv-nav` (`overflow:auto`) by 20px → a horizontal scrollbar in the sidebar. Fix: scope `box-sizing: border-box` to `.ctv-nav-item`. Verified nav scrollWidth==clientWidth (216==216) and no horizontal scroll after the fix; the intended vertical nav scroll is unaffected. Docs: spa-conventions.md §1 records the base reset and the deliberate absence of a global box-sizing reset (set border-box locally when combining width:100% + padding). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
31 KiB
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.
- Composition root:
web/src/App.tsx— owns the active route, approved Libraries sub-path, navigation guard integration, and theme; it composes the shell with the matched screen. - Routes + nav:
web/src/app/routes.tsx— the ONE stable module-level array ofScreenRouteobjects (path,label,title,kicker,icon, etc.) plusallowSubPaths?: boolean, the matcher, and sidebar group definitions. Route objects must never be rebuilt per render. - Shell + screen dispatch:
web/src/app/AppShell.tsxowns Sidebar/TopBar/Connect/version/theme chrome;web/src/app/ScreenContent.tsxexhaustively maps a matched route to its screen and owns the Media/Libraries route wrappers. - 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 actv-prefix (~690 occurrences across those two files). Reuse an existingctv-*class before inventing a new one.shell.csscarries the only base reset —html, body { margin: 0 }plusbody { background: var(--surface-app) }(the 8px default body margin otherwise frames every full-viewport layout with a light border, #373). There is no globalbox-sizingreset (the SPA is authored under the defaultcontent-box), so any element that combineswidth: 100%with padding/border must setbox-sizing: border-boxlocally or it overflows its container — e.g..ctv-nav-item(#377). Preferwidth: auto(shrink-to-fit) overwidth: 100%+ padding where you can.
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 the composition root re-rendering ScreenContent when the sub-path
changes.
Why: app/routes.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 abovePlayoutsRouteScreeninscreens/PlayoutsScreen.tsx 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 colocated in
PlayoutsScreen.tsx (which owns two sibling sub-paths, /playouts/{id}/alternate-schedules and
/playouts/{id}/templates, dispatching internally via parsePlayoutSubRoute).
Exception — a guarded sub-path route defers pathname ownership to App. When a sub-path route's
screens ALSO register a dirty guard (§8), the wrapper must not self-listen for popstate; App
owns the pathname and passes the approved sub-path down as a prop. The LibrariesRouteScreen wrapper
in app/ScreenContent.tsx is the exemplar (sub-path parsed by parseLibrariesSubRoute, dispatched
via a flat switch, sub-path supplied by App's librariesSubPath state). See §8 for why child-before-parent
effect order makes the self-listening pattern unsafe here.
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/unmountuseEffect) 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 separateuseEffect(() => { load(); }, [load]). - Lint rule —
react-hooks"no set-state-in-effect": never callsetStatesynchronously in the body of auseEffect. State transitions happen only inside event handlers or promise.then()/.catch()callbacks (as inLogsScreen'sload). This is enforced byeslint-plugin-react-hooksinweb/eslint.config.js— a synchronoussetStatein an effect body will failnpm 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: thequery;MediaBrowseScreen: akind|query|pagekey), set in the seq-guarded.then. Deriveconst refreshing = state.status === 'success' && state.<key> !== <current params>;in render. Prefer this over a synchronously-setrefreshingflag: setting state synchronously from the load path trips thereact-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. Ifload()early-returns for some param value (e.g.SearchScreen's blank-query guard),statenever updates for that value and a stalestatus: '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 stucktrueonce the query was cleared to empty, see PR discussion for #221). - While
refreshing: show a visible cue (arole="status""Refreshing…" row with<Spinner>plus the.ctv-media-grid-dimopacity class on the grid) and disable every mutation surface — per-card Add-to menu (withhold theactionsnode), select toggle + in-grid selection (const canSelect = selectMode && !refreshing;gatesonToggleSelect), the selection action bar, "Add all", "Save as smart collection". Card navigation (onOpen) may stay live — but only outside select mode: whileselectMode && refreshing,MediaPosterCardfalls back toonOpenwheneveronToggleSelectis 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 —SearchScreenreuseslastQueryRef) and discard otherwise. Checking onlyactiveRef(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
URLSearchParamsfrom only the params that are set, then calls the sharedrequest<T>(url)helper from./client. - An error-message helper (e.g.
messageFromLogsError) that narrowsunknown→ApiError(from./client) → a human string, with a fallback message — screens use this instead of stringifying errors themselves. web/src/api/index.tsre-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.
4a. Optimistic-concurrency editors (ETag / If-Match / 412)
Replace-all editors (Blocks is the reference; see api-conventions.md §7a for the server contract,
issue #253) must round-trip the aggregate's concurrency ETag so a stale tab can't silently overwrite a
fresher edit:
- Transport seam (
web/src/api/client.ts):requestWithMeta<T>(path, options)returns{ data, etag }(reads theETagresponse header).request<T>delegates to it and drops the meta — keep usingrequestfor endpoints without a concurrency token. - Domain module (
web/src/api/blocks.ts): expose a…WithMetaload (getBlockItemsWithMeta→{ data, etag }) and make the replace accept the last-seen ETag and return the new one:replaceBlock(id, body, ifMatch?)→requestWithMeta(..., { headers: ifMatch ? {'If-Match': ifMatch} : undefined }). - Editor: hold the ETag in a
useRef; set it from the load GET, and replace it from the PUT response's ETag on every successful save (a same-tab second save otherwise 412s against its own write). Keyed reload: areloadKeystate in the loaduseEffectdep array lets the conflict flow re-fetch. - 412 UX: catch
error instanceof ApiError && error.status === 412on save and open a blocking "changed elsewhere — reload (unsaved changes discarded)"ConfirmDialog(Reload bumpsreloadKey), distinct from a 409 ("build in progress — retry shortly"). All other errors stay the generic save-error path. Reference:web/src/screens/BlocksScreen.tsxBlockEditor. - List-derived editors (Multi/Rerun collections open the editor from a paged-list row, not a per-record
GET): fetch the single record via the
…WithMetaby-id helper (getMultiCollectionWithMeta/getRerunCollectionWithMeta) when the editor mounts, and build the draft from that response — so the ETag and the draft data come from one read and stay consistent (the fail-safe ordering; don't pair a list-row draft with a separately-fetched ETag). Editors that navigate back to the list on save (Playout, Multi/Rerun, Collection reorder) need only the 412 branch — no post-save ETag rotation.
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.
5e. Session auth: the boot gate, the machine-key screen + the global 401 signal (#295)
Since #295 the browser authenticates with a session cookie, not an API key. The whole /api
surface still answers a missing/expired session with 401, and mutating verbs additionally require a
CSRF header. The former keyless-bootstrap ApiKeyScreen (the X-Api-Key/localStorage model of #197)
is gone. The SPA seams now are:
- The request client sends no API key; it relies on the cookie and adds CSRF automatically
(
web/src/api/client.ts): the session cookie rides along with every same-origin request, and every mutating verb (POST/PUT/PATCH/DELETE) gets anX-Csrfheader set automatically — screens making a POST get CSRF for free. There is nogetStoredApiKey/X-Api-Keypath anymore; the only residuallocalStoragetouch isclearLegacyStoredApiKey()(web/src/api/auth.ts), a one-shot cleanup the boot gate runs once to purge any stalectv-api-key. - The boot gate owns login/setup (
web/src/AuthGate.tsx), wrapping<App/>outside the shell/router. It asks the PUBLIC/api/v1/auth/config, then/api/v1/auth/session, and renders Setup / Login / the app. It also publishesAuthContext({ username, method, signOut, requireLogin }) — readmethodviauseContext(AuthContext)to branch on the auth kind (e.g.'local'vs OIDC). - The machine-key screen is a normal authenticated screen (
web/src/screens/ApiKeyScreen.tsx, route/app/api-key, System nav group). It loads the server machine key from/api/v1/auth/machine-key(viagetMachineKey()) and displays it masked (a masked<code>that never puts the key in the DOM until Reveal) with Reveal + Copy affordances — the key is for MCP / external REST clients; the browser no longer uses it. For local accounts only (AuthContext.method === 'local') it also renders a "Local admin password" card that callschangePassword(current, new), mapping a wrong- current-password 401 to the server ProblemDetails detail inline. - File-download endpoints go through a fetch-blob helper, never a browser tab. Endpoints that
return a binary body (the troubleshooting archive / media-sample POSTs) must not go through
request()(it JSON-parses) and must not be opened withwindow.open(a session-cookie GET in a new tab can't carry the CSRF header and bypasses error handling). Insteadfetch(path, { method: 'POST', headers: { 'X-Csrf': '1' } }), thenresponse.blob()→ object URL → anchor-click → revoke, reading the filename fromContent-Disposition; a non-ok response throws anApiError. SeedownloadTroubleshootingArchive/downloadTroubleshootingMediaSampleinweb/src/api/troubleshoot.ts. - 401 UX is one shell-level banner, not per-screen handling. Rather than special-casing 401 in
every screen's error path, the request client emits a single app-wide signal on 401
(
auth.tsnotifyUnauthorized/subscribeUnauthorized), andweb/src/UnauthorizedBanner.tsx(rendered once in the App shell) subscribes and prompts re-login. This is the DRY seam for "the server rejected us" — reuse it instead of adding bespoke 401 branches. Auth flows that expect a 401 as an inline answer (login, change-password) passsuppressUnauthorizedSignalso they don't trip it.
6. Tests
- vitest, colocated
*.test.ts/*.test.tsxnext 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.tsnext tologs.ts). web/src/App.test.tsxcovers composition, navigation, shell behavior, and integration regressions such as sub-path-to-sub-path rendering and guarded popstate restoration. Detailed screen behavior belongs in the screen's colocated test.web/src/app/routes.test.tsxindependently pins stable route-object identity and query/path matching.- 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 ingetByRolename matchers whenever a new label could be a substring of (or share a substring with) an existing one — checkapp/routes.tsx's navlabel:list for collisions before picking a new label. - Extracted-screen tests own their own fetch mock (Schedules #207, Channels #244, Playouts
#245): when a
screen is pulled out of
App.tsxintoweb/src/screens/<Name>Screen.tsx, its colocated<Name>Screen.test.tsxbuilds a self-containedvi.spyOn(window, 'fetch')mock scoped to that screen's own endpoints (plus localjsonResponse/fixture-factory helpers) and renders the screen component directly — it must not import fromApp.test.tsxor reuse the monolithicmockDashboardApi().App.test.tsxkeeps only a thin nav-smoke test for the extracted screen (route to it, assert it mounted and hit its own endpoint) plus genuinely cross-cutting shell concerns (route table, sub-path ownership §2, the unsaved-changes/popstate guard §8, the TopBar action wiring). Detailed screen behavior lives in the screen's own test file. SeeChannelsScreen.test.tsx/SchedulesScreen.test.tsx/PlayoutsScreen.test.tsxfor the shape.
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 mountuseEffectfor cleanup. Only one guard is active at a time (the mounted screen); a stale unregister only clears its own guard. -
App.tsx'snavigatehandler callscanLeaveCurrentScreen()beforepushState— afalsereturn aborts the in-app sidebar/nav click. -
Browser Back/Forward (
popstate) is also covered.App.tsx'spopstatehandler consultscanLeaveCurrentScreen()too. Apopstatecannot be cancelled — by the time it fires the URL has already changed — so on a veto the handler re-pushes the pre-pop path (tracked in acurrentPathRefupdated on every approved navigation) viahistory.pushStateand leavesactiveRouteuntouched, effectively undoing the browser's URL change. This same handler also runs for the synthetic popnavigateToPath()dispatches, so programmatic in-app navigation is guarded as well. The re-push does not re-firepopstate; that's safe because only one screen is mounted at a time. -
Guarded sub-path routes: App owns pathname/popstate (resolves the old §2 caveat). A sub-path wrapper that BOTH tracks its own pathname (§2) AND whose sub-screens register a dirty guard cannot self-listen for
popstate: React commits child passive effects before the parent, so a wrapper-ownedpopstatelistener fires before App's guard-restore handler and would switch sub-screen before App could veto and re-push — desyncing the two. Resolution (design #202 §D.2, finding 4): App is the singlepopstateowner. On a pop it consultscanLeaveCurrentScreen(), and only on approval does it update acurrentSubPathstate value (librariesSubPath) that it passes down into the wrapper; on a veto it re-pushes the pre-pop path and updates nothing, so the wrapper never sees a vetoed path. The wrapper (LibrariesRouteScreen) derives its sub-path purely from that prop — never from the raw event — so effect-commit order is irrelevant. Scope the state write to the guarded route only: thesetLibrariesSubPathcall is gated on the arrived route beinglibraries, so unguarded sub-path routes (Playouts/Media) skip it entirely and stay byte-identical (they still bail App'ssetActiveRouteviaObject.isand self-own their pathname per §2). The regression test is the App-owned-popstate case inApp.test.tsx(dirty editor at/app/libraries/local/3:confirm→falsekeeps URL + mounted sub-screen;confirm→truenavigates). Unguarded sub-path screens (PlayoutsRouteScreen,MediaRouteScreen) keep self-owning their pathname — only screens that register a dirty guard defer to App. -
The screen also guards the paths the module still can't see — an in-screen action that would replace the draft (schedule switch, opening the properties editor) uses its own
window.confirm(...), and abeforeunloadlistener (installed whiledirty) 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.
- Successful save + same-callback navigation:
useDirtyGuardreturns amarkCleancallback for the narrow case where a successful save queues the new draft/baseline and immediately callsnavigateToPath(...)in that same promise callback. React has not committed the clean render before the syntheticpopstate, so callmarkClean()after the durable save succeeds and immediately before navigating. Still queue the real clean draft/baseline state; the callback only transfers the synchronous guard truth across that one event boundary. Only do this while the successful request still owns the current draft: disable or otherwise gate draft mutations for the entire in-flight save, or revision-check the completion before marking clean. Otherwise the completion can discard edits made after the request started. Do not replace the callback with a timer or a forced render. Screens that remain mounted after saving do not need the callback.
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
lastQueryRefno-change guard exists precisely for that); a.thencan resolve after the params it was launched for have moved on (§3a, therefreshinggate 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.
10. TopBar primary-action button (usePrimaryAction)
The shell TopBar (app/AppShell.tsx) renders at most one primary-action button (top-right, Plus
icon) for the active screen. App.tsx wraps the shell and matched screen in the minimal
PrimaryActionProvider from web/src/primaryAction.ts: the active screen explicitly registers one
handler, and the TopBar reads that matching registration directly from React context. No window event,
string-keyed dispatcher, or generic screen-action framework participates in normal screen actions.
- A screen opts in by calling
usePrimaryAction(routeId, handler)at the top of the component, before any early return (it's a hook). The handler must be reachable there — a create/navigate handler or auseStatesetter, not something defined below a loading/errorreturn. Reference:SchedulesScreen(usePrimaryAction('schedules', () => setForm('create'))). - The provider owns exactly one registration. Each mounted hook gets an opaque owner token; registering a newer screen replaces the previous registration, and cleanup clears only the registration owned by that hook. A stale unmount therefore cannot erase the newer screen's action. The hook keeps the latest handler in a ref, so ordinary screen renders update behavior without reclaiming/churning ownership. Directly-rendered screen tests outside the provider remain harmless; there is simply no shell action consumer.
- The route must ALSO declare a matching non-empty
primaryActionlabel in the stableapp/routes.tsxtable. The TopBar renders the button only when that label is non-empty and the active route owns a matching registration — metadata alone can no longer produce a dead button, while a registration without a label remains intentionally invisible. Keep the two in lockstep. The#238tests inApp.test.tsxguard this: a data-drivenit.eachasserts each URL-navigating create screen's banner actually navigates (so a typo'd route id → a button that navigates nowhere → red), plus a drop test that an action screen shows no banner button.primaryAction.test.tsxseparately pins matching, latest-handler, route-change, cleanup, and stale-owner semantics. The dialog/editor create screens (schedules, multi/rerun collections, trakt) are exercised by their own create-flow tests. - When to WIRE vs DROP (issue #238). The Plus icon makes the button semantically a "create new item"
affordance. Keep + wire it only on list screens with a single, unambiguous create flow ("Add Channel",
"Add Schedule", "Add Multi-Collection", "Add Rerun Collection", "Add Trakt List", "Add Filler Preset", "Add
Profile", "Add Watermark"). Drop it (
primaryAction: '') where the primary action is not a create (Save / Refresh / Play / Validate / Reset / Scan — the "+" is wrong, and those screens already carry the correct in-body control), or where a single banner action would be ambiguous (Collections has two create types behind tabs), a silent no-op (the Channel builder's Create is disabled until the form is valid), or semantically misplaced (Dashboard is a status page; Libraries "Scan" is per-row). Coexisting with an in-body create control is fine (Schedules has both) — the banner is a convenience, not the sole entry point. Seedocs/decisions.md2026-07-12 for the full rationale.