From 57d128fa72a535ea9b06064eac167c7ee06f40e7 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Date: Fri, 14 Aug 2026 16:24:08 +0000 Subject: [PATCH] fix(ui): seventeen error messages that were never red, and a test so it cannot happen again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--color-danger` was never defined. Seventeen places say `text-danger` or `bg-danger-soft` — the failed import, the calendar that would not fetch, the model that would not download, the agent that refused, every `role="alert"` in onboarding — and Tailwind v4 emits a utility only when it can resolve the token behind it. Emitting nothing is not an error: no rule, no warning, and no visible sign except that the alert is the same colour as the sentence above it. `.text-danger` appears zero times in the built stylesheet. The same fault in a second place: `border-ok/30 bg-ok-soft` on the onboarding success panel, against a `--color-ok` that does not exist either. Pointed at the accent, which is what that panel meant; a fifth green would have been a fifth thing to keep in step. ## The test `theme.test.ts` checked that what the theme *declares* survives into the bundle. It now also checks the other direction — that every `text-`, `bg-`, `border-` and `ring-` colour the interface *names* was ever declared. Nothing else could have caught this. The type checker sees a string, the linter sees a string, and the screenshot audit is happy: grey text on the page background is a perfectly legible contrast pair, so it scored those alerts as passing. Tailwind overloads these prefixes, so the check resolves a name against `--color-*` first, then the size, radius, font and shadow scales (`text-meta` is a size, `rounded-card` a radius), then peels off an edge or an offset (`border-l-accent`, `ring-offset-bg`). Comments and arbitrary values are stripped before scanning: a doc comment saying why `bg-soft` was rejected must not read as a use of it, and `bg-[linear-gradient(…var(--color-bg-elevated)…)]` is CSS passing through rather than a utility. ## Two empty screens that read as broken The voice book put "GIỌNG ĐÃ BIẾT" at the top of a blank pane with "no voices yet" four hundred pixels below — two answers to the same question with a void between them. The heading now only appears over a list. The calendar centred its empty state in what was left under the subscribe form, which put the explanation of why there is nothing three hundred pixels beneath the box that fixes it. That screen no longer asks for `full`, which means "this is the whole screen" and was never true here. Capping `full` centrally was tried first and reverted: it made every screen where the empty state genuinely *is* the content fail `e2e/density.mjs` with four hundred pixels of background under a centred message. The comment in `Empty` now says so, because the next person to notice the calendar gap will reach for the same fix. Also: the notes pane's empty state offers the button that fixes it. It was a dead end whose only way out was a 288px column away in the corner of another pane. All 18 browser suites green, 298 unit tests, 44 screenshots re-shot in both schemes with no overflow and contrast AA everywhere. --- apps/web/src/components/People.tsx | 14 ++- apps/web/src/components/onboarding/Setup.tsx | 2 +- apps/web/src/components/ui/Empty.tsx | 6 ++ apps/web/src/screens/AgendaScreen.tsx | 8 +- apps/web/src/screens/NotesScreen.tsx | 14 ++- apps/web/src/styles/theme.css | 20 ++++ apps/web/src/styles/theme.test.ts | 108 ++++++++++++++++++- 7 files changed, 163 insertions(+), 9 deletions(-) diff --git a/apps/web/src/components/People.tsx b/apps/web/src/components/People.tsx index e105641..d54eb34 100644 --- a/apps/web/src/components/People.tsx +++ b/apps/web/src/components/People.tsx @@ -212,9 +212,17 @@ export function People({ client, meeting }: Props) { )} - {t("people.known")} - {space && ( -

{t("people.identified_by", { space })}

+ {/* Only over a list. A heading above nothing is a heading that promises content the screen + does not have, and on a new vault this one sat alone at the top of an empty pane with the + "no voices yet" message four hundred pixels below it — two separate answers to the same + question, neither next to the other. */} + {people.length > 0 && ( + <> + {t("people.known")} + {space && ( +

{t("people.identified_by", { space })}

+ )} + )} {people.length === 0 ? ( diff --git a/apps/web/src/components/onboarding/Setup.tsx b/apps/web/src/components/onboarding/Setup.tsx index 7c0d7f0..3bc51e8 100644 --- a/apps/web/src/components/onboarding/Setup.tsx +++ b/apps/web/src/components/onboarding/Setup.tsx @@ -302,7 +302,7 @@ export function Setup({ onDone }: { onDone: () => void }) { ) : ( -
+

{t("setup.ready")}

{t("setup.ready_hint")}

diff --git a/apps/web/src/components/ui/Empty.tsx b/apps/web/src/components/ui/Empty.tsx index e4aecdb..3a6091e 100644 --- a/apps/web/src/components/ui/Empty.tsx +++ b/apps/web/src/components/ui/Empty.tsx @@ -55,6 +55,12 @@ export function Empty({ transition={GENTLE} className={cn( "flex flex-col items-center justify-center gap-3 px-6 text-center", + // Uncapped on purpose, and it was tried the other way. Bounding this so it would sit closer + // to a form above it made every screen where the empty state *is* the content fail + // `e2e/density.mjs` — four hundred pixels of background under a message that had been + // centred. The gap-after-a-form problem is real but it belongs to the caller: `full` means + // "this is the whole screen", and the screen with a subscribe form above it should not be + // asking for it. See `AgendaScreen`. full ? "h-full py-10" : "py-14", className, )} diff --git a/apps/web/src/screens/AgendaScreen.tsx b/apps/web/src/screens/AgendaScreen.tsx index c443bf2..5ec957d 100644 --- a/apps/web/src/screens/AgendaScreen.tsx +++ b/apps/web/src/screens/AgendaScreen.tsx @@ -56,9 +56,11 @@ export function AgendaScreen() { void refresh()} /> {entries.length === 0 ? ( - // `full`, so an empty calendar centres itself in what is left of the pane instead of - // hanging in the upper third with a screen-height of background under it. - + // Not `full`. This screen has a subscribe form above it, and centring in what is left put + // the explanation of *why there is nothing* three hundred pixels below the box that fixes + // it — two halves of one sentence with a void between them. `full` is for a screen whose + // only content is the empty state, which this is not. + ) : (
{grouped.map(([day, items]) => ( diff --git a/apps/web/src/screens/NotesScreen.tsx b/apps/web/src/screens/NotesScreen.tsx index 4642b81..5ece3b8 100644 --- a/apps/web/src/screens/NotesScreen.tsx +++ b/apps/web/src/screens/NotesScreen.tsx @@ -267,7 +267,19 @@ export function NotesScreen() { // Centred in the pane rather than pinned a fixed distance from the top: `mt-24` puts a // grey sentence in the upper third of a tall empty column, which is what the whole // interface used to look like. - + // With a way out. An empty state that only describes the emptiness is a dead end, and + // this one is the largest surface on the screen for anybody who has not written a note + // yet — the button they need is a 288px column away in the corner of another pane. + void create("blank")}> + {t("notes.new")} + + } + /> ) : ( <>
diff --git a/apps/web/src/styles/theme.css b/apps/web/src/styles/theme.css index 3918bfb..ff00adc 100644 --- a/apps/web/src/styles/theme.css +++ b/apps/web/src/styles/theme.css @@ -67,6 +67,22 @@ --color-rec: #fb6b70; --color-rec-soft: #fb6b701a; + /* Something went wrong. + * + * This token did not exist. Seventeen places said `text-danger` or `bg-danger-soft` — the failed + * import, the calendar that would not fetch, the model that would not download, the agent that + * refused, every `role="alert"` in onboarding — and Tailwind v4 emits only the utilities whose + * variable it can find, so `.text-danger` was never in the bundle at all. Every error message in + * this app has been rendering in ordinary body colour, which is exactly as loud as the sentence + * above it. + * + * The same red as recording, deliberately. Two reds would be two things to learn, and the + * contexts never meet: recording is a disc and a chip in the header, an error is a sentence + * announced to a screen reader. Kept as its own name so a future change to one does not silently + * move the other. */ + --color-danger: #fb6b70; + --color-danger-soft: #fb6b701a; + /* Task and job states. Semantic, not decorative. */ --color-done: #64e3a1; --color-running: #7aa2f7; @@ -190,6 +206,8 @@ --color-ai-fg: #ffffff; --color-rec: #c62f32; --color-rec-soft: #c62f3214; + --color-danger: #c62f32; + --color-danger-soft: #c62f3214; --color-done: #0f7350; --color-running: #2f5fc0; --color-running-soft: #2f5fc014; @@ -284,6 +302,8 @@ --color-ai-fg: #120a26; --color-rec: #fb6b70; --color-rec-soft: #fb6b701a; + --color-danger: #fb6b70; + --color-danger-soft: #fb6b701a; --avatar-fill-l: 0.72; --avatar-fill-c: 0.11; --avatar-fill-a: 0.22; diff --git a/apps/web/src/styles/theme.test.ts b/apps/web/src/styles/theme.test.ts index f47a89a..b662a1b 100644 --- a/apps/web/src/styles/theme.test.ts +++ b/apps/web/src/styles/theme.test.ts @@ -1,4 +1,5 @@ -import { readFileSync } from "node:fs"; +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; @@ -67,3 +68,108 @@ describe("swatch tokens", () => { expect(theme).not.toContain("--color-swatch-"); }); }); + +/** + * Every colour a component asks for is a colour the theme defines. + * + * The failure this catches is silent in a way that is worth spelling out. Tailwind v4 generates a + * utility only when it can resolve the token behind it, and generating nothing is not an error — + * `class="text-danger"` with no `--color-danger` produces no rule, no warning, and no visible sign + * except that the element is the colour it would have been anyway. `text-danger` was used in + * seventeen places for over a year: the failed import, the calendar that would not fetch, the model + * that would not download, every `role="alert"` in onboarding. All of them rendered in ordinary + * body colour, and every check in this repository passed. The screenshot audit could not see it + * either — grey text on the page background is a perfectly legible contrast pair. + * + * So the direction matters: the other tests here check that what the theme declares survives into + * the bundle, and this one checks that what the interface *asks for* was ever declared. + */ +describe("colour utilities", () => { + const css = read("./theme.css"); + + /** Every `--color-*` the theme defines, anywhere in the file. */ + const defined = new Set([...css.matchAll(/--color-([a-z0-9-]+):/g)].map((m) => m[1]!)); + + /** + * The four prefixes that are colours often enough to be worth checking, and the values they take + * that are *not* colours. + * + * Tailwind overloads these: `text-sm` is a size, `bg-cover` is a fit, `border-b` is an edge. The + * list is static, it is Tailwind's rather than ours, and a false positive costs one line here — + * which is the right trade against a whole class of silently-dead styling. + */ + const NOT_A_COLOUR = new Set([ + // text- + ...["xs", "sm", "base", "lg", "xl", "2xl", "3xl", "4xl", "5xl", "6xl", "7xl", "8xl", "9xl"], + ...["left", "center", "right", "justify", "start", "end", "balance", "pretty"], + ...["wrap", "nowrap", "ellipsis", "clip"], + // bg- + ...["fixed", "local", "scroll", "cover", "contain", "none", "repeat", "no-repeat"], + ...["top", "bottom", "auto", "clip-text", "clip-border", "clip-padding", "clip-content"], + // border- + ...["0", "2", "4", "8", "x", "y", "s", "e", "t", "r", "b", "l"], + ...["solid", "dashed", "dotted", "double", "hidden", "collapse", "separate"], + // ring- + ...["1", "3", "inset", "offset"], + // Tailwind's own palette, used directly in a few places where a literal is honest. + ...["white", "black", "transparent", "current", "inherit"], + ]); + + /** Every `.tsx` and `.ts` under `src`. */ + function sources(dir: string): string[] { + const here = fileURLToPath(new URL(dir, import.meta.url)); + const out: string[] = []; + for (const entry of readdirSync(here, { withFileTypes: true })) { + const path = join(dir, entry.name); + if (entry.isDirectory()) out.push(...sources(`${path}/`)); + else if (/\.tsx?$/.test(entry.name) && !entry.name.endsWith(".test.ts")) out.push(path); + } + return out; + } + + it("has a value behind every text-, bg-, border- and ring- colour the interface names", () => { + const files = sources("../"); + expect(files.length, "no sources found — this test is checking nothing").toBeGreaterThan(20); + + /** Whether Tailwind can resolve `-` to something real. */ + const resolves = (name: string): boolean => { + if (defined.has(name) || NOT_A_COLOUR.has(name)) return true; + // `text-meta` is a size, not a colour, and `rounded-card` a radius. Same shape, other token. + if (new RegExp(`--(?:text|radius|font|shadow)-${name}:`).test(css)) return true; + // `border-l-accent`, `border-b-0`, `ring-offset-bg`: an edge or an offset, then the value. + const [head, ...rest] = name.split("-"); + if (rest.length > 0 && /^(?:x|y|s|e|t|r|b|l|offset)$/.test(head!)) { + return resolves(rest.join("-")); + } + return false; + }; + + const missing = new Map(); + for (const file of files) { + // Comments first: this file's own prose names utilities in backticks, and a doc comment + // explaining why `bg-soft` was rejected must not read as a use of it. + // + // Arbitrary values are Tailwind passing CSS straight through, and the CSS inside them says + // things like `var(--color-bg-elevated)` and `transition-[background,border-color]` that look + // exactly like a utility and are not one. Dropped before scanning rather than pattern-matched + // around, which is what a lookbehind alone could not do. + const source = read(file) + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/[^\n]*/g, "") + .replace(/\[[^\]]*\]/g, ""); + // The name only, without the opacity suffix: `text-accent/60` is `accent`. The lookbehind is + // what stops `--avatar-text-c` and `--color-bg-elevated` reading as utilities. + for (const [, name] of source.matchAll( + /(? `${name} (first seen in ${file})`), + "these utilities generate no CSS at all", + ).toEqual([]); + }); +});