Public: anyone can read this; the crew can change it.
Home: buttons never show a loading spinner at rest #3040
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3039-home-buttons-never-show-a-loading-spinne → main 4 commits fork 3 threads · 3 unresolved
src/web/style/components.ts +3
| @@ -181,6 +181,9 @@ fieldset.field > .check + .check { margin-top: 8px; } | ||
| 181 | 181 | .badge .ico { width: 12px; height: 12px; } |
| 182 | 182 | /* A loader that turns: the one icon that means "still going". */ |
| 183 | 183 | .ico-loader { animation: spin 1.1s linear infinite; } |
| 184 | /* A button never spins at rest: only its own busy state turns the loader. */ | |
| 185 | .btn .ico-loader { animation-play-state: paused; } | |
Good: it only changes the play state, so no new animation and the reduced-motion ratchet is untouched. Follow-up: no code adds aria-busy to a submitting button yet, so this running rule is currently unreachable. |
||
| 186 | .btn[aria-busy="true"] .ico-loader { animation-play-state: running; } | |
| 184 | 187 | @keyframes spin { to { transform: rotate(360deg); } } |
| 185 | 188 | |
| 186 | 189 | /* One stored hex, derived three ways, so a chip reads correctly in all six |
test/button-rest.test.ts +29 −21
| @@ -1,26 +1,34 @@ | ||
| 1 | import { expect, test } from "bun:test"; | |
| 1 | /** #3039: a button never shows a spinner at rest; only its own busy state turns the loader. */ | |
| 2 | import { describe, expect, test } from "bun:test"; | |
| 2 | 3 | import { Glob } from "bun"; |
| 4 | import { components } from "../src/web/style/components.ts"; | |
| 3 | 5 | |
| 4 | // A button at rest must not spin: `.ico-loader` always animates, so a | |
| 5 | // btn(...) whose icon is "loader" looks busy before anyone clicks it. | |
| 6 | test("no btn(...) call in src/web rests on the loader icon", async () => { | |
| 7 | const offenders: string[] = []; | |
| 8 | for (const path of new Glob("src/web/**/*.ts").scanSync(".")) { | |
| 9 | const text = await Bun.file(path).text(); | |
| 10 | for (const m of text.matchAll(/\bbtn\(/g)) { | |
| 11 | // Read the call's argument list by matching parentheses. | |
| 12 | let depth = 0; | |
| 13 | let end = m.index! + m[0].length - 1; | |
| 14 | for (; end < text.length; end++) { | |
| 15 | if (text[end] === "(") depth++; | |
| 16 | else if (text[end] === ")" && --depth === 0) break; | |
| 17 | } | |
| 18 | const call = text.slice(m.index!, end + 1); | |
| 19 | if (/\bicon:\s*["']loader["']/.test(call)) { | |
| 20 | const line = text.slice(0, m.index).split("\n").length; | |
| 21 | offenders.push(`${path}:${line}`); | |
| 6 | describe("buttons at rest", () => { | |
| 7 | test("stylesheet stops the loader inside a button unless it is aria-busy", () => { | |
| 8 | expect(components).toContain(".btn .ico-loader { animation-play-state: paused; }"); | |
outdated
This pins the exact CSS text, so reformatting the rule fails the test even when the behaviour is unchanged. Prefer rendering btn() and the two pages and asserting on the output. Asserting the exact rule string is brittle. A whitespace edit fails it with no behaviour change. Prefer a regex tolerant of spacing. |
||
| 9 | expect(components).toContain('.btn[aria-busy="true"] .ico-loader'); | |
outdated
Scan every file under src/web (Bun.Glob) rather than a hard-coded pair, so any new btn() with the loader icon fails the test. |
||
| 10 | }); | |
| 11 | // `.ico-loader` animates, so a btn(...) whose icon is "loader" looks busy | |
| 12 | // before anyone clicks it. | |
| 13 | test("no btn(...) call in src/web rests on the loader icon", async () => { | |
| 14 | const offenders: string[] = []; | |
| 15 | for (const path of new Glob("src/web/**/*.ts").scanSync(".")) { | |
| 16 | const text = await Bun.file(path).text(); | |
| 17 | for (const m of text.matchAll(/\bbtn\(/g)) { | |
| 18 | // Read the call's argument list by matching parentheses. | |
| 19 | let depth = 0; | |
| 20 | let end = m.index! + m[0].length - 1; | |
| 21 | for (; end < text.length; end++) { | |
| 22 | if (text[end] === "(") depth++; | |
| 23 | else if (text[end] === ")" && --depth === 0) break; | |
| 24 | } | |
| 25 | const call = text.slice(m.index!, end + 1); | |
| 26 | if (/\bicon:\s*["']loader["']/.test(call)) { | |
| 27 | const line = text.slice(0, m.index).split("\n").length; | |
| 28 | offenders.push(`${path}:${line}`); | |
| 29 | } | |
| 22 | 30 | } |
| 23 | 31 | } |
| 24 | } | |
| 25 | expect(offenders).toEqual([]); | |
| 32 | expect(offenders).toEqual([]); | |
| 33 | }); | |
| 26 | 34 | }); |
Good guard. Per docs/DESIGN.md the button already lists [aria-busy] as a state, so this matches the system. Nothing sets aria-busy yet, so the second rule is currently unused.