fix(721,740): align Auto-Tune proposal rows on a grid; guard AddItemsDialog's async searches #831
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#831
Reference in New Issue
Block a user
fixes #721
fixes #740
Two
frontendfixes swept in one session — disjoint files, one shared component change.#721 — Auto-Tune proposal rows
The channel number started wherever the preceding text ended: the row was a flex line whose first child was
<Checkbox label={displayName}>, sominWidth: 42reserved width for the number without being able to align it. The name then rendered a second time in the detail column, and the group header above the rows had the same duplicate.The row is a grid now. What makes the column line up is that everything left of the number is fixed-width — a label-less
Checkboxrenders one fixed-size control — plus a fixed number track, because each row is its own grid container and anauto/max-contenttrack resolves per row.No fixed track bounds the value, and the code says so rather than pretending otherwise:
AutoTuneNumberAllocator.Allocateemitsint.ToString()from aStartingNumberthatPreviewAutoTuneChannelsHandleronly floors at 1, and a per-proposal override is free text checked againstChannel.NumberValidator(^[0-9]+(\.[0-9]{1,2})?$). So the cell clips, with the full value on hover. That is a real trade — the flex box it replaces grew instead of truncating — so thetitlefallback is part of the fix and is pinned by a test.CheckboxgainshideLabel, mirroring the propSwitchin the same file already has.labelstill supplies the accessible name, so every existinggetByRole('checkbox', { name })assertion is untouched.#740 — AddItemsDialog async guards
Neither guard §3b mandates was present, and their triggers differ:
runSearchis reachable twice concurrently — a form submit fanning out overDEFAULT_SEARCH_KINDS, a kind-chip click issuing one request. A superseded response settling later repainted the list with every kind under an active "Movies" chip, wiped a newer successful result with a superseded search's error, or clearedsearchingwhile the other request was still in flight.submitis unmount-reachable mid-request: its Add button is disabled while adding, butDialogcloses on Escape, backdrop click and the header close button, none of which consultadding.onAdded/onCloseare deliberately not guarded — they belong to the parent, which owns its own guarding, and dropping the list reload after a write that succeeded would leave the collection stale.Verification
Every clause ships with a test that reddens when that clause alone is removed — 14/14, each mutation applied to pristine source, asserted to match exactly once, read back, then re-run.
Two of those were green until the tests were strengthened, and both are the kind of green that means nothing:
submitguards masked each other — removing either alone left the whole suite green, because the survivor still read the ref. That test counts the reads now.queryByText('Unable to search library'), whichmessageFromCollectionErroronly produces for a non-Errorthrow — null-vs-null, passing with the guard removed. It matchesrole="alert"instead, verified to fail on its own with the test's other assertions removed.Web suite 115 files / 1206 tests green;
npm run typecheckandnpm run lintclean; repo guard suitescripts/tests956 passed / 2 skipped.Live-E2E not required —
docs/e2e-local.mdscopes it to API write-path handler changes; this is pure-SPA and read-only. No.csfiles touched.Review
Independent cold adversarial review (worktree-isolated, review-only brief), three rounds: BLOCKED → BLOCKED (narrowly) → MERGEABLE. Round 1 caught a false mutation-proof claim in the commit message plus a genuinely unpinned catch-path guard whose removal reintroduced a worse bug than #740 itself; round 2 caught the vacuous error assertion and two unpinned style clauses. Both rounds' findings were taken in full, none argued away.
Docs
docs/spa-conventions.md§3b — theAddItemsDialogcaveat naming #740 as outstanding is replaced by the shipped rule, with the two guards' different triggers stated separately (concurrent reachability vs unmount reachability) and the own-state-only rule spelled out.Deferred
#830 — a failed add is silently swallowed when the dialog is closed mid-request. Pre-existing; the guard here makes the drop deliberate rather than accidental, but fixing the UX needs a parent-owned surface or a change to the shared
Dialog, which is its own blast radius.Design system
The prototypes already had a label-less proposal checkbox — the SPA had drifted from them — so the mirrored delta is the number's right-align. The prototypes keep their flex row; converting them to the grid is a layout change to a mockup, not part of this fix.
🤖 Generated with Claude Code
Review-verdict: MERGEABLE @
4d33462Independent cold adversarial review, 3 rounds (BLOCKED, BLOCKED-narrowly, MERGEABLE). Round 1 found a false mutation-proof claim and an unpinned catch-path guard whose removal reintroduced a worse defect than #740; round 2 found a vacuous error assertion and 2 unpinned style clauses. All taken in full. Final: 14/14 clauses redden individually, 0 unpinned, reproduced independently by the reviewer. Verdict posted by the AUTHOR on the reviewer's behalf.