Skip to content

Give the archive browser a theme switch - #383

Open
Babissimo wants to merge 1 commit into
mainfrom
feat/data-explorer-dark-mode
Open

Babissimo wants to merge 1 commit into
mainfrom
feat/data-explorer-dark-mode

Conversation

@Babissimo

Copy link
Copy Markdown
Contributor

Adds a light/dark/system switch to data.retina.fm and moves its dark palette
onto the map's.

Sibling of #382, which does the same for dash and
admin. The two share no code — different apps, different build story — so this
branches from main rather than stacking, and either can land first.

What was already there

This surface is not gaining dark mode; it has answered prefers-color-scheme
since it was written. What it lacked was a way to say so: someone on a light
OS who wants dark, or on a dark OS reading in daylight, had no control. The OS
preference stays the default and gains two overrides.

What changes visibly

The dark values move onto the map's — canvas #0d1b2a, card #132240, accent
#38bdf8, and the brightened semantics — from the slate set they had. That set
was effectively a third palette: the light tokens were already verbatim from the
console, but the dark ones matched neither the console nor the map, and the brand
guide's §10 names exactly this drift as the estate's main failure mode. Anyone
browsing in dark today will notice
; nobody loses a theme.

--accent-ink is new because the dark accent is a bright sky blue: the #fff
that inked the brand mark and the primary button sits at about 1.8:1 on it.

Two details worth a reviewer's attention

The pre-paint script is a file, not an inline tag — for precisely the reason
vendor/ exists. This vhost sends script-src 'self', which does not cover
inline code, while a laptop serving these files sends no CSP at all: an inline
version works in every local check and silently never runs once deployed.

Opening the page persists nothing. This surface is public and
unauthenticated, so showTheme (stamp and mark the control) and chooseTheme
(also write storage) are separate, and only a click writes.

The retina.theme key and the data-theme attribute are the names the dashboard
console uses for the same job. Separate origins, so a stored choice cannot travel
between them — the matching names are what make the two one idiom rather than two.

Verification

No test suite or build step here, so this was checked in a browser:

storage data-theme canvas
first visit not written absent follows the OS
clicked Dark dark dark rgb(13, 27, 42)
back to System system absent follows the OS again

An explicit choice survives a reload, and an explicit light beats a dark OS.

Follow-up

The brand guide in claude-shared describes this surface as untheme-able and the
map as "the only surface that themes". Worth a small PR there once both of these
land.

🤖 Generated with Claude Code

It has answered `prefers-color-scheme` since it was written, with a
comment saying there is deliberately no switch — so someone on a light
OS who wants dark, or on a dark OS reading in daylight, has no way to
say so. The OS preference stays the default and gains two overrides.

The dark values move onto the map's (#0d1b2a canvas, #132240 card,
#38bdf8 accent, and the brightened semantics) from the slate set they
had. That set was a third palette: the light tokens were already
verbatim from the console, but the dark ones answered to neither the
console nor the map, and the brand guide's §10 names drift as the
estate's main failure mode. Anyone browsing in dark today will see the
change; nobody loses a theme.

`system` stamps no attribute and lets the media query answer, which is
what keeps the OS preference working when it changes mid-session;
`:root:not([data-theme="light"])` is what lets an explicit light choice
win over a dark OS. Stamping a resolved value instead would pin the
page to whatever the OS was at load.

Two details the diff does not explain:

- The pre-paint script is a file rather than an inline tag, for exactly
  the reason vendor/ exists: this vhost sends `script-src 'self'`,
  which does not cover inline code, while a laptop serving these files
  sends none — so an inline version works in every local check and
  silently never runs once deployed.

- Merely opening the page persists nothing. This surface is public and
  unauthenticated, so `showTheme` and `chooseTheme` are separate and
  only a click on the control writes storage.

`--accent-ink` is new because the dark accent is a bright sky blue: the
white that inked the brand mark and the primary button is about 1.8:1
on it.

Verified in a browser: a first visit writes no storage and follows the
OS, a click persists and survives a reload, and returning to system
clears the attribute and hands control back to the media query.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 14, 2026

Copy link
Copy Markdown

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


Code review

  • Check PR eligibility for review (open, non-draft, not already reviewed)
  • Gather relevant CLAUDE.md files
  • Summarize the PR changes
  • Review for CLAUDE.md compliance (2 parallel passes)
  • Review for bugs / correctness issues (2 parallel passes)
  • Validate flagged issues
  • Post final findings

No issues found. Checked for bugs and CLAUDE.md compliance.

Summary: This PR adds a System/Light/Dark switch to data-explorer (index.html, app.js, app.css) plus a pre-paint theme-boot.js to avoid a flash of the wrong theme, and moves the dark palette onto the map's own tokens (frontend/src/map-surface.css) instead of the previous ad-hoc slate set.

What I checked in detail:

  • Three-state CSS cascade — traced all combinations of {system, light, dark} × {OS light, OS dark} through :root, @media (prefers-color-scheme:dark){:root:not([data-theme="light"])}, and :root[data-theme="dark"] in data-explorer/app.css. Each resolves to the palette described in the PR's verification table.
  • Script order / DOM timingtheme-boot.js loads without defer in <head> before first paint; app.js loads at the end of <body> after the #theme-switch markup already exists, so the click listener and showTheme(storedTheme()) in app.js don't hit a null element.
  • theme-boot.js vs app.js — the two are redundant-but-harmless (idempotent), not conflicting; theme-boot.js can't touch the switch buttons since they don't exist yet when it runs.
  • localStorage access — both read (storedTheme) and write (chooseTheme) paths are wrapped in try/catch with sensible fallbacks for private browsing.
  • Color claims — spot-checked that the new dark hex values (#0d1b2a, #132240, #38bdf8, #0f2035, etc.) genuinely match frontend/src/map-surface.css, and that the CSP claim (script-src 'self', motivating theme-boot.js as a file rather than inline) matches deploy/nginx/snippets/security-headers-page.conf.
  • CLAUDE.md compliance — diff touches only data-explorer/, no backend/nodes-API/contract files, no backend/.env, no deploy tooling or host addresses, and no credentials/secrets in the added comments.

No high-signal bugs or CLAUDE.md violations found by either independent review pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant