Offer a dark theme on dash and admin by Babissimo · Pull Request #382 · offworldlabs/retina-server

Babissimo · 2026-09-14T16:52:50Z

Adds a light/dark/system appearance control to dash.retina.fm and
admin.retina.fm (one bundle, one change), modelled on the map's theming and
taking the map's dark palette rather than inventing a sixth.

The data explorer is the other half of the request and lands separately, since
it is a different app with no code in common: #383.

How it works

:root keeps today's light tokens and dark arrives behind [data-theme="dark"]
— the opposite way round from the map, which carries dark on its base selector.
Same reasoning both times: the attribute is stamped from JavaScript, so
whichever theme depends on it is the one that can flash the other on first
paint, and that must never be the default. The guide makes light the default for
anything that explains or configures.

The control has three states rather than the map's two. System stamps no
attribute at all and lets a prefers-color-scheme block answer, so the OS
preference needs no JavaScript and keeps working when the OS changes mid-session;
light and dark override it via :root:not([data-theme="light"]). It lives in the
existing header dropdown.

Three things worth a reviewer's attention

Recharts cannot read the cascade. It paints from props, so the chart chrome
is a second, hand-kept copy of the tokens in utils/chartTheme.ts;
chartTheme.test.ts asserts the two agree. Related: AnomalyPage was already
passing stroke="var(--border)" into SVG presentation attributes, where CSS
does not resolve var() — those axes have been drawing at the initial value,
and now draw.

--accent-ink is new. Dark's accent is a bright sky blue, so the white
that inked the primary button and the sidebar mark drops to about 1.8:1 there.
Both now take a token: white on light, near-black on dark. The map still inks
#fff on the same fill — that is a real contrast bug there, left alone rather
than copied.

The pre-paint script is a file, not an inline tag. Every page vhost sends
script-src 'self' (deploy/nginx/snippets/security-headers-page.conf), which
does not cover inline code, while the dev server sends no CSP at all — so an
inline version works in every local check and silently never runs once deployed.
public/theme-boot.js is outside the module graph by necessity, so a test
asserts it uses the same storage key and attribute as ThemeContext.

Two smaller things: vite.config.js gains css: true so the token tests can
read App.css?raw (vitest stubs CSS imports with an empty string otherwise,
the raw query included), and NetworkHealthPage's node markers deliberately
keep their light values — they are drawn onto OSM tiles, which do not theme.

Verification

typecheck, lint and build clean; 95 tests pass (62 before, 33 new
covering the preference, the attribute, OS-following, storage failure, the two
dark blocks agreeing, the chart palette and the boot script).

Checked in a real browser across all three settings: system follows the OS and
follows it changing, an explicit light beats a dark OS, an explicit choice
survives a reload, and the tooltip/grid/axes take themed values
(background: rgb(19, 34, 64), border: rgba(100, 180, 255, 0.28),
color: rgb(226, 232, 240)).

Not verified, and worth a look after deploy: anything behind auth — the
sidebar, tables and the NetworkHealthPage Leaflet map under the new dark tile
filter — since there is no backend locally to log into.

Follow-ups

The brand guide in claude-shared now describes the estate inaccurately: it
calls the map "the only surface that themes". Worth a small PR there.

🤖 Generated with Claude Code

Babissimo · 2026-09-14T16:59:30Z

Fixed the OverviewPage Area fill — good catch, and a real mismatch: stroke would have followed the palette into dark while the fill stayed on light's blue.

Rather than just fixing the one site, I added the sweep that would have caught it. chartTheme.test.ts now reads every page under src/pages/ and asserts none hardcodes a colour, with three documented exemptions (the Google mark, the OSM-bound node markers, and AnomalyPage's per-theme domain palette). Confirmed it fails by reintroducing the bug before restoring the fix — the stylesheet's own sweep only ever looked at App.css, and it matched #hex but not rgba(), which is exactly how this slipped past.

Folded into the original commit rather than stacked on top, per the repo's commit guidance. 118 tests pass; typecheck, lint and build clean.

Babissimo · 2026-09-14T17:06:13Z

CI is green. One note on the way there: backend-tests failed once on this branch and it was not this change.

test_known_lane.py::TestMaybeRunPass::test_interval_gate_holds_between_passes failed its first assert (assert 0 == 1), meaning the pass-interval clock was already closed when the test body started. This branch touches no Python at all; the same commit's earlier run passed, #383 off the same base passed, and a re-run of the identical commit passed.

It is a known race rather than a new one: the autouse fixture in that file already carries a comment describing this exact symptom (dated 2026-09-06) and calls known_lane._reset_for_tests(), which does zero _last_pass_ts. So the gate is being re-closed after the fixture runs — the leaked solver-worker daemons the same docstring warns about, polling maybe_run_pass concurrently. Under --dist worksteal the interleaving varies, which is why it is rare.

Filed as https://app.clickup.com/t/123zgec2ry0 rather than fixed here, since a harder reset would not address it — the race is with a live daemon, not with leftover state.

Babissimo · 2026-09-14T17:08:16Z

Fixed — and this one was a better catch than the last, because it is the same bug class --accent-ink exists to fix, hiding where the token could not reach it.

Both Discord links carried an inline color: "white" on a .btn-primary. An inline style outranks the class rule, so those two buttons would have kept white on the dark accent (~1.8:1) while every other primary button took the themed ink. Removing the inline style is enough: .btn-primary (0,1,0) already outranks the a element rule (0,0,1), so they stay white in light mode via --accent-ink: #ffffff.

Verified in a browser with data-theme="dark": the button now computes rgb(8, 47, 73) on rgb(56, 189, 248) — about 9.4:1.

You also identified precisely why my sweep missed it, so I extended it rather than only fixing the two sites: the page sweep now also rejects a named colour in a style object (color, background, fill, stroke…), alongside the hex/rgba() check. Confirmed it fails by reintroducing the bug before restoring the fix. 143 tests pass; typecheck, lint and build clean.

Folded into the same commit again.

The console has only ever had the light palette, which makes it the odd one out next to the map — the map has themed since it borrowed dash's whole vocabulary, and the data explorer has answered the OS preference all along. Someone working across the three at night meets a white page on two of them. The mechanism is the map's, with the two themes the other way round. The map carries dark on its base selector and light behind the attribute, because dark is its default and the default must never be the theme that flashes on first paint; light is the default here, so light stays on the bare selector and dark is what the attribute buys. The palette is the map's dark column wholesale rather than a new one: the brand guide already records the two as counterparts, and the estate's own §10 says drift is its main failure mode. The control has three states rather than the map's two. `system` stamps no attribute and lets a prefers-color-scheme block answer, so the OS preference is honoured with no JavaScript and keeps working when the OS changes its mind mid-session; light and dark override it. That also means the data explorer, which has behaved that way since it was written, can gain a switch without regressing anyone. Three things the diff does not explain on its own: - Recharts paints from props, not from the cascade, so `var()` cannot reach it. The chart chrome moves into a second, hand-kept copy of the tokens, and a test asserts the two agree. AnomalyPage was already passing `stroke="var(--border)"` into SVG presentation attributes, where CSS does not resolve var() at all — those axes were drawing at the initial value, and now draw. - Dark's accent is a bright sky blue, so the white that inks the primary button and the sidebar mark drops to about 1.8:1 there. Both now take `--accent-ink`, near-black on dark. The map still inks `#fff` on the same fill; that is its bug to fix, not one to copy. - The pre-paint script is a file, not an inline tag. Every page vhost sends `script-src 'self'`, which does not cover inline code, while the dev server sends no CSP — an inline version works in every local check and silently never runs once deployed. A colour written into a page is pinned to one theme, and no token block covers a chart's props or an inline style, so the pages get their own sweep. It catches both shapes this change had: an Area whose stroke follows the palette while its fill does not, and a `color: "white"` sitting on a .btn-primary, where an inline style outranks the class rule and quietly undoes --accent-ink. Leaflet needs three rules rather than one. Its tiles take a filter, since OSM does not theme; its container takes the sunk tier; and its own attribution and zoom controls are hardcoded white and #333 in leaflet.css, so on a dark card they read as two bright chips stuck to the corners until they are told otherwise. The switch is three icons rather than three words: Feather's sun, monitor and moon, ordered light to dark with the neutral between them, hand-inlined in the same house style as the sidebar's set (24-unit box, no fill, 2-unit round-capped stroke in currentColor) since no icon package is installed and three glyphs do not justify adding one. Drawing in currentColor is what lets the active segment tint itself from the same rule that sets its ink. A glyph names nothing on its own, so each button carries an aria-label and a matching title, and the svg is aria-hidden so the name is not read twice. Choosing role=radiogroup over three independent toggles is what obliges the keyboard half: the set is announced as one control with three options, so it is one tab stop rather than three (a roving tabIndex on whichever is checked) and the arrows move within it, selecting as they go. Home and End reach the ends. Everything else passes through — without preventDefault the arrows scroll the dropdown while the selection moves underneath. Verified in a browser across all three settings: system follows the OS and follows it changing, an explicit choice overrides it and survives a reload, and the tooltip, grid and axes take their themed values. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude · 2026-09-15T10:59:18Z

Claude finished @Babissimo's task in 2m 14s —— View job

Review: dark theme for dash and admin (PR #382)

Gather context (PR body, changed files, CLAUDE.md, prior review rounds + author responses)
Understand the request (custom instruction: run /code-review on PR Offer a dark theme on dash and admin #382)
Run code review over the latest diff (796c8a1..HEAD, this commit's sole parent — same shallow-clone quirk as prior passes: origin/main has moved past this PR via later unrelated merges, so there's no merge-base for origin/main...HEAD)
Post final review feedback

This synchronize addresses the one open item from the last review round: Header.tsx's appearance switch now implements roving-tabIndex/arrow-key navigation for its role="radiogroup". I traced that addition line-by-line rather than taking the diff size at face value, and independently re-verified the core theming mechanism (not just the delta) since a fresh pass shouldn't just trust the prior summary.

Verified correct:

Header.tsx:70-102 (onRadioKeyDown) — ArrowLeft/Up and ArrowRight/Down move with wraparound, Home/End jump to the ends, unhandled keys return before preventDefault() so they aren't swallowed. Only the checked button carries tabIndex={0} (Header.tsx:169), the rest -1 — correct roving-tabindex pattern, and .focus() on a tabIndex={-1} element still works since it's explicitly focusable.
dashboard/src/test/appearanceSwitch.test.tsx:102-167 covers all of it: one tab stop, left/right movement with focus following selection, wraparound both directions, Home/End, "swallows only the keys it handles" (asserts fireEvent.keyDown returns false for ArrowRight, true for an unhandled key), and that an arrow selection reaches document.documentElement's data-theme attribute, not just component state.
ThemeContext.tsx — system/light/dark resolution, storage-read/write wrapped in try/catch, matchMedia listener cleanup, and the useResolvedTheme() no-provider fallback for chart-only tests all check out.
theme-boot.js / index.html — script tag precedes the stylesheet with no defer, agrees with ThemeContext.tsx on both the storage key (retina.theme) and the attribute, and only stamps an explicit light/dark (leaves system unstamped for the CSS media query to answer, avoiding a flash either way).
chartTheme.ts / chartTheme.test.ts — chart chrome tokens match App.css's --border/--text-muted/--bg-card/--text-primary per theme via a live read of App.css?raw, the series palettes share no colour between themes, and seriesColour wraps past the 10th category.
--accent-ink (App.css:40,92,122) — #ffffff on light's #3b82f6 accent, #082f49 on dark's #38bdf8, applied consistently at all three usage sites (App.css:207,279,706).
AnomalyPage.tsx:68-75 — typeColour() uses Object.prototype.hasOwnProperty.call against the backend-supplied type string, closing the constructor/toString prototype-chain footgun a plain palette[type] lookup would have.

No blocking issues found, and no new issues introduced by this push. This PR is in good shape.
(branch: feat/dashboard-dark-mode)