trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns18g rEventsFlows

The design lint refuses raw colours, z-index outside the tokens, and fixed boxes outside the overlay host #12019

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/12017-the-design-lint-refuses-raw-colours-z-in → main 2 commits fork

  • Merged. scout/12017-the-design-lint-refuses-raw-colours-z-in was merged into main as e119dca.

Review

Sign in to review this change.
5 files changed +192 −5
docs/design/baseline.json +41 −1
@@ -1,3 +1,43 @@
11 {
2 "files": {}
2 "files": {},
3 "layout": {
4 "src/web/pages/board_style.ts": {
5 "viewport": 2
6 },
7 "src/web/pages/build_info.ts": {
8 "space": 8
9 },
10 "src/web/pages/iterations.ts": {
11 "zindex": 3,
12 "space": 7
13 },
14 "src/web/pages/talk_client.ts": {
15 "zindex": 2,
16 "space": 1
17 },
18 "src/web/pick.ts": {
19 "space": 1
20 },
21 "src/web/space_client.ts": {
22 "fixed": 1
23 },
24 "src/web/studio/feed_style.ts": {
25 "zindex": 1,
26 "space": 1
27 },
28 "src/web/style/base.ts": {
29 "zindex": 2,
30 "viewport": 1,
31 "space": 7
32 },
33 "src/web/style/components.ts": {
34 "zindex": 7,
35 "fixed": 3,
36 "space": 68
37 },
38 "src/web/style/content.ts": {
39 "zindex": 3,
40 "space": 80
41 }
42 }
343 }
docs/design/sections/4-space-radius-motion.md +5
@@ -18,3 +18,8 @@
1818 and inset (−2px) on full-bleed rows so nothing reflows. Never `outline: none`.
1919 - **Sticky, and only these**: the topbar, a diff file's header, the file path
2020 header. Everything anchorable carries `scroll-margin-top`.
21 - **Layout is tokens.** A `z-index` is `var(--z-…)`, spacing is on the scale above,
22 and `position: fixed` or a full-viewport box belongs to `src/web/overlay_host.ts`
23 (open a layer with `forgeOverlay.open`). Enforced: `src/design/layout.ts`, run by
24 `bun scripts/design-lint.ts`; today's debt is named per file under `layout` in
25 `docs/design/baseline.json` and only shrinks.
scripts/design-lint.ts +9 −3
@@ -30,6 +30,10 @@
3030 * target a button, link, input or select under 24px wide or high, WCAG 2.5.8
3131 * (specification.website, target size)
3232 *
33 * The layout rules (`src/design/layout.ts`, #12017) are held the same way, under `layout` in the baseline:
34 * z-index outside the `--z-` tokens, `position: fixed` or a full-viewport box outside src/web/overlay_host.ts,
35 * spacing off the scale.
36 *
3337 * The quiet-page rules (#8775, #8719) are held per file in `docs/design/quiet-debt.json`,
3438 * the same ratchet, and may only go down: `label` ("Status: Open"), `caps`
3539 * (`text-transform: uppercase`), `echo` (an icon's aria-label equal to its visible
@@ -62,9 +66,11 @@
6266 import { readdirSync, readFileSync, writeFileSync } from "node:fs";
6367 import { join, relative } from "node:path";
6468 import { STRANGER_NEVER_SEES } from "../src/design/first_five_minutes.ts";
69 import { compareLayout, layoutCounts, scanLayout } from "../src/design/layout.ts";
6570 import { cssBlocksAt, scanOverlay, sourceFiles } from "../src/design/overlay.ts";
6671
6772 // The covered-page rule lives in src/design/ so the release preflight runs it too (#11916).
73 export { compareLayout, layoutCounts, layoutIn, scanLayout } from "../src/design/layout.ts";
6874 export { matchesClosedOverlay, overlay, overlayClassesIn, type OverlayFinding, scanOverlay, sourceFiles } from "../src/design/overlay.ts";
6975
7076 /** The type scale in docs/DESIGN.md §3, in px. */
@@ -90,7 +96,7 @@ const WEB_MESSAGE: Record<(typeof WEB_KEYS)[number], string> = {
9096 input: "form input under 16px, which zooms iOS (specification.website, input size)",
9197 target: "interactive target under 24x24 CSS px, WCAG 2.5.8 (specification.website, target size)",
9298 };
93 export type Baseline = { files: Record<string, Counts> };
99 export type Baseline = { files: Record<string, Counts>; layout?: Record<string, Partial<Record<"zindex" | "fixed" | "viewport" | "space", number>>> };
94100 /** One offence, where it is and what the system offers instead. */
95101 export type Finding = { line: number; kind: "colour" | "size"; text: string; use: string };
96102 export type Scan = Baseline & { classes: string[]; found?: Record<string, Finding[]> };
@@ -564,14 +570,14 @@ if (import.meta.main) {
564570 const root = process.cwd();
565571 const now = scan(root);
566572 if (process.argv.includes("--update")) {
567 const baseline = { ...JSON.parse(readFileSync(join(root, BASELINE), "utf8")), files: now.files }; // keeps `coldRead` (#5882)
573 const baseline = { ...JSON.parse(readFileSync(join(root, BASELINE), "utf8")), files: now.files, layout: layoutCounts(scanLayout(root)) }; // keeps `coldRead` (#5882)
568574 writeFileSync(join(root, BASELINE), JSON.stringify(baseline, null, 2) + "\n");
569575 writeFileSync(join(root, QUIET_DEBT), JSON.stringify(quietCounts(scanQuiet(root)), null, 2) + "\n");
570576 console.log(`wrote ${BASELINE}`);
571577 } else {
572578 const base: Baseline = JSON.parse(readFileSync(join(root, BASELINE), "utf8"));
573579 const debt: QuietDebt = JSON.parse(readFileSync(join(root, QUIET_DEBT), "utf8"));
574 const problems = [...compare(base, now, designText(root), areaFiles(root)), ...compareQuiet(debt, scanQuiet(root)), ...scanBanner(root), ...scanOverlay(root), ...scanCards(root), ...scanStranger(root)];
580 const problems = [...compare(base, now, designText(root), areaFiles(root)), ...compareQuiet(debt, scanQuiet(root)), ...scanBanner(root), ...compareLayout(base.layout ?? {}, scanLayout(root)), ...scanOverlay(root), ...scanCards(root), ...scanStranger(root)];
575581 for (const p of problems) console.error(p);
576582 if (problems.length) process.exit(1);
577583 console.log("design: no colour, size or component added outside the system");
src/design/layout.ts added +99
@@ -0,0 +1,99 @@
1 /**
2 * The layout rules of the design check (#12017): z-index, spacing and boxes that
3 * escape the page are tokens or the overlay host's, never a literal in a stylesheet.
4 * Read from every `/* css *\/` template in src/web. Plain data in, plain data out:
5 * `layoutIn` takes one file's text and a path; `scanLayout` the tree.
6 *
7 * zindex a `z-index` that is not `var(--z-…)` (or 0, auto): stacking order is a token
8 * fixed `position: fixed`, outside src/web/overlay_host.ts: a layer opens through
9 * forgeOverlay
10 * viewport a box sized 100vw, 100vh, 100dvh or 100svh (not inside calc/min/max/clamp,
11 * where it is a bound): a full-viewport box is an overlay, the host's to open
12 * space a padding, margin or gap in px off the scale `2 4 6 8 12 16 24 32 48 64`:
13 * spacing is the scale
14 *
15 * Like the other rules this is a ratchet: `layout` in docs/design/baseline.json
16 * names today's debt per file, and a pull may lower it, never raise it.
17 */
18 import { readFileSync } from "node:fs";
19 import { relative } from "node:path";
20 import { cssBlocksAt, sourceFiles } from "./overlay.ts";
21
22 export type LayoutRule = "zindex" | "fixed" | "viewport" | "space";
23 export const LAYOUT_RULES: readonly LayoutRule[] = ["zindex", "fixed", "viewport", "space"];
24 export type LayoutFinding = { line: number; rule: LayoutRule; text: string };
25 export type LayoutDebt = Record<string, Partial<Record<LayoutRule, number>>>;
26
27 export const OVERLAY_HOST = "src/web/overlay_host.ts";
28 export const SPACE_SCALE: readonly number[] = [0, 2, 4, 6, 8, 12, 16, 24, 32, 48, 64];
29
30 /** One line each: the rule and the fix. */
31 export const LAYOUT_FIX: Record<LayoutRule, string> = {
32 zindex: "stacking order is a token: z-index: var(--z-…), or open a layer through forgeOverlay",
33 fixed: "position: fixed belongs to src/web/overlay_host.ts: open the layer with forgeOverlay.open",
34 viewport: "a full-viewport box is an overlay: open it with forgeOverlay.open, or size it with a bound (min(), max())",
35 space: `spacing is the scale ${SPACE_SCALE.slice(1).join(" ")}px: pick the nearest`,
36 };
37
38 const lineOf = (text: string, at: number) => text.slice(0, at).split("\n").length;
39
40 /** Whether `value` has a px length off the scale, outside a var() fallback. */
41 function offScale(value: string): boolean {
42 const bare = value.replace(/var\([^)]*\)/g, "");
43 for (const m of bare.matchAll(/(?<![\w.])(-?[0-9]*\.?[0-9]+)px\b/g)) {
44 if (!SPACE_SCALE.includes(Math.abs(Number(m[1])))) return true;
45 }
46 return false;
47 }
48
49 export function layoutIn(text: string, path: string): LayoutFinding[] {
50 const out: LayoutFinding[] = [];
51 const host = relative("", path).replace(/\\/g, "/").endsWith(OVERLAY_HOST);
52 for (const { css, at } of cssBlocksAt(text)) {
53 const code = css.replace(/\/\*[\s\S]*?\*\//g, (c) => c.replace(/[^\n]/g, " "));
54 for (const m of code.matchAll(/([a-z-]+)\s*:\s*([^;{}]+)/gi)) {
55 const prop = m[1]!.toLowerCase();
56 const value = m[2]!.trim();
57 const line = lineOf(text, at + m.index!);
58 const add = (rule: LayoutRule) => out.push({ line, rule, text: `${prop}: ${value}` });
59 if (prop === "z-index" && !/^(var\(\s*--z-[\w-]+\s*\)|0|auto)$/.test(value)) add("zindex");
60 else if (prop === "position" && value === "fixed" && !host) add("fixed");
61 else if (/^(width|height|min-width|min-height|inline-size|block-size)$/.test(prop) && /^(100(vw|vh|dvh|svh|lvh)|var\(--vv-h[^)]*\))$/.test(value) && !host) add("viewport");
62 else if (/^(padding|margin|gap|row-gap|column-gap)(-(top|right|bottom|left|inline|block)(-(start|end))?)?$/.test(prop) && offScale(value)) add("space");
63 }
64 }
65 return out;
66 }
67
68 export function scanLayout(root: string): Record<string, LayoutFinding[]> {
69 const files: Record<string, LayoutFinding[]> = {};
70 for (const f of sourceFiles(root)) {
71 const found = layoutIn(readFileSync(f, "utf8"), relative(root, f));
72 if (found.length) files[relative(root, f)] = found;
73 }
74 return files;
75 }
76
77 export function layoutCounts(files: Record<string, LayoutFinding[]>): LayoutDebt {
78 const out: LayoutDebt = {};
79 for (const [file, found] of Object.entries(files)) {
80 for (const r of LAYOUT_RULES) {
81 const n = found.filter((f) => f.rule === r).length;
82 if (n) (out[file] ??= {})[r] = n;
83 }
84 }
85 return out;
86 }
87
88 /** What a pull made worse, one line each: file:line, the rule and the fix. */
89 export function compareLayout(debt: LayoutDebt, files: Record<string, LayoutFinding[]>): string[] {
90 const out: string[] = [];
91 for (const [file, found] of Object.entries(files)) {
92 for (const r of LAYOUT_RULES) {
93 const here = found.filter((f) => f.rule === r);
94 if (here.length <= (debt[file]?.[r] ?? 0)) continue;
95 for (const f of here) out.push(`${file}:${f.line} ${r}: ${f.text} → ${LAYOUT_FIX[r]} (${debt[file]?.[r] ?? 0} already named in docs/design/baseline.json)`);
96 }
97 }
98 return out;
99 }
test/design-lint.test.ts +38 −1
@@ -1,6 +1,6 @@
11 import { describe, expect, test } from "bun:test";
22 import { readdirSync, readFileSync } from "node:fs";
3 import { areaFiles, classesIn, compare, compareQuiet, quiet, scanQuiet, scanCards, scanStranger, strangerWords, STRANGER_FILES, countFile, designText, documentedClasses, locate, loneSentenceCards, registeredClasses, overlay, overlayClassesIn, scanOverlay, scan, sectionLinks, webCss } from "../scripts/design-lint.ts";
3 import { areaFiles, compareLayout, layoutCounts, layoutIn, scanLayout, classesIn, compare, compareQuiet, quiet, scanQuiet, scanCards, scanStranger, strangerWords, STRANGER_FILES, countFile, designText, documentedClasses, locate, loneSentenceCards, registeredClasses, overlay, overlayClassesIn, scanOverlay, scan, sectionLinks, webCss } from "../scripts/design-lint.ts";
44
55 /** A DESIGN.md in miniature: the §5 table, a §5d register, and a §6 that names a class too. */
66 const DESIGN = [
@@ -348,3 +348,40 @@ describe("overlay: nothing closed is drawn (#11831)", () => {
348348 expect(scanOverlay(process.cwd())).toEqual([]);
349349 });
350350 });
351
352 describe("design check: the layout rules (#12017)", () => {
353 const css = (body: string) => "export const s = /* css */ `\n" + body + "\n`;";
354 const rules = (body: string, path = "src/web/x.ts") => layoutIn(css(body), path).map((f) => f.rule);
355
356 test("a raw z-index is refused, a token or 0 is not", () => {
357 expect(rules(".a { z-index: 40; }")).toEqual(["zindex"]);
358 expect(rules(".a { z-index: var(--z-menu); }")).toEqual([]);
359 expect(rules(".a { z-index: 0; }")).toEqual([]);
360 });
361 test("position: fixed is refused outside the overlay host", () => {
362 expect(rules(".a { position: fixed; inset: 0; }")).toEqual(["fixed"]);
363 expect(rules(".a { position: fixed; }", "src/web/overlay_host.ts")).toEqual([]);
364 expect(rules(".a { position: sticky; }")).toEqual([]);
365 });
366 test("a full-viewport box is refused, a viewport bound is not", () => {
367 expect(rules(".a { height: 100dvh; }")).toEqual(["viewport"]);
368 expect(rules(".a { width: 100vw; }")).toEqual(["viewport"]);
369 expect(rules(".a { max-width: min(560px, calc(100vw - 32px)); }")).toEqual([]);
370 });
371 test("spacing off the scale is refused, on it is not", () => {
372 expect(rules(".a { padding: 5px 8px; }")).toEqual(["space"]);
373 expect(rules(".a { margin-top: -10px; }")).toEqual(["space"]);
374 expect(rules(".a { gap: 12px; padding: 0 var(--pad, 7px); }")).toEqual([]);
375 });
376 test("each refusal is one line naming the rule and the fix", () => {
377 const files = { "src/web/x.ts": layoutIn(css(".a { z-index: 9; position: fixed; width: 100vw; gap: 5px; }"), "src/web/x.ts") };
378 const lines = compareLayout({}, files);
379 expect(lines).toHaveLength(4);
380 for (const l of lines) expect(l).toMatch(/^src\/web\/x\.ts:2 {2}(zindex|fixed|viewport|space): .* → /);
381 expect(compareLayout(layoutCounts(files), files)).toEqual([]);
382 });
383 test("the tree is no worse than its named debt", () => {
384 const base = JSON.parse(readFileSync("docs/design/baseline.json", "utf8"));
385 expect(compareLayout(base.layout ?? {}, scanLayout("."))).toEqual([]);
386 }, SLOW);
387 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs