diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d19da430..ef826872a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -436,6 +436,11 @@ jobs: - name: Validate form-field defaults run: pnpm run styles:form-field-defaults:validate + # m3-theme.scss themes only the Material components the app + # imports; a new import without a theme entry renders unthemed. + - name: Validate the Material theme scope + run: pnpm run styles:material-theme-scope:validate + # Nx rejects a non-parallel task with continuous dependencies, and # the Playwright plugin infers serve dependencies only when CI is # unset, so this checks both the local and the CI inference. diff --git a/apps/web/src/m3-theme.scss b/apps/web/src/m3-theme.scss index f7e189a8f..2a39a33e1 100644 --- a/apps/web/src/m3-theme.scss +++ b/apps/web/src/m3-theme.scss @@ -1,4 +1,5 @@ @use 'sass:map'; +@use 'sass:meta'; @use '@angular/material' as mat; @use 'styles'; @@ -57,9 +58,58 @@ $dark-theme: mat.define-theme( ) ); +// ─── Themed components ─────────────────────────────────────────────────────── +// Only the Material components the app imports, in Material's own order. +// `mat.all-component-themes` also emitted about 24 KB of tokens for 17 +// components nothing imports. Add an entry with a new +// `@angular/material/` import and remove it with the last one: +// `pnpm run styles:material-theme-scope:validate` (CI) checks both +// directions, and `core` (ripples, options, pseudo-checkboxes) always stays. +$material-components: ( + core, + card, + progress-bar, + tooltip, + form-field, + input, + select, + dialog, + slide-toggle, + slider, + menu, + list, + checkbox, + button, + icon-button, + fab, + snack-bar, + progress-spinner, + button-toggle, + divider, + icon +); + +@mixin component-themes($theme) { + @each $component in $material-components { + @include meta.apply( + meta.get-mixin('#{$component}-theme', 'mat'), + $theme + ); + } +} + +@mixin component-colors($theme) { + @each $component in $material-components { + @include meta.apply( + meta.get-mixin('#{$component}-color', 'mat'), + $theme + ); + } +} + // ─── Apply Themes ──────────────────────────────────────────────────────────── html { - @include mat.all-component-themes($light-theme); + @include component-themes($light-theme); // The theme is built with the legacy `define-theme` config, whose // component mixins emit resolved component tokens but never declare the @@ -170,7 +220,7 @@ html { --app-rating-color: #7a4a00; .dark-theme { - @include mat.all-component-colors($dark-theme); + @include component-colors($dark-theme); // Only the color roles differ per theme: the elevation shadows are // the same black in both, so html's declarations already apply here. @include mat.system-level-colors($dark-theme); diff --git a/apps/web/src/styles.scss b/apps/web/src/styles.scss index e2a6d22dc..c196cd71e 100644 --- a/apps/web/src/styles.scss +++ b/apps/web/src/styles.scss @@ -238,10 +238,6 @@ body.dark-theme { overflow-x: hidden !important; } -.mat-drawer-inner-container { - overflow: hidden !important; -} - .cdk-overlay-backdrop { backdrop-filter: blur(4px); } diff --git a/package.json b/package.json index a3ef63593..4445c6d3d 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,9 @@ "styles:form-field-defaults:test": "node --test tools/nx/check-form-field-defaults.test.mjs", "styles:form-field-defaults:check": "node tools/nx/check-form-field-defaults.mjs", "styles:form-field-defaults:validate": "pnpm run styles:form-field-defaults:test && pnpm run styles:form-field-defaults:check", + "styles:material-theme-scope:test": "node --test tools/nx/check-material-theme-scope.test.mjs", + "styles:material-theme-scope:check": "node tools/nx/check-material-theme-scope.mjs", + "styles:material-theme-scope:validate": "pnpm run styles:material-theme-scope:test && pnpm run styles:material-theme-scope:check", "e2e:task-graphs:test": "node --test tools/nx/check-e2e-task-graphs.test.mjs", "e2e:task-graphs:check": "node tools/nx/check-e2e-task-graphs.mjs", "e2e:task-graphs:validate": "pnpm run e2e:task-graphs:test && pnpm run e2e:task-graphs:check", diff --git a/tools/nx/check-material-theme-scope.mjs b/tools/nx/check-material-theme-scope.mjs new file mode 100644 index 000000000..8b14c431e --- /dev/null +++ b/tools/nx/check-material-theme-scope.mjs @@ -0,0 +1,199 @@ +import { execFileSync } from 'node:child_process'; +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { isScanned } from './check-theme-references.mjs'; + +/** + * `apps/web/src/m3-theme.scss` themes only the Material components the app + * imports, through its `$material-components` list; `all-component-themes` + * also emitted about 24 KB of tokens for components nothing uses. This guard + * keeps the list in step with the imports in both directions: a component a + * new `@angular/material/` import brings in must be themed, or it + * renders with no colours at all, and a component whose last import went + * must leave the list. + */ + +/** Angular renderer sources that load the theme (the remote control has its own styles). */ +const SCANNED_PATHSPECS = ['apps/web/*.ts', 'libs/*.ts']; + +export const THEME_FILE = 'apps/web/src/m3-theme.scss'; + +const ENTRY_POINT_IMPORT = /from\s+['"]@angular\/material\/([a-z-]+)['"]/g; + +/** + * The theme mixins (`mat.-theme`, `mat.-color`) each Material + * entry point needs. An entry point that is not listed has no themed + * component. + */ +export const THEMES_BY_ENTRY_POINT = new Map([ + ['autocomplete', ['autocomplete']], + ['badge', ['badge']], + ['bottom-sheet', ['bottom-sheet']], + ['button', ['button', 'icon-button', 'fab']], + ['button-toggle', ['button-toggle']], + ['card', ['card']], + ['checkbox', ['checkbox']], + ['chips', ['chips']], + ['core', ['core']], + ['datepicker', ['datepicker']], + ['dialog', ['dialog']], + ['divider', ['divider']], + ['expansion', ['expansion']], + ['form-field', ['form-field']], + ['grid-list', ['grid-list']], + ['icon', ['icon']], + ['input', ['input']], + ['list', ['list']], + ['menu', ['menu']], + ['paginator', ['paginator']], + ['progress-bar', ['progress-bar']], + ['progress-spinner', ['progress-spinner']], + ['radio', ['radio']], + ['select', ['select']], + ['sidenav', ['sidenav']], + ['slide-toggle', ['slide-toggle']], + ['slider', ['slider']], + ['snack-bar', ['snack-bar']], + ['sort', ['sort']], + ['stepper', ['stepper']], + ['table', ['table']], + ['tabs', ['tabs']], + ['timepicker', ['timepicker']], + ['toolbar', ['toolbar']], + ['tooltip', ['tooltip']], + ['tree', ['tree']], +]); + +/** + * Themed whatever is imported: `core` carries the ripple, option, optgroup + * and pseudo-checkbox tokens that select, menu and list render with. + */ +export const ALWAYS_THEMED = ['core']; + +/** Material entry points the runtime sources import, with one importer each. */ +export function usedEntryPoints(sources) { + const importers = new Map(); + for (const { file, source } of sources) { + for (const match of source.matchAll(ENTRY_POINT_IMPORT)) { + if (!importers.has(match[1])) importers.set(match[1], file); + } + } + return importers; +} + +/** The theme names the imported entry points need, each with a reason. */ +export function requiredThemes(importers) { + const required = new Map( + ALWAYS_THEMED.map((name) => [name, 'always themed']) + ); + for (const [entryPoint, file] of importers) { + for (const name of THEMES_BY_ENTRY_POINT.get(entryPoint) ?? []) { + if (!required.has(name)) { + required.set( + name, + `@angular/material/${entryPoint} in ${file}` + ); + } + } + } + return required; +} + +/** + * The components the theme file themes: the `$material-components` list + * plus any `mat.-theme` included directly. `all` means + * `all-component-themes` is still included, which themes everything. + */ +export function themedComponents(scss) { + const themed = new Set(); + const list = scss.match(/\$material-components\s*:\s*\(([^)]*)\)/); + if (list) { + for (const item of list[1].split(',')) { + const name = item.replace(/\/\/.*$/gm, '').trim(); + if (name) themed.add(name); + } + } + for (const match of scss.matchAll(/@include\s+mat\.([a-z-]+)-theme\(/g)) { + themed.add(match[1]); + } + return { + themed, + all: /@include\s+mat\.all-component-themes\(/.test(scss), + }; +} + +/** + * Themes that imports need but the file lacks, and themes the file carries + * that no import needs. + */ +export function compareThemeScope(required, { themed, all }) { + const unthemed = all + ? [] + : [...required].filter(([name]) => !themed.has(name)); + const unused = [...themed].filter((name) => !required.has(name)); + return { unthemed, unused }; +} + +/** Tracked renderer sources the guard reads, at any depth. */ +export function listScannedFiles(rootDir) { + // No shell: `cmd.exe` treats single quotes as literal characters, so a + // POSIX-quoted pathspec reaches git intact on Windows and matches nothing. + return execFileSync('git', ['ls-files', ...SCANNED_PATHSPECS], { + cwd: rootDir, + encoding: 'utf8', + maxBuffer: 32 * 1024 * 1024, + }) + .trim() + .split('\n') + .filter(Boolean) + .filter(isScanned); +} + +const isMain = + process.argv[1] && + path.resolve(process.argv[1]) === + path.resolve(fileURLToPath(import.meta.url)); + +if (isMain) { + const rootDir = process.cwd(); + const files = listScannedFiles(rootDir); + const sources = await Promise.all( + files.map(async (file) => ({ + file, + source: await readFile(path.join(rootDir, file), 'utf8'), + })) + ); + const required = requiredThemes(usedEntryPoints(sources)); + const scope = themedComponents( + await readFile(path.join(rootDir, THEME_FILE), 'utf8') + ); + const { unthemed, unused } = compareThemeScope(required, scope); + + if (unthemed.length > 0) { + console.error( + `Material components the app imports but ${THEME_FILE} does not theme; they render without colours. Add them to $material-components:` + ); + for (const [name, reason] of unthemed) { + console.error(`- ${name} (${reason})`); + } + process.exitCode = 1; + } + if (unused.length > 0) { + console.error( + `Material components ${THEME_FILE} themes although no runtime source imports them; their tokens only add to the initial stylesheet. Remove them from $material-components:` + ); + for (const name of unused) { + console.error(`- ${name}`); + } + process.exitCode = 1; + } + if (!process.exitCode) { + console.log( + scope.all + ? `Checked ${files.length} renderer sources; ${THEME_FILE} themes every Material component through all-component-themes.` + : `Checked ${files.length} renderer sources; ${THEME_FILE} themes exactly the ${scope.themed.size} Material components they import.` + ); + } +} diff --git a/tools/nx/check-material-theme-scope.test.mjs b/tools/nx/check-material-theme-scope.test.mjs new file mode 100644 index 000000000..4843f750a --- /dev/null +++ b/tools/nx/check-material-theme-scope.test.mjs @@ -0,0 +1,107 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +import { + compareThemeScope, + requiredThemes, + themedComponents, + usedEntryPoints, +} from './check-material-theme-scope.mjs'; + +const lines = (...rows) => rows.join('\n'); + +test('usedEntryPoints lists each Material entry point with its first importer', () => { + const importers = usedEntryPoints([ + { + file: 'a.component.ts', + source: lines( + "import { MatButtonModule } from '@angular/material/button';", + 'import {', + ' MatDialog,', + "} from '@angular/material/dialog';", + 'import { MatIcon } from "@angular/material/icon";' + ), + }, + { + file: 'b.component.ts', + source: "import { MatButton } from '@angular/material/button';", + }, + ]); + + assert.deepEqual( + [...importers], + [ + ['button', 'a.component.ts'], + ['dialog', 'a.component.ts'], + ['icon', 'a.component.ts'], + ] + ); +}); + +test('requiredThemes always keeps core and expands the button entry point', () => { + const required = requiredThemes( + new Map([ + ['button', 'a.ts'], + ['tooltip', 'b.ts'], + ['testing', 'c.ts'], + ]) + ); + + assert.deepEqual( + [...required], + [ + ['core', 'always themed'], + ['button', '@angular/material/button in a.ts'], + ['icon-button', '@angular/material/button in a.ts'], + ['fab', '@angular/material/button in a.ts'], + ['tooltip', '@angular/material/tooltip in b.ts'], + ] + ); +}); + +test('themedComponents reads the list, direct includes and the all-components mixin', () => { + assert.deepEqual( + themedComponents( + lines( + '$material-components: (', + ' core,', + ' button, // used by every dialog', + ' icon-button,', + ');', + '@include mat.tooltip-theme($theme);' + ) + ), + { + themed: new Set(['core', 'button', 'icon-button', 'tooltip']), + all: false, + } + ); + assert.deepEqual( + themedComponents('html { @include mat.all-component-themes($t); }'), + { themed: new Set(), all: true } + ); +}); + +test('compareThemeScope reports missing and surplus themes', () => { + const required = new Map([ + ['core', 'always themed'], + ['button', '@angular/material/button in a.ts'], + ]); + + assert.deepEqual( + compareThemeScope(required, { + themed: new Set(['core', 'tabs']), + all: false, + }), + { + unthemed: [['button', '@angular/material/button in a.ts']], + unused: ['tabs'], + } + ); + // all-component-themes leaves nothing unthemed, but is still surplus-free + // only when the list is empty. + assert.deepEqual( + compareThemeScope(required, { themed: new Set(), all: true }), + { unthemed: [], unused: [] } + ); +});