fix(theme): tokenise the accents, so light stops painting dark-tuned colour (#402) - #607
Merged
Conversation
Every contrast figure in the repository is prose: measured once by hand and written into a comment. Prose does not fail when the value it describes moves, which is how a token tuned against a near-black ground survived into a light theme at 1.46:1 (#402). tests/helpers/contrast.ts does the arithmetic the browser does — oklch through Oklab to clipped sRGB, alpha compositing in the gamma-encoded space, WCAG luminance in the linear one — and reads the Tailwind palette out of node_modules rather than transcribing it, so a Tailwind upgrade that restyles blue-400 reports itself as the dark-mode change it is. theme.css gains the accent layer #384 left out of scope: four state roles with two text steps, a wash and a filled ground each, plus a fifteen-hue identity palette selected per mode the way charts/palette.ts selects. Light floor 4.79:1, dark floor 4.97:1, and the light identity set is no tighter anywhere than the dark set it is modelled on.
…colour (#402) 697 colour literals across 68 files move onto the token layer #384 built. Two families, because colour here answers two different questions: brand/warning/ success/danger where it tracks a condition, and a fifteen-hue identity palette where it labels a thing — which engine, which panel, added vs removed, primary key vs foreign key. The state family is brand, not accent. shadcn owns --accent (its neutral hover ground, #f5f5f5) and globals.css maps --color-accent to it AFTER importing the studio layer, so a token of that name loses the cascade in silence and paints near-white text on a white page. The compiled stylesheet is the only place that shows; no unit test can see it. Dark is a byte-for-byte no-op except where a -500 text literal was drift — the codebase used three steps of one hue for one meaning, and those collapse onto the base token. Every such site is listed in the PR. Also: the tests that asserted a hue name now assert the token, and are renamed where the name said 'returns green'.
…nverted hover, and nine repainted dark grounds Three defects an independent review found, none of which any gate could see. The Oklab a-row in the contrast helper was transcribed wrong: -2.4285922050 became -0.2429228246 and the sign on the s term flipped. Contrast never touches Oklab so every ratio stayed correct, but every separation figure was measured in a space that is not Oklab — including the ones that chose the palette. Corrected against Ottosson's published values, with a round-trip check so the next slip fails instead of measuring quietly. In the right space the natural assignment wins: hue-amber returns to amber-800 (the step warning already uses) and yellow's alt to -900; orange moves to -900 instead, because Tailwind's amber ramp rotates into orange as it darkens. Three error pages hovered DOWN the ramp where the rest of the app hovers up. Flattening them onto -solid-hover inverted the direction and took white-on-hover from 6.8:1 to 3.8:1. They get -solid-active, and a test now measures what a filled control's label survives — the AA sweep had excluded solids as grounds, so nothing did. Nine monitoring grounds and borders were routed through success/warning, which are emerald and amber, while the literals were green and yellow: a hue change in dark, outside the stated exception. They take the identity hues that reproduce them exactly. A pair-by-pair audit of the whole diff now reports 93 differences, all of them the sanctioned -500/-200 text drift, and zero others. Also: two assertions that compared an object to itself, a GROUNDS.dark that was never read, a tint ceiling asserted rather than derived (four live sites use /25, one of them the pill this issue is about), and a word boundary that let bg-brand-tint satisfy --studio-brand.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Foreground opacity and a fixed black preview surface still create untested light-mode contrast failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Migrates accent colours to theme-aware state and identity tokens, improving light-mode contrast while preserving dark-mode styling and package consumers.
Changes:
- Adds and documents shared accent tokens.
- Migrates UI colour literals to semantic utilities.
- Adds contrast, token-usage, and regression tests.
File summaries
| File | Description |
|---|---|
| docs/ui/theming.md | Documents accent tokens and validation. |
| src/app/admin/error.tsx | Tokenises error action colour. |
| src/app/error.tsx | Tokenises error action colour. |
| src/app/login/login-form.tsx | Tokenises login accents. |
| src/app/not-found.tsx | Tokenises navigation action colour. |
| src/components/CodeGenerator.tsx | Tokenises generator accents. |
| src/components/CommandPalette.tsx | Tokenises command icons and states. |
| src/components/ConnectionModal.tsx | Tokenises connection UI accents. |
| src/components/CreateTableModal.tsx | Tokenises form and preview accents. |
| src/components/DataCharts.tsx | Tokenises chart controls. |
| src/components/DataImportModal.tsx | Tokenises import workflow states. |
| src/components/DataProfiler.tsx | Tokenises profiler states and identities. |
| src/components/DatabaseDocs.tsx | Tokenises documentation panel colours. |
| src/components/MaskingSettings.tsx | Tokenises masking UI accents. |
| src/components/MobileNav.tsx | Tokenises active navigation state. |
| src/components/PivotTable.tsx | Tokenises pivot controls and values. |
| src/components/QueryEditor.tsx | Tokenises editor actions. |
| src/components/QueryHistory.tsx | Tokenises history states and controls. |
| src/components/QuerySafetyDialog.tsx | Tokenises risk-level styling. |
| src/components/ResultsGrid.tsx | Tokenises grid interactions and values. |
| src/components/SaveQueryModal.tsx | Tokenises save-dialog controls. |
| src/components/SavedQueries.tsx | Tokenises saved-query actions. |
| src/components/SchemaDiagram.tsx | Tokenises diagram controls and warnings. |
| src/components/SchemaDiff.tsx | Tokenises schema-difference identities. |
| src/components/SnapshotTimeline.tsx | Tokenises selection and deletion states. |
| src/components/Studio.tsx | Tokenises handles and warning dialog. |
| src/components/TestDataGenerator.tsx | Tokenises generator colours. |
| src/components/VisualExplain.tsx | Tokenises plan and AI-analysis colours. |
| src/components/admin/AdminDashboard.tsx | Tokenises admin navigation. |
| src/components/admin/tabs/AuditTab.tsx | Tokenises audit states. |
| src/components/admin/tabs/OperationsTab.tsx | Tokenises operation accents. |
| src/components/admin/tabs/OverviewTab.tsx | Tokenises overview accents. |
| src/components/admin/tabs/SecurityTab.tsx | Tokenises security controls. |
| src/components/agent/AgentRail.tsx | Tokenises agent states and controls. |
| src/components/agent/AnswerCard.tsx | Tokenises answer states and identities. |
| src/components/agent/ConsentCard.tsx | Tokenises consent states. |
| src/components/agent/SafetyStrip.tsx | Tokenises posture tones. |
| src/components/community-section.tsx | Tokenises focus rings. |
| src/components/copy-button.tsx | Tokenises copy-failure state. |
| src/components/login/connection-signature.tsx | Tokenises signature accent. |
| src/components/monitoring/MonitoringDashboard.tsx | Tokenises refresh indicator. |
| src/components/monitoring/tabs/OverviewTab.tsx | Tokenises monitoring summary icons. |
| src/components/monitoring/tabs/PerformanceTab.tsx | Tokenises health states. |
| src/components/monitoring/tabs/PoolTab.tsx | Tokenises pool metrics. |
| src/components/monitoring/tabs/QueriesTab.tsx | Tokenises slow-query warning. |
| src/components/monitoring/tabs/SessionsTab.tsx | Tokenises session states. |
| src/components/monitoring/tabs/StorageTab.tsx | Tokenises storage identities. |
| src/components/monitoring/tabs/TablesTab.tsx | Tokenises vacuum states. |
| src/components/results-grid/ResultCard.tsx | Tokenises result-card accents. |
| src/components/results-grid/RowDetailSheet.tsx | Tokenises copy and masking states. |
| src/components/results-grid/StatsBar.tsx | Tokenises result statistics and actions. |
| src/components/results-grid/renderers/binary.ts | Tokenises binary values. |
| src/components/results-grid/renderers/json.ts | Tokenises JSON values. |
| src/components/results-grid/renderers/scalar.ts | Tokenises scalar values. |
| src/components/schema-diagram/TableNode.tsx | Tokenises schema-node identities. |
| src/components/schema-explorer/ColumnList.tsx | Tokenises key and index icons. |
| src/components/schema-explorer/SchemaExplorer.tsx | Tokenises explorer controls. |
| src/components/schema-explorer/TableItem.tsx | Tokenises table actions. |
| src/components/sidebar/ConnectionItem.tsx | Tokenises connection states. |
| src/components/sidebar/Sidebar.tsx | Tokenises sidebar indicators. |
| src/components/studio/BottomPanel.tsx | Tokenises panel identities and notices. |
| src/components/studio/QueryToolbar.tsx | Tokenises query states and actions. |
| src/components/studio/StudioDesktopHeader.tsx | Tokenises desktop header states. |
| src/components/studio/StudioMobileHeader.tsx | Tokenises mobile header states. |
| src/components/studio/StudioTabBar.tsx | Tokenises active tab styling. |
| src/lib/db-ui-config.ts | Assigns tokenised engine identities. |
| src/lib/monitoring-thresholds.ts | Returns tokenised threshold borders. |
| src/styles/theme.css | Defines light and dark accent palettes. |
| src/workspace/StudioWorkspace.tsx | Tokenises embedded workspace accents. |
| tests/components/CommandPalette.test.tsx | Updates palette mocks. |
| tests/components/ConnectionModal.mobile.test.tsx | Updates mobile colour mocks. |
| tests/components/ConnectionModal.test.tsx | Updates connection colour mocks. |
| tests/components/DataProfiler.test.tsx | Verifies profiler tokens. |
| tests/components/MaskingSettings.test.tsx | Verifies danger styling. |
| tests/components/QueryHistory.test.tsx | Verifies history tokens. |
| tests/components/QuerySafetyDialog.test.tsx | Verifies risk tokens. |
| tests/components/ResultsGrid.test.tsx | Updates edit-border selector. |
| tests/components/SchemaDiagram.test.tsx | Verifies diagram highlights. |
| tests/components/SchemaDiff.test.tsx | Verifies identity tints. |
| tests/components/SnapshotTimeline.test.tsx | Verifies selection tokens. |
| tests/components/VisualExplain.test.tsx | Verifies explanation tokens. |
| tests/components/admin/OperationsTab.test.tsx | Updates engine-colour mock. |
| tests/components/admin/OverviewTab.test.tsx | Updates engine-colour mocks. |
| tests/components/agent/AgentRail.test.tsx | Updates token terminology. |
| tests/components/monitoring/PerformanceTab.test.tsx | Verifies monitoring tokens. |
| tests/components/results-grid/RowDetailSheet.test.tsx | Verifies copy-state tokens. |
| tests/components/schema-explorer/ColumnList.test.tsx | Verifies key token. |
| tests/components/sidebar/ConnectionItem.test.tsx | Verifies active token. |
| tests/components/sidebar/ConnectionsList.test.tsx | Verifies list token. |
| tests/components/studio/BottomPanel.test.tsx | Verifies panel identity. |
| tests/components/studio/QueryToolbar.test.tsx | Verifies toolbar states. |
| tests/components/studio/StudioDesktopHeader.test.tsx | Verifies header states. |
| tests/components/studio/StudioTabBar.test.tsx | Verifies active tab token. |
| tests/helpers/contrast.ts | Adds colour and contrast arithmetic. |
| tests/unit/components/column-list.test.tsx | Updates key-colour assertions. |
| tests/unit/components/results-grid-renderers.test.ts | Updates renderer expectations. |
| tests/unit/components/results-grid-utils.test.ts | Updates value-colour expectations. |
| tests/unit/components/studio-mobile-header.test.tsx | Updates mobile-header assertions. |
| tests/unit/lib/monitoring-thresholds.test.ts | Verifies threshold token output. |
| tests/unit/theme-accent-contrast.test.ts | Adds accent contrast gates. |
| tests/unit/theme-token-usage.test.ts | Prevents literals and unused tokens. |
Review details
- Files reviewed: 101/101 changed files
- Comments generated: 12
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -243,12 +243,12 @@ export function CreateTableModal({ isOpen, onClose, onTableCreated }: CreateTabl | |||
| <div className="p-4 rounded-lg bg-black border border-hairline font-mono"> | |||
| col.faker.generator === "autoIncrement" | ||
| ? "bg-overlay text-fg-subtle line-through" | ||
| : "bg-amber-500/10 text-amber-400/80", | ||
| : "bg-hue-amber-tint/10 text-hue-amber/80", |
| {node["Filter"] && ( | ||
| <div className="flex items-start gap-2 py-1 text-xs"> | ||
| <span className="text-amber-500/70 font-medium shrink-0">Filter:</span> | ||
| <span className="text-hue-amber/70 font-medium shrink-0">Filter:</span> |
| <div className="flex items-center gap-2 py-1 text-xs"> | ||
| <span className="text-emerald-500/70 font-medium">Index:</span> | ||
| <span className="text-emerald-400 font-mono">{node["Index Name"]}</span> | ||
| <span className="text-hue-emerald/70 font-medium">Index:</span> |
| data-testid="agent-handover-declined" | ||
| className="mt-0.5 pl-3.5 text-xs text-amber-400/80" | ||
| > | ||
| <p key={declined.id} data-testid="agent-handover-declined" className="mt-0.5 pl-3.5 text-xs text-warning/80"> |
Comment on lines
+145
to
+146
| function worstGround(palettes: Map<string, string>, token: string, ownTint: string): { ratio: number; where: string } { | ||
| const foreground = rgb(palettes, token); |
Comment on lines
+58
to
+59
| | Accent, state | `brand` · `warning` · `success` · `danger`, each with `-bright`, `-tint`, `-solid`, `-solid-hover` | | ||
| | Accent, identity | `hue-<name>`, one per hue the app uses, some with `-alt` (a second step) and `-tint` (its wash) | |
Comment on lines
+379
to
+380
| - **4.5:1** on the five studio grounds, on a wash of its own hue up to `/20`, and on the brand | ||
| tile — WCAG AA for text, in both palettes. |
…ht on a black ground Two external reviews on #607. One found a real regression, one found a real hole in the gate; the smaller items are fixed here too. CreateTableModal's SQL preview is a terminal: a hard `bg-black` in both themes. Routing its text through a mode-aware token made light resolve blue-700 on black — 5.27:1 on main, 2.30:1 here. It is now scoped to the dark palette the way the login hero scopes its own pinned-dark column, and measures 5.27:1 again. The contrast gate only ever measured an OPAQUE foreground, so it could not see `text-warning/80` and its 45 siblings: an opacity modifier composites before anyone reads it, and a token that clears AA at full strength can fall under it faded. The suite now measures those, and holds them to the rule the rest of the issue works to — light no worse than dark — with the sites that still fail pinned by name rather than described in a sentence. Measured against main, this migration improved every one of those 38 sites in light (worst 1.35:1 → 6.80:1) and regressed none, which is why they are a second failure class rather than this one. Also: the Explain button's hover had collapsed onto its own base, because the sanctioned -500 text drift moved the base to what the hover used to be; getThresholdColor drew two of three rings from the identity family and one from the state family, at identical values, so the mix was invisible and is now asserted against; the mode-independence check enumerated suffixes by hand and so skipped brand-solid-active entirely; and both the token comment and the theming doc said nineteen engines where DatabaseType has seventeen.
Member
Author
|
Both reviews checked against the code and re-measured rather than taken on trust. Between them they found one real regression and one real hole in the gate; both are fixed in 0856ac9, along with five smaller confirmed items. Confirmed and fixedThe regression — @copilot-pull-request-reviewer on The gate hole — foreground opacity. Also correct: What the measurement says once it exists, though, refutes the framing that these "reintroduce the contrast defect the migration is intended to remove". Against main, every one of the 38 faded sites improved in light and none regressed:
21 are still under AA, and 9 of those fail in dark too, at values byte-identical to what shipped before this PR. By the issue's own calibration — "accent-on-accent-tint is the one failure class that light has and dark does not" — faded text is a second, symmetric class, and clearing it needs a per-mode alpha (a faded token whose dark value carries the alpha and whose light value is opaque), which is its own design. So: measured, held to light no worse than dark, and the survivors pinned by name so the list can only shrink and the day one is fixed the suite says so. Filing the follow-up. Docs — all three correct. @deepseek — all three correct.
One correction
The VerificationAll three new contracts were mutation-probed: flipping |
…lesystem order CI failed on a list with identical membership and a different order. The scans that read `src` walked it with `readdirSync`, which returns filesystem order, so every list built on them was machine-ordered: the pinned set happened to match the order of the machine it was written on and matched nothing else. Locally it was green, which is the worst version of this bug — the suite was reporting the developer's inode order as a fact about the codebase. Sorted at the walk rather than only at the assertion, in both files that do it, so nothing else built on those scans inherits it. Verified by reversing the walk and re-running: still green, which is the control that a sort matching one machine would not have passed.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #402.
What the issue claimed, and what actually measured
The defect is real and the direction was right. Two figures in the table were not.
text-blue-300onbg-blue-500/15over#fafafa#0a0a0atext-blue-400on its groundtext-amber-300on#fafafatext-amber-500on#fafafaThe two wrong ones look like a mislabelled row and a pre-v4 palette value; neither changes the conclusion. One premise did not survive: "one deliberate literal remains in
src" — there are ~32*-whitesites, though those are #384's neutral scope, not this one.Every line reference in the issue had drifted (
AgentRail.tsx:1028→:1989,BottomPanel.tsx:204→:256, and so on), and the surface is larger than the table: 955 colour literals across 68 files.The load-bearing correction: the obvious light steps do not work.
blue-600measures 4.23 andamber-7004.06 on their own/15tint — both fail. The tint is ~1.19× harder than the plain ground across the whole ramp, and it flips the verdict for exactly the two steps a reviewer reaches for first.And the two acceptance criteria are not in tension. Dark
blue-300sits at 9.48 on its own tint. Nothing in dark needs to move.What this does
Two families, because colour here answers two different questions.
State —
brand/warning/success/danger, each with-bright(emphasis),-tint(the wash it sits on),-solid/-solid-hover(filled grounds). Used where colour tracks a changing condition.Identity —
--studio-hue-<h>for the hues the app uses, some with-alt(a second step) and-tint. Used where colour is a fixed label: which engine, which panel, added vs removed, primary key vs foreign key, number vs boolean. Folding these into four state roles would repaint nineteen engines in four colours. Selected per mode the waylib/charts/palette.tsselects, rather than flipping a ramp — the two modes run out of room in different places.It is
brand, notaccent. shadcn owns--accent(its neutral hover ground,#f5f5f5) andglobals.cssmaps--color-accentto it after importing the studio layer, so a token of that name loses the cascade in silence:.text-accent{color:var(--accent)}— near-white text on a white page. Only the compiled stylesheet shows this; no unit test can see it. Caught in the browser.Acceptance
dist/styles.csscarries all 135 declarations;build:lib+attwgreen.The one deliberate dark change
A
-500(or-200) text literal was drift — the codebase used three steps of one hue for one meaning. Those collapse onto the base token and move one step in dark. 93 sites, all text, no backgrounds or borders. Everything else is byte-for-byte identical, resolved against the installed Tailwind palette rather than transcribed, so a Tailwind upgrade that restylesblue-400fails the suite instead of shipping quietly.Verified in a real browser
Chrome, production build, live PostgreSQL, both themes, resolving computed colour through a canvas because Tailwind emits
color-mix(in oklab, …)for every opacity modifier.The issue's own element — the active agent-mode pill,
bg-blue-500/15— composites to#dbe8fb, exactly the ground the issue predicted:#8ec5ffon#dbe8fb— 1.46 : 1#193cb8on#dbe8fb— 7.11 : 1#8ec5ffon#0e1c2f— 9.48 : 1#8ec5ffon#0e1c2f— 9.48 : 1, unchangedSweeping every accent-coloured node on the studio screen: zero below AA in light, worst 4.51:1.
The measurement is now a gate
Every contrast figure this repo has ever recorded is prose —
charts/palette.ts's header,AgentRail.tsx:2583, the login showcase comment. Prose does not fail when the value it describes moves, which is exactly how a dark-tuned token survived into a light theme for four months.tests/helpers/contrast.tsdoes the arithmetic the browser does (oklch → Oklab → clipped sRGB, compositing in the gamma space, luminance in the linear one) and reads the Tailwind palette out ofnode_modulesrather than transcribing it.theme-accent-contrast.test.tsre-measures every token on every ground each run;theme-token-usage.test.tsfails on a colour literal insrcand on a token nothing reaches.Both were mutation-probed: reverting the light accent to
blue-300, moving a dark value one step, collapsingteal-altontoemerald-alt, colliding amber with orange, lightening a solid below AA, and dropping a token from the@thememap each fail; the unmutated tree is clean.What an independent review found, and what it changed
A five-lens adversarial review raised 49 findings; 17 survived refutation. Three mattered:
The Oklab matrix in my own helper was wrong. The a-row had
-0.2429228246where Ottosson has-2.4285922050, with the sign on the s term flipped. Contrast never touches Oklab, so every ratio stayed correct — but every separation figure was measured in a space that is not Oklab, including the ones that chose the identity palette. Corrected, verified against Ottosson's published values, and given a round-trip check. In the right space the natural assignment wins:hue-amberreturns to amber-800 (the stepwarningalready uses) and yellow's alt to -900; orange moves to -900 instead, because Tailwind's amber ramp rotates into orange as it darkens.Three error pages hovered the wrong way. They used
bg-blue-600 hover:bg-blue-700— hover darkens — where the rest of the app lightens. Flattening them onto-solid-hoverinverted the direction and took white-on-hover from 6.8:1 to 3.8:1. They get-solid-active, and a new test measures what a filled control's label survives; the AA sweep had excluded solids as grounds, so nothing did.Nine monitoring grounds repainted in dark.
bg-green-500→bg-success-tintis emerald, not green;bg-yellow-500→bg-warning-tintis amber. A hue change in dark, outside the stated exception. They now take the identity hues that reproduce them exactly.Also fixed: two assertions that compared an object to itself and could never fail, a
GROUNDS.darkthat was never read, a tint ceiling asserted rather than derived (four live sites use/25, one of them the pill this issue is about — the ladder is now scanned out ofsrc), and a word boundary that letbg-brand-tintsatisfy--studio-brand.Deliberately not done
warning/success/ teal buttons carry a white label at 3.2–3.7:1. Under AA, in both themes, and under AA before this migration. Darkening them is a visible change to a control Accent colours are not tokenised, so they keep dark-tuned values on light grounds #402 never claimed. Pinned by name in the suite so the deferral is a fact rather than a sentence; when it is fixed, that test fails and says so.bg-clip-textheadline) sits inside a subtree that re-declares the dark palette, so the light ramp never reaches it.SchemaDifflikewise. Preserving dark meant preserving the drift. Worth a follow-up; not worth changing dark pixels here.Verification
format·lint(134 warnings, vs 135 on main) ·typecheck·knip·chart:check·channels:showcase:check·readme:check·security:check·test(10771 pass, 0 fail) ·build·build:lib+attw· coverage 45983/45983 lines (100.00%).cc @omerfarukbolat — you filed this; the calibration in it is what made it tractable. Two numbers in the table did not reproduce (noted above), and the tint effect turned out to be the thing that decides which step works.