feat(lint): enforce no hardcoded colour in TS/TSX DOM code (code-colors)
Add a blocking 'code-colors' mode to theme-lint: default-deny with layered exemptions (structural; var()/readColor/canvas/pdf-lib contexts; a // theme-allow-color opt-out; exempt paths for rendering/vendor/config/ illustration areas). File list from 'git ls-files' (no directory walk). Wired into 'task frontend:lint:colors'; README + script header updated. Also removes the temporary color-migration-audit.md scratch file.
This commit is contained in:
EthanHealy01
@@ -211,6 +211,7 @@ tasks:
|
||||
cmds:
|
||||
- node editor/scripts/lint/theme-lint.mjs
|
||||
- node editor/scripts/lint/theme-lint.mjs css-colors
|
||||
- node editor/scripts/lint/theme-lint.mjs code-colors
|
||||
|
||||
contrast:
|
||||
desc: "Report low-contrast theme token pairs (warning only, never blocks)"
|
||||
|
||||
@@ -1,38 +0,0 @@
|
||||
# Colour migration audit (temporary — remove before final merge)
|
||||
|
||||
Tracks colours that were **not** tokenized during the consolidation, and the semantic tokens still needed to finish the job. Generated from the agent consolidation pass.
|
||||
|
||||
## Needed new `--c-*` tokens (would let the flagged colours below migrate)
|
||||
|
||||
| Token | Value / role | Wanted by |
|
||||
|---|---|---|
|
||||
| `--c-warning` | amber status (mid) | Payg status chips, Procurement pending-step/projection, billing spend-projection |
|
||||
| `--c-warning-subtle` / `--c-success-subtle` / `--c-danger-subtle` (+ `-border`) | pale status alert bg/border (3-step chip ramp: bg / text / dot) | auth error/success boxes, AuthCallback, Payg/billing status chips |
|
||||
| `--c-neutral-*` accent family (fill / hover / text / border / tint) | low-emphasis grey *fill* button (the neutrals are surface/text/border only, no fill) | `core/ui/accents.css` `.sui-acc-neutral` |
|
||||
|
||||
## Genuinely-unique / fixed colours KEPT (with reasons)
|
||||
|
||||
**Brand**
|
||||
- `#0a8bff` azure + rgba tints — cloud billing (`Payg`/`SpendCapControl`/`UpgradeModal`/`billing.css .scc`). Distinct from `--c-primary`.
|
||||
- `#af3434` / `#9a2e2e` / `#9c2f30` Stirling red — `auth.css`, `saas-auth.css`.
|
||||
- `#8e3131`… red family + AI multi-hue gradient (`#8b5cf6`/`#6366f1`/`#22d3ee`…) — `core/ui/accents.css`.
|
||||
- `#16213e` navy — portal hero CTA text (fixed dark on white CTA).
|
||||
- OAuth provider treatments (`#0f172a`, `#eef2f7`, provider button chrome) — `auth.css`.
|
||||
|
||||
**Data-viz / status (fixed by design)**
|
||||
- Usage-bar / segment / dot gradients: `#0a8bff→#38bdf8`, `#8b5cf6`, `#06b6d4` — `billing.css`, `Payg.css`.
|
||||
- Status-chip palette `#dcfce7`/`#fef3c7`/`#fee2e2` + dot rings — `billing.css`, `Payg.css` (marked "semantic, not theme-bound" in-file).
|
||||
- Emerald `#10b981`/`#4ade80` (free tier), purple `#6c5ce7`, pink `#ec4899` — cloud billing.
|
||||
- "viewed" teal/green `#10b981` / `--p-green-500` — `FilesPage.css`, `FileSidebarFileItem.css`.
|
||||
|
||||
**Illustration / bespoke**
|
||||
- File-card navy header banner `#3b4b6e` (light) / `#0d1020` (dark) — `FileEditor.module.css`; adaptive but no "banner surface" token — decision needed (dedicated token vs fixed brand navy).
|
||||
- Onboarding hero tiles/gradients (`#0f1626`, `linear-gradient(#eef1fb…)`) + pin gold `#ffc107` — `FileEditor`/`InitialOnboardingModal`.
|
||||
- `.portal-qb*` Notion light-only paper palette (`#37352f`, `#e3e1dc`, `#2383e2`…) — `Procurement.css`.
|
||||
- QR/logo whites, Calendly white — various (must stay).
|
||||
|
||||
**Per-theme, no token (OS-media / rgb-triplet contexts)**
|
||||
- `AuthCallback.module.css` `@media (prefers-color-scheme: dark)` block — `--c-*` follow the *app* theme not the OS query, so can't use them here.
|
||||
- `auth-theme.css` `--text-divider-*-rgb` navy triplets (consumed as `rgb(var(--x)/.4)`).
|
||||
|
||||
**Structural** (allowed anywhere): `rgba(0,0,0,…)` / `rgba(255,255,255,…)` scrims/shadows/hovers; pure black/white; the deliberate flash-yellow animation.
|
||||
@@ -1,5 +1,5 @@
|
||||
#!/usr/bin/env node
|
||||
// Theme colour lint — guards the theme SYSTEM (core/theme/). Two modes:
|
||||
// Theme colour lint. Modes:
|
||||
//
|
||||
// node theme-lint.mjs enforce: literal colours live ONLY in
|
||||
// primitives.css; colors/dimensions must
|
||||
@@ -8,6 +8,12 @@
|
||||
// node theme-lint.mjs css-colors enforce: NO hardcoded colour in any source
|
||||
// .css (primitives.css + generated output.css
|
||||
// exempt). Scope: all editor/src. (blocking)
|
||||
// node theme-lint.mjs code-colors enforce: no hardcoded colour in TS/TSX DOM
|
||||
// code. Default-deny with layered exemptions
|
||||
// (structural; var()/readColor/canvas/pdf-lib
|
||||
// contexts; `// theme-allow-color` opt-out;
|
||||
// exempt PATHS for rendering/vendor/config).
|
||||
// Scope: all editor/src. (blocking)
|
||||
// node theme-lint.mjs contrast warn-only WCAG contrast report (never blocks)
|
||||
//
|
||||
// Structural black / white / transparent (shadows, scrims) are always allowed.
|
||||
@@ -360,12 +366,114 @@ function checkAppCss() {
|
||||
return violations;
|
||||
}
|
||||
|
||||
// ── code-colors (blocking): no NEW hardcoded colour in TS/TSX DOM code ───────
|
||||
// TS/TSX can't be zeroed like CSS — canvas/PDF/pdfium rendering, colour maths,
|
||||
// colour pickers, vendor brand and self-contained docs legitimately need
|
||||
// numeric colour. So this is default-deny with layered exemptions:
|
||||
// 1. structural black/white/transparent
|
||||
// 2. detected safe CONTEXTS on the line (var()/readColor fallback, canvas
|
||||
// assignment, pdf-lib 0..1 rgb) and an explicit `// theme-allow-color` opt-out
|
||||
// 3. exempt PATHS — whole areas where colour literals are inherent
|
||||
// Anything else (a raw colour in a normal DOM component) fails. File list from
|
||||
// `git ls-files` (no directory walk). Comments and HTML entities are stripped.
|
||||
const CODE_EXEMPT_PATH = [
|
||||
/Thumbnail|Overlay|DrawingCanvas|PageEditor|MobileScannerPage/, // canvas/PDF render
|
||||
/[Pp]df|PDF|pixelCompare|\/compare\.ts$|customPrimary|accentColors/,
|
||||
/validateSignature\/outputtedPDFSections|CenteredMessageSection|StatusBadgeSection/,
|
||||
/\/viewer\/|Annotation|useViewerReadAloud|CommentsSidebar|\/constants\/search\.ts$|SignaturePreview/,
|
||||
/ColorPicker|ColorControl|WatchedFolderManagementModal|watchedFolderPresets|fileColors|unifiedBackground|folder\.ts$|policyFolders/, // pickers/palettes as data
|
||||
/OAuthButtons|oauthCallbackHtml/, // vendor brand / self-contained doc
|
||||
/mantineTheme|\/theme\.ts$|toolsTaxonomy|LayoutPreview|PageNumberPreview|CloudStorageIcons/, // theme/config objects
|
||||
/\/onboarding\//, // decorative onboarding illustration gradients
|
||||
/addStamp|addWatermark|\/tooltips\//, // colour params fed to PDF + tooltip prose
|
||||
/UpgradeBanner|AdminPlanSection/, // bespoke marketing/upsell banner palette
|
||||
/\.test\.[jt]sx?$|\.stories\.[jt]sx?$|\/types\//, // tests, stories, type decls
|
||||
];
|
||||
const CODE_HEX =
|
||||
/#(?:[0-9a-fA-F]{8}|[0-9a-fA-F]{6}|[0-9a-fA-F]{4}|[0-9a-fA-F]{3})(?![0-9a-fA-F])/g;
|
||||
const CODE_RGB = /rgba?\(([^)]*)\)/gi;
|
||||
const codeStructuralHex = (h) =>
|
||||
/^#(?:000|fff|000f|ffff|000000|ffffff|00000000|ffffffff)$/i.test(h);
|
||||
const CODE_SKIP_LINE =
|
||||
/theme-allow-color|var\(\s*--[a-z0-9-]+\s*,|readColor\(|getPropertyValue|\.colors\.[a-z]+\?\.\[|fillStyle|strokeStyle|shadowColor|addColorStop|createLinearGradient|ctx\.|getContext/i;
|
||||
function codeStripNonCode(text) {
|
||||
return text
|
||||
.replace(/\/\*[\s\S]*?\*\//g, (m) => m.replace(/[^\n]/g, " "))
|
||||
.replace(
|
||||
/(^|[^:])\/\/[^\n]*/g,
|
||||
(m, p) => p + m.slice(p.length).replace(/[^\n]/g, " "),
|
||||
)
|
||||
.replace(/&#\d+;/g, " ");
|
||||
}
|
||||
function codeRgbIsColour(inner) {
|
||||
if (/var\(/.test(inner)) return false;
|
||||
const p = inner
|
||||
.split(/[, /]+/)
|
||||
.map((x) => x.trim())
|
||||
.filter(Boolean);
|
||||
if (!/^\d/.test(p[0] || "")) return false;
|
||||
const nums = p.slice(0, 3).map(Number);
|
||||
if (nums.some((n) => Number.isNaN(n))) return false;
|
||||
if (nums.every((n) => n <= 1)) return false; // pdf-lib rgb(0..1)
|
||||
const k = `${nums[0]},${nums[1]},${nums[2]}`;
|
||||
return k !== "0,0,0" && k !== "255,255,255";
|
||||
}
|
||||
function checkCodeColors() {
|
||||
const files = execSync("git ls-files -- editor/src", { encoding: "utf8" })
|
||||
.split("\n")
|
||||
.map((l) => l.trim())
|
||||
.filter(
|
||||
(l) =>
|
||||
/\.(ts|tsx)$/.test(l) && !CODE_EXEMPT_PATH.some((re) => re.test(l)),
|
||||
);
|
||||
const violations = [];
|
||||
for (const rel of files) {
|
||||
const raw = readFileSync(rel, "utf8");
|
||||
const rawLines = raw.split("\n");
|
||||
const text = codeStripNonCode(raw);
|
||||
text.split("\n").forEach((line, i) => {
|
||||
// marker lives in a comment (stripped from `line`), so test the raw line
|
||||
if (/theme-allow-color/.test(rawLines[i] || "")) return;
|
||||
if (CODE_SKIP_LINE.test(line)) return;
|
||||
const hits = [];
|
||||
for (const h of line.match(CODE_HEX) || [])
|
||||
if (!codeStructuralHex(h)) hits.push(h);
|
||||
for (const m of line.match(CODE_RGB) || [])
|
||||
if (codeRgbIsColour(m.replace(/rgba?\(|\)/gi, ""))) hits.push(m);
|
||||
for (const h of hits)
|
||||
violations.push({
|
||||
file: rel,
|
||||
line: i + 1,
|
||||
msg: `hardcoded colour ${h} — use a var(--c-*/--p-*) token, or add \`// theme-allow-color <reason>\` if this must be a literal`,
|
||||
});
|
||||
});
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
// ── CLI ──────────────────────────────────────────────────────────────────────
|
||||
if (process.argv.includes("contrast")) {
|
||||
reportContrast();
|
||||
process.exit(0); // never blocks
|
||||
}
|
||||
|
||||
if (process.argv.includes("code-colors")) {
|
||||
const violations = checkCodeColors();
|
||||
if (violations.length) {
|
||||
console.error(
|
||||
`\n✖ theme-lint code-colors: ${violations.length} hardcoded colour(s) in TS/TSX:\n`,
|
||||
);
|
||||
for (const v of violations)
|
||||
console.error(` ${v.file}:${v.line} ${v.msg}`);
|
||||
console.error("");
|
||||
process.exit(1);
|
||||
}
|
||||
console.log(
|
||||
"✓ theme-lint code-colors: no hardcoded colour in TS/TSX DOM code (rendering/vendor/config areas exempt)",
|
||||
);
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
if (process.argv.includes("css-colors")) {
|
||||
const violations = checkAppCss();
|
||||
if (violations.length) {
|
||||
|
||||
@@ -62,9 +62,10 @@ One file: `editor/scripts/lint/theme-lint.mjs` (no baseline). Run via `task fron
|
||||
|
||||
- **Default (blocking):** enforces "literals only in `primitives.css`; everything else in `core/theme/` references tokens; no duplicate primitives." Scope: `core/theme/`.
|
||||
- **`css-colors` (blocking):** enforces **no hardcoded colour in any source `.css`** across `editor/src` — `primitives.css` (the literal home) and generated `output.css` are the only exemptions. The file list comes from `git ls-files` (not a directory walk), and comments/structural black-white-transparent are ignored.
|
||||
- **`code-colors` (blocking):** enforces **no hardcoded colour in TS/TSX DOM code**. TS/TSX can't be zeroed like CSS (canvas/PDF/pdfium rendering, colour maths, colour pickers, vendor brand and self-contained docs legitimately need numeric colour), so it's **default-deny with layered exemptions**: (1) structural black/white/transparent; (2) detected safe contexts on the line — `var(--x, …)` / `readColor(…)` fallbacks, canvas assignments (`fillStyle`/`ctx.`…), pdf-lib `rgb(0..1)`, and an explicit **`// theme-allow-color <reason>`** opt-out; (3) exempt **paths** for whole rendering/vendor/config/illustration areas (viewer/annotation, `*Thumbnail`/`*Overlay`, `*Pdf*`, colour pickers, `onboarding/`, `mantineTheme`/`theme.ts`, tests, stories, type decls, marketing banners). A raw colour anywhere else fails. Adding a new colour to a normal component → blocked; genuine exceptions get the marker.
|
||||
- **`node theme-lint.mjs contrast`** (task `frontend:contrast`, non-blocking): WCAG contrast report for text-on-surface / on-primary pairs per theme.
|
||||
|
||||
Both blocking modes run under `task frontend:lint:colors`. Source CSS is at **zero** hardcoded colour; keep it there — add the literal to `primitives.css` and reference `var(--p-…)`. (Colour literals in TS/TSX that drive canvas/PDF/pdfium rendering are out of scope — those APIs take numbers, not CSS vars.)
|
||||
All three blocking modes run under `task frontend:lint:colors`. Source **CSS is at zero** hardcoded colour and **TS/TSX DOM code is clean** (only the exempt rendering/vendor/config areas hold literals). Keep it there — add the literal to `primitives.css` and reference `var(--p-…)`, or for an unavoidable TS/TSX literal add `// theme-allow-color <reason>`.
|
||||
|
||||
## Gotchas
|
||||
|
||||
|
||||
Reference in New Issue
Block a user