Repository navigation
F131: move add/edit capture into accessible modal dialogs - #132
Conversation
Inline create bars and in-row edit modes shoved the list around on every open — the form competed with the data it mutated. Add/edit now happens in a shared modal, so the list stays put and the form gets full attention. New `Dialog` component (web/src/components/Dialog.tsx): - role="dialog" + aria-modal, named by its heading via aria-labelledby - focus moves to the first field on open and returns to the trigger on close - Tab is trapped inside the panel in both directions - Escape, the close button, and a backdrop click all dismiss; a click that lands on the panel does not - body scroll is locked while open and restored on close - portalled to <body>, styled with the #52 tokens, and a bottom sheet below 900px; the entry animation respects prefers-reduced-motion Migrated (create, in-row edit, and per-row actions): - Grades, Products (+ packed units), Customers, Users, Flocks, Inventory - Flocks: bird movement; Inventory: purchase, feed usage, stock correction - Expenses: category + correction; History: entry adjust Behaviour is unchanged by design: same request bodies, validation, role gates, idempotency-key replay/rotate, and version guards. A failed save keeps the dialog open with the error beside the form it asks you to re-apply; a success closes it and resets. The screens whose whole job is capture — Daily entry, Water, recording an expense — keep their form inline and are tracked separately. The Expenses and History correction panels no longer scroll-and-focus themselves; the dialog takes focus, so that workaround is gone. Tests: new Dialog suite (7) covers the a11y contract; every migrated screen opens the dialog first and keeps its existing assertions, plus a dismissal test proving Cancel writes nothing. 261 -> 285 tests. Coverage floor re-baselined on this state: lines/statements 80 -> 84, functions 63 -> 64. Docs: glossary gains a Dialog entry; the Help page gains an "Adding & correcting" section explaining the popup flow and which screens keep an inline form. Also fixes a latent style bug the dialogs exposed: `.inline-form input` (10rem) outranked `input[type="checkbox"]`, stretching checkboxes to 160px. Closes #131 for the CRUD and per-row surfaces.
Dialog (web/src/components/Dialog.tsx):
- Focus return survives a save that closes while still busy. The screens
render their row trigger `disabled={busy}`, and History/Expenses close the
dialog before the write settles, so the restore hit a disabled button —
focus() is a silent no-op there and focus fell to <body>. The restore now
verifies it landed and retries on the next frame, guarded on <body> so a
retry can never steal focus the user has since moved. (codex, high)
- New `focusKey` prop. The focus effect keyed only on `open`, so a 409 rebind
— which swaps the server's newer record into the still-open dialog — left
focus wherever it was while the form was replaced underneath. History and
Expenses now pass their record; changing it pulls focus back to the first
field. This restores what the removed `[adjusting]`/`[editing]` scroll-and-
focus effects did on main. (feature-dev)
- The focusable selector no longer matches things the browser will not tab
to: hidden inputs, [hidden]/aria-hidden nodes, tabindex="-1". A hidden
first field swallowed the initial focus and a hidden last field stopped
being the trap boundary, letting Tab escape. Focus calls are now verified
rather than assumed. (codex, medium)
- Escape reads the latest onClose through a ref, so the keydown listener
binds once per open instead of being torn down and re-added on every
keystroke (callers pass inline lambdas). (cavecrew)
Behaviour parity the migration had drifted on:
- Admin-only dialogs are gated on `isAdmin`, not just their trigger. Roles
are re-read on token refresh, so a demotion mid-edit used to leave a
submittable form open where the inline `{isAdmin && ...}` form would have
vanished. Grades, Products (+ packed units), Inventory (create/edit/
correction), Flocks (edit, movement). (codex, medium)
- `noValidate` on the five edit forms whose save used to be a plain button.
Making them submit buttons activated min/step/required constraints that
had never run, so the browser started blocking input before the screens'
own parser could report the useful message ("At most 3 decimal places for
this currency"). Also drops the `required` this PR had added to the Grades
edit name. (codex, medium)
- Products edit no longer exposes Notes. The inline edit had no notes field;
#131 changes the shape of capture, not what it can do. editNotes stays
seeded so the request body is unchanged. (codex, low)
Not changed: Grades/Products leave gradeType/unit/grade unreset after a
create. Both are verbatim from main — sticky pickers help when adding
several of a kind — so "fixing" them here would itself be the drift this PR
promises not to introduce. (cavecrew)
Tests: 285 -> 291. New Dialog cases cover the disabled-trigger focus retry,
focusKey rebind (and that an unrelated re-render does NOT re-grab focus),
and the trap skipping hidden / tabindex="-1" controls. ProductsPage gains a
validation-parity case pinning that an over-precise price still produces the
screen's currency-scale message. Coverage 84.4 -> 84.6 lines, gate exit 0.
Review: codex + feature-dev + cavecrewAll findings triaged; fixes in 1c29cf9. FixedFocus return broke on a successful save (codex, high). The screens render their row trigger The 409 rebind lost focus (feature-dev). The focus effect keyed only on Focus trap matched untabbable controls (codex, medium). The selector took hidden inputs, Admin dialogs survived a live role demotion (codex, medium). Only the triggers were gated. Roles are re-read on token refresh, so a demotion mid-edit left a submittable form open where the inline New native validation on converted edit forms (codex, medium). Turning a plain "save" button into a submit activated Products edit exposed a Notes field (codex, low) the inline edit never had. Removed — #131 changes the shape of capture, not what it can do. Keydown listener churn (cavecrew). Escape now reads the latest Deliberately not changedGrades and Products leave Verification285 → 291 tests, all passing. New Dialog cases cover the disabled-trigger focus retry, the One gap worth naming: the |
…alog Creating a flock from the capture screen still unfolded an inline form that pushed the whole entry grid down the page — the exact pattern #131 set out to remove, and the same entity the Flocks screen already creates through a dialog. The toggle becomes a plain opener (it no longer doubles as "cancel" — the dialog owns dismissal). It slipped the first pass because no test touched it: the screen's suite covers capture, and the create-flock path had no case at all. Added one that pins the full request body with every field off its default plus the idempotency key, that success closes the dialog and selects the new flock, and that Cancel writes nothing. DailyEntryPage 68.0 -> 80.3% lines. Still inline by design (tracked in #133): the entry grid itself, the Water reading, and recording an expense — those screens exist to capture. 291 -> 293 tests; coverage 84.6 -> 85.3 lines, gate exit 0.
Completes the sweep. Sales was the last screen with capture controls sitting inline; starting an order and taking a payment are discrete actions, so they move behind "New order" (page head) and "Record payment" (order panel). Adding lines to a draft stays inline — that panel IS the work surface, same call as the Daily entry grid. Deliberately NOT wrapped in <form>: these controls were always button-driven onClick, so introducing a form would newly enforce min/step and swallow the screen's own money messages — the exact drift codex flagged on the converted edit forms earlier in this PR. They keep their handlers and gain a dialog-foot; success closes the dialog from inside the try, so a throw leaves it open with the error. Payments had NO coverage before this — recordPayment was stubbed but never asserted. Three cases added: the full request body at a 3-decimal scale (BHD "1.5" -> 1500, so a hard-coded x100 dies), blank optional fields nulled, and Cancel writing nothing. SalesPage 60.9 -> 77.9% lines. Coverage floor re-baselined on the finished state: lines/statements 84 -> 86, functions 64 -> 66, branches unchanged at 82 (83.1 actual). 293 -> 296 tests; global coverage 85.3 -> 86.8 lines, gate exit 0. Docs: glossary + Help "Adding & correcting" list orders and payments, and name adding order lines among the deliberately-inline capture forms.
Sweep completed — two surfaces were still inlineAudited every Daily entry's Sales — new order + record payment (fe90ad1). Starting an order and taking a payment are discrete actions, so they move behind New order (page head) and Record payment (order panel). Adding lines to a draft stays inline — that panel is the work surface, same call as the Daily entry grid. Worth noting: those Sales controls were always button-driven Payments had no coverage at all — Deliberately still inline
State296 tests (261 at branch start), coverage 86.8% lines / 83.1 branches / 67.3 functions. Floor re-baselined on the finished state: lines/statements 84 → 86, functions 64 → 66, branches unchanged at 82. Gate exit 0, tsc 0, build 0, CI green. Docs updated with it: the glossary entry and the Help "Adding & correcting" section now list orders and payments, and name order lines among the deliberately-inline capture forms. |
Asked whether the F131 dialogs work on a phone, I drove them at 390x844 and found the page itself was broken: it laid out ~510px wide on a 390px screen. Cause is the shell grid, not the dialogs. `grid-template-columns: 1fr` floors the track at the item's min-content, and the mobile nav bar's min-content is wider than a phone, so the track won a 510px width the viewport could not show. The page-head sits at the top of that overflowing column, which put each screen's "New …" button underneath the wrapped nav — Playwright could not tap it, and neither could a thumb. `minmax(0, 1fr)` lets the track shrink to the container. Measured on an iPhone-13 viewport before -> after: grid column 510px -> 390px document scrollWidth 510 -> 390 (no horizontal scroll) "New grade" BLOCKED -> tappable The bug predates this PR (it came in with the #52 shell) but it is what "are the modals mobile friendly?" actually surfaces, so it belongs here. The dialogs themselves check out on a phone: full-width bottom sheet anchored to the bottom edge, rounded top corners, scrim over the nav, and the tallest form in the app (Inventory purchase, 6 fields) fits without internal scrolling with both footer buttons in thumb reach. Verified light and dark. Desktop is unchanged: 244px sidebar + content at 1280/1024/901, single column at 899/768, no horizontal overflow at any of them.
1 participant
Closes #131 (CRUD + per-row surfaces). Follow-up to the #52 revamp — Phase 1.1, epic #14.
Why
On the CRUD screens the add form was a bar above the table and editing swapped a row into inputs. As the lists fill up that reads as intrusive: the form competes with the data it mutates, and opening an editor shoves everything below it down the page.
Add/edit now happens in a modal. The list stays put; the form gets full attention.
The Dialog component
web/src/components/Dialog.tsx— one shared shell, portalled to<body>:role="dialog"+aria-modal, named by its heading througharia-labelledbyprefers-reduced-motionWhat moved
Each screen grows a New … button beside its title;
edit/correctlinks open the same dialog seeded from the row.What did not change
Same request bodies, validation, role gates, idempotency-key replay-on-failure / rotate-on-success, and version guards. A failed save keeps the dialog open with the error beside the form it is telling you to re-apply; a success closes it and resets. Read-only drill-downs (ledgers, lots, movements) stay inline.
Screens whose whole job is capture — Daily entry, Water, recording an expense — keep their form on the page. Migrating those is a separate call and is tracked in the follow-up issue below.
Bonus removal: the Expenses and History correction panels used to
scrollIntoView+ focus themselves because they rendered above the table. The dialog takes focus, so that workaround is gone.Tests
Dialog.test.tsx(7) pins the a11y contract: modal semantics, first-field focus, focus return, Tab trap both ways, Escape / close / backdrop-vs-panel click, scroll lock + restore261 → 285 tests. Coverage floor re-baselined on this state: lines/statements 80 → 84, functions 63 → 64, branches unchanged at 82.
Verification
vitest run --coverage— 285 passed, gate exit 0 (84.5 lines / 83.0 branches / 64.6 functions)tsc --noEmit— 0npm run build— 0Docs
Per the standing rule, both doc surfaces move with the change: glossary gains a Dialog entry; the Help page gains an Adding & correcting section (new TOC entry) covering the popup flow, dismissal, retry safety, and which screens deliberately keep an inline form.
Incidental fix
The dialogs exposed a latent style bug:
.inline-form input(10rem) outrankedinput[type="checkbox"], stretching checkboxes to 160px. Pinned inside dialogs.