Adopt the console design system, with the map's palette as dark - #35
Conversation
|
Claude finished @Babissimo's task in 5m 41s —— View job Claude is reviewing this PR
|
9e7558f to
4d89abd
Compare
|
Claude finished @Babissimo's task in 3m 0s —— View job Claude's review
Went through the full diff: Overall: this is unusually careful work — the light/dark token duplication is guarded by One inconsistency worth fixing before merge: PR description contradicts the shipped code on
but
So the actual (correct, and better) decision was to avoid adding No other issues found. Fix this → |
…dark The UI was styled to nothing in particular: a navy gradient header over gradient buttons, drop shadows where the rest of the estate uses hairlines, and forty-odd colour literals spread across four stylesheets and three components. It read as a different product from the consoles it sits beside. Take dash.retina.fm's system wholesale and map.retina.fm's palette as the dark theme, per claude-shared's brand guide. The pairing is the one the estate already makes: dash carries the map's dark ramp as its own dark theme, so this is the same two palettes under the same names, and retina-server's dashboard, its map-surface.css and this now carry one vocabulary. The appearance switch is dash's, ported class for class: the three-state .theme-switch radiogroup, the retina.theme storage key, the ThemeContext whose `system` stamps no attribute and lets the prefers-color-scheme block answer. That third state is why the dark palette is written twice, once per selector — CSS cannot share a declaration block across a media query boundary, so a test guards the two copies against drifting rather than the stylesheet. The console hangs the switch under an "Appearance" label in its avatar menu; there is no signed-in user on this surface, so it sits in the header bar. Only the placement differs. Three things are deliberate departures from the surfaces being copied, each because the guide records the original as a gap rather than an intent: a focus ring (dash has none anywhere), a disabled state on the primary button (dash attaches it to .btn-secondary alone), and a styled error boundary (dash and the map both fall back to an unstyled heading, which on dark is black on white in the middle of the console). The search spinner also stays, against dash's no-spinner-anywhere rule: this query warns it may take a minute. One trap found while building it, silent and now pinned by a test: react-leaflet replays `pathOptions` through setStyle, which drops `className`, so the search-radius ring kept Leaflet's own blue in both themes with nothing erroring. The class is a top-level prop, and an e2e test asserts the computed stroke per theme. The rank ramp and the band chips become tokens rather than literals so they brighten on navy; the bands take owl-os's palette, which exists there because picking a transmitter is the same job this page does. The stylesheet lint is JavaScript rather than TypeScript, which is the one odd-looking thing in here. It reads surface.css off disk, because vite's ?raw transform returns an empty string in this repo and a lint over an empty file passes every assertion in it. Reading a file needs node:fs, which needs @types/node, and adding that rewrites package-lock.json: npm on macOS prunes the other platforms' optional @esbuild builds out of the lock, and CI's clean install then refuses it. tsconfig already sets checkJs: false, so a .js test is run by vitest and skipped by tsc, and the lockfile is left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4d89abd to
4898878
Compare
|
Claude finished @Babissimo's task in 1m 45s —— View job Claude finished reviewing this PR
SummaryThis is a large, well-executed styling migration: 21 files, one design-token system ( Findings1. PR description is out of sync with the actual implementation (minor, but worth fixing) The PR body states:
But This looks like the PR description is a stale draft written before the approach was changed to sidestep the lockfile churn — the in-code comment ( What I verified
I was not able to run |
|
Fixed the stale description: the On the sandbox note — the three suites were run locally before each push, and CI's own |
1 participant
The UI was styled to nothing in particular: a navy gradient header over gradient
buttons, drop shadows where the rest of the estate uses hairlines, and forty-odd
colour literals spread across four stylesheets and three components. It read as a
different product from the consoles it sits beside.
This takes dash.retina.fm's design system wholesale, with map.retina.fm's palette
as the dark theme, per
claude-shared/docs/brand/brand-guide.md. dash alreadycarries the map's dark ramp as its own dark theme, so this is the same two
palettes under the same token names:
retina-server's dashboard, itsmap-surface.cssand this now share one vocabulary.The appearance switch is dash's, ported class for class
Not an approximation of it.
ThemeContextwith the same three states and thesame
retina.themestorage key, and the same.theme-switchradiogroup:Feather sun/monitor/moon, ordered light → system → dark, roving
tabIndex,arrows moving selection and focus together,
aria-labelplustitleon eachbutton since they carry no text.
systemis the default and stamps no attribute, letting theprefers-color-schemeblock answer, so the OS preference needs no JavaScript andkeeps working when the OS changes its mind mid-session. The cost is the dark
palette written twice, once per selector, because CSS cannot share a declaration
block across a media query boundary;
themeTokens.test.ts(ported from dash)guards the two copies against drifting, and also enforces that no palette value
is hardcoded outside the token block.
The console hangs the switch under an "Appearance" label in its avatar menu.
There is no signed-in user on this surface, so it sits in the header bar. Only
the placement differs.
Deliberate departures
Three, each where the guide records the original as a gap rather than an intent:
a focus ring (dash has none anywhere), a disabled state on the primary button
(dash attaches it to
.btn-secondaryalone), and a styled error boundary (dashand the map both fall back to an unstyled heading, which on dark is black on
white in the middle of the console). The search spinner also stays, against
dash's no-spinner-anywhere rule: this query warns it may take a minute.
One silent trap, now pinned by a test
react-leaflet replays
pathOptionsthroughsetStyle, which dropsclassName,so the search-radius ring kept Leaflet's own
#3388ffin both themes withnothing erroring; the build, the type check and the whole suite passed. The class
is a top-level prop now, and an e2e test asserts the computed stroke per theme.
Also
take owl-os's palette, which exists there because picking a transmitter is the
same job this page does.
ResizeObservercallinginvalidateSize: Leaflet caches the container size at init, and adding afrequency row changes it with no window resize to notice.
themeTokens.test.js, JavaScript rather thanTypeScript, which is the one odd-looking thing in here. It reads
surface.cssoff disk because vite's?rawtransform returns an emptystring in this repo, and a lint over an empty file passes every assertion in
it. Reading a file needs
node:fs, which needs@types/node, and addingthat rewrites
package-lock.json: npm on macOS prunes the other platforms'optional
@esbuildbuilds out of the lock and CI's clean install thenrefuses it.
tsconfigalready setscheckJs: false, so a.jstest is runby vitest and skipped by
tsc, andpackage.json,package-lock.jsonandtsconfig.jsonstay byte-identical with main.Verification
90 unit, 19 e2e (Chromium), 681 backend, typecheck and build all green.
Rendered and checked at 1440px, 390px and 320px in both themes; no horizontal
overflow at either phone width.
Brand-guide side: offworldlabs/claude-shared#23.
🤖 Generated with Claude Code