Migrate the remaining inline confirmations to ConfirmationModal #213

Merged

rob merged 1 commit from migrate-inline-confirmations into main

2026-08-21 15:30:08 +00:00

rob commented

2026-08-21 15:19:10 +00:00

Owner

Closes task 221. One visual language for "are you sure?" rather than two. Presentation consolidation, not a redesign — every site keeps its own wording, destructive styling, disabled-while-in-flight states, focus behaviour and Escape semantics.

Migrated: PlaceFormPanel delete, GroupFormPanel delete (its cascading place-count wording preserved — that text is load-bearing, not decoration), and PendingInvitationRow's decline.

ConfirmationModal grew Busy, disabling Confirm and Cancel while a mutation is in flight. ADR-0136's own Alternatives anticipated and deferred exactly this, so no new ADR was needed — the shared component grew rather than a second variant being forked.

The decline site was structurally the hardest, not a markup swap. Nesting ConfirmationModal inside a row that itself goes inert around it would have made the confirmation inert too — the precise bug ADR-0136's SuppressedByStackedOverlay saga already found once in a real browser while every bUnit assertion passed. The confirmation and its state moved up to Account, which is also the first ConfirmationModal stacked over ordinary page content rather than another ModalOverlay-hosted panel, so Account.razor's .page-content now carries its own inert binding.

Excluded, each with the reason recorded in code:

PlaceForm's move confirmation — stays a real <form> submit. A button in a separate stacked modal sits outside the form, reopening the second-entry-point problem ADR-0066 forbids. This is the honest exception the ticket allowed.
GroupMemberRow (remove/promote/leave) — ADR-0065 deliberately chose inline for this list-scoped case, and ADR-0136 never names it among the sites task 221 converts. Converting it would silently reopen a decision nobody revisited.
Account deletion — no such UI exists in the WebUI at all, only the API client method. The ticket assumed a confirm site that has never been built; nothing to migrate.
GroupMembersPanel — surveyed, has no confirm-shaped markup of its own; all of it delegates to GroupMemberRow.

Watched to fail, per site, as the ticket required — each mutation wired the destructive action to fire without confirmation:

PlaceFormPanel — PlaceFormPanel_DeleteButtonClicked_ShowsAConfirmationNamingThePermanenceRatherThanDeletingImmediately reddened on an unexpected DELETE.
GroupFormPanel — GroupFormPanel_DeleteButtonClicked_NamesTheCascadingPlaceCountInTheConfirmation reddened the same way.
Account — Account_DeclineCancelled_LeavesTheInvitationListedAndCallsNothing reddened, the confirmation vanishing before Cancel could be clicked because the decline had already happened.

E2E: added DeclineInvitationJourneyTests, the only genuinely new browser-only plumbing here, since Account is the first non-panel ConfirmationModal host. PlaceFormPanel's stacked-confirmation mechanism is already covered by EditAndDeletePlaceJourneyTests and DiscardConfirmationJourneyTests and is centralised in ModalOverlay, unchanged. GroupFormPanel's delete has no E2E coverage — a pre-existing gap, not deepened here, recorded rather than left silent. Run confirmed genuinely executing with PLACEMARK_RUN_E2E_TESTS=true, and caught a real locator bug in the new test on its first run.

Suite totals re-derived, not carried over: Domain 39, Contracts 139, Architecture 148, WebUI 923 passed / 4 skipped, Infrastructure 360, Api 622, E2E 61/61. Coverage ratchet held from a Release build with the merged Cobertura report — WebUI 91.7% against a 91.6% baseline.

Closes task 221. One visual language for "are you sure?" rather than two. **Presentation consolidation, not a redesign** — every site keeps its own wording, destructive styling, disabled-while-in-flight states, focus behaviour and Escape semantics. **Migrated:** `PlaceFormPanel` delete, `GroupFormPanel` delete (its cascading place-count wording preserved — that text is load-bearing, not decoration), and `PendingInvitationRow`'s decline. **`ConfirmationModal` grew `Busy`**, disabling Confirm and Cancel while a mutation is in flight. ADR-0136's own Alternatives anticipated and deferred exactly this, so no new ADR was needed — the shared component grew rather than a second variant being forked. **The decline site was structurally the hardest, not a markup swap.** Nesting `ConfirmationModal` inside a row that itself goes `inert` around it would have made the confirmation inert too — the precise bug ADR-0136's `SuppressedByStackedOverlay` saga already found once in a real browser while every bUnit assertion passed. The confirmation and its state moved up to `Account`, which is also the **first `ConfirmationModal` stacked over ordinary page content** rather than another `ModalOverlay`-hosted panel, so `Account.razor`'s `.page-content` now carries its own `inert` binding. **Excluded, each with the reason recorded in code:** - **`PlaceForm`'s move confirmation** — stays a real `<form>` submit. A button in a separate stacked modal sits outside the form, reopening the second-entry-point problem ADR-0066 forbids. This is the honest exception the ticket allowed. - **`GroupMemberRow`** (remove/promote/leave) — ADR-0065 deliberately chose inline for this list-scoped case, and ADR-0136 never names it among the sites task 221 converts. Converting it would silently reopen a decision nobody revisited. - **`Account` deletion** — **no such UI exists in the WebUI at all**, only the API client method. The ticket assumed a confirm site that has never been built; nothing to migrate. - **`GroupMembersPanel`** — surveyed, has no confirm-shaped markup of its own; all of it delegates to `GroupMemberRow`. **Watched to fail, per site**, as the ticket required — each mutation wired the destructive action to fire without confirmation: - `PlaceFormPanel` — `PlaceFormPanel_DeleteButtonClicked_ShowsAConfirmationNamingThePermanenceRatherThanDeletingImmediately` reddened on an unexpected DELETE. - `GroupFormPanel` — `GroupFormPanel_DeleteButtonClicked_NamesTheCascadingPlaceCountInTheConfirmation` reddened the same way. - `Account` — `Account_DeclineCancelled_LeavesTheInvitationListedAndCallsNothing` reddened, the confirmation vanishing before Cancel could be clicked because the decline had already happened. **E2E:** added `DeclineInvitationJourneyTests`, the only genuinely new browser-only plumbing here, since `Account` is the first non-panel `ConfirmationModal` host. `PlaceFormPanel`'s stacked-confirmation mechanism is already covered by `EditAndDeletePlaceJourneyTests` and `DiscardConfirmationJourneyTests` and is centralised in `ModalOverlay`, unchanged. **`GroupFormPanel`'s delete has no E2E coverage — a pre-existing gap, not deepened here, recorded rather than left silent.** Run confirmed genuinely executing with `PLACEMARK_RUN_E2E_TESTS=true`, and caught a real locator bug in the new test on its first run. **Suite totals re-derived, not carried over:** Domain 39, Contracts 139, Architecture 148, WebUI 923 passed / 4 skipped, Infrastructure 360, Api 622, E2E 61/61. Coverage ratchet held from a Release build with the merged Cobertura report — WebUI **91.7%** against a 91.6% baseline.

rob added 1 commit

2026-08-21 15:19:10 +00:00

Migrate the remaining inline confirmations to ConfirmationModal

CI / build (pull_request) Successful in 5m38s

Details

CI / container-images (pull_request) Successful in 5s

Details

CI / e2e (pull_request) Successful in 4m48s

Details

d755c66dcb

rob reviewed

2026-08-21 15:26:22 +00:00

rob left a comment

Verdict: mergeable

Checked against task 221 and ADR-0136:

Behaviour preserved. GroupFormPanel.DeleteConfirmPrompt's place-count wording is untouched (read the getter directly — cascading-count logic identical). Test assertion changes are selector-only or expand to the synchronous modal-overlay-panel-closing check pattern PlaceFormPanel's discard tests already established for task 220 — none change what behaviour is under test.
Account restructure is sound. ConfirmationModal renders as .page-content's sibling, before the div that goes inert, not nested inside it — the exact class of bug ADR-0136's SuppressedByStackedOverlay saga found is avoided here by construction. DeclineInvitationJourneyTests is real browser coverage of exactly this new stacking shape, which is the right place to have spent the E2E budget on this PR.
ConfirmationModal.Busy: read ADR-0136's Alternatives directly — it was explicitly proposed and rejected as premature, deferred by name to task 221 ("task 221 extends the component when a real caller needs it"). No ADR-0137 needed; the PR's claim holds.
The four exclusions all check out as honest:
- PlaceForm move confirmation — ADR-0066's single-entry-point constraint is real.
- GroupMemberRow — ADR-0065's "Inline confirmation, not a modal" section genuinely justifies the list-scoped inline pattern on its own merits; nothing here reopens it.
- Account deletion — verified independently: DeleteAccount/DeleteMeAsync appears only in PlaceMarkApiClient.Users.cs; no .razor caller exists.
- GroupMembersPanel — read the file; it has no confirm markup of its own, everything delegates to GroupMemberRow via events.
Per-site mutations: could not re-derive one myself — the sandbox blocked file edits in the read-only worktree (as flagged as a possibility), so I'm reporting that rather than implying I ran it. The wiring in Account.razor/GroupFormPanel/PlaceFormPanel is consistent with the claimed shape.

Non-blocking observations:

GroupFormPanel's delete has no E2E coverage, same as before this PR. Lower risk than Account's new stacking shape (it reuses PlaceFormPanel's already-covered mechanism verbatim), but given inert/stacking defects here have twice only been found in a real browser, a follow-up ticket for it is reasonable rather than required.
Coverage ratchet (91.6% → 91.7%) held; raising the baseline in this PR vs. a separate one is a toss-up, not acted on either way.

CI (run #892, SHA d755c66 — matches head) was still running at review time; confirm it finished green (apart from the known RememberViewportJourneyTests flake on main, task 277) before merging.

Verdict: mergeable Checked against task 221 and ADR-0136: - **Behaviour preserved.** `GroupFormPanel.DeleteConfirmPrompt`'s place-count wording is untouched (read the getter directly — cascading-count logic identical). Test assertion changes are selector-only or expand to the synchronous `modal-overlay-panel-closing` check pattern `PlaceFormPanel`'s discard tests already established for task 220 — none change what behaviour is under test. - **Account restructure is sound.** `ConfirmationModal` renders as `.page-content`'s sibling, before the div that goes `inert`, not nested inside it — the exact class of bug ADR-0136's `SuppressedByStackedOverlay` saga found is avoided here by construction. `DeclineInvitationJourneyTests` is real browser coverage of exactly this new stacking shape, which is the right place to have spent the E2E budget on this PR. - **`ConfirmationModal.Busy`**: read ADR-0136's Alternatives directly — it was explicitly proposed and rejected as premature, deferred by name to task 221 ("task 221 extends the component when a real caller needs it"). No ADR-0137 needed; the PR's claim holds. - **The four exclusions all check out as honest:** - `PlaceForm` move confirmation — ADR-0066's single-entry-point constraint is real. - `GroupMemberRow` — ADR-0065's "Inline confirmation, not a modal" section genuinely justifies the list-scoped inline pattern on its own merits; nothing here reopens it. - Account deletion — verified independently: `DeleteAccount`/`DeleteMeAsync` appears only in `PlaceMarkApiClient.Users.cs`; no `.razor` caller exists. - `GroupMembersPanel` — read the file; it has no confirm markup of its own, everything delegates to `GroupMemberRow` via events. - **Per-site mutations**: could not re-derive one myself — the sandbox blocked file edits in the read-only worktree (as flagged as a possibility), so I'm reporting that rather than implying I ran it. The wiring in `Account.razor`/`GroupFormPanel`/`PlaceFormPanel` is consistent with the claimed shape. Non-blocking observations: - `GroupFormPanel`'s delete has no E2E coverage, same as before this PR. Lower risk than `Account`'s new stacking shape (it reuses `PlaceFormPanel`'s already-covered mechanism verbatim), but given `inert`/stacking defects here have twice only been found in a real browser, a follow-up ticket for it is reasonable rather than required. - Coverage ratchet (91.6% → 91.7%) held; raising the baseline in this PR vs. a separate one is a toss-up, not acted on either way. CI (run #892, SHA `d755c66` — matches head) was still `running` at review time; confirm it finished green (apart from the known `RememberViewportJourneyTests` flake on `main`, task 277) before merging.

rob merged commit 426a7e9175 into main

2026-08-21 15:30:08 +00:00

rob deleted branch migrate-inline-confirmations

2026-08-21 15:30:08 +00:00

rob referenced this pull request from a commit

2026-08-21 15:30:10 +00:00

Migrate the remaining inline confirmations to ConfirmationModal (#213)