Repository navigation
feat(web): redesign the setup lists as a table with a bottom inspector - #939
Conversation
…stomers #908 Concept B: the entity table keeps its scroll and comparison columns; selecting a row updates a separate inspector docked at the bottom of the list region instead of overlaying the table. New shared pieces in FieldConsole.tsx (RecordInspector, ListInspectorPane, selectableRowProps, STICKY_TABLE_HEAD_SX) plus a combined swipe+scroll phone cue on LedgerTableContainer. CustomersPage is the first of five screens to adopt it.
… source policy guard (#908)
The owner-approved table+bottom-inspector direction lab (crud-direction-lab.html), copied from the shared checkout's untracked docs/designs/ so this branch carries the reference the implementation follows.
…t prop Self-review pass (pstack:deslop) over the #908 diff before opening the PR. No caller ever passed it; ListInspectorPane's own bounded height already does the job. No other slop found (no any/eslint-disable/defensive try-catch/narrative comments in the branch's diff).
Customers — before / after (1280×800 and 390×844, light/dark)After frames captured at 6576010. Before is
|
Grades — before / after (1280×800 and 390×844, light/dark)After frames captured at 6576010.
|
Flocks — before / after (1280×800 and 390×844, light/dark)After frames captured at 6576010.
|
Users — before / after (1280×800 and 390×844, light/dark)After frames captured at 6576010.
|
Products — before / after (1280×800 and 390×844, light/dark)After frames captured at 6576010. Products tab (the default view; the before stack has no Packed units tab to diff against).
|
After-only: a row selected with the inspector showing (net-new UI, no before to diff)After frames captured at 6576010. One screen per pair, 1280 then 390, light.
|
Create dialogs — before / after (1280×800 and 390×844, light)After frames captured at 6576010. Before is
|
After-only: Products' Packed units tab, and the Flocks phone table at max scrollAfter frames captured at 6576010. The last frame proves the inspector's own in-flow layout never overlaps the fixed bottom nav.
|
… mode MUI's Tabs default (textColor="primary") paints the selected label and indicator with palette.primary.main (raw --brand), which styles.css keeps mode-independent on purpose — so it clears roughly 1:1 against a dark surface. Products' tabs were the first Tabs in the app, surfacing a defect MuiBottomNavigationAction already had to fix the same way (#829). Theme-level fix: MuiTabs/MuiTab now use --stat-accent (the same already-computed, per-brand "active" accent the bottom nav and sidebar's active-item rule use), which clears 4.5:1 against both --surface and --canvas in light and dark mode for all four brand palettes. farmTheme.policy.test.ts pins the computed contrast (not just "uses the token") across every brand x mode combination; confirmed red against the pre-fix theme (MuiTab/MuiTabs overrides absent) before restoring the fix.
Fix: selected Tabs label/indicator invisible in dark mode (owner-reported)New head: Cause: MUI's Fix, at the theme level: Current frames: see the Products before/after comment above (captured at 6576010): #939 (comment) |
Scan of every other dark after-frame for brand-coloured text on a dark surfaceRe-viewed all 10 previously-posted dark frames (Customers, Grades, Flocks, Users, Products — 1280 and 390 each) against this defect class specifically (brand-hued text sitting directly on Found only on Products — the one screen with a The theme-level fix above covers every current and future |
…ctually sticks Codex review of 1b13ad7, finding 1 (P2). LedgerTableContainer's TableContainer had overflow-x: auto (which computes overflow-y: auto too), making it position: sticky's containing block — but ListInspectorPane's outer Box was the one that actually scrolled. The header stuck to a scroller that never moved, and slid away with the content the moment the real scroller (the outer Box) was scrolled. Nests LedgerTableContainer and its TableContainer into the same flex-column chain ListInspectorPane already uses, so TableContainer is now the ONE element that both scrolls and anchors the sticky header. Outside a flex parent (this component's seven other, unbounded ledger callers) the added flex/minHeight properties are inert. New permanent Playwright spec (tools/simulation/ui/specs/) scrolls Flocks' real bounded table region in a live browser and asserts the header's bounding-box top does not move — a screenshot at rest cannot catch this, only scrolling and re-measuring can. Confirmed genuinely red against the pre-fix code first (header moved from y=181 to y=-19 on a 200px scroll), then green after the fix.
Codex review of 1b13ad7, finding 2 (P2). FlocksPage's selectedId was never cleared when the selected flock left `visible` (the show-archived toggle, or an archive/reactivate write): selectedFlock derives from visible and so the inspector correctly went blank, but the stale id survived in state and could resurface as "selected" — with no new choice from the user — the moment that row became visible again. Fixed with useClampSelection (FieldConsole.tsx, added alongside the sticky- header fix), wired into Flocks and, defensively and uniformly per the review's explicit request, into the other four selection points (Customers, Grades, Users, Products' two tabs) — none of which can currently drop a row out of its own list (no client-side filter, no hard delete anywhere in this app), confirmed by reading each screen's fetch/filter logic, but the same shape is now wired the same way everywhere rather than left correct by accident. Two new FlocksPage tests, confirmed genuinely red first (toggling the archived filter back on, or reshowing after an archive write, both left the row marked aria-selected="true" with no new click) then green after the fix.
… def Codex review of 1b13ad7, finding 3 (P3). The new tl.ts glossary prose named the five destinations in English (Customers, Products, Grades, Flocks, Users) instead of the labels the Tagalog sidebar and More menu actually show (Mga Customer, Mga Produkto, Mga Grado, Mga Kawan, Mga User), violating #688 — a user reading Help in Tagalog would be told to find screen names that don't match the nav. Re-checked the es prose the same way (read nav.customers/products/grades/ flocks/users in es.ts and compared): it already names the exact es nav labels (Clientes, Productos, Grados, Lotes, Usuarios) — no change needed there. en's prose already matches en's own nav labels too.
…-proof identity Codex review of 1b13ad7, finding 4 (P3). The MUI source policy guard identified a site only as file:literal, so FieldConsole.tsx's two approved uppercase sites (the table-head style and the inspector eyebrow, two different top-level components) both read as the identical components/FieldConsole.tsx:"uppercase" — the duplicated expected string checked only the COUNT, not which locations carried it. Removing uppercase from one and adding it to an unrelated style in the same file would leave the guard green. Tags each site with its nearest top-level export/component name (walkTopLevel + containerName), so the two FieldConsole.tsx sites are now distinguishable (#FieldConsole vs #RecordInspector) while sites that genuinely share one enclosing declaration (createFarmTheme's two shadow sites) are still allowed to collide, honestly. Mutation-checked the exact substitution the finding describes (removed uppercase from RecordInspector's eyebrow, added an unrelated uppercase inside ConsoleSubhead): confirmed the guard now fails on it before reverting the mutation and confirming green again.
…t any dialog Codex review of 1b13ad7, finding 5 (P3). "opens the same edit dialog from the inspector's own edit action" asserted only that some dialog existed, so a miswiring of the inspector's Edit button to a different dialog (e.g. openPassword) would still pass a bare findByRole("dialog"). Now identifies the dialog by its own accessible name ("Edit user — worker@farm.test") and checks the seeded Name value ("Wendy"), matching the stronger customer and product inspector tests. Mutation-checked by wiring the inspector's Edit button to openPassword: confirmed the strengthened assertion goes red (the edit dialog by that name never opens) before reverting the mutation and confirming green.
Codex (gpt-5.6-sol) review round 1 of
|
…w site's identity Codex review round 2 of 5f4f753, finding 1 (P3, a partial fix of round 1's finding 4). The container alone still let a same-component substitution through: moving textTransform: "uppercase" from FieldConsole's "& .MuiTableCell-head" rule to its "&& h2" rule kept the same container and the same collected literal, so the guard stayed green with the actual approved site gone. walkTopLevel now threads the chain of enclosing object-property keys between the container and the property itself, so the two FieldConsole sites are components/FieldConsole.tsx#FieldConsole > & .MuiTableCell-head > textTransform vs components/FieldConsole.tsx#RecordInspector > textTransform — genuinely different identities. As a side effect this also separates FarmThemeProvider's two boxShadow sites, which used to be an honest collision (MuiAutocomplete/paper vs MuiTooltip/tooltip, now both distinguishable too). Mutation-checked the exact scenario from the finding (moved the real uppercase from .MuiTableCell-head to && h2): confirmed the guard fails on the substitution (identity read "&& h2" instead of "& .MuiTableCell-head"), then reverted and confirmed green.
Codex (gpt-5.6-sol) review round 2 of
|
Ran the deslop skill over the full origin/main...HEAD diff — the seven commits after 1b13ad7 had never been passed through it. All findings were comment-only: - Comments narrating "codex review", "codex/owner review", or a specific round number, added while fixing PR review findings, rewritten to cite the actual issue (#908 for the feature, #824 for the guard the comments live in) and state the technical why without narrating the review process that produced it. - Several of those comments ran to 6-9 lines; trimmed to the 4-line cap, keeping only the non-obvious constraint. No `any`/`as any`/`eslint-disable`, no defensive try/catch on a trusted path, and no nesting an early return would flatten were found anywhere in the branch's diff. Behavior unchanged: typecheck clean, full web suite 3390/3390.
… last The shared MuiTableCell padding gave every cell 1rem on the right and none on the left, correct for the borderless ledger it was written against. Every table now sits inside a bordered FieldConsole panel, so the first cell's text sat flush against that border on every screen with no local override (Customers, Grades, Flocks, Products, Users). Add the inset at the base rule's own specificity via :where(:first-of-type), so routes that already carry their own higher-specificity override (FieldConsole's ledgers, Dashboard's inline table) keep their existing, already-symmetric padding.
…908) Users' Status column and header were blank for active users, leaving row status implicit. Show an explicit Active/Disabled badge in both the table and inspector, and give the Actions column the header the other screens already have (tc("actions"), Products' own precedent). Destructive row/inspector actions (deactivate, archive, deplete, disable) read identically to the primary edit link across all five screens, colour being their only distinguishing signal once colour vision is set aside. Add CONSOLE_DESTRUCTIVE_LINK_SX (FieldConsole's shared token, not per-screen sx) plus a leading TriangleAlert/Ban icon at each destructive call site, so shape carries the signal alongside colour.
) CONSOLE_DESTRUCTIVE_LINK_SX used --danger, a token designed as a filled- button background. Dark mode's --danger measures ~3:1 against --surface and its own hover fill, under AA's 4.5:1 floor for text. Switch to --error, the token already tuned for small-text contrast; add a red-first styles.test.ts assertion that reads the actual style value (not an assumed token name) and checks contrast against both rest and hover fill, every brand and mode. The destructive/status marker tests asserted only colour and text, so removing the icon or swapping StatusBadge for plain text kept them green. Assert the actual aria-hidden icon and StatusBadge class now, mutation- checked (icon removed, badge swapped for text) to confirm each goes red. Users' Active badge read a bespoke `activeBadge` catalog entry, duplicating enums:status.Active in three locales. Render statusLabel("Active") instead, i18n/enums.ts's one sanctioned renderer for this vocabulary, and delete the three now-redundant catalog entries.
Codex (gpt-5.6-sol) round 3 at 5418f87No P1 findings. Fixed both flagged product defects and the noted test gap; new head
Verification: |
|
Codex (gpt-5.6-sol) review round 4 at It checked the round-3 fixes: |
1 participant
Closes #908
822 component-plan row
This PR doesn't add a new #822 pair — it's a later-phase UX redesign layered on top of pair 9 (
table.data→TableContainer/Table size="small"/TableHead/TableRow/TableCell), which #897 already landed on these five screens. "Reused, never rebuilt": the new shared pieces extendweb/src/components/FieldConsole.tsx(the #899 Field Console layout family) rather than a new file —LedgerTableContainergained ascrollHintvariant for the combined "Swipe columns ↔ · Scroll rows ↕" cue, and three new exports (RecordInspector,ListInspectorPane,selectableRowProps,STICKY_TABLE_HEAD_SX) implement the bottom inspector and row-selection wiring shared by all five screens.What changed, per screen
All five screens (Customers, Grades, Flocks, Users, Products) keep their existing table columns, dialogs, business logic and RBAC gates untouched. Each row is now click/keyboard-selectable (
aria-selected, Enter/Space, a visible accent bar); the table is wrapped in a bounded, independently-scrolling region (ListInspectorPane) with a dockedRecordInspectorpanel showing the selected row's facts and the SAME action handlers the row's own Actions cell already calls (extracted into onerenderActions/renderProductActions-style function per screen, not duplicated). Per-row actions stay in the table too — the approval comment names the inspector as an addition, not a replacement, and removing a working affordance without an explicit ask was not this slice's call to make.myId !== u.id) preserved.Tabs("Products" first, "Packed units" second), each with its own table+inspector and its own, independent selection state (switching tabs does not clear or carry over the other tab's selection — confirmed by a dedicated test).Individual's fixed-1 packed-unit behavior is unchanged.Phone: all five tables carry the combined
Swipe columns ↔ · Scroll rows ↕cue (a newLedgerTableContainervariant +common:swipeColumnsScrollRowsi18n key, since the table now also scrolls vertically in its own bounded region, not just horizontally). Verified at 390×844 that the inspector's own in-flow layout never overlaps the fixed bottom nav (screenshots below).Docs sync
Per #688 (help prose must use each locale's own control label): added a Selected-record inspector entry to
specs/product/GLOSSARY.md's "Getting around" section, a matchinghelpGlossary.tsrow, andglossarySelectedRecordInspectorTerm/Def+swipeColumnsScrollRows/inspectorEmptyPrompt/inspectorLabel/entitySingularcatalog keys in en/es/tl (es/tl machine-drafted, matching the existing "PENDING NATIVE-SPEAKER REVIEW" convention for those files).Tests (measured before/after, not remembered)
switchToPackedUnits()tab click first — the row they targeted moved to a second, initially-hidden tab)Every new test asserts a literal expected value (row
aria-selected, inspector heading/field text, dialog reopened, tab-independent selection) — not just "doesn't crash".Full suite:
npm run typecheckclean;npm test -- --run3387/3387 (zero pre-existing failures on a clean run — oneDailyEntryPage.test.tsxtest flakes under full-suite parallel load, confirmed pre-existing and unrelated: passes standalone, and this PR never touches that file);npm run test:coverage92.16/88.96/88.24/94.83 (statements/branches/functions/lines) against the 89/80/85/92 floors;npm run buildandnpm run verify:swclean (precache 1843.52 KiB, 56.48 KiB under the 1900 KiB ceiling).Guards
styles.harness-selectors.test.ts,styles.declared-tokens.test.ts,farmTheme.policy.test.ts,styles.elevation.test.ts: green throughout, by construction — every new style issxon existing MUI components using already-declaredvar(--…)tokens (--tint-accent,--brand,--rule,--surface), sostyles.cssitself was never touched.styles.conversion.test.ts(web: the postcss style guards go blind as screens convert to MUI #824 MUI source policy): the inspector's eyebrow label added a secondtextTransform: "uppercase"site inFieldConsole.tsx(the first is the existing table-head style). Ran RED first (confirmed the guard's pinned census failed with the real diff before I touched the test), then registered the new site as a second, deliberate entry — not an allow-list, the guard's own documented mechanism for tracking new reviewed uppercase sites.catalogParity.test.ts+helpGlossary.test.ts: green after the glossary/help additions (206 tests, includes the cross-check betweenspecs/product/GLOSSARY.md's bold terms and the in-app catalog).dotnet test tests/Cluckwork.Application.Tests --filter "FullyQualifiedName~ImagePin|FullyQualifiedName~RealTree": 14/14, run before both markdown-touching commits (the GLOSSARY.md entry and the copied design artifact).Verification
cluckwork-sim.cw908b(before, built fromorigin/mainat190dcd6, the commit this branch forked from) andcw908a(after, this PR's head,1b13ad7), both via a standalone copy ofdocker-compose.sim.yml(project name + published port 8108 overridden, torn down and images removed after use). Both farms provisioned on each (default-farmsimulation fixture,readme-farmdemo).cw908a(this head): 92 passed, 1 intentionally skipped (the slow session-refresh spec, same as every other PR here), 0 failures — includingphone.spec.ts's "no walked screen overflows the viewport horizontally" (the historical class of regression the web: convert the CRUD list screens to MUI — Customers, Products, Grades, Flocks, Users #832 slice hit) and the pagination specs that exercise Customers' and Flocks' paged lists directly.readme-farm, 1:1, same scenario both sides: all five screens (Products both tabs) at 1280×800/390×844, light/dark — before frames fromcw908b, after fromcw908a. Plus after-only (net-new UI, no before to diff against): a selected row with its inspector open at both widths for all five screens, an open create dialog at both widths for all five screens, the Products "Packed units" tab at both widths, and the Flocks phone table scrolled to its end showing clearance above the bottom nav. Compared every after frame againstdocs/designs/674-crud-redesign/crud-direction-lab.html's Concept B myself before attaching: table-above/inspector-below layout, dark inspector header, two-column fact list, phone swipe+scroll cue, and independent Products-tab table+inspector pairs all match the approved direction.Left out / open items
DailyEntryPage.test.tsx) is pre-existing and untouched by this branch.Also fixes: flush-left first column on every MUI table (theme-level)
Owner-reported: the first column's text sat almost flush against the bordered
pane's left border on all five of this PR's screens. Root cause was the shared
MuiTableCellpadding intheme/FarmThemeProvider.tsx(0.6rem 1rem 0.6rem 0,zero left inset), written for a borderless ledger before every table sat inside
a bordered panel. Fixed at the theme root with
"&:where(:first-of-type)": { paddingLeft: "1rem" }—:where()holds it at the SAME specificity as thebase rule, so any route with its own higher-specificity padding override keeps
that override unchanged.
Walked every route rendering a MUI
<Table>(Customers,Dashboard,Expenses,Feed,Flocks,Grades,History,Inventory,Products,Reports,Sales,Stock,Users,Water;Audit/Exportrender no MUItable) at 1280 and 390, light. Visibly affected: only this PR's five screens
(Customers, Grades, Flocks, Products, Users) — the routes with no local
MuiTableCelloverride.FieldConsole-wrapped ledgers (Expenses, Feed,History, Inventory, Reports, Sales, Stock, Water) and
Dashboard's own inlinetable already carry their own equal-or-higher-specificity padding override
(8px and 6px scales respectively) and render byte-identical before and after
this fix — confirmed via computed-style measurement, not inference, so no
before/after pair is posted for those routes.
No new overflow at 1280 or 390 on any of the 14 routes (
document.scrollWidth==
clientWidtheverywhere).Flocks' own bounded table region alreadyscrolled horizontally before this fix (its widest row exceeded its container);
the fix adds 16px to that pre-existing, by-design scroll distance (the same
LedgerTableContainerswipe-hint pattern already used byHistory), not a newbreak. Separately found and not fixed (pre-existing, out of scope):
Users'Actions column (5 buttons) already overflowed its table's bounded region by
about 48px at 1280 before this fix; confirmed via DOM-simulated removal of the
fix that the overflow predates it.
Covered by a red-first
farmTheme.policy.test.tsassertion (the themecontract) and a real-browser Playwright geometry check
(
table-cell-first-column-inset.spec.ts) on bothFlocks(this fix's own16px scale) and
History(an existing #899 ledger's untouched 8px scale),each asserting the first column's left gap is close to the last column's right
gap.
Acceptance follow-up: row identity/status/actions distinct without colour alone, and dialog before/after
Two #908 acceptance criteria were unmet at
e6246b6and are fixed here:Activebadge (matching Grades'/Products' own status chip, table and inspector) alongside the existingDisabledbadge, and anActionsheader matching Products' precedent.activeBadgeadded to en/es/tl, reusingenums:status.Active's existing es/tl wording rather than drafting new copy.FieldConsole.tsxgainedCONSOLE_DESTRUCTIVE_LINK_SX(one shared token, not per-screen sx); every deactivate/archive/deplete/disable call site across Grades, Flocks, Products and Users now pairs it with a leadingTriangleAlert/Banicon. Customers has no destructive row action, so it is unaffected. Corrective actions (activate/reactivate/enable) are untouched. Accessible names are unchanged (icons arearia-hidden).Verified red-first per screen: reverted each screen's own change via
git stash, confirmed the new test failed oncolor: var(--link)instead ofvar(--danger), then restored. Full web suite green (3398/3398), typecheck clean, full quick Playwright green on a rebuiltcw908a.Screenshots: recaptured every after-frame whose appearance changed (Grades, Flocks, Products, Users — Customers excluded, unaffected) across the five main comments, the row-selected comment, and the create-dialog comment. Separately, the create-dialog comment previously had after-only frames; it now also carries before frames from an isolated stack built at
origin/main's190dcd6(the same base commit the original before-frames used), for all five screens including Customers.Screenshots below, laid out as before/after pairs per screen, followed by the after-only inspector/dialog/tabs/phone-scroll evidence.