Repository navigation
feat(web): adopt MUI, themed from the farm palette tokens (#674) - #860
Conversation
The record's Consequences section says tech_spec §8.1 and web/README.md are amended. Neither was. Amend both and index the record.
… outside React (#674) The observer is the provider's reason to exist and had no test. Removing observer.observe makes the second case fail.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChangesMUI adoption
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant App
participant FarmThemeProvider
participant document.documentElement
participant MUI ThemeProvider
App->>FarmThemeProvider: Render application content
FarmThemeProvider->>document.documentElement: Read theme attributes and CSS tokens
FarmThemeProvider->>MUI ThemeProvider: Provide theme from resolved tokens
document.documentElement-->>FarmThemeProvider: Report data-brand or data-theme mutation
FarmThemeProvider->>MUI ThemeProvider: Rebuild and provide updated theme
Merge Risk: ⚪ Minimal · up to No merge-blocking behavior issue is identified in the MUI theme bridge or dependency integration. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
#862) Closes #822 Builds on #860 (merged as 6c83c5c), which landed the MUI theme bridge and the decision record this doc builds on. ## Why Sequence step 1 of the SPA revamp. Nothing else on #674 starts until the design exists, and the epic's own issue texts are stale in ways that change the work: no role renders an 18-link sidebar, the dialog family is 996 lines rather than 640, MUI 9.4 ships no `NumberField`, and the Inter `opsz` axis the epic says costs nothing is not loaded at all (+118.9 KiB when it is). The doc verifies every such claim at the base commit and decides the component map, layout system, IA, whole-app baseline, identity direction and slice cut so that #823 to #836, #740 and #50 can each be cut from it alone. ## Scope - `docs/designs/822-mui-revamp.md`: fact base, walked inventories, decisions D1 to D10, guards G1 and G2 with mutation rows, the slice cut in order, and §7 collecting the product calls for the owner with a named alternative each. - `docs/plans/822-mui-revamp/synthesis.md`: how the doc was produced and what the grill changed. - Out of scope: any code. The record amendment lands with #823, per the issue. ## Tradeoffs Owner review rows rather than decisions where the call is a product one: `CssBaseline`, the 8px spacing scale, the More sheet component, the `NumberField` alternative, full-screen phone dialogs, the link colour source, the precache ceiling, and the daily-entry footer stacking. Each names the alternative so the choice is decidable at its slice. ## Blast Radius Documentation only. The `changes` job classifies it as such and skips `web` and `image`. Nothing the doc says is enforced until #823/#824 ship the guards it specifies. ## Verification - Three independent drafts against one brief, cross-judged on a fourth model, merged by hand. Recorded in the synthesis file. - Grill: three adversarial reviewers, 33 findings, each re-verified against `styles.css`, the test files and `@mui/material@9.4.0` before it changed the doc. Two consensus criticals were real and are fixed: an outlined `Paper` default would have flattened every float, and `Autocomplete`'s popover had no default elevation to map. One finding dismissed with the code line that refutes it. - No em-dash in either file; no bare `postgres:` tag (the #508 tracked-file pin). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Clarified implementation details in the MUI revamp design documentation, including responsive layout behavior, loading controls, toolbar testing, styling, and screenshot requirements. - Added a synthesis record documenting the design review process, evaluated alternatives, verified findings, and resulting decisions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…one action rule (#823) (#871) ## Why Slice 1 of the #674 SPA revamp, scoped by `docs/designs/822-mui-revamp.md` D8 row 1. It is mechanics, not looks: every screen still renders its own hand-built markup, and this puts the theme, the guard and the phone action rule underneath them so the screen slices that follow inherit a decided baseline instead of each deciding again. **It also found the thing that blocks the rest of the epic, and that finding outweighs the slice.** #822 D6 decided to adopt `CssBaseline`; this branch adopted it, and it does not apply. The production CSP is `style-src 'self'` (`src/Cluckwork.Api/Security/SecurityHeaders.cs`) and Emotion delivers every style by injecting a stylesheet, so the browser refuses all of them. Measured against the sim harness at 390: the `<style data-emotion>` tag is in the document, `html` computes `box-sizing: content-box` instead of CssBaseline's `border-box`, `/daily-entry` lays out 419px wide inside a 390px frame, and the console reads *"Applying inline style violates the following Content Security Policy directive `style-src 'self'`"*. **The same path carries every `sx`, `styled()` and `styleOverrides` value #826 to #836 will add**, so nothing MUI renders will be styled until this is settled. Nobody could have seen it before now: the app renders no MUI component, so the theme had nothing to emit. So this PR takes D6's own recorded alternative — keep `styles.css`'s baseline, neutralise the bare elements anyway — and the only thing it changes on screen is the phone action rows, which closes #740. The theme and its guard land as groundwork and are honest about being inert. **The CSP question wants its own decision** and I have not filed an issue for it; see the bottom of this description for what the three candidate answers cost. ## Scope - `web/src/theme/FarmThemeProvider.tsx`. `createFarmTheme` grows the elevation array, the three-step radius scale, the typography variants the stylesheet already renders, 8px spacing, sentence case on `button` and `overline`, `MuiPaper.defaultProps.elevation = 0`, `MuiCard variant="outlined"`, the `MuiAutocomplete` paper shadow, `MuiButton disableElevation` plus its pill radius and phone-only 44px floor, and the `MuiDialogActions` phone stack. `CssBaseline` stays unrendered, now for a measured reason rather than a deferred decision. - `web/src/theme/farmTokens.ts`. `THEME_TOKENS` grows by `--surface-2`, the five `--tint-*`, `--stat-accent`, `--r-input`, `--r-panel`, `--r-pill`, `--shadow-bar`, `--shadow-dialog`. `farmTokens.test.ts` still proves every one is declared in `styles.css`. - `web/src/theme/farmTheme.policy.test.ts`. G2, walked over all four palettes and both modes. - `web/src/styles.bare-elements.test.ts`. The postcss walk over every rule whose selector names an element, with the 11 that reach MUI's DOM anywhere in the app pinned by selector and by declared property. - `web/src/styles.css`. The bare-element rules wrapped in `:where()`; the phone action rules rewritten. `*` and `body` stay and are recorded in the guard as deliberate: they are the baseline `CssBaseline` would have taken over. - `tools/simulation/ui/specs/phone.spec.ts` and `src/mutants.ts`. The action walk now covers the Sales draft panel and asserts width share as well as ratio; the mutant reverts the rule. - `docs/decisions/674-ui-component-library.md`. The §9 amendment. The six `.actions` sites, all unchanged in markup: `SalesPage.tsx:1492` (the #740 reproduction) and `:1612`, `ExpensesPage.tsx:700`, `InventoryPage.tsx:703`, `SettingsPage.tsx:831`, `DailyEntryPage.tsx:859`. The two dialog-foot rules changed are `styles.css:3072-3075` and `3077-3080`. **Every component that reads `theme.shadows` by index**, walked with `grep -rn "shadows\[" node_modules/@mui/material --include='*.js'` before the map was written: `Button` (2 rest, 4 hover, 6 focus, 8 press, 0 disabled — handled by `disableElevation`), `Paper` (dynamic, by `elevation` prop), `ButtonGroup` (2), `Chip` (1), `Switch` (1), `SpeedDialAction` (1), `Slider` (2), `Fab` (6 rest, 12 hover, 0), `BreadcrumbCollapsed` (0). Under this map every one of those except `Button` and `Fab` resolves to `none`. None of them renders in the app today; the ones worth knowing about when they arrive are `Fab`, which would carry the bar shadow at rest, and `Switch`, whose thumb loses its shadow. ## Tradeoffs **`:where()` demotion instead of per-property `components` overrides.** The design asked for theme overrides. They cannot do this job for the compound forms: `button:hover:not(:disabled)` is 0,2,1 and `.MuiButton-contained:hover` is 0,2,0, so any override strong enough to beat the stylesheet also beats MUI's own variant and has to decide the hover colour for every variant and colour rather than hand it back. `:where()` contributes no specificity, so one Emotion class outranks the whole family and MUI decides. Nothing is deleted and no raw consumer changes; `styles.bare-elements.test.ts` holds it, and mutation M10 (un-demote one selector) reddens it. **Nothing is deleted from `styles.css:299-315`, where D6 and D8 say to delete it.** `*` and `body` stay because their replacement does not apply (above). `h1, h2, h3, h4` stays for a second, independent reason: no screen renders `Typography` yet, so it has no successor until #829 to #833, and deleting it now would drop every heading in the app from weight 800 with -0.02em tracking to the browser default. **`.empty-state` (`styles.css:1255-1274`) is not deleted**, though D8 row 1 lists it. Deleting it needs `EmptyState`'s internals rendered as `Typography` and `Button`, which is a visible change. It moves to the slice that converts the component. **The daily-entry footer pair stays side by side**, which is #822 §7's owner-review row resolved the other way from the design's default. The first version of this branch stacked it, and CodeRabbit caught that it then contradicted the #864 mockup, which keeps the pair at 390 (`docs/designs/864-visual-language/daily-entry.html`, `grid-template-columns: 1fr 1fr`). The owner confirmed that mockup after seeing it, so the mockup is the authority and the CSS follows it: F134's pair is restored and `styles.css:2993-2996` keeps buying back the 2.3rem of width that makes it fit a thumb. Everything else stacks, including the Sales draft panel, which is where #740 actually reproduced. ## Blast Radius Every screen at once, which is the point and the risk: `CssBaseline` and the `:where()` demotion are global. The demotion changes no cascade outcome that exists today, because nothing in the stylesheet declares these properties at zero specificity, so the only rules that gain are Emotion's — and Emotion renders nothing on any screen yet. The line-height change is real and app-wide, and the captures are where it is judged. Nothing server-side moves: no `.cs`, no migration, no config key, so the sim harness and the AppHost need no teaching. ## Verification **Web suite.** 3020 tests in 128 files pass, from 2998 in 126 at the base: +22 across `farmTheme.policy.test.ts` (G2) and `styles.bare-elements.test.ts`. `npm run typecheck` clean. `npm run test:coverage` at 94.16 lines / 91.23 statements / 86.77 functions / 87.77 branches, every one above both the `vite.config.ts` floor and the 93.86 / 90.9 / 86.21 / 87.64 the design measured at #860. Nothing was re-baselined. No `.cs` moved, so `dotnet build` is not in play; the two `ImagePin_IsOneIdenticalStringAcrossEveryTrackedFile` tests were run before the record amendment was committed and both pass. **No test was retired.** `styles.elevation.test.ts`'s `declarationsFor` now looks through a `:where()` wrapper, so its `input` radius assertion keeps the reach it had rather than losing it to a selector rename. Every other `styles.*.test.ts` passes unchanged. **Mutations, each run red before the claim was written.** | # | Mutation | Result | |---|---|---| | M3 | `shadows[1] = --shadow-bar` | red — `aubergine/light shadows: expected [ 'none', …(24) ] to deeply equal […]` | | M4 | `typography.button.textTransform = "uppercase"` | red — `expected 'uppercase' to be 'none'` | | M5a | `MuiPaper.defaultProps.elevation = 1` | red — `expected 1 to be +0` | | M5b | `MuiPaper.defaultProps.variant = "outlined"` | red — `expected { elevation: +0, variant: 'outlined' } to not have property "variant"` | | M6 | `shape.borderRadius` from `--r-card` | red — `aubergine/light default radius: expected 16 to be 10` | | M8 | remove the `MuiAutocomplete` paper override | red — `MuiAutocomplete paper is missing: expected undefined to be defined` | | M9 | remove `MuiButton disableElevation` | red — `expected undefined to be true` | | M10 | un-demote `button:hover:not(:disabled)` | red — the bare-element pin | M10 is not in the design's table. It was added because the demotion is this slice's whole neutralisation mechanism and nothing else would have noticed it being undone. **M7 and its mirror, against the live stack.** `bash mutation-check.sh phone-action-label-wrapped phone-entry-foot-stacked`: both killed, baseline green, restore green, and each one's `MUST_STAY_GREEN_ON` re-run of the whole desktop project came back green, which is what makes the rules width-scoped rather than merely broken. M7 no longer lengthens a label — a full-width button cannot be made taller than it is wide, so the old mutant would have survived — it reverts the stacking rule instead. `phone-entry-foot-stacked` is new and is the mirror: with the daily-entry pair exempt, M7 can no longer reach that row, which left the walk's side-by-side branch an assertion nothing could falsify. It stacks that row and reddens it alone. **The bare-element guard's own mutation.** A global `fieldset { border }` added to `styles.css`: red on two assertions, naming `fieldset {border}`. The control is the finding — the same rule under the pre-fix fixed element list left the pin **green**, and only the non-vacuity floor moved. **Playwright against the sim stack at this head.** `chromium-phone` 5 of 5 pass, including the walk extended to `/sales` as `phone.spec.ts`'s own comment asked (that comment is deleted with it). `chromium` 41 pass, 1 skipped (the 15-minute boundary). The walk now declares the layout each row must have and asserts the width share in both directions, because "full width everywhere" would fail on the one row the design says must not be full width. **The #740 measurements, at 390.** | Row | Before | After | |---|---|---| | Sales draft, `en` | `91.5 x 103.2`, `89.8 x 103.2`, `89.9 x 103.2` in a 295.2 row | all three `295.2` wide, 46.2 and 44.2 tall | | Sales draft, `tl` | — | all three `295.2` wide; the longest label wraps to two lines at 65.2 and stays far wider than tall | | Daily entry saves | `170.6 x 65.2` each, 48% of a 353.2 row | unchanged, and deliberately so | | Sales draft at 1280 | `285.7 x 40.1`, `98.8`, `50.8` | unchanged | **Captures.** 1:1 at `deviceScaleFactor: 1`, 1280x800 and 390x844, Dashboard, Daily entry and Sales, plus the Sales draft-order panel because that is where `.actions` lives and it renders only while a draft is open. The before set is the sim stack built at `16d0350`; the after set is the same stack rebuilt at this branch's head. One `main` commit sits between them (#869) and it touches no file under `web/`. With `CssBaseline` declined there should now be exactly one difference anywhere: the phone action rows stack. Every other pair should be identical. Anything else is a leaked override. **A note on the harness, because it cost a wrong conclusion.** The sim stack is one shared docker compose project (`cluckwork-sim`, hard-gated by `reset.sh`), and another worktree rebuilt it during the first after pass. Those captures showed clipped content at 390 and read exactly like a leaked override. They were the stale-bytes failure AGENTS.md names, arriving as a live race rather than an old container. The capture run now asserts the served stylesheet contains this branch's own selector before it believes a pixel, and the same comparison was reproduced independently against two Vite dev servers, one at `origin/main` and one at this head, which measured identical `documentElement.scrollWidth` on all six screen-and-width pairs. ## The CSP question, for whoever picks it up Three candidate answers, with what each costs, so nobody re-derives them: - **`'unsafe-inline'` on `style-src`.** One line, and a real relaxation — it re-opens CSS injection, including the attribute-selector exfiltration class. It is a security decision, not a build one. - **A per-request nonce threaded into `@emotion/cache`.** Keeps the policy strict. It needs `index.html` templated per request rather than served as a static file from `wwwroot`, which changes how the SPA is served. - **Build-time extraction**, so MUI's CSS lands in a real stylesheet the CSP already allows. Heaviest, and it constrains what the runtime theme can compute — which matters here, because this app's palette is read off the document at runtime by design (#674). I have not opened an issue for it. ## Documentation `specs/product/GLOSSARY.md` and the Help page do not change: no concept appears or changes meaning. No new user-facing string, so nothing to translate. A deslop pass was run over the diff against `main` before each commit. Closes #823 Closes #740 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added dashboard and daily-entry design mockups with responsive desktop and mobile layouts. * Added light/dark themes and multiple brand color options for design previews. * Added dashboard views for egg counts, recent sales, inventory, and 14-day trends. * Added daily-entry controls for egg counts, feed, water, and mortality, with draft and submission actions. * **Style** * Improved theme consistency with updated typography, spacing, elevation, borders, and component defaults. * Mobile action controls now stack vertically and provide larger touch targets. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🤖 I have created a release *beep* *boop* --- ## [0.1.2](v0.1.1...v0.1.2) (2026-09-16) ### Features * **data:** standardize business record chronology ([#820](#820)) ([6231b31](6231b31)) * **infra:** optional leader-lease endpoint for pooled deploys ([#869](#869)) ([e9bc6a7](e9bc6a7)) * **sim:** seed a second farm for the README dashboard capture ([#867](#867)) ([de407c6](de407c6)) * **web:** adopt MUI, themed from the farm palette tokens ([#674](#674)) ([#860](#860)) ([6c83c5c](6c83c5c)) * **web:** convert Daily entry to MUI, field-first on the phone ([#888](#888)) ([b66f8b8](b66f8b8)) * **web:** convert the Dashboard and app shell to MUI ([#829](#829)) ([#883](#883)) ([2e94277](2e94277)) * **web:** retire the Slack-blue link colour for ink + a rule underline ([#884](#884)) ([c08f9d8](c08f9d8)) * **web:** serve a per-request CSP nonce so Emotion's styles apply under style-src 'self' ([#874](#874)) ([ba4e6f3](ba4e6f3)) * **web:** visual language theme overrides for the MUI revamp ([#864](#864)) ([#882](#882)) ([0bb6b73](0bb6b73)) * **web:** whole-app MUI baseline, theme policy guard and the [#740](#740) phone action rule ([#823](#823)) ([#871](#871)) ([af565e4](af565e4)) ### Bug fixes * **auth:** fail closed on unresolved flock-scope actors ([#787](#787)) ([#868](#868)) ([16d0350](16d0350)) * **auth:** make farm configuration owner-only ([#870](#870)) ([42f9036](42f9036)) * **e2e:** repoint the canary at the markup two PRs replaced ([#844](#844)) ([18b45dc](18b45dc)) * **i18n:** tl glossary uses the standard passive of ilagay ([#813](#813)) ([20dec10](20dec10)), closes [#738](#738) * **sim:** stop the k6-baseline EXIT trap masking a clean run as failed ([#838](#838)) ([f5ec96f](f5ec96f)) * **web:** declare the rule tokens the Dashboard reads, and guard undeclared custom properties ([#885](#885)) ([5bead1f](5bead1f)) ### Performance * **ci:** start the serialized integration collection first ([#861](#861)) ([1dcc7f6](1dcc7f6)), closes [#839](#839) ### Documentation * **auth:** record the OAuth 2.1 decision for MCP authentication ([#801](#801)) ([0510854](0510854)) * **designs:** MUI revamp design doc, component map, layout system, IA ([#862](#862)) ([da49481](da49481)) * **readme:** recapture the daily entry, reports and sales screenshots ([#865](#865)) ([f18e336](f18e336)) * **specs:** correct the sales_order_items column list in §10.5 ([#812](#812)) ([afe4a02](afe4a02)), closes [#737](#737) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: cluckwork-lockfix[bot] <309265648+cluckwork-lockfix[bot]@users.noreply.github.com>
Part of #674. Groundwork for every slice on the SPA revamp milestone; closes nothing.
Why
Every slice on #674 is filed against MUI, and MUI is not on main. This PR lands the decision record and the theme bridge the slices cite, so the milestone can start.
The bridge reads the farm palette tokens back off the document with
getComputedStyleand hands MUI concrete colours.styles.cssstays the single source of truth for colour, and a fifth palette added there reaches MUI with no code change. AMutationObserverondata-brandanddata-themekeeps MUI in step with the pre-paint script and the night toggle, neither of which goes through React.Scope
docs/decisions/674-ui-component-library.md: MUI adopted, Tailwind + shadcn/ui declined, bundle and runtime cost measured.web/src/theme/farmTokens.tsandFarmThemeProvider.tsx: the bridge.CssBaselineis deliberately not rendered; adopting it is web: settle the whole-app MUI baseline (CssBaseline, type, spacing, elevation) #823.web/src/App.tsx: the provider mounts outsideAuthProviderso the login screen is themed from the cached palette (SPA: per-farm brand palette, keyed by slug so it survives pre-paint #586).specs/technical/tech_spec.md§8.1,web/README.md,docs/decisions/README.md: the consequences the record claims.Blast Radius
MUI becomes a production dependency under the #146 audit gate. No component renders yet, so no screen changes. Precache grows from 1312.45 KiB to 1397.79 KiB for the provider alone; #825 sets the ceiling before the first component lands.
Verification
npm run typecheckclean.npm run test:coverage: 125 files, 2996 tests, coverage ratchet green.farmTokens.test.tsproves all four palettes in both modes reachpalette.primary.maindistinctly and produce derived states.FarmThemeProvider.test.tsxproves the observer path. Mutation check: withobserver.observeremoved, the data-theme case fails.npm run build: precache 1397.79 KiB, matching the record's provider-only row.Summary by CodeRabbit
New Features
Documentation