diff --git a/README.md b/README.md index 689338ff3..ea8e774c6 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,138 @@ npm run lint # Check code rules npm run dist # Build distribution files ``` +## Styling + +Components are styled with plain CSS in a sibling `.css` file, imported for side effects. + +### Theme tokens + +Theme values are defined once, in JavaScript, and projected into CSS custom properties by +`src/theme/theme.css`. That file is **generated** from `src/theme/palette.ts` and +`src/theme.ts` — never edit it by hand. `npm run build` regenerates it as its first step, so +a published package can never ship a stale one. To refresh it without a full build: + +``` +npm run generate:theme-css +``` + +It is committed as well as generated, because jest and ladle read `src/` directly and never +run the build — `src/theme/tokens.spec.ts` fails when the committed copy is stale, which is +what makes CI (which runs lint and test, not build) catch it. + +**Never write a theme value as a literal in a component stylesheet** — reference the token +instead: + +```css +/* no */ +.thing { border-color: #d5d5d5; } + +/* yes */ +.thing { border-color: var(--ox-color-pale); } +``` + +Tokens are `--ox-`-prefixed, so they will not collide with a consuming app's own variables. +Color tokens are the kebab-case form of the `src/theme/palette.ts` key +(`palette.neutralLighter` → `--ox-color-neutral-lighter`); there are also `--ox-color-link`, +`--ox-color-link-hover`, `--ox-z-index-*` and `--ox-padding-navbar-*`. + +Any component whose CSS uses a token must import the token file alongside its own +stylesheet: + +```ts +import './MyComponent.css'; +import '../theme/theme.css'; +``` + +There is no bundler in the build — `build.bash` copies CSS 1:1 — so nothing resolves an +`@import` on our behalf and a stylesheet does not drag `theme.css` in by itself. Forget the +import and every `var(--ox-*)` in that file quietly takes its fallback, which usually looks +right on screen because the fallback is the literal the token replaced. `tokens.spec.ts` +fails when a stylesheet reads a token and the module importing it does not import the +tokens, so this cannot be forgotten rather than merely being documented. + +Consumers of the package need do nothing: the import rides along with the component, +bundlers deduplicate it, and `sideEffects` in `package.json` keeps it from being +tree-shaken. An app that wants the tokens without rendering one of our components — to +build its own styles on the palette, say — can load the file directly: + +```ts +import '@openstax/ui-components/theme/theme.css'; +``` + +### Component override hooks and when to bind in JS + +A component may expose its own `--component-*` custom property so consumers can restyle it. +Declare the **default in the stylesheet** with the token as the fallback, and do not bind it +from JavaScript: + +```css +.tabs [role="tab"] { border-color: var(--tabs-active-border-color, var(--ox-color-dark-green)); } +``` + +Bind a custom property from JavaScript only when its value genuinely varies at runtime — a +variant lookup, a numeric prop, a disabled state. A static color pushed through an inline +style is duplication with extra steps, and it wins over the cascade in ways callers do not +expect. + +Widen the component's `style` prop to `CSSPropertiesWithVariables` (from `src/types`) so +callers can set these without a cast. + +### What CI enforces + +`src/theme/tokens.spec.ts` fails the build if: + +- the committed `theme.css` is not what the generator produces from the JS theme (the build + regenerates it; this is what stops a stale copy reaching jest, ladle or a reviewer) +- a component stylesheet writes a color literal that duplicates a theme value +- a component stylesheet introduces a color that is not a theme value and not in the + `KNOWN_OFF_PALETTE` allowlist +- a component stylesheet reads an `--ox-*` token that does not exist +- a stylesheet reads a token and the module that imports it does not import `theme.css` + +The color check parses declarations rather than grepping for hex, so it reads a value the +way the property does. What that covers: + +- Hex, and the color functions — `rgb()`, `hsl()`, `hwb()`, `lab()`, `lch()`, `oklab()`, + `oklch()`, `color()` and their `a` variants — in any declaration, whatever the property. +- A bare name such as `whitesmoke` only where the property could take a color: a property + whose name contains `color`, one of the color shorthands (`background`, `border`, + `outline`, `box-shadow`, `text-decoration`, `fill` and the rest), or a custom property, + which has no grammar to go on. `var()` keeps the gate of the property it is written in, + since its fallback is whatever that property makes of it. +- Functions that merely *contain* colors (`var()`, `color-mix()`, `light-dark()`, the + gradients) are descended into rather than read as literals, so a value built out of + tokens stays clean. The gradients and `color-mix()` open the gate on their own, because + `linear-gradient(red, blue)` is a gradient between two colors whatever it is assigned to. +- A translucent color where its opaque channels are a theme value — `rgba(0, 0, 0, 0.2)` is + black at 20% and there is no token form for it. A new hue smuggled in through `rgba()` is + still refused. + +What it does not cover, deliberately: a bare name outside a color context. `animation-name: +red`, `font-family: white` and `grid-area: gold` are identifiers that happen to spell +colors, and are not reported. + +What it does not cover, for now: only the literal ASCII spelling of a *name* is recognized. +Escapes are decoded in color names themselves, but not in property or function names, so +`c\6f lor: red` — which is `color: red` to a browser — is missed. Nothing we ship is +written that way; CORE-2885 tracks it along with the rest of the lexing work. + +The check has its own tests, so what is claimed above is pinned rather than asserted. + +The stylesheets migrated before the tokens existed are listed in `PENDING_SWEEP` in +`tokens.spec.ts` and are exempt from the duplicate-literal check until they are swept — +from that check only. A color the theme does not have is still refused in those files, so +the list defers work already owed rather than opening a gap. The list is asserted to be +exactly the set of files that still carry duplicates, so it cannot drift: you cannot exempt +a clean file, and you cannot sweep a file without removing it from the list. Do not add to +it — new stylesheets are expected to use the tokens from the start. + +Adding a genuinely new color is therefore a deliberate act: put it in `palette.ts` if it is +part of the design, or in the allowlist with a reason if it is a one-off we are keeping. + +Breakpoints are the known gap — `@media (min-width: var(--x))` is not valid CSS, so +breakpoint values are still repeated in media queries and are not covered by the tests. + ## Testing Across Projects To test changes before tagging: diff --git a/package.json b/package.json index 3384fd4dd..b17f20e7d 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,11 @@ "import": "./dist/esm/index.js", "require": "./dist/cjs/index.js" }, + "./*.css": { + "browser": "./dist/esm/*.css", + "import": "./dist/esm/*.css", + "require": "./dist/cjs/*.css" + }, "./*": { "browser": "./dist/esm/*.js", "import": "./dist/esm/*.js", @@ -40,6 +45,7 @@ ], "scripts": { "build": "./scripts/build.bash", + "generate:theme-css": "./scripts/generate-theme-css.bash", "clean": "rm -rf ./dist", "build:clean": "npm-run-all clean build", "dist": "npm-run-all clean build", diff --git a/scripts/build.bash b/scripts/build.bash index c86b3997a..ec96d6725 100755 --- a/scripts/build.bash +++ b/scripts/build.bash @@ -6,6 +6,11 @@ project_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." >/dev/null 2>&1 && pwd cd "$project_dir" +# theme.css is generated from the JS theme, so regenerate before anything copies it. +# Committed too, because jest and ladle read src/ directly and never run this script; +# src/theme/tokens.spec.ts fails when the committed file is stale. +./scripts/generate-theme-css.bash + tsc_args=(--noEmit false --declaration) npx tsc --project tsconfig.without-specs.esm.json "${tsc_args[@]}" diff --git a/scripts/generate-theme-css.bash b/scripts/generate-theme-css.bash new file mode 100755 index 000000000..818103f1c --- /dev/null +++ b/scripts/generate-theme-css.bash @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# spell-checker: ignore pipefail outdir +# +# Regenerates src/theme/theme.css from the JavaScript theme. The committed file is checked +# against this output by src/theme/tokens.spec.ts, so CI fails if palette.ts or theme.ts +# changes without this being re-run. +set -euo pipefail; if [ -n "${DEBUG-}" ]; then set -x; fi + +project_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." >/dev/null 2>&1 && pwd )" +cd "$project_dir" + +# The theme is TypeScript and the repo has no TS runner outside jest, so compile the +# renderer and its (small, pure) import graph to a throwaway directory and run that. +outdir="$(mktemp -d)" +trap 'rm -rf "$outdir"' EXIT + +npx tsc src/theme/themeCss.ts \ + --outDir "$outdir" --rootDir . \ + --module commonjs --target es2019 --moduleResolution node --esModuleInterop --skipLibCheck + +node -e " + const fs = require('fs'); + const { renderThemeCss } = require('$outdir/src/theme/themeCss'); + fs.writeFileSync('src/theme/theme.css', renderThemeCss()); + console.log('wrote src/theme/theme.css'); +" diff --git a/src/theme/buttons.ts b/src/theme/buttons.ts index 994c82e28..aa821eca3 100644 --- a/src/theme/buttons.ts +++ b/src/theme/buttons.ts @@ -19,8 +19,8 @@ const asButtonStyleSetTypes = (st: { [K in keyof T]: ButtonStyleSet }) => st; const buttonStyleSets = asButtonStyleSetTypes({ primary: { background: palette.orange, - backgroundActive: "#b03808", - backgroundHover: "#be3c08", + backgroundActive: palette.orangeActive, + backgroundHover: palette.orangeHover, color: palette.white, outline: palette.white, shadow: palette.black, @@ -36,8 +36,8 @@ const buttonStyleSets = asButtonStyleSetTypes({ }, secondary: { background: palette.darkGray, - backgroundActive: "#4c4c4c", - backgroundHover: "#646464", + backgroundActive: palette.darkGrayActive, + backgroundHover: palette.darkGrayHover, color: palette.white, outline: palette.white, shadow: palette.black, diff --git a/src/theme/palette.ts b/src/theme/palette.ts index e24d4ca17..d4d213a91 100644 --- a/src/theme/palette.ts +++ b/src/theme/palette.ts @@ -19,6 +19,9 @@ export const palette = { tangerine: "#ffbd3e", gray: "#5e5e5e", darkGray: "#757575", + // button variants + darkGrayHover: "#646464", + darkGrayActive: "#4c4c4c", pale: "#d5d5d5", light: "#e4e4e4", white: "#ffffff", @@ -34,5 +37,8 @@ export const palette = { neutralFeedback: "#555", // another dark gray neutralDarker: "#424242", // very dark gray black: "#000000", - orange: "#D4450C" + orange: "#D4450C", + // button variants + orangeHover: "#be3c08", + orangeActive: "#b03808" } as const; diff --git a/src/theme/theme.css b/src/theme/theme.css new file mode 100644 index 000000000..68f6774df --- /dev/null +++ b/src/theme/theme.css @@ -0,0 +1,74 @@ +/* + * Global theme tokens. + * + * GENERATED FILE — do not edit by hand. Run `npm run generate:theme-css`. + * + * Projected from the JavaScript theme, which stays the single source of truth: + * src/theme/palette.ts (colors) and src/theme.ts (link colors, z-index, padding). + * src/theme/tokens.spec.ts fails if this file is out of date, so add the value to the + * JS theme and regenerate rather than editing here. + * + * Component CSS should reference these rather than repeating a literal. Where a + * component exposes its own --component-* override hook, use the token as the + * fallback: var(--tabs-border-color, var(--ox-color-pale)). + * + * Imported for side effects by every component whose CSS depends on it, the same + * way component stylesheets are imported. Consumers need do nothing. + */ +:root { + /* palette — src/theme/palette.ts */ + --ox-color-red: #ca2026; + --ox-color-danger: #c2002f; + --ox-color-dark-red: #c22032; + --ox-color-light-red: #e298a0; + --ox-color-pale-red: #FBE7EA; + --ox-color-green: #77af42; + --ox-color-light-green: #8bc753; + --ox-color-pale-green: #e0edd3; + --ox-color-dark-green: #63a524; + --ox-color-darker-green: #4e7226; + --ox-color-pale-yellow: #ffffbb; + --ox-color-teal: #0dc0de; + --ox-color-dark-teal: #007297; + --ox-color-blue: #007da4; + --ox-color-medium-blue: #026AA1; + --ox-color-light-blue: #34bdd8; + --ox-color-neutral-light-blue: #0dc0dc; + --ox-color-tangerine: #ffbd3e; + --ox-color-gray: #5e5e5e; + --ox-color-dark-gray: #757575; + --ox-color-dark-gray-hover: #646464; + --ox-color-dark-gray-active: #4c4c4c; + --ox-color-pale: #d5d5d5; + --ox-color-light: #e4e4e4; + --ox-color-white: #ffffff; + --ox-color-neutral-lightest: #f9f9f9; + --ox-color-neutral-cool: #f6f7f8; + --ox-color-neutral-bright: #f5f5f5; + --ox-color-neutral-lighter: #f1f1f1; + --ox-color-neutral-light: #e5e5e5; + --ox-color-neutral-medium: #a0a0a0; + --ox-color-neutral: #818181; + --ox-color-neutral-thin: #6f6f6f; + --ox-color-neutral-dark: #5f6163; + --ox-color-neutral-feedback: #555; + --ox-color-neutral-darker: #424242; + --ox-color-black: #000000; + --ox-color-orange: #D4450C; + --ox-color-orange-hover: #be3c08; + --ox-color-orange-active: #b03808; + + /* link colors — src/theme.ts */ + --ox-color-link: #026AA1; + --ox-color-link-hover: #005481; + + /* z-index scale — src/theme.ts */ + --ox-z-index-navbar: 10; + --ox-z-index-sidebar: 20; + --ox-z-index-modals: 30; + --ox-z-index-toasts: 40; + + /* padding — src/theme.ts */ + --ox-padding-navbar-mobile: 1.6rem; + --ox-padding-navbar-desktop: 3.2rem; +} diff --git a/src/theme/themeCss.ts b/src/theme/themeCss.ts new file mode 100644 index 000000000..ceb605e7f --- /dev/null +++ b/src/theme/themeCss.ts @@ -0,0 +1,69 @@ +import { palette } from "./palette"; +import { colors, padding, zIndex } from "../theme"; + +const kebab = (key: string) => key.replace(/([a-z0-9])([A-Z])/g, "$1-$2").toLowerCase(); + +type Token = readonly [name: string, value: string]; + +/** + * The tokens theme.css is made of, grouped the way the file is laid out. This is the + * single description of the projection from the JavaScript theme into CSS custom + * properties: `renderThemeCss` writes it out, and `tokens.spec.ts` checks the committed + * file still matches. Nothing reads the CSS to decide what the tokens are. + */ +export const themeTokenGroups: ReadonlyArray<{ label: string; tokens: Token[] }> = [ + { + label: "palette — src/theme/palette.ts", + tokens: Object.entries(palette).map(([key, value]) => [`--ox-color-${kebab(key)}`, value] as const), + }, + { + label: "link colors — src/theme.ts", + tokens: [ + ["--ox-color-link", colors.link.color], + ["--ox-color-link-hover", colors.link.hover], + ], + }, + { + label: "z-index scale — src/theme.ts", + tokens: Object.entries(zIndex).map(([key, value]) => [`--ox-z-index-${kebab(key)}`, String(value)] as const), + }, + { + label: "padding — src/theme.ts", + tokens: [ + ["--ox-padding-navbar-mobile", `${padding.navbar.mobile}rem`], + ["--ox-padding-navbar-desktop", `${padding.navbar.desktop}rem`], + ], + }, +]; + +/** Every token as a flat name -> value map. */ +export const themeTokens = (): Map => + new Map(themeTokenGroups.flatMap((group) => group.tokens)); + +const header = `/* + * Global theme tokens. + * + * GENERATED FILE — do not edit by hand. Run \`npm run generate:theme-css\`. + * + * Projected from the JavaScript theme, which stays the single source of truth: + * src/theme/palette.ts (colors) and src/theme.ts (link colors, z-index, padding). + * src/theme/tokens.spec.ts fails if this file is out of date, so add the value to the + * JS theme and regenerate rather than editing here. + * + * Component CSS should reference these rather than repeating a literal. Where a + * component exposes its own --component-* override hook, use the token as the + * fallback: var(--tabs-border-color, var(--ox-color-pale)). + * + * Imported for side effects by every component whose CSS depends on it, the same + * way component stylesheets are imported. Consumers need do nothing. + */`; + +export const renderThemeCss = () => { + const body = themeTokenGroups + .map(({ label, tokens }) => + [` /* ${label} */`, ...tokens.map(([name, value]) => ` ${name}: ${value};`)].join("\n") + ) + .join("\n\n"); + + return `${header}\n:root {\n${body}\n}\n`; +}; diff --git a/src/theme/tokens.spec.ts b/src/theme/tokens.spec.ts new file mode 100644 index 000000000..06ec4380a --- /dev/null +++ b/src/theme/tokens.spec.ts @@ -0,0 +1,581 @@ +import fs from 'fs'; +import path from 'path'; +import ts from 'typescript'; +import { + colorKey, describeColor, FoundColor, opaqueKey, stripNoise, stylesheetColors, +} from './cssColors'; +import { renderThemeCss, themeTokens } from './themeCss'; + +const srcDir = path.join(__dirname, '..'); +const themeCssPath = path.join(__dirname, 'theme.css'); + +/** + * Colors that appear in component CSS but are deliberately not theme values. Anything + * here is a value we inherited from the styled-components originals and chose not to snap + * to the nearest palette entry, because doing so would be a visual change rather than a + * refactor. Adding to this list should be a deliberate act — prefer adding the color to + * palette.ts if it is really part of the design. + * + * Keyed by the form the checker computes: `colorKey` for a color it can resolve, or the + * whitespace-collapsed literal for one it cannot. See `allowlistKey` below. + * + * Translucent colors do not need an entry when their opaque channels are a theme value — + * `rgba(0, 0, 0, 0.2)` is black at 20% and passes on its own. That rule is what lets + * shadows and overlays stay readable without allowlisting every alpha we happen to use, + * while still refusing a new hue smuggled in through rgba(). + */ +const KNOWN_OFF_PALETTE = new Map([ + ['#cccccc', 'Tooltip border and the uncontrolled-form h3 rule. Predates the palette; nearest entry is pale (#d5d5d5).'], + ['#dddddd', 'Modal header bottom rule. Predates the palette; nearest entry is pale (#d5d5d5).'], +]); + +/** + * How a color is looked up in KNOWN_OFF_PALETTE. + * + * A resolvable color is keyed by its channels, so the entry covers every spelling of it + * at once. One we cannot resolve has no channels to key by, so it falls back to the + * literal as written — meaning `hsl()` and friends have to be allowlisted per spelling, + * which is the right amount of friction for a value the checker cannot reason about. + */ +const allowlistKey = ({ literal, rgba }: FoundColor) => + rgba === null ? literal.replace(/\s+/g, ' ').trim().toLowerCase() : colorKey(rgba); + +const walk = ( + dir: string, matches: (fileName: string) => boolean, out: string[] = [] +): string[] => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + walk(full, matches, out); + } else if (matches(entry.name)) { + out.push(full); + } + } + return out; +}; + +const isStylesheet = (fileName: string) => fileName.endsWith('.css'); + +/** Source modules, specs excluded: a spec's CSS import is mapped away by jest anyway. */ +const isModule = (fileName: string) => + /\.tsx?$/.test(fileName) && !/\.spec\.tsx?$/.test(fileName); + +const here = path.basename(__filename); + +/** + * Every color the theme defines, as token name -> the value written in the JS theme. + * + * Read off the projection in themeCss.ts rather than re-derived from palette.ts and + * theme.ts, so the set the checks below run over is by construction the set theme.css is + * generated from. Restating the list here is what let `--ox-color-link` go missing: the + * generator had it, this file did not, and nothing tied the two together. + */ +const themeColors: ReadonlyArray = [...themeTokens()].filter( + ([name]) => name.startsWith('--ox-color-') +); + +/** + * Theme colors describeColor cannot reduce to channels. Asserted empty below rather than + * cast away: such an entry would drop out of themeValues, and the color would then read + * as off-palette everywhere it is used — a confusing failure a long way from its cause. + * + * Taken as a function of the entries so that the guard itself can be tested against a + * malformed value, rather than only ever being run over a theme that happens to be sound. + */ +const unresolvableColors = (entries: ReadonlyArray) => entries + .filter(([, value]) => describeColor(value) === null) + .map(([token, value]) => `${token}: ${value}`); + +const unresolvableThemeColors = unresolvableColors(themeColors); + +/** + * Every theme color, by opaque channels, so a literal can be traced back to its token. + * + * A value can carry more than one token — `--ox-color-link` and `--ox-color-medium-blue` + * are both #026AA1 — so every token holding a value is kept. Reporting all of them lets + * the author pick the one that says what they mean, rather than being sent to whichever + * entry happened to be written last. + */ +const themeValues = themeColors.reduce((byValue, [token, value]) => { + const rgba = describeColor(value); + if (rgba !== null) { + const key = opaqueKey(rgba); + byValue.set(key, [...(byValue.get(key) ?? []), token]); + } + return byValue; +}, new Map()); + +/** + * Everything wrong with the colors in one stylesheet, split by what it would take to fix. + * + * `duplicates` are literals the theme already holds a token for: a mechanical swap that + * changes nothing on screen, and the only kind of finding PENDING_SWEEP defers. + * + * `offPalette` are colors the theme does not have at all, including the ones the checker + * cannot resolve. Introducing one is a design decision rather than a missed swap, so it is + * refused in every stylesheet — a file awaiting the sweep is no more entitled to a new + * color than a clean one. Keeping the two apart is what stops a pending file from + * smuggling one in under cover of the literals it is already known to carry. + * + * Both empty means the file is clean. + */ +interface ColorProblems { duplicates: string[]; offPalette: string[] } + +const colorProblems = (css: string): ColorProblems => { + const duplicates: string[] = []; + const offPalette: string[] = []; + + for (const found of stylesheetColors(css)) { + const { literal, rgba } = found; + const key = allowlistKey(found); + + if (KNOWN_OFF_PALETTE.has(key)) { continue; } + + if (rgba === null) { + offPalette.push( + `"${literal}" is a color this check cannot resolve — build it from a theme token, or add "${key}" to KNOWN_OFF_PALETTE in ${here} with a reason` + ); + continue; + } + + const hex = opaqueKey(rgba); + const tokens = themeValues.get(hex); + + if (rgba.a < 1) { + // An alpha variant of a theme color is fine — there is no token form for it. + if (!tokens) { + offPalette.push( + `"${literal}" is translucent and its channels (${hex}) are not a theme value — add ${hex} to palette.ts, or "${key}" to KNOWN_OFF_PALETTE in ${here} with a reason` + ); + } + } else if (tokens) { + duplicates.push( + `${literal} duplicates the theme — use ${tokens.map((name) => `var(${name})`).join(' or ')}` + ); + } else { + offPalette.push( + `${literal} is not a theme value — add it to palette.ts, or to KNOWN_OFF_PALETTE in ${here} with a reason` + ); + } + } + + return { duplicates, offPalette }; +}; + +/** Both kinds of finding together, for the cases where the distinction does not matter. */ +const allColorProblems = (css: string): string[] => { + const { duplicates, offPalette } = colorProblems(css); + return [...duplicates, ...offPalette]; +}; + +/** + * --ox-* tokens a stylesheet reads but theme.css does not define. + * + * The match is case-insensitive because CSS function names are: `VAR(--ox-color-pale)` is + * the same reference as `var(--ox-color-pale)`, and a check that only knew the lowercase + * spelling would let a typo through in the other one. The lookup stays case-sensitive, + * because custom property *names* are — `var(--OX-color-pale)` really is a reference to + * something nothing defines, and silently falling through to its fallback is the failure + * this check exists to catch. + * + * The name runs to the end of the CSS identifier, non-ASCII included, because anything at + * U+0080 or above is a name code point. `[\w-]+` stopped at the first of them and handed + * back a prefix, so `var(--ox-color-redé)` was looked up as `--ox-color-red`, found, and + * passed — the undefined reference reported as fine, which is the one answer this check + * must never give. + * + * Escaped spellings are still read literally: `var(--ox-color-r\65 d)` is a reference to + * `--ox-color-red` that this reads as `--ox-color-r` and reports as undefined. Wrong, but + * wrong in the direction of a failure rather than a pass, and it is the same + * decode-the-escapes work as the rest of CORE-2885 rather than a boundary this regex can + * fix. + */ +const unknownTokenReferences = (css: string, defined: Map) => [ + ...new Set( + [...stripNoise(css).matchAll(/var\(\s*(--ox-(?:[\w-]|[^\x00-\x7f])+)/gi)] + .map((match) => match[1]) + .filter((name) => !defined.has(name)) + ), +]; + +describe('theme.css', () => { + it('is what the generator produces from the JS theme', () => { + // theme.css is generated, not written: `npm run generate:theme-css`. If this fails, + // the JS theme moved and the committed CSS did not — regenerate rather than editing + // theme.css by hand. + expect(fs.readFileSync(themeCssPath, 'utf8')).toEqual(renderThemeCss()); + }); +}); + +/** + * The checker below is only worth anything if it fails on the things it claims to fail on. + * These cases are the contract: every color syntax reaches the palette check, and the + * ways of writing a color that are legitimately fine stay quiet. + * + * The parsing underneath is covered in cssColors.spec.ts. What is tested here is the layer + * this file adds: which colors the ui-components palette recognizes, and what an author + * is told about the ones it does not. + */ +describe('the color check itself', () => { + const rule = (declaration: string) => allColorProblems(`.x { ${declaration} }`); + + it.each([ + ['hex', 'color: #d5d5d5;', '--ox-color-pale'], + ['short hex', 'color: #FFF;', '--ox-color-white'], + ['named color', 'color: white;', '--ox-color-white'], + ['named color in a shorthand', 'border: 1px solid whitesmoke;', '--ox-color-neutral-bright'], + ['functional rgb', 'color: rgb(213, 213, 213);', '--ox-color-pale'], + ['space-separated rgb', 'color: rgb(213 213 213 / 100%);', '--ox-color-pale'], + ['percentage rgb', 'color: rgb(100%, 100%, 100%);', '--ox-color-white'], + ['hex in a var() fallback', 'color: var(--thing, #d5d5d5);', '--ox-color-pale'], + ['hex in an uppercase var() fallback', 'color: VAR(--thing, #d5d5d5);', '--ox-color-pale'], + ['color in a gradient stop', 'background: linear-gradient(to right, #d5d5d5, transparent);', '--ox-color-pale'], + ['named color in a custom property', '--tabs-border-color: whitesmoke;', '--ox-color-neutral-bright'], + ['named color in box-shadow', 'box-shadow: 0 0 0.2rem white;', '--ox-color-white'], + ['named color in a vendor-prefixed property', '-webkit-text-fill-color: white;', '--ox-color-white'], + ['hex outside a color property', 'animation-name: #d5d5d5;', '--ox-color-pale'], + ])('flags a %s that duplicates a token', (_case, declaration, token) => { + expect(rule(declaration)).toEqual([expect.stringContaining(`use var(${token})`)]); + }); + + it.each([ + ['hex', 'color: #123456;'], + ['named color', 'color: tan;'], + ['named color in a longhand', 'color: red;'], + ['named color in a shorthand', 'border: 1px solid red;'], + ['named color in a gradient', 'background: linear-gradient(to right, tan, transparent);'], + ['rgb', 'color: rgb(1, 2, 3);'], + ['hsl', 'color: hsl(200 50% 50%);'], + ['oklch', 'color: oklch(70% 0.1 200);'], + ['color()', 'color: color(display-p3 1 0 0);'], + ['translucent off-palette color', 'background: rgba(1, 2, 3, 0.5);'], + // rgb() may legally hold var() channels, but then we cannot tell what color it is; + // flagging beats skipping, which would let an off-palette value through unchecked. + ['rgb() with var() channels', 'background: rgba(var(--channels), 0.2);'], + ])('flags an untokenized %s', (_case, declaration) => { + expect(rule(declaration)).toHaveLength(1); + }); + + it.each([ + ['a token reference', 'color: var(--ox-color-pale);'], + ['a nested token fallback', 'color: var(--tabs-border-color, var(--ox-color-pale));'], + ['an uppercase token reference', 'color: VAR(--ox-color-pale);'], + ['color-mix over tokens', 'background: color-mix(in srgb, var(--ox-color-black) 20%, transparent);'], + ['transparent', 'background: transparent;'], + ['currentcolor', 'border-color: currentcolor;'], + ['a system color', 'outline: 0.2rem auto Highlight;'], + ['an allowlisted color', 'border-color: #ccc;'], + ['alpha over a theme color', 'box-shadow: 0 0 0.2rem rgba(0, 0, 0, 0.2);'], + ['a keyword that merely contains a color name', 'animation-name: moveblue;'], + ['an animation named after a color', 'animation-name: red;'], + ['a font named after a color', 'font-family: white;'], + ['a grid area named after a color', 'grid-area: gold;'], + ['a non-color value', 'filter: grayscale(1);'], + ])('stays quiet for %s', (_case, declaration) => { + expect(rule(declaration)).toEqual([]); + }); + + it('ignores color-shaped text outside declaration values', () => { + expect(allColorProblems('.red { }')).toEqual([]); + expect(allColorProblems('.x { content: "tan"; }')).toEqual([]); + expect(allColorProblems('.x { /* #d5d5d5 */ color: var(--ox-color-pale); }')).toEqual([]); + }); + + it('checks declarations nested in at-rules', () => { + const css = '@media screen and (min-width: 75em) { .x { color: #d5d5d5; } }'; + expect(allColorProblems(css)).toEqual([expect.stringContaining('use var(--ox-color-pale)')]); + }); + + it('sorts a finding by whether the theme already has the color', () => { + // The split is what PENDING_SWEEP keys off, so it is worth stating directly: a file + // may be excused the literals it copied from the theme, never a color the theme + // does not have. Both kinds in one stylesheet, to show neither absorbs the other. + const css = '.x { color: #d5d5d5; border-color: #123456; background: hsl(200 50% 50%); }'; + expect(colorProblems(css)).toEqual({ + duplicates: [expect.stringContaining('use var(--ox-color-pale)')], + offPalette: [ + expect.stringContaining('#123456 is not a theme value'), + expect.stringContaining('cannot resolve'), + ], + }); + }); + + it('can reduce every theme color to channels', () => { + // Guards the themeValues map: see unresolvableThemeColors above for why a silent drop + // would be worse than a failure here. + expect(unresolvableThemeColors).toEqual([]); + }); + + it('would fail if a theme color were malformed', () => { + // The guard above only means something if it can fail. `#ggg` is the case that used to + // slip through it: expanded to six characters it looked like a color, so it entered + // themeValues under a key nothing could ever match. + expect(unresolvableColors([['--ox-color-bad', '#ggg']])).toEqual(['--ox-color-bad: #ggg']); + }); + + it('checks every color token the theme projects, semantic ones included', () => { + // themeColors is derived from the projection, so this cannot drift the way the + // hand-written list did — --ox-color-link was absent from it, leaving the link color + // outside both the resolvability guard and the duplicate check. + expect(themeColors.map(([name]) => name)).toEqual( + [...themeTokens().keys()].filter((name) => name.startsWith('--ox-color-')) + ); + expect(themeColors.map(([name]) => name)).toEqual( + expect.arrayContaining(['--ox-color-link', '--ox-color-link-hover']) + ); + }); + + it('names every token that carries a color when more than one does', () => { + // #026AA1 is both palette.mediumBlue and colors.link.color. Naming only one would send + // half the authors who hit this to a token that does not say what they mean. + expect(rule('color: #026AA1;')).toEqual([ + '#026AA1 duplicates the theme — use var(--ox-color-medium-blue) or var(--ox-color-link)', + ]); + }); + + it('flags a reference to a token that does not exist', () => { + const defined = themeTokens(); + expect(unknownTokenReferences('.x { color: var(--ox-color-pale); }', defined)).toEqual([]); + expect(unknownTokenReferences('.x { color: var(--ox-color-palee); }', defined)) + .toEqual(['--ox-color-palee']); + }); + + it('reads a reference however the var() is spelled', () => { + // CSS function names are ASCII case-insensitive. Knowing only the lowercase spelling + // would mean an uppercase typo fell through to its fallback unreported — the one + // thing this check is for. + const defined = themeTokens(); + expect(unknownTokenReferences('.x { color: VAR(--ox-color-pale); }', defined)).toEqual([]); + expect(unknownTokenReferences('.x { color: VAR(--ox-color-palee); }', defined)) + .toEqual(['--ox-color-palee']); + expect(unknownTokenReferences('.x { color: Var( --ox-color-palee ); }', defined)) + .toEqual(['--ox-color-palee']); + }); + + it('reads the whole name when it carries a character outside ASCII', () => { + // CSS name code points include everything at U+0080 and above, so `--ox-color-redé` is + // a valid name and a different one from `--ox-color-red`. Truncating to the prefix + // found a token that exists and passed the reference that does not — a missing token + // reported as present, which is the failure mode this check cannot have. + const defined = themeTokens(); + expect(defined.has('--ox-color-red')).toBe(true); + expect(unknownTokenReferences('.x { color: var(--ox-color-redé); }', defined)) + .toEqual(['--ox-color-redé']); + expect(unknownTokenReferences('.x { color: var(--ox-color-red🎨); }', defined)) + .toEqual(['--ox-color-red🎨']); + expect(unknownTokenReferences('.x { color: var(--ox-cölor-pale); }', defined)) + .toEqual(['--ox-cölor-pale']); + // and the name it is a prefix of still passes + expect(unknownTokenReferences('.x { color: var(--ox-color-red); }', defined)).toEqual([]); + }); + + it('flags a token name whose case does not match', () => { + // Property names, unlike function names, are case-sensitive: --OX-color-pale is not + // the token, so the declaration resolves to its fallback or nothing at all. + expect(unknownTokenReferences('.x { color: var(--OX-color-pale); }', themeTokens())) + .toEqual(['--OX-color-pale']); + }); +}); + +/** + * Stylesheets migrated before the tokens existed, still carrying hand-copied literals. + * Each is removed by the PR that sweeps it; the list is expected to reach empty, at which + * point it and the assertion below go away with it. + * + * The exemption is narrow: only the duplicate-literal check. The off-palette check below + * runs over these files too, so being on this list defers a swap that is already owed and + * grants nothing else — a new color in one of them fails exactly as it would anywhere. + * + * The point of listing them rather than skipping the check is that the list is asserted to + * be *exactly* the set with duplicates left, so it cannot rot in either direction: dropping + * a name without sweeping the file fails, and sweeping a file without dropping its name + * fails too. + */ +const PENDING_SWEEP = new Set([ + 'components/Button.css', + 'components/ButtonBar.css', + 'components/Checkbox/Checkbox.css', + 'components/CloseModalButton.css', + 'components/DropdownMenu.css', + 'components/Modal.css', + 'components/NavBar.css', + 'components/NavBar.stories.css', + 'components/NavBarMenuButtons.css', + 'components/Overlay.css', + 'components/Radio.css', + 'components/Tabs.css', + 'components/Text.css', + 'components/Toast.css', + 'components/Tooltip.css', + 'components/forms/uncontrolled/inputTypes.css', +]); + +describe('component CSS', () => { + const cssFiles = walk(srcDir, isStylesheet).filter((file) => file !== themeCssPath); + const tokens = themeTokens(); + const name = (file: string) => path.relative(srcDir, file); + + it('has files to check', () => { + // Guards against the walk silently finding nothing and the suite passing vacuously. + expect(cssFiles.length).toBeGreaterThan(0); + }); + + it('lists exactly the stylesheets whose literals still duplicate the theme', () => { + const failing = cssFiles + .filter((file) => colorProblems(fs.readFileSync(file, 'utf8')).duplicates.length > 0) + .map(name); + expect(failing.sort()).toEqual([...PENDING_SWEEP].sort()); + }); + + it.each(cssFiles.map((file) => [name(file), file]))( + '%s introduces no color the theme does not have', + (_name, file) => { + // Every stylesheet, PENDING_SWEEP included: the exemption is for literals that + // duplicate a token, not a licence to add a color while the file waits its turn. + expect(colorProblems(fs.readFileSync(file, 'utf8')).offPalette).toEqual([]); + } + ); + + it.each( + cssFiles.filter((file) => !PENDING_SWEEP.has(name(file))).map((file) => [name(file), file]) + )( + '%s uses tokens rather than repeating theme values', + (_name, file) => { + expect(colorProblems(fs.readFileSync(file, 'utf8')).duplicates).toEqual([]); + } + ); + + it.each(cssFiles.map((file) => [name(file), file]))( + '%s only references tokens that exist', + (_name, file) => { + expect(unknownTokenReferences(fs.readFileSync(file, 'utf8'), tokens)).toEqual([]); + } + ); +}); + +/** + * Stylesheets a module imports, as absolute paths. + * + * Parsed rather than matched. A regex over the source also sees `// import './x.css';` and + * the same text inside a string, and reading either as an import is the wrong way round for + * a check like this: it says the token file is present when it is not, so deleting a real + * import next to a commented-out one would pass. + * + * Only relative specifiers, because that is how a component reaches its own CSS and the + * token file; a package-relative spelling would not resolve inside src/ anyway. Only static + * imports, which is what a stylesheet side effect is written as here — `require` and dynamic + * `import()` of CSS appear nowhere in src, and would need their own handling if they did. + */ +const importedStylesheets = (moduleFile: string, source: string): string[] => { + const parsed = ts.createSourceFile( + moduleFile, + source, + ts.ScriptTarget.Latest, + /* setParentNodes */ false, + moduleFile.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS + ); + + return parsed.statements + .filter(ts.isImportDeclaration) + .map(({ moduleSpecifier }) => moduleSpecifier) + .filter(ts.isStringLiteral) + .map(({ text }) => text) + .filter((specifier) => specifier.startsWith('.') && specifier.endsWith('.css')) + .map((specifier) => path.resolve(path.dirname(moduleFile), specifier)); +}; + +/** + * Stylesheets this module pulls in that read a token, when the module does not also pull in + * the file that defines them. Empty means the module is sound. + * + * There is no bundler here: build.bash rsyncs CSS 1:1, so nothing resolves an `@import` for + * us and a stylesheet does not drag theme.css in by itself. The component that imports the + * stylesheet has to import the token file too, or every `var(--ox-*)` in it silently takes + * its fallback — which is exactly the failure that is invisible in review, because a + * fallback is usually the literal the token replaced and so looks right on screen. + */ +const missingThemeImport = ( + moduleFile: string, source: string, readsTokens: ReadonlySet +): string[] => { + const imported = importedStylesheets(moduleFile, source); + const needy = imported.filter((file) => readsTokens.has(file)); + + return imported.includes(themeCssPath) ? [] : needy; +}; + +/** Whether a stylesheet reads a theme token at all. */ +const readsThemeToken = (css: string) => /var\(\s*--ox-/i.test(stripNoise(css)); + +describe('the token import rule', () => { + const moduleFile = path.join(srcDir, 'components/Thing.tsx'); + const ownStyles = path.join(srcDir, 'components/Thing.css'); + const readsTokens = new Set([ownStyles]); + + it('flags a component that imports a token-reading stylesheet and not the tokens', () => { + expect(missingThemeImport(moduleFile, "import './Thing.css';", readsTokens)) + .toEqual([ownStyles]); + }); + + it('stays quiet when the component imports the token file too', () => { + const source = "import './Thing.css';\nimport '../theme/theme.css';"; + expect(missingThemeImport(moduleFile, source, readsTokens)).toEqual([]); + }); + + it('stays quiet when the stylesheet reads no token', () => { + expect(missingThemeImport(moduleFile, "import './Thing.css';", new Set())).toEqual([]); + }); + + it('does not count a commented-out import of the token file', () => { + // The failure this rule exists to catch is a missing import, so anything that reads as + // present when it is absent defeats it — deleting the real import and leaving the + // comment behind is the exact shape of that. + const source = "import './Thing.css';\n// import '../theme/theme.css';"; + expect(missingThemeImport(moduleFile, source, readsTokens)).toEqual([ownStyles]); + + const block = "import './Thing.css';\n/* import '../theme/theme.css'; */"; + expect(missingThemeImport(moduleFile, block, readsTokens)).toEqual([ownStyles]); + }); + + it('does not count the specifier appearing in a string', () => { + const source = "import './Thing.css';\nexport const doc = \"import '../theme/theme.css';\";"; + expect(missingThemeImport(moduleFile, source, readsTokens)).toEqual([ownStyles]); + }); + + it('counts a real import written with a comment beside it', () => { + const source = "import './Thing.css';\nimport '../theme/theme.css'; // tokens\n"; + expect(missingThemeImport(moduleFile, source, readsTokens)).toEqual([]); + }); + + it('parses a .tsx module', () => { + // TS and TSX disagree about `x`, so the kind is chosen from the extension rather + // than left at the default. A component file is the case that actually matters here. + const tsx = "import './Thing.css';\nexport const T = () =>
;"; + expect(missingThemeImport(moduleFile, tsx, readsTokens)).toEqual([ownStyles]); + }); + + it('resolves an import from a subdirectory', () => { + const nested = path.join(srcDir, 'components/Thing/Thing.tsx'); + const nestedStyles = path.join(srcDir, 'components/Thing/Thing.css'); + expect(missingThemeImport(nested, "import './Thing.css';", new Set([nestedStyles]))) + .toEqual([nestedStyles]); + }); + + it('reads a token reference through comments and however var() is spelled', () => { + expect(readsThemeToken('.x { color: VAR(--ox-color-pale); }')).toBe(true); + expect(readsThemeToken('.x { /* var(--ox-color-pale) */ color: red; }')).toBe(false); + expect(readsThemeToken('.x { color: var(--tabs-border-color); }')).toBe(false); + }); + + it.each(walk(srcDir, isModule).map((file) => [path.relative(srcDir, file), file]))( + '%s imports theme.css if its stylesheet needs it', + (_name, file) => { + // Vacuous on this branch by construction — no stylesheet reads a token until the + // sweep in #143 converts one, and the rule exists so that sweep cannot forget the + // import. The helper above is tested on its own, so the rule itself is pinned now. + const readsTokens = new Set( + walk(srcDir, isStylesheet).filter((css) => readsThemeToken(fs.readFileSync(css, 'utf8'))) + ); + expect(missingThemeImport(file, fs.readFileSync(file, 'utf8'), readsTokens)).toEqual([]); + } + ); +});