Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8377096
feat(web): replace NamedEntityPicker's combobox with MUI Autocomplete
mforce Sep 17, 2026
99c0d0f
chore(web): retire the named-picker combobox CSS the Autocomplete con…
mforce Sep 17, 2026
cddae7b
test(web): fix the six page suites' picker-trigger queries for the ne…
mforce Sep 17, 2026
8928999
test(e2e): fix the Playwright harness's picker-trigger mechanics for …
mforce Sep 17, 2026
a6699c1
docs(design): amend 822-mui-revamp.md pair 1 for what #826 actually s…
mforce Sep 17, 2026
5ae46aa
fix(web): stop ArrowDown-at-the-end from wrapping the highlight to th…
mforce Sep 17, 2026
42a8be7
fix(web): disable Autocomplete's list-wrap so the TRUE final option d…
mforce Sep 17, 2026
c7ffb6e
test(e2e): track aria-activedescendant instead of Mui-focused for the…
mforce Sep 17, 2026
a3065e3
fix(web): use a chevron, not a magnifier, for the closed-state trigge…
mforce Sep 17, 2026
6543da3
refactor(web): stop building the footer-ref object literal twice
mforce Sep 17, 2026
56f70f7
fix(web): use TextField size="small" to stop the picker truncating it…
mforce Sep 17, 2026
ebb9c4d
fix(web): widen the popper and stop it wrapping options at phone widt…
mforce Sep 17, 2026
d504ae4
fix(web): force scrollLeft=0 after the open-focus selection — directi…
mforce Sep 17, 2026
1a9e8f0
fix(web): stack Daily entry's Flock picker full-width above Date at 390
mforce Sep 17, 2026
21b3c14
fix(web): keep Daily entry's date underline styling after the 390 row…
mforce Sep 17, 2026
f770bab
feat(web): make Daily entry's Date field an outlined MUI TextField (#…
mforce Sep 17, 2026
4a08662
docs(designs): record #898's phone-stacking and outlined-vs-underline…
mforce Sep 17, 2026
3674fac
fix(web): shrink Daily entry's Date label so it never overlaps the mm…
mforce Sep 17, 2026
4a525f2
fix(web,tools): address Codex review of #898 (head 4a08662), six find…
mforce Sep 17, 2026
0f23fc0
fix(e2e): two more commitNamedPicker/worker.spec.ts defects found by …
mforce Sep 17, 2026
bf5c81b
test: pin all four highlightedIdRef clear sites; correct finding 2's …
mforce Sep 17, 2026
d10d892
test: fourth attempt at finding 2's regression test, real browser, st…
mforce Sep 17, 2026
59f193e
chore(web): trim review-round narration from #826's source comments t…
mforce Sep 17, 2026
5541ab0
test(web,tools): deslop review-round narration in test comments; drop…
mforce Sep 17, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/designs/822-mui-revamp.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,19 @@ All from `@mui/material` 9.4.0 (`package-lock.json`). No `@mui/x-*` package; no
| 21 | `DayStrip`, `StockBar`, `.meter`, `.meter-stack`, `GradingChip`, `BrandSplash`, `ProvenanceCell`, `FarmDate` | **kept as they are** | bespoke data marks and interactions (#654, #777, F134, #179); their CSS stays (§2.2). `StockBar` is a multi-grade stacked bar and `LinearProgress` is single-value; `GradingChip` is a drag source (§2.1). No alternative meets the functionality, so these are not owner-review rows. | none |
| 22 | `usePagedList` "Load more" | `Button` | the hook (381 lines, ticket discipline) is not presentation and stays; there is no offset `Pagination` to adopt. | none |

**Amendment on pair 1 (#826, landed 2026-09-17).** Six places where what shipped differs from the row above, each found by running the rewritten suite against the real installed `@mui/material@9.4.0` rather than assumed from the API surface, plus three presentation changes the owner made after the first captures (mid-implementation, and two more in a #898 review round):

- **`Autocomplete`'s own Escape handling calls `event.stopPropagation()`**, which the row's plan did not anticipate and which broke a real caller: a picker nested inside a Dialog (Sales' new-order customer picker) used to let a single Escape press both cancel exploration AND bubble to close the dialog. `onClose`'s `reason === "escape"` branch never fires that stop; the fix moves Escape handling into the picker's own root `onKeyDown` (which `useAutocomplete.js`'s `getRootProps` calls BEFORE its internal switch) and sets `event.defaultMuiPrevented = true` so `Autocomplete`'s own Escape case — and its `stopPropagation` — never runs. Caught by `SalesPage.test.tsx`'s existing "lets the dialog own Escape" test, not by the four picker test files.
- **`useAutocomplete.js`'s `handleValue` bails out of calling `onChange` when the newly selected option is `===` the current controlled `value`** — reference equality, not `isOptionEqualToValue`. A picker whose committed entity and discovery window share the same object reference (FR-037's page-level default customer, fetched once and handed to both `controlledCommitted` and the picker's own discovery request) silently no-ops on re-clicking that already-committed option. Fixed by passing a shallow clone as `value`, so it is never reference-equal to a discovery row. Also caught by `SalesPage.test.tsx`, not the four picker files — the row's plan had no reason to anticipate a same-reference collision.
- **`getOptionKey` needed explicit wiring, by id.** Without it, `useAutocomplete.js` falls back to `getOptionLabel(option)` as the React list key, and two rows can share a name (the discovery contract explicitly allows it — the duplicate-name pair test in `NamedEntityPicker.test.tsx`) — a real key collision the row's plan did not name.
- **The listbox's own AX status element could not be reused for loading/no-results text**, contrary to what "`noOptionsText`/`loadingText`… all five come from `t()`" implied. `Autocomplete`'s `StatusSlot` (role="status") mounts UNCONDITIONALLY whenever the popper is open — even with both its loading and no-options children null — because `hasPopupContent` is unconditionally true whenever `freeSolo` is false. A second, usually-empty `role="status"` node coexisting with the engine's own would make `getByRole("status")` ambiguous, and `Autocomplete`'s own text has no way to express "loading while retained rows are still shown" (Load more in flight), which the engine's own status markup must keep handling regardless. Fixed by nulling `slots.status` and keeping the engine's own status/alert spans verbatim; `noOptionsText`/`loadingText`/`clearText`/`closeText`/`openText` are still supplied from `t()` (the five icons/text they would otherwise leak into are also nulled or hidden), so the letter of "no baked-in English" holds even though the visible path never exercises them.
- **`disablePortal` is required**, not just elevation. Without it, `Autocomplete`'s popper renders in a React portal to `document.body`, which defeats the engine's own outside-click `mousedown` listener (`containerRef.current.contains(event.target)` — a portalled option is never a DOM descendant of the container) and would have broken US2's outside-click cancellation.
- **Presentation change (owner, 2026-09-17), beyond the row's scope.** The committed/closed state no longer renders the page-supplied `trigger` element's own markup (a `<button className="named-picker-trigger">`): it now reads as the SAME outlined MUI field as the open search — a read-only `TextField` with the picker's `label` floating in the border and a `lucide-react` `ChevronDown` end adornment (checked against #864's own Daily Entry mockup, `docs/designs/864-visual-language/daily-entry-390.png`, which draws this exact affordance as a chevron rather than the magnifier the owner's instruction also allowed) — because the old trigger read, per the #896 review, as "a plain dark box with no affordance that it reopens a search." The `trigger` prop's `onClick`/`disabled` are extracted and reapplied to the new field (click, Enter and Space all open it); its CHILDREN (the displayed value or a per-screen placeholder — "No flocks yet", "All", a loading/unavailable fallback) still come from the caller, unchanged, since the engine's own `committedText` has no placeholder to fall back to. The open state's `<label>` also moved onto the `TextField` itself (`label`/`required` props), dropping the separate external element. `.named-picker-trigger` (base and `button`-qualified) CSS retires — nothing renders it anymore — `.named-picker-label` and `.named-picker-committed` do not (both still have a live render site). This changed the picker's closed-state ACCESSIBLE NAME from "`<label> <value>`" (a hand-built `aria-labelledby` chain) to the label alone, which broke 69 tests across six page files (role `button`→`textbox`, name matcher from the value/placeholder to the plain label) — none of them in the four picker test files, all fixed the same mechanical way.
- **Presentation change (owner, 2026-09-17, #898 review round): Daily entry's Flock+Date row stacks at 390.** The shipped two-up row put the Flock field at roughly 110px wide once Date and "+ new flock" shared it — narrow enough that a real committed flock name ("House 1 layers") truncated to its first few characters with no way to read the rest, caught on a #898 review capture. The owner was offered two fixes: widen the picker while open, or stack Flock full-width above Date at 390. Stacking won because a field whose width changes between its closed and open states makes the whole row jump; widening only the open state was rejected for exactly that reason. `DailyEntryPage.tsx`'s row `Box` now switches `flexDirection` to `"column"` only below `md` (`{ xs: "column", md: "row" }`); Date and "+ new flock" stay grouped in their own nested row `Box` so their relative spacing at 1280 is pixel-identical to the old flat three-item row, and 1280 itself is unchanged by construction (`md` is the same 900px breakpoint D3 names). Recapturing the "after" state caught a second-order regression from the restructure itself: the Date `<label>`'s new nesting depth broke the outer Box's `& > label input[type='date']` **direct-child** sx selector (the label moved one level deeper), so Date silently fell back to the browser's default boxed input at 390 with no visible error — fixed by widening the selector to `& label` (descendant) before that commit landed. (That selector, and the native `<input type="date">` it targeted, are both since retired by the next bullet.)
- **Presentation change (owner, 2026-09-17, #898 review round): Daily entry's Flock and Date fields both go outlined, not underlined.** #826's own row above already changed the picker's closed state to an outlined `TextField`, but left "whether Daily Entry's flock control should keep the underlined-select language or fold into the new global outlined look" as an explicit open design call (see the presentation-change bullet above), and Date kept a hand-rolled underline (`border: 0; borderBottom: 1px solid var(--rule-strong)`) meant to visually match it. The owner resolved this by direct A/B comparison: two throwaway builds (A — outlined for both; B — underlined "standard" variant for both, matching #830's confirmed mockup) were captured at 1280 and 390, attached to #898 in one comment, and the owner picked **A**. Date converts from the native `<input type="date">` to an outlined MUI `TextField` (`type="date"`, `size="small"` to match the picker's height); the picker itself needed no code change — outlined is already `NamedEntityPickerEngine`'s shipped default, so nothing passes a `variant` prop at all, and the A/B comparison's own throwaway `variant` prop (which the B build would have needed) was deleted rather than wired through, since nothing selects anything but the default now. The underline sx rule the previous bullet's selector fix was preserving is retired with the native input it targeted; it was scoped to this one `Box`'s own inline `sx`, never a shared CSS class, so there was nothing in `styles.css` to delete alongside it (confirmed by grepping the whole repo, including `tools/simulation/ui`, before removing it).

Test-count consequence: the four picker test files total 1,851 lines (1,420 / 241 / 138 / 52) across 61 tests — effectively unchanged from the 1,852 lines this row's own diagnosis measured, since the brief was "rewrite what the mechanism change breaks," not "shrink." `NamedEntityPicker.tsx` itself grew from 1,147 to 1,286 lines: the engine (discovery, selection-transition, generation discipline) is untouched, but the render layer gained the closed-state `TextField` redesign, the `slots.paper` footer component and its `footerRef`/lazy-`useRef` stability wiring, `getOptionKey`, and several long explanatory comments recording the divergences above — a net INCREASE the row's plan (implicitly, "replace the combobox") did not predict, because it did not anticipate the mid-implementation trigger redesign.

**Amendment on pair 2 (#827, landed 2026-09-16).** Four places where what shipped differs from the row above, each found by running the rewritten test suite against the real installed `@mui/material@9.4.0` rather than assumed from the API surface:

- **`disableEscapeKeyDown` does not exist in this version.** Checked directly against the installed package (zero matches for the prop name anywhere under `node_modules/@mui/material`, in both the implementation and its `.d.ts`) — it was removed from `Modal` at some point between the version this row's plan assumed and 9.4.0. `closeDisabled` is therefore two mechanisms, not three: an `onClose` wrapper that gates on `closeDisabled` for both `escapeKeyDown` and `backdropClick` reasons (Modal calls it identically for both), plus the disabled close button. #609 rests on those two.
Expand Down
91 changes: 75 additions & 16 deletions tools/simulation/ui/specs/named-entity-picker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ test.describe("Searchable named-entity picker (#512)", () => {
await signIn(castMember("Manager"));
await nav.link("nav:dailyEntry").click();

// The trigger's accessible name is "<label> <current value>" (aria-labelledby),
// and the current value may already be a remembered/default flock rather than
// the empty "Select a flock" placeholder — match on the stable label prefix.
await page.getByRole("button", { name: new RegExp(`^${tEn("dailyEntry:flockLabel")} `) }).click();
// #826 — the closed-state trigger is a read-only MUI field now (role
// "textbox"); its accessible name is the picker's label ALONE (the
// current value — possibly already a remembered/default flock rather
// than the empty placeholder — lives in the field's `value`, not its
// name).
await page.getByRole("textbox", { name: tEn("dailyEntry:flockLabel") }).click();
const combobox = page.getByRole("combobox", { name: tEn("dailyEntry:flockLabel") });
await expect(combobox).toBeVisible();

Expand Down Expand Up @@ -93,16 +95,57 @@ test.describe("Searchable named-entity picker (#512)", () => {
// flocks of their own, so keep arrowing (never assume exactly one more
// press) until it is genuinely the active option, then commit with
// Enter — also part of the Keyboard Contract, and never a click.
for (let i = 0; i < 60; i++) {
const activeClass = await sentinel.getAttribute("class");
if (activeClass?.includes("active")) break;
// #826 — tracked via `aria-activedescendant` on the combobox, not a CSS
// class: `useAutocomplete.js`'s `syncHighlightedIndexToDOM` sets that
// attribute unconditionally from the highlighted INDEX, whereas its
// `Mui-focused` class is applied by querying the listbox DOM for a
// `[data-option-index]` match at sync time — which this suite found is
// NOT reliably present yet immediately after an extension's rows commit,
// so polling for the class flaked where the attribute does not.
const sentinelId = await sentinel.getAttribute("id");
for (let i = 0; i < 260; i++) {
if ((await combobox.getAttribute("aria-activedescendant")) === sentinelId) break;
await combobox.press("ArrowDown");
}
await expect(sentinel).toHaveClass(/active/);
await expect(combobox).toHaveAttribute("aria-activedescendant", sentinelId!);

// Pins `NamedEntityPicker.tsx`'s `disableListWrap`: ArrowDown at the
// true final option (hasMore exhausted) must not wrap to the first row.
// The sentinel is not necessarily that row (other specs may add flocks
// that sort after it), so keep arrowing until the highlight stops
// advancing before checking the next press leaves it there.
let activeId = sentinelId;
for (let i = 0; i < 260; i++) {
await combobox.press("ArrowDown");
const next = await combobox.getAttribute("aria-activedescendant");
if (next === activeId) break;
activeId = next;
}
const trueEndId = activeId;
await combobox.press("ArrowDown");
await expect(
combobox,
"ArrowDown at the picker's true final option (hasMore exhausted) must not wrap back to the first row",
).toHaveAttribute("aria-activedescendant", trueEndId!);

// Navigate back up to the named sentinel to commit it — this test's own
// promise (reaches AND commits the sentinel through paging), unaffected
// by the wrap check above.
for (let i = 0; i < 260 && (await combobox.getAttribute("aria-activedescendant")) !== sentinelId; i++) {
await combobox.press("ArrowUp");
}
await expect(combobox).toHaveAttribute("aria-activedescendant", sentinelId!);
await combobox.press("Enter");
await expect(combobox).toHaveValue(FLOCK_SENTINEL);
});

// A real-browser construction for the `clonedSelectedValue` memoisation
// (commit a flock, page to the true end, hold the extension's request via
// `page.route`, assert `aria-activedescendant` mid-flight and after)
// passed both with the fix and against a rebuilt image with it reverted —
// vacuous here too, so it is not shipped. Full account in
// `NamedEntityPicker.test.tsx`, above the picker's T023-7 block.

test("a new Sales order's customer picker reaches and commits the page-two sentinel through search", async ({
page,
signIn,
Expand All @@ -119,6 +162,20 @@ test.describe("Searchable named-entity picker (#512)", () => {
const combobox = dialog.getByRole("combobox", { name: tEn("sales:customer") });
await expect(combobox).toBeVisible();

// #826 (D2 pair 1) — the Load more button renders through a custom
// `slots.paper` component precisely so it lands OUTSIDE `<ul
// role="listbox">`, as a sibling: ARIA only allows `option`/`group`
// inside a listbox, and `slotProps.listbox` (rather than `slots.paper`)
// would have put it inside. Assert every DIRECT child of the listbox is
// itself role="option" — the AX guarantee that placement depends on.
const listbox = dialog.getByRole("listbox", { name: tEn("sales:customer") });
await expect(listbox).toBeVisible();
const listboxChildren = await listbox.locator(":scope > *").all();
expect(listboxChildren.length).toBeGreaterThan(0);
for (const child of listboxChildren) {
expect(await child.getAttribute("role")).toBe("option");
}

// Absent before: the picker opens on the unfiltered first page (up to 50
// of 101 customers), and the sentinel — lexically last — is not on it.
const sentinel = page.getByRole("option", { name: CUSTOMER_SENTINEL });
Expand All @@ -130,10 +187,11 @@ test.describe("Searchable named-entity picker (#512)", () => {
await expect(sentinel, "search for \"Page Two\" never surfaced the customer page-two sentinel").toHaveCount(1);

await sentinel.click();
await expect(dialog.getByRole("button", {
name: `${tEn("sales:customer")} ${CUSTOMER_SENTINEL}`,
exact: true,
})).toBeVisible();
// #826 — the committed trigger's accessible name is the picker's label
// ALONE now; the committed value moved to the field's `value`.
const committedTrigger = dialog.getByRole("textbox", { name: tEn("sales:customer"), exact: true });
await expect(committedTrigger).toBeVisible();
await expect(committedTrigger).toHaveValue(CUSTOMER_SENTINEL);
});

test("recovers from a failed customer search with Retry, then reaches the sentinel", async ({
Expand Down Expand Up @@ -190,9 +248,10 @@ test.describe("Searchable named-entity picker (#512)", () => {
await expect(sentinel).toHaveCount(1);

await sentinel.click();
await expect(dialog.getByRole("button", {
name: `${tEn("sales:customer")} ${CUSTOMER_SENTINEL}`,
exact: true,
})).toBeVisible();
// #826 — the committed trigger's accessible name is the picker's label
// ALONE now; the committed value moved to the field's `value`.
const recoveredTrigger = dialog.getByRole("textbox", { name: tEn("sales:customer"), exact: true });
await expect(recoveredTrigger).toBeVisible();
await expect(recoveredTrigger).toHaveValue(CUSTOMER_SENTINEL);
});
});
Loading
Loading