fix(web): make the small spinner actually small

docs/badges.json

@@ -10,7 +10,7 @@
   "packages": 43,
   "goFiles": 192,
   "goLines": 48278,
-  "testFiles": 931,
+  "testFiles": 932,
   "directDeps": 3,
   "transitiveDeps": 15,
   "totalTests": 6479,

tests/spec/css_modifier_layer_test.go

@@ -0,0 +1,109 @@
+package spec_test
+
+import (
+	"path/filepath"
+	"regexp"
+	"sort"
+	"strings"
+	"testing"
+)
+
+// A size modifier has to be able to beat the class it modifies.
+//
+// Tailwind v4 compiles every @utility into @layer utilities. A plain CSS rule
+// written outside any @layer is unlayered, and unlayered CSS wins over layered
+// CSS in the cascade whatever the specificity. So when the base class is a plain
+// rule and its modifier is an @utility, the base overrides the modifier and the
+// modifier does nothing at all.
+//
+// That is what happened to vault42-spinner-sm. .vault42-spinner was a plain rule
+// setting w-5 h-5; vault42-spinner-sm was an @utility setting w-4 h-4. Every
+// "small" spinner in the app rendered at the default size, on two call sites,
+// and nothing could see it: the class was present in the markup, the rule was
+// present in the stylesheet, and the build emitted both. Only the layer differed.
+// vault42-spinner-lg escaped only because somebody happened to write it as a
+// plain rule next to the base.
+//
+// This gate holds the pairing rather than the sizes: a modifier and its base
+// must be declared the same way.
+//
+// That is a necessary condition and not a sufficient one, and the difference is
+// worth stating here so nobody reads a pass as proof. Order inside @layer
+// utilities is Tailwind's property-set sort, not source order, so two utilities
+// declared identically can still emit in the wrong order -- measured, adding
+// w-full to vault42-btn-sm moves it ahead of vault42-btn and makes it inert at
+// every one of its nine call sites, and this gate passes throughout. The gate
+// that settles it compiles the stylesheet and reads the emitted offsets:
+// web/src/__tests__/spinnerCascade.test.ts. This one is the cheap early signal
+// that runs with the Go suite.
+//
+// The tests are read-only. They never write to the source tree.
+
+var (
+	styleSheet = filepath.Join("web", "src", "style.css")
+
+	// `@utility name {` -- the layered form.
+	utilityDecl = regexp.MustCompile(`(?m)^@utility\s+([a-zA-Z0-9_-]+)\s*\{`)
+	// `.name {` at column zero -- the unlayered form.
+	plainDecl = regexp.MustCompile(`(?m)^\.([a-zA-Z0-9_-]+)\s*\{`)
+
+	// Suffixes that mean "this class modifies another one" rather than standing
+	// alone. A class ending in one of these is checked against its base.
+	modifierSuffixes = []string{"-sm", "-lg", "-xs", "-xl"}
+)
+
+func TestACSSModifierIsDeclaredTheSameWayAsTheClassItModifies(t *testing.T) {
+	css := readFileString(t, filepath.Join(repoRoot(t), styleSheet))
+
+	// class name -> "utility" or "plain"
+	form := map[string]string{}
+	for _, m := range utilityDecl.FindAllStringSubmatch(css, -1) {
+		form[m[1]] = "@utility"
+	}
+	for _, m := range plainDecl.FindAllStringSubmatch(css, -1) {
+		form[m[1]] = "a plain rule"
+	}
+
+	if len(form) == 0 {
+		t.Fatalf("no class declarations found in %s. If the stylesheet moved, move this "+
+			"gate with it: what it holds is that a size modifier can actually beat the "+
+			"class it modifies.", styleSheet)
+	}
+
+	var checked int
+	names := make([]string, 0, len(form))
+	for name := range form {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+
+	for _, name := range names {
+		for _, suffix := range modifierSuffixes {
+			if !strings.HasSuffix(name, suffix) {
+				continue
+			}
+			base := strings.TrimSuffix(name, suffix)
+			baseForm, ok := form[base]
+			if !ok {
+				// Not a modifier of anything in this sheet; it just ends that way.
+				continue
+			}
+			checked++
+			if baseForm != form[name] {
+				t.Errorf("%s is %s and %s is %s.\n"+
+					"@utility rules land in @layer utilities and plain rules are unlayered, "+
+					"and unlayered beats layered whatever the specificity -- so the plain one "+
+					"wins and the other is inert no matter what it declares. Declare both the "+
+					"same way and let source order decide.",
+					name, form[name], base, baseForm)
+			}
+		}
+	}
+
+	// A gate that compares no pairs proves nothing.
+	if checked == 0 {
+		t.Fatalf("no modifier/base pairs found in %s, so this gate compared nothing. "+
+			"If the naming convention changed, update modifierSuffixes.", styleSheet)
+	}
+	t.Logf("checked %d modifier/base pairs", checked)
+}

web/src/__tests__/spinnerCascade.test.ts

@@ -0,0 +1,120 @@
+import { describe, it, expect } from 'vitest'
+import { readFileSync, readdirSync } from 'node:fs'
+import { fileURLToPath } from 'node:url'
+import { dirname, join, resolve as resolvePath } from 'node:path'
+import { compile } from 'tailwindcss'
+
+/**
+ * A size modifier has to actually beat the class it modifies, in the output.
+ *
+ * Declaring both the same way is necessary and not sufficient, which is the
+ * whole reason this gate compiles instead of reading. Tailwind orders rules
+ * inside @layer utilities by a property-set sort, not by source order, so two
+ * utilities written adjacently can still emit in the wrong order -- measured:
+ * adding w-full, h-4, block, inline-flex or z-10 to vault42-btn-sm moves it
+ * ahead of vault42-btn and makes it inert at every one of its call sites, while
+ * adding rounded-md or leading-none does not. Nothing about the source says
+ * which of those you just did.
+ *
+ * The defect this was written for: vault42-spinner-sm set w-4 h-4 as an
+ * @utility, which lands in @layer utilities, while .vault42-spinner was a plain
+ * unlayered rule setting w-5 h-5. Unlayered beats layered whatever the
+ * specificity, so every "small" spinner rendered at the default size. The class
+ * was in the markup, the rule was in the stylesheet, the build emitted both.
+ * Only the layer differed.
+ */
+
+const srcDir = resolvePath(dirname(fileURLToPath(import.meta.url)), '..')
+const repoRoot = resolvePath(srcDir, '../..')
+
+/** Modifier/base pairs whose relative order in the output is load-bearing. */
+const PAIRS: Array<[string, string]> = [
+  ['vault42-spinner-sm', 'vault42-spinner'],
+  ['vault42-spinner-lg', 'vault42-spinner'],
+  ['vault42-btn-sm', 'vault42-btn'],
+]
+
+function sourceFiles(dir: string, out: string[] = []): string[] {
+  for (const e of readdirSync(dir, { withFileTypes: true })) {
+    if (e.name === 'node_modules' || e.name === 'dist' || e.name === '.git') continue
+    const p = join(dir, e.name)
+    if (e.isDirectory()) sourceFiles(p, out)
+    else if (/\.(vue|ts|js|html)$/.test(e.name)) out.push(p)
+  }
+  return out
+}
+
+async function buildCSS(): Promise<string> {
+  const files = [
+    ...sourceFiles(srcDir),
+    ...sourceFiles(resolvePath(repoRoot, 'packages/vue/src')),
+    resolvePath(repoRoot, 'web/index.html'),
+  ]
+  const candidates = new Set<string>()
+  for (const f of files) {
+    for (const m of readFileSync(f, 'utf8').matchAll(/[^\s"'`<>=(){};,]+/g)) candidates.add(m[0])
+  }
+
+  const twBase = resolvePath(repoRoot, 'web/node_modules/tailwindcss')
+  const compiler = await compile(readFileSync(join(srcDir, 'style.css'), 'utf8'), {
+    base: srcDir,
+    loadStylesheet: async (id: string, base: string) => {
+      const file = id === 'tailwindcss'
+        ? join(twBase, 'index.css')
+        : id.startsWith('tailwindcss/')
+          ? join(twBase, id.slice('tailwindcss/'.length))
+          : resolvePath(base, id)
+      return { path: file, base: dirname(file), content: readFileSync(file, 'utf8') }
+    },
+    loadModule: async () => {
+      throw new Error('no modules')
+    },
+  })
+  return compiler.build([...candidates])
+}
+
+/**
+ * Offset of a class's own rule in the emitted CSS, or -1.
+ *
+ * Plain string scanning rather than a regex built from the class name. The
+ * first version escaped the name with cls.replace(/[-]/g, ...), which handles
+ * exactly one metacharacter and is the shape CodeQL calls incomplete
+ * sanitization -- correctly, even though every name here is a literal from the
+ * table above. Not building a pattern from a value is simpler than escaping one.
+ *
+ * The delimiter check is what stops `.vault42-spinner` matching inside
+ * `.vault42-spinner-sm`: a class name ends where the selector does.
+ */
+function ruleOffset(css: string, cls: string): number {
+  const needle = `.${cls}`
+  for (let i = css.indexOf(needle); i !== -1; i = css.indexOf(needle, i + 1)) {
+    const after = css[i + needle.length]
+    if (after === undefined) continue
+    if (after === '{' || after === ',' || after === ' ' || after === '\n' || after === ':') {
+      return i
+    }
+  }
+  return -1
+}
+
+describe('utility cascade', () => {
+  it('emits every size modifier after the class it modifies', async () => {
+    const css = await buildCSS()
+
+    for (const [modifier, base] of PAIRS) {
+      const mOff = ruleOffset(css, modifier)
+      const bOff = ruleOffset(css, base)
+
+      expect(mOff, `${modifier} is not in the built CSS at all`).toBeGreaterThan(-1)
+      expect(bOff, `${base} is not in the built CSS at all`).toBeGreaterThan(-1)
+
+      expect(
+        mOff > bOff,
+        `${modifier} is emitted at ${mOff}, before ${base} at ${bOff}, so ${base} wins and ` +
+        `${modifier} does nothing. Order inside @layer utilities is Tailwind's property-set ` +
+        `sort, not source order -- adding a property to the modifier can move it ahead of its ` +
+        `base without anything in the source looking different.`,
+      ).toBe(true)
+    }
+  })
+})

web/src/style.css

@@ -148,11 +148,6 @@
            max-w-md w-full mx-4 shadow-lg;
 }
 
-@utility vault42-spinner-sm {
-  /* Small spinner variant */
-  @apply w-4 h-4;
-}
-
 @layer base {
   body {
     font-family: 'Inter', system-ui, -apple-system, sans-serif;
@@ -288,11 +283,29 @@
 @keyframes vault42-spin {
   to { transform: rotate(360deg); }
 }
-.vault42-spinner {
+/* All three are @utility, and that is the point rather than a formality.
+   Tailwind compiles @utility into @layer utilities; a plain rule written outside
+   any @layer is unlayered, and unlayered beats layered whatever the specificity.
+   With the base plain and vault42-spinner-sm an @utility, the base simply
+   overrode it and every "small" spinner rendered at 20px. vault42-spinner-lg
+   escaped only because somebody happened to write it plain beside the base.
+
+   Making all three layered fixes that and one more thing: while .vault42-spinner
+   was unlayered it silently beat 45 of the utilities present in the bundle, so
+   an ordinary h-4 or w-4 on a spinner did nothing either. Moving the modifier
+   out instead would have restored the 16px and left that hazard in place.
+
+   Order within the layer is Tailwind's property-set sort, not source order, so
+   this is measured rather than assumed -- see the compile-time gate in
+   web/src/__tests__/spinnerCascade.test.ts. */
+@utility vault42-spinner {
   @apply inline-block w-5 h-5 border-2 border-vault42-control border-t-vault42-accent rounded-full;
   animation: vault42-spin 0.6s linear infinite;
 }
-.vault42-spinner-lg {
+@utility vault42-spinner-sm {
+  @apply w-4 h-4;
+}
+@utility vault42-spinner-lg {
   @apply w-8 h-8;
 }