Gates / frontend (push) Successful in 1m2s
Gates / test (push) Successful in 1m38s
Gates / package (push) Successful in 5m5s
Gates / test-aarch64 (push) Successful in 6m30s
Gates / container (push) Successful in 15s
CI / gates (push) Successful in 13m30s
297 lines
30 KiB
Markdown
297 lines
30 KiB
Markdown
# Milestone 23: StyleX and React Aria replace Tailwind
|
|
|
|
Goal: every styled element in `web/src` renders through StyleX (`@stylexjs/unplugin` at build time); Dialog, AlertDialog, Tabs and Select come from `react-aria-components`; Tailwind is removed. TanStack Router and TanStack Query stay (decided 2026-08-12 after review; the router swap is out of scope permanently, not deferred).
|
|
|
|
Design written 2026-08-12 against HEAD `91a0aa9`. Needs milestone 22 landed (the StyleX and RAC `.d.ts` surfaces are checked by the TS7 `tsc -b`).
|
|
|
|
Inventory this spec rests on: 522 `className=` occurrences across 31 `web/src` files; 17 shared class constants in `web/src/ui/classes.ts`; `web/src/styles.css` is the single line `@import "tailwindcss"` (so dark mode is Tailwind 4's default `prefers-color-scheme` media strategy — no class toggle exists); the Tailwind plugin sits at `web/vite.config.ts:3,8`; `tailwindcss` and `@tailwindcss/vite` are devDependencies (`web/package.json:34,44`) and appear in neither the runtime closure nor the bundled set of `licenses/dependency-identity.txt`.
|
|
|
|
## Implementation contract (read first)
|
|
|
|
- Read `AGENTS.md`, then this spec whole, before session work starts.
|
|
- `web/src/lib/contractSamples.gen.ts` and `web/src/lib/types.ts` are API contract surfaces; this milestone is styling-only and touches neither.
|
|
- Every session leaves all gates green: `npm run format:check`, `npm run lint`, `npm run typecheck`, `npm test`, `npm run build`, `npm run assert-bundled` (from `web/`), and `zig build test`. The release gate (`zig build dist` + `verify-dist`, `.gitea/workflows/gates.yml`) keeps its byte budgets; they are not tight — `web/dist` is 440,924 bytes raw against the 15 MiB binary budget — but `verify-dist` remains the arbiter.
|
|
- `assert-bundled` fails whenever the bundled npm package set changes. That failure is the license workflow trigger, not an obstacle: review each new package into `licenses/inventory.zon`, then record the new set in `licenses/dependency-identity.txt` (procedure in that file's header). Never update the recorded list without the inventory review.
|
|
|
|
## Rulings (binding)
|
|
|
|
### 1. Build integration is `@stylexjs/unplugin`, proven before any page moves
|
|
|
|
`stylex.vite()` from `@stylexjs/unplugin` goes into `web/vite.config.ts` alongside the existing plugins (Tailwind stays until S3). Configuration per the official Vite guide (stylexjs.com/docs/learn/installation/vite): `useCSSLayers: true`, `runtimeInjection: false`, `dev` keyed off the Vite mode. The generated CSS injects into an existing CSS asset, so `web/src/styles.css` (imported by `main.tsx:8`) remains the CSS entry for the whole milestone; the dev/HMR virtual-module wiring from that guide is part of S1. The unmaintained community `vite-plugin-stylex` is forbidden. S1's acceptance proves dev server, `vitest run` and `vite build` all handle a StyleX component before S2 or S3 convert anything — this is a gate, not an assumption.
|
|
|
|
### 2. Tokens live in `web/src/ui/tokens.stylex.ts`
|
|
|
|
`stylex.defineVars` with role names, not raw shades: surface, surface-raised, border, border-strong, text, text-muted (the zinc ramp today), primary and primary-text (blue-600/white), danger, danger-surface, danger-border (the red ramp), focus (blue-600). Each var carries its dark value under `@media (prefers-color-scheme: dark)` inside `defineVars` — same media strategy the app has now. No theme toggle, no `data-theme` attribute.
|
|
|
|
### 3. `web/src/ui/styles.ts` replaces `ui/classes.ts`
|
|
|
|
The 17 exports of `web/src/ui/classes.ts` (focusRing, insetFocusRing, input, smallInput, button, smallButton, largeButton, primaryButton, largePrimaryButton, rowButton, linkButton, dangerLinkButton, retryButton, th, td, tableWrap, formCard) become `stylex.create` style objects in `web/src/ui/styles.ts`, composed at call sites via `{...stylex.props(styles.button, extra)}`. The milestone-9 accessibility floor restated by milestone-18 ruling 10 carries over verbatim: every interactive element renders the focus ring (`:focus-visible` outline, 2px, offset 2, focus token; the inset variant for controls flush against panel edges). `ui/classes.ts` is deleted in S3 when its last importer converts.
|
|
|
|
### 4. React Aria scope is exactly four components — end state, not a phase
|
|
|
|
From `react-aria-components`: Dialog + Modal (replacing the hand-rolled `role="dialog"` overlay in `web/src/features/clients/ClientEditDialog.tsx:27-28`), AlertDialog composition (ruling 5), Tabs (replacing the button-pair tab switcher in `web/src/features/local/LocalDnsPage.tsx`), Select (replacing the 7 files carrying native `<select>`: LookupPage, QueryLogPage, RecordsTab, RulesPage, SettingsPage, PrefixesEditor, ClientEditDialog). Everything else — buttons, text inputs, tables, links, banners — stays native HTML styled with StyleX. A native styled `<button>` is the finished design, not an interim one. S2 wraps the four as shared components: `web/src/ui/Dialog.tsx`, `web/src/ui/ConfirmDialog.tsx`, `web/src/ui/Tabs.tsx`, `web/src/ui/Select.tsx`, each styled with StyleX through RAC's render-prop state booleans (`stylex.props(isSelected && styles.selected)`) — never data-attribute selectors, which StyleX cannot express.
|
|
|
|
### 5. `window.confirm` dies with the conversion
|
|
|
|
All call sites become `ConfirmDialog` (RAC AlertDialog: `role="alertdialog"`, focus trapped, danger-styled confirm button): `web/src/features/blocklists/BlocklistsPage.tsx:61`, `web/src/features/upstreams/UpstreamsPage.tsx:43`, `web/src/features/rules/RulesPage.tsx:34`, and `web/src/ui/useCrudForm.ts:49` (which carries the confirm for ZonesTab and RecordsTab). `useCrudForm`'s `CrudFormSpec.confirmDelete` keeps its signature; the hook returns the dialog state instead of calling `window.confirm`. The tests that stub confirm today (`web/src/features/upstreams/UpstreamsPage.test.tsx:140,144,188`, `web/src/ui/useCrudForm.test.tsx:102`) are rewritten to drive the real dialog: open, assert `role="alertdialog"` and the entity name in the message, confirm and cancel paths both asserted. BlocklistsPage and RulesPage gain the same coverage for their delete paths.
|
|
|
|
### 6. Styling mechanics
|
|
|
|
`stylex.create`/`stylex.props` only. Conditions are boolean-guarded style objects; variants are separate named styles; responsive behavior (the `md:` sidebar grid in `web/src/shell/AppShell.tsx:90-98`, the mobile drawer) becomes `@media` conditions inside style values. No dynamic class-name strings, no `clsx`-style helpers, no `className` literals left anywhere in `web/src` after S3 — `className`/`style` attributes appear only as the spread of `stylex.props(...)` (RAC render-prop `className` functions included). TanStack `Link` `activeProps`/`inactiveProps` (`AppShell.tsx:35-42`) pass `stylex.props(...).className` outputs.
|
|
|
|
### 7. Visual contract: drift accepted, structure pinned
|
|
|
|
No pixel parity requirement and no visual-regression harness. Pinned instead, checkable by reading the converted file against the original: same layout structure (grid/flex relationships, table columns, heading levels), token roles per ruling 2 (zinc surfaces, blue primary actions, red danger), the focus floor of ruling 3 on every interactive element, dark mode via media query, `overflow-x-auto` table wrappers preserved.
|
|
|
|
### 8. License inventory, named per step
|
|
|
|
- S1 adds `@stylexjs/stylex` (runtime dependency; `stylex.props` ships in the bundle) to the runtime closure and the bundled set. `@stylexjs/unplugin` is a devDependency — runtime closure and bundled set untouched by it.
|
|
- S2 adds `react-aria-components` and its transitive closure (`@react-aria/*`, `@react-stately/*`, `@react-types/*`, `@internationalized/*`, `@swc/helpers` — the exact bundled subset is whatever `npm run assert-bundled` reports after S2's conversions; every reported name gets a reviewed `licenses/inventory.zon` entry before the recorded list moves). This is the milestone's largest review surface.
|
|
- S3 removes nothing from the license files: Tailwind was dev-only and never recorded. The `@tanstack/*` entries stay untouched.
|
|
|
|
### 9. PLAN.md wording moves in S3
|
|
|
|
`PLAN.md:138` "Vite + React + TypeScript + Tailwind; TanStack Router + TanStack Query" → "Vite + React + TypeScript + StyleX + React Aria; TanStack Router + TanStack Query". `PLAN.md:235` directory comment updates the same way. Line 562 names loaders and Query only — untouched. No other PLAN amendment.
|
|
|
|
### 10. Conversion order is ascending `className` count, one commit per file
|
|
|
|
So a regression bisects to one page. Counts at spec time:
|
|
|
|
| file | count |
|
|
|---|---|
|
|
| lib/InlineError.tsx | 2 |
|
|
| features/settings/ReadOnlyConfigBanner.tsx | 2 |
|
|
| features/dashboard/HealthBanners.tsx | 2 |
|
|
| features/local/LocalDnsPage.tsx | 3 |
|
|
| features/settings/RestartBanner.tsx | 3 |
|
|
| features/dashboard/StatCards.tsx | 5 |
|
|
| routes.tsx | 6 |
|
|
| auth/LoginPage.tsx | 9 |
|
|
| features/dashboard/DashboardPage.tsx | 9 |
|
|
| features/groups/GroupSourcesEditor.tsx | 9 |
|
|
| features/pause/PauseWidget.tsx | 9 |
|
|
| features/blocklists/BlocklistForm.tsx | 11 |
|
|
| features/clients/ClientEditDialog.tsx | 11 |
|
|
| features/dashboard/DiskCard.tsx | 13 |
|
|
| features/upstreams/UpstreamForm.tsx | 14 |
|
|
| features/clients/PrefixesEditor.tsx | 15 |
|
|
| shell/AppShell.tsx | 17 |
|
|
| features/live/LiveLogPage.tsx | 18 |
|
|
| features/dashboard/TimeseriesChart.tsx | 21 |
|
|
| features/dashboard/UpstreamHealthTable.tsx | 21 |
|
|
| features/upstreams/UpstreamsPage.tsx | 21 |
|
|
| features/groups/GroupsPage.tsx | 22 |
|
|
| features/lookup/LookupPage.tsx | 25 |
|
|
| features/local/ZonesTab.tsx | 26 |
|
|
| features/blocklists/SourceStatusSection.tsx | 27 |
|
|
| features/clients/ClientsPage.tsx | 28 |
|
|
| features/rules/RulesPage.tsx | 31 |
|
|
| features/blocklists/BlocklistsPage.tsx | 31 |
|
|
| features/settings/SettingsPage.tsx | 31 |
|
|
| features/local/RecordsTab.tsx | 34 |
|
|
| features/queries/QueryLogPage.tsx | 46 |
|
|
|
|
S2 takes its owned cluster out of this order (converted together because the RAC wrappers land there); S3 walks the remainder smallest-first.
|
|
|
|
### 11. Tests that must change, known in advance
|
|
|
|
RAC changes DOM and focus behavior; these edits are expected work, not regressions to paper over:
|
|
- Dialog: `ClientsPage.test.tsx` (ClientEditDialog now portals into an RAC Modal; queries move to `within(screen.getByRole("dialog"))`).
|
|
- Tabs: `LocalDnsPage.test.tsx` (tab switcher becomes `role="tablist"`/ `role="tab"`; `getByRole("button", { name: ... })` queries move to `getByRole("tab", ...)`).
|
|
- AlertDialog: `UpstreamsPage.test.tsx`, `useCrudForm.test.tsx` per ruling 5.
|
|
- Select: RAC Select renders a button + listbox, not `<select>`, so `getByLabelText(...)` + `fireEvent.change` patterns break in the tests for the 7 Select files (LookupPage, QueryLogPage, LocalDnsPage/RecordsTab, RulesPage, SettingsPage, ClientsPage). They move to `userEvent`-style open-then-select interactions. All other tests query by role/text and are expected to survive; a test that asserted a Tailwind class is rewritten against role or accessible name.
|
|
|
|
## Sessions
|
|
|
|
Three sessions, strictly sequential: S1 → S2 → S3. (S2 and S3 both write `web/package.json` and the license files; the RAC wrappers S3 consumes are built in S2.)
|
|
|
|
### Session S1: StyleX foundation, proven end to end
|
|
|
|
Owns: `web/package.json`, `web/package-lock.json`, `web/vite.config.ts`, `web/src/styles.css`, `web/src/ui/tokens.stylex.ts` (new), `web/src/ui/styles.ts` (new), `web/src/ui/styles.test.tsx` (new), `licenses/inventory.zon`, `licenses/dependency-identity.txt`.
|
|
|
|
- S1.1 add `@stylexjs/stylex` (dependency) and `@stylexjs/unplugin` (devDependency), exact pins; wire `stylex.vite()` per ruling 1. Tailwind plugin stays.
|
|
- S1.2 tokens per ruling 2; shared styles per ruling 3. `ui/classes.ts` is not deleted yet — both vocabularies coexist until S3.
|
|
- S1.3 `styles.test.tsx`: render a probe component using `stylex.props(styles.button)` under vitest; assert a non-empty `className` lands on the element (proves the transform runs in the test pipeline, since `runtimeInjection` is off).
|
|
- S1.4 license entries per ruling 8.
|
|
|
|
Acceptance (S1):
|
|
- [ ] `npm run dev` serves a probe page where a StyleX rule visibly applies
|
|
and edits hot-reload (checked by hand, result recorded).
|
|
- [ ] `npm test` passes including `styles.test.tsx`.
|
|
- [ ] `npm run build` emits StyleX CSS into the `web/dist` assets; the built
|
|
`index.html` page renders with the probe styled.
|
|
- [ ] `npm run assert-bundled` passes with `@stylexjs/stylex` in the recorded
|
|
set; `zig build test` passes (drift guard agrees).
|
|
- [ ] All remaining gates green.
|
|
|
|
### Session S2: React Aria primitives and their cluster
|
|
|
|
Owns: `web/package.json`, `web/package-lock.json`, `web/src/ui/Dialog.tsx` (new), `web/src/ui/ConfirmDialog.tsx` (new), `web/src/ui/Tabs.tsx` (new), `web/src/ui/Select.tsx` (new), `web/src/ui/useCrudForm.ts`, `web/src/ui/useCrudForm.test.tsx`, `web/src/features/clients/*`, `web/src/features/local/*`, `web/src/features/upstreams/*`, `web/src/features/blocklists/BlocklistsPage.tsx` and its test, `web/src/features/rules/RulesPage.tsx` and its test, `licenses/inventory.zon`, `licenses/dependency-identity.txt`.
|
|
|
|
- S2.1 add `react-aria-components`, exact pin; build the four wrappers per ruling 4, styled with tokens and `ui/styles.ts`.
|
|
- S2.2 convert the owned cluster fully to StyleX while adopting the wrappers: ClientEditDialog + PrefixesEditor + ClientsPage (Dialog, Select); LocalDnsPage + ZonesTab + RecordsTab (Tabs, Select, ConfirmDialog via useCrudForm); UpstreamsPage + UpstreamForm, BlocklistsPage, RulesPage (ConfirmDialog, Select). No `className` literal survives in an owned file.
|
|
- S2.3 `window.confirm` removal and test rewrites per rulings 5 and 11.
|
|
- S2.4 license review per ruling 8 — every package `assert-bundled` names.
|
|
|
|
Acceptance (S2):
|
|
- [ ] `grep -rn 'window\.confirm(' web/src` → zero matches. The bare name
|
|
still appears in three comments in `ui/` that record what replaced it,
|
|
which is why the check is for a call and not for the name.
|
|
- [ ] Delete flows on upstreams, blocklists, rules, zones and records drive a
|
|
`role="alertdialog"` element; confirm and cancel both covered by tests.
|
|
- [ ] Tabs on `/local-dns` expose `role="tablist"`; keyboard arrow keys move
|
|
between tabs (asserted in `LocalDnsPage.test.tsx`).
|
|
- [ ] No `className=` literal in S2-owned files
|
|
(`grep -c 'className="' <file>` = 0; `stylex.props` spreads excepted).
|
|
- [ ] `assert-bundled` passes with the RAC closure recorded;
|
|
`zig build test` passes.
|
|
- [ ] All remaining gates green.
|
|
|
|
### Session S3: the sweep and the removal
|
|
|
|
Owns: every `web/src` file not owned by S1/S2 that carries `className=` (the ruling-10 remainder: InlineError, ReadOnlyConfigBanner, HealthBanners, RestartBanner, StatCards, routes.tsx, LoginPage, DashboardPage, GroupSourcesEditor, GroupsPage, PauseWidget, BlocklistForm, SourceStatusSection, DiskCard, AppShell, LiveLogPage, TimeseriesChart, UpstreamHealthTable, LookupPage, QueryLogPage, SettingsPage) plus their tests, `web/src/ui/classes.ts` (delete), `web/package.json`, `web/package-lock.json`, `web/vite.config.ts`, `web/src/styles.css`, `PLAN.md`.
|
|
|
|
- S3.1 convert the remainder smallest-first per ruling 10, one commit per file, gates green at every commit.
|
|
- S3.2 delete `ui/classes.ts` when its import count reaches zero.
|
|
- S3.3 remove Tailwind: drop `tailwindcss` and `@tailwindcss/vite` from `package.json`, the plugin from `vite.config.ts:3,8`, and the `@import "tailwindcss"` line from `styles.css` (the file itself stays as the StyleX CSS entry per ruling 1).
|
|
- S3.4 PLAN.md per ruling 9.
|
|
|
|
Acceptance (S3):
|
|
- [ ] `grep -rn 'className="' web/src` returns zero matches. No Tailwind
|
|
dependency in `web/package.json`, no plugin in `web/vite.config.ts`, no
|
|
`@import "tailwindcss"` in `web/src/styles.css`. The word still appears
|
|
in seven comments in `web/src` that name the ramps the tokens were taken
|
|
from and the reset the preflight used to supply; those are the record of
|
|
where the values came from, so the check is not for the name.
|
|
- [ ] `ui/classes.ts` is gone; nothing imports it.
|
|
- [ ] Dark scheme still follows `prefers-color-scheme` (vitest: token var
|
|
resolves differently under a mocked dark media query, or a recorded
|
|
manual check in both schemes).
|
|
- [ ] `PLAN.md:138` and `:235` name StyleX + React Aria; `git diff PLAN.md`
|
|
touches nothing else.
|
|
- [ ] All gates green, including `zig build dist` + `zig build verify-dist`
|
|
with `-Dweb-dist=web/dist` (run once at milestone end).
|
|
|
|
### Orchestrator
|
|
|
|
Verify S1 acceptance (the ruling-1 gate) before S2 starts; S2 before S3. After S3: full gate set, then a live smoke — `zig build` a server with the fresh `web/dist`, click through every page in light and dark scheme, exercise one delete confirm and the local-dns tabs. Record deviations in `## Recorded (implementation)`.
|
|
|
|
## Module layout
|
|
|
|
New files:
|
|
- `web/src/ui/tokens.stylex.ts` — the design tokens (ruling 2).
|
|
- `web/src/ui/styles.ts` — the shared style vocabulary (ruling 3).
|
|
- `web/src/ui/styles.test.tsx` — the transform-pipeline probe (S1.3).
|
|
- `web/src/ui/Dialog.tsx`, `web/src/ui/ConfirmDialog.tsx`, `web/src/ui/Tabs.tsx`, `web/src/ui/Select.tsx` — the RAC wrappers (ruling 4).
|
|
|
|
Deleted surface: `web/src/ui/classes.ts`; the `window.confirm` calls; the Tailwind dependency pair and its vite plugin line and CSS import.
|
|
|
|
## Acceptance (milestone complete)
|
|
|
|
- [ ] All session boxes above.
|
|
- [ ] Zero Tailwind: no dependency, no plugin, no `@import`, no utility
|
|
`className` literal in `web/src`.
|
|
- [ ] RAC appears in exactly four wrapper files plus their consumers; no
|
|
other `react-aria-components` import (`grep -rln 'react-aria-components'
|
|
web/src` ⊆ the four wrappers).
|
|
- [ ] Focus ring visible via keyboard on every interactive element of every
|
|
page (spot-checked live, recorded).
|
|
- [ ] `licenses/dependency-identity.txt`, `licenses/inventory.zon` and the
|
|
bundled reality agree (`assert-bundled` and `zig build test` both
|
|
green).
|
|
- [ ] `web/src/lib/types.ts` and `web/src/lib/contractSamples.gen.ts` are
|
|
byte-identical to HEAD.
|
|
|
|
## Recorded (implementation)
|
|
|
|
Written during S1 and the S2 hand-off. Each entry is a place the spec was wrong or a decision the spec did not have.
|
|
|
|
### S1 could not put StyleX in the bundle, and its acceptance says otherwise
|
|
|
|
S1 owns no page file, so no production module imports StyleX and rollup emits none of it. Two S1 boxes — "`npm run build` emits StyleX CSS into the `web/dist` assets" and "`assert-bundled` passes with `@stylexjs/stylex` in the recorded set" — are therefore unmeetable inside S1 as scoped, and the recorded bundled set is unchanged at the S1 commit. Both were proven instead against a temporary probe component, then deferred to S2, which converts real pages and does change the bundled set. Ruling 8's "S1 adds `@stylexjs/stylex` ... to the runtime closure and the bundled set" is half right: the closure moves in S1, the bundled set in S2. The inventory entry for StyleX is anticipatory at the S1 commit, which is deliberate — see the styleq entry for why it cannot wait.
|
|
|
|
### The probe results, recorded rather than left as a claim
|
|
|
|
Against the temporary probe: the built CSS carries `@layer priority1..N` and the token variables, resolving to `#fafafa` light and `#09090b` dark with lightningcss `lab()` fallbacks. The dev server needs no wiring of its own — the plugin injects `<link rel="stylesheet" href="/virtual:stylex.css">` into `index.html`, so the `DevStyleXInject` component in StyleX's Vite guide is for frameworks that bypass Vite's HTML transform and is not used here. Editing a token regenerates the virtual stylesheet and the dev server issues a page reload.
|
|
|
|
### S1 wrote outside its owned files, and outside an anti-requirement
|
|
|
|
The licence guard forced it: `licenses/stylex-mit.txt`, `licenses/styleq-mit.txt` and their registration in `licenses/licenses.zig`, plus the `npm_not_shipped` array in `src/licenses_drift_test.zig`. The last contradicts the "no edits ... under `src/`" anti-requirement below. The anti-requirement is amended, not ignored: `src/licenses_drift_test.zig` is writable for its licence-fact arrays and, from S2, for the per-package licence change described next. The rest of `src/` stays closed.
|
|
|
|
### Apache-2.0 inbound: a decision, not a deduction
|
|
|
|
`react-aria-components` 1.20.0 brings eight Apache-2.0 packages (react-aria-components, react-aria, react-stately, `@react-types/shared`, three `@internationalized/*`, `@swc/helpers`) and `tslib` under 0BSD into an EUPL-1.2 project. The user accepted Apache-2.0 inbound on 2026-08-12 and authorised the guard change it needs. This is recorded as their decision because nothing in the licence settles it: EUPL-1.2's appendix does not list Apache-2.0 — it names GPL, AGPL, OSL, EPL, CeCILL, MPL, LGPL, CC BY-SA, EUPL and LiLiQ — and that appendix governs Article 5 outbound relicensing, not inbound consumption. Any claim that the appendix permits this is false and must not be written into the inventory. `src/licenses_drift_test.zig:122` hardcodes a single expected licence and becomes per-package in S2; the guard stays strict, an unexpected licence still fails.
|
|
|
|
### Ruling 8's picture of the React Aria closure is wrong
|
|
|
|
There are no `@react-aria/*` or `@react-stately/*` scoped packages. RAC 1.20.0 ships consolidated `react-aria` and `react-stately`. The runtime closure it adds is the eleven packages named above plus `aria-hidden`, `clsx` and `client-only` under MIT — not the "dozens" the ruling anticipated.
|
|
|
|
### Ruling 11's claim about the tab tests is wrong
|
|
|
|
`LocalDnsPage.tsx` already renders `role="tablist"` and `role="tab"` by hand, so no test query moves from `getByRole("button")` to `getByRole("tab")`. What RAC adds there is keyboard arrow-key navigation, which the hand-rolled switcher lacks, and that is what the S2 acceptance box asserts.
|
|
|
|
### `formCard` is a layout change, not a port
|
|
|
|
`space-y-3` set sibling margins on a block container; the StyleX version is a flex column with `gap`. StyleX cannot express the `> * + *` selector `space-y` compiles to, so `gap` is the only mechanism available. Both call sites (`RecordsTab`, `ZonesTab`) are plain vertical form stacks, where the difference in child sizing and margin behaviour does not show. Recorded against ruling 7, which pins layout structure.
|
|
|
|
### S2 needed two build-config changes the spec did not anticipate
|
|
|
|
Both are in files S1 and S3 own, both were forced, and neither is optional.
|
|
|
|
`web/vitest.setup.ts` (new) plus `setupFiles` in `web/vite.config.ts`: jsdom implements no `CSS` interface at all, and React Aria's collection code calls `CSS.escape` unguarded when it looks an item up by key (`react-aria/dist/private/selection/utils.mjs:22`). Every RAC Select and Tabs test threw `Cannot read properties of undefined (reading 'escape')` before this landed. The setup file implements CSSOM's *serialize an identifier* algorithm rather than approximating it, because a wrong escape would break the key lookups silently instead of loudly.
|
|
|
|
The `@` alias passed to `stylex.vite()` in `web/vite.config.ts`: StyleX resolves the `.stylex.ts` theme file itself and cannot see Vite's `resolve.alias`, so `import { colors } from "@/ui/tokens.stylex"` in a feature file failed the build with "Could not resolve the path to the imported file". Relative imports were the alternative; passing the alias keeps the repo's `@/` convention instead.
|
|
|
|
### RAC Select changes the accessible name of a labelled control
|
|
|
|
`useSelect` labels the trigger button with `aria-labelledby="<value> <label>"`, so a select labelled "Type" showing "A" has the accessible name "A Type", and `getByLabelText("Type")` no longer finds it. Converted tests query `getByRole("button", { name: /Type$/ })` and read the options by opening the listbox. RAC also renders a hidden native `<select>` for form submission; it sits inside an `aria-hidden` container, so role and label queries skip it.
|
|
|
|
### The RAC anti-requirement forbids parts of a component it requires
|
|
|
|
`Select.tsx` imports RAC `Button`, `Popover`, `Label`, `ListBox`, `ListBoxItem` and `SelectValue`. The anti-requirement below names `Button` and `Popover` among the components RAC must not bring. Those two are not adopted components here — RAC composes a Select out of exactly these parts, and there is no RAC Select without them. The anti-requirement is amended: it forbids adopting a RAC component as an app-wide primitive, and the internal parts of the four adopted components are permitted where no file outside their wrapper imports them. That last clause is the check that keeps the scope honest, and it holds: `react-aria-components` is imported only by the four `ui/` wrappers. A RAC `Button` in a feature file would still be a breach.
|
|
|
|
### The focus-ring floor was briefly amended on a false premise
|
|
|
|
S2 left the Select's options with their outline removed, showing keyboard focus as a background inversion alone. This section previously amended ruling 3 to excuse that, arguing RAC drives option focus through an `isFocused` render prop so a `:focus-visible` rule could never match. That argument is wrong. `useSelectableItem` gives options a roving `tabIndex` and focuses the option's own DOM node (`react-aria/dist/private/selection/useSelectableItem.mjs:64`), which `ListBoxItem` spreads onto its element — so `:focus-visible` matches and the ring was simply missing. The amendment is withdrawn and ruling 3 stands unchanged. `Select.tsx` now composes the shared inset ring onto every option: inset because an option flush against a scrolling popover clips an outset outline, and recoloured to `primaryText` on the highlighted row because the focus token is the same blue that row paints behind it. The inversion stays as the listbox convention; it is no longer the only indicator.
|
|
|
|
### Two notes for S3
|
|
|
|
`textMuted` resolves to zinc-400 in dark mode. Several current `text-zinc-500` call sites carry no dark override and stay zinc-500, so the token cannot replace those without a shade shift — decide per call site rather than sweeping.
|
|
|
|
Vitest began reporting "Tests closed successfully but something prevents Vite server from exiting" with the StyleX plugin in the pipeline. The suite passes and exits 0. It should not be allowed to become the accepted baseline.
|
|
|
|
### Both S3 notes are now settled
|
|
|
|
**The vitest warning is fixed, and it was not our code.** Bisecting a trivial one-assertion test against a bare config showed `react()` exits clean and `stylex.vite()` does not. `@stylexjs/unplugin@0.19.0`'s `lib/es/vite.mjs:59` starts a 150ms `setInterval` in `configureServer` to poll its CSS store for HMR, and clears it from the `close` event of `server.httpServer`. A probe plugin confirmed vitest resolves `httpServer` to `null`, because it runs the dev server in middleware mode — so the timer is never cleared and holds the event loop open until vitest kills the worker on a ten-second timeout. `web/vite.config.ts` now strips that one hook when `process.env.VITEST` is set; the transform hook, which the tests do need, is untouched, and `ui/styles.test.tsx` still proves it runs. `npm test` reports 31 files and 183 tests passed, with no warning, and exits 0.
|
|
|
|
**`textMuted` was swept; here is the per-call-site judgement.** Measuring the two shifts against their grounds settles it in opposite directions:
|
|
|
|
- 39 call sites were a flat `text-zinc-500` with no dark override. On the dark ground that measures **4.12:1**, which fails WCAG AA. The token's zinc-400 dark value measures **7.56:1**. The sweep repaired a real accessibility defect there, so it stands, and the token comment now records why.
|
|
- 4 call sites were `text-zinc-600 dark:text-zinc-400` — already adaptive. The sweep moved their light value to zinc-500, from **7.40:1** to **4.62:1**. That clears AA by a hair on small text: the query-log table header, the dashboard period button, the inactive nav item and the chart legend. It buys nothing, so a second token `textSecondary` (zinc-600 light, zinc-400 dark) restores the original pair at exactly those four.
|
|
|
|
Contrast was computed from the tokens' own oklch values through linear sRGB to WCAG relative luminance, not read off a chart.
|
|
|
|
### The reset's font stack is not Tailwind's, deliberately
|
|
|
|
Tailwind is uninstalled, so its default stack cannot be reproduced from the tree, and ruling 7 accepts the metric drift. One part is not drift: `Select.tsx` renders U+25BE for its chevron, and Segoe UI does not carry that glyph, so on Windows the stack has to reach a font that does. `"Segoe UI Symbol"` and `"Noto Sans Symbols 2"` are appended for that glyph. No emoji families are listed, because the app renders no emoji.
|
|
|
|
### Two review findings closed by decision, not by a change
|
|
|
|
Both were put to Mokhtar Mial on 2026-08-12 and both are closed. Neither is to be re-opened by a later review reading the history and inferring an oversight.
|
|
|
|
`0f58966` carries a `README.md` edit that was already in the working tree and is not part of the `srOnly` work its message describes. The content is intact and only the attribution is wrong; rewriting published history to correct that was declined.
|
|
|
|
Ruling 10 asks for one commit per file in ascending `className` count. S3 did not hold to the order strictly. Bisectability does not depend on it and the commits were already published, so the order stands as it is.
|
|
|
|
## Anti-requirements
|
|
|
|
- No router or query changes: TanStack Router and TanStack Query stay, untouched. That decision is closed.
|
|
- No RAC beyond Dialog, AlertDialog, Tabs, Select — no RAC Button, TextField, Table, Menu, DatePicker adopted as an app primitive. Native elements styled with StyleX are the end state. The internal parts of the four adopted components (Button, Popover, Label, ListBox, ListBoxItem, SelectValue inside Select) are permitted, and only inside their own wrapper file — see the recorded amendment above.
|
|
- No runtime style injection (`runtimeInjection: false` stays), no CSS-in-JS at runtime, no styled-components-like patterns.
|
|
- No dynamic class-name assembly, no `clsx`/`classnames` dependency, no Tailwind-compatibility shim.
|
|
- No theme toggle, no `data-theme`, no stored preference — media-query dark mode only, as today.
|
|
- No redesign: layout structure is pinned by ruling 7. Drift in shades and spacing is accepted; new page structures are not.
|
|
- No visual-regression harness, no screenshot tooling.
|
|
- No edits to `web/src/lib/types.ts`, `web/src/lib/contractSamples.gen.ts`, `web/src/lib/api.ts`, or anything under `src/` (the Zig tree).
|
|
- No `vite-plugin-stylex` (unmaintained community plugin).
|