From 52948cd3f0c30f894f13ac475fdb18f627934316 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Mon, 7 Sep 2026 16:16:44 +0100 Subject: [PATCH] fix(web): stabilize archive confirmation surface (#3460) Co-authored-by: Kandev Agent --- .../task/task-archive-confirmation.test.tsx | 132 ++++++++++++++-- .../task/task-archive-confirmation.tsx | 65 ++++---- .../kanban/cascade-subtasks-toggle.spec.ts | 84 ++++++++++ .../plan.md | 144 ++++++++++++++++++ ...tabilize-archive-confirmation-selection.md | 128 ++++++++++++++++ .../confirmation-warning-hierarchy.md | 20 ++- .../confirmation-warning-hierarchy.md | 74 +++++++-- 7 files changed, 578 insertions(+), 69 deletions(-) create mode 100644 docs/plans/archive-confirmation-stable-surface/plan.md create mode 100644 docs/plans/archive-confirmation-stable-surface/task-01-stabilize-archive-confirmation-selection.md diff --git a/apps/web/components/task/task-archive-confirmation.test.tsx b/apps/web/components/task/task-archive-confirmation.test.tsx index b0756e914a..ed5fc30aad 100644 --- a/apps/web/components/task/task-archive-confirmation.test.tsx +++ b/apps/web/components/task/task-archive-confirmation.test.tsx @@ -1,6 +1,6 @@ -import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { useRef } from "react"; +import { useCallback, useRef, useState } from "react"; import { StateProvider } from "@/components/state-provider"; const getSubtaskCountMock = vi.hoisted(() => vi.fn()); @@ -21,6 +21,10 @@ vi.mock("@/hooks/use-responsive-breakpoint", () => ({ import { TaskArchiveConfirmation } from "./task-archive-confirmation"; afterEach(cleanup); +beforeEach(() => { + pointerState.isFinePointer = false; + getSubtaskCountMock.mockReset(); +}); function ConfirmationHarness({ onConfirm, @@ -37,6 +41,9 @@ function ConfirmationHarness({ + void; +}) { + const [open, setOpen] = useState(true); + const anchorRef = useRef(null); + const handleOpenChange = useCallback( + (nextOpen: boolean) => { + setOpen(nextOpen); + onOpenChange(nextOpen); + }, + [onOpenChange], + ); + + return ( + <> + {showAnchor ? ( + + ) : null} + + + ); +} + function renderConfirmation(onConfirm = vi.fn(), onOpenChange = vi.fn(), forceDialog = false) { return render( @@ -64,12 +108,66 @@ function renderConfirmation(onConfirm = vi.fn(), onOpenChange = vi.fn(), forceDi ); } -describe("TaskArchiveConfirmation classification", () => { - beforeEach(() => { - pointerState.isFinePointer = false; - getSubtaskCountMock.mockReset(); +function deferredSubtaskCount() { + let resolve: (result: { count: number }) => void = () => {}; + const promise = new Promise<{ count: number }>((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + +describe("TaskArchiveConfirmation pending dismissal", () => { + it("dismisses the hidden desktop request on Escape and restores trigger focus", async () => { + pointerState.isFinePointer = true; + getSubtaskCountMock.mockReturnValue(new Promise(() => undefined)); + const onOpenChange = vi.fn(); + + renderConfirmation(vi.fn(), onOpenChange); + await waitFor(() => expect(getSubtaskCountMock).toHaveBeenCalledWith("task-1")); + + const outsideAction = screen.getByTestId("outside-action"); + outsideAction.focus(); + fireEvent.keyDown(outsideAction, { key: "Escape" }); + + expect(onOpenChange).toHaveBeenCalledWith(false); + expect(document.activeElement).toBe(screen.getByTestId("archive-anchor")); }); + it("dismisses the hidden desktop request on outside pointer intent", async () => { + pointerState.isFinePointer = true; + getSubtaskCountMock.mockReturnValue(new Promise(() => undefined)); + const onOpenChange = vi.fn(); + + renderConfirmation(vi.fn(), onOpenChange); + await waitFor(() => expect(getSubtaskCountMock).toHaveBeenCalledWith("task-1")); + + fireEvent.pointerDown(screen.getByTestId("outside-action")); + + expect(onOpenChange).toHaveBeenCalledWith(false); + }); + + it("dismisses the hidden desktop request when its anchor disappears", async () => { + pointerState.isFinePointer = true; + const deferredCount = deferredSubtaskCount(); + getSubtaskCountMock.mockReturnValue(deferredCount.promise); + const onOpenChange = vi.fn(); + const renderHarness = (showAnchor: boolean) => ( + + + + ); + const view = render(renderHarness(true)); + await waitFor(() => expect(getSubtaskCountMock).toHaveBeenCalledWith("task-1")); + + view.rerender(renderHarness(false)); + await act(async () => deferredCount.resolve({ count: 2 })); + + expect(screen.queryByRole("alertdialog")).toBeNull(); + expect(onOpenChange).toHaveBeenCalledWith(false); + }); +}); + +describe("TaskArchiveConfirmation classification", () => { it("does not expose an archive action while descendant classification is pending", () => { getSubtaskCountMock.mockReturnValue(new Promise(() => undefined)); @@ -79,20 +177,24 @@ describe("TaskArchiveConfirmation classification", () => { expect(screen.queryByRole("alertdialog")).toBeNull(); }); - it("shows a disabled anchored loading surface while desktop classification is pending", () => { + // @covers AC-TASKS-CONFIRMATION-SURFACE-002.4 + it("waits for desktop classification before showing only the cascade dialog", async () => { pointerState.isFinePointer = true; - getSubtaskCountMock.mockReturnValue(new Promise(() => undefined)); - const onOpenChange = vi.fn(); + const deferredCount = deferredSubtaskCount(); + getSubtaskCountMock.mockReturnValue(deferredCount.promise); - renderConfirmation(vi.fn(), onOpenChange); + renderConfirmation(); + await waitFor(() => expect(getSubtaskCountMock).toHaveBeenCalledWith("task-1")); - expect(screen.getByTestId("task-archive-confirm-popover")).toBeTruthy(); - expect(screen.getByText("Loading…")).toBeTruthy(); - expect(screen.getByTestId(CONFIRM_TEST_ID).hasAttribute("disabled")).toBe(true); + expect(screen.queryByTestId("task-archive-confirm-popover")).toBeNull(); expect(screen.queryByRole("alertdialog")).toBeNull(); + expect(screen.queryByTestId(CONFIRM_TEST_ID)).toBeNull(); - fireEvent.click(screen.getByRole("button", { name: "Cancel" })); - expect(onOpenChange).toHaveBeenCalledWith(false); + await act(async () => deferredCount.resolve({ count: 2 })); + + expect(await screen.findByRole("alertdialog")).toBeTruthy(); + expect(screen.getByTestId("archive-cascade-checkbox")).toBeTruthy(); + expect(screen.queryByTestId("task-archive-confirm-popover")).toBeNull(); }); it("uses touch-sized local actions after a resolved zero-descendant result", async () => { diff --git a/apps/web/components/task/task-archive-confirmation.tsx b/apps/web/components/task/task-archive-confirmation.tsx index f38ef30eb5..c24fcc1b9b 100644 --- a/apps/web/components/task/task-archive-confirmation.tsx +++ b/apps/web/components/task/task-archive-confirmation.tsx @@ -1,8 +1,7 @@ "use client"; -import type { RefObject } from "react"; +import { useEffect, useLayoutEffect, type RefObject } from "react"; import { useTranslation } from "react-i18next"; -import { IconLoader } from "@tabler/icons-react"; import { useAppStore } from "@/components/state-provider"; import { ActionConfirmPopover } from "@/components/confirmation/action-confirm-popover"; import { InlineConfirmActions } from "@/components/confirmation/inline-confirm-actions"; @@ -157,45 +156,41 @@ function ArchiveConfirmPopover({ ); } -function ArchiveClassifyingPopover({ +function PendingArchiveDismissal({ anchorRef, focusReturnRef, - focusBoundaryRef, onOpenChange, - confirmTestId, }: { anchorRef: RefObject; focusReturnRef?: RefObject; - focusBoundaryRef?: RefObject; onOpenChange: (open: boolean) => void; - confirmTestId: string; }) { - const { t } = useTranslation(); - return ( - - - {t("common:loading")} - - } - cancelLabel={t("common:cancel")} - confirmLabel={t(ARCHIVE_LABEL_KEY)} - confirmAriaLabel={t(ARCHIVE_LABEL_KEY)} - confirmTestId={confirmTestId} - confirmDisabled - testId="task-archive-confirm-popover" - onOpenChange={onOpenChange} - onCancel={() => onOpenChange(false)} - onConfirm={() => undefined} - /> - ); + useLayoutEffect(() => { + if (anchorRef.current?.isConnected) return; + onOpenChange(false); + }); + + useEffect(() => { + const dismiss = () => onOpenChange(false); + const handlePointerDown = () => dismiss(); + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key !== "Escape") return; + event.preventDefault(); + event.stopPropagation(); + const focusTarget = focusReturnRef?.current ?? anchorRef.current; + dismiss(); + if (focusTarget?.isConnected) focusTarget.focus(); + }; + + document.addEventListener("pointerdown", handlePointerDown, true); + document.addEventListener("keydown", handleKeyDown, true); + return () => { + document.removeEventListener("pointerdown", handlePointerDown, true); + document.removeEventListener("keydown", handleKeyDown, true); + }; + }, [anchorRef, focusReturnRef, onOpenChange]); + + return null; } type ArchiveDialogProps = Pick< @@ -272,12 +267,10 @@ function ArchiveConfirmationContent({ } if (!isFinePointer) return null; return ( - ); } diff --git a/apps/web/e2e/tests/kanban/cascade-subtasks-toggle.spec.ts b/apps/web/e2e/tests/kanban/cascade-subtasks-toggle.spec.ts index 59e5f58b85..ea40cee67f 100644 --- a/apps/web/e2e/tests/kanban/cascade-subtasks-toggle.spec.ts +++ b/apps/web/e2e/tests/kanban/cascade-subtasks-toggle.spec.ts @@ -1,3 +1,4 @@ +import type { Route } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; import { KanbanPage } from "../../pages/kanban-page"; @@ -8,6 +9,89 @@ const VISIBLE_TIMEOUT = 10_000; // has live subtasks, default to unchecked (subtasks preserved), and // propagate the choice to the backend. test.describe("Kanban card archive — cascade subtasks toggle", () => { + // @covers AC-TASKS-CONFIRMATION-SURFACE-002.4 + test("keeps pending classification hidden and dismissible before the cascade dialog", async ({ + testPage, + apiClient, + seedData, + prCapture, + }) => { + const parent = await apiClient.createTask(seedData.workspaceId, "Stable Archive Parent", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + await apiClient.createTask(seedData.workspaceId, "Stable Archive Child", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + parent_id: parent.id, + }); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await expect(kanban.taskCard(parent.id)).toBeVisible({ timeout: VISIBLE_TIMEOUT }); + + let releaseResponse: () => void = () => {}; + const responseHeld = new Promise((resolve) => { + releaseResponse = resolve; + }); + let markRequestObserved: () => void = () => {}; + const requestObserved = new Promise((resolve) => { + markRequestObserved = resolve; + }); + let requestStarted = false; + let markHandlerSettled: () => void = () => {}; + const handlerSettled = new Promise((resolve) => { + markHandlerSettled = resolve; + }); + const routePattern = `**/api/v1/tasks/${parent.id}/subtask-count`; + const holdSubtaskCount = async (route: Route) => { + requestStarted = true; + markRequestObserved(); + await responseHeld; + try { + await route.continue(); + } finally { + markHandlerSettled(); + } + }; + + await testPage.route(routePattern, holdSubtaskCount); + try { + await kanban.openTaskActionsMenu(parent.id); + await testPage.getByRole("menuitem", { name: "Archive" }).click(); + await requestObserved; + + const popover = testPage.getByTestId("task-archive-confirm-popover"); + const dialog = testPage.getByRole("alertdialog", { name: "Archive task" }); + await expect(popover).toHaveCount(0); + await expect(dialog).toHaveCount(0); + + await testPage.keyboard.press("Escape"); + await expect(kanban.taskCard(parent.id).getByLabel("More options")).toBeFocused(); + + releaseResponse(); + await handlerSettled; + await testPage.unroute(routePattern, holdSubtaskCount); + + await expect(dialog).toHaveCount(0); + await expect(popover).toHaveCount(0); + + await kanban.openTaskActionsMenu(parent.id); + await testPage.getByRole("menuitem", { name: "Archive" }).click(); + + await expect(dialog).toBeVisible(); + await expect(dialog.getByTestId("archive-cascade-checkbox")).toBeVisible(); + await expect(popover).toHaveCount(0); + await prCapture.screenshot("stable-archive-cascade-dialog", { + caption: "A parent task opens only the final cascade archive dialog.", + }); + } finally { + releaseResponse(); + if (requestStarted) await handlerSettled; + await testPage.unroute(routePattern, holdSubtaskCount); + } + }); + test("renders the cascade checkbox only when the task has subtasks", async ({ testPage, apiClient, diff --git a/docs/plans/archive-confirmation-stable-surface/plan.md b/docs/plans/archive-confirmation-stable-surface/plan.md new file mode 100644 index 0000000000..071b945f90 --- /dev/null +++ b/docs/plans/archive-confirmation-stable-surface/plan.md @@ -0,0 +1,144 @@ +--- +created: 2026-09-06 +status: done +requirements: + - REQ-TASKS-CONFIRMATION-SURFACE-002 + - REQ-UI-TASK-CLEANUP-CONFIRMATION-001 +system_design: + - ../../specs/ui/system-design/confirmation-warning-hierarchy.md +legacy_specs: [] +--- + +# Implementation Plan: Stable Archive Confirmation Surface + +## Overview + +Prevent a parent-task archive request from showing an anchored loading popover +and then replacing it with the cascade dialog. Gate the standard fine-pointer +surface until descendant classification settles, then mount only the existing +final popover or dialog. One sequential work order keeps the rendering change, +regression tests, and desktop/mobile compatibility evidence together. + +## Scope + +### In scope + +- Keep the confirmation request open while descendant classification is + pending without mounting a provisional fine-pointer confirmation, while + retaining Escape and outside-pointer dismissal. +- Preserve the anchored popover for a resolved zero-descendant task and the + full cascade dialog for positive or unavailable descendant counts. +- Prove the async transition at component and desktop Kanban E2E boundaries. +- Re-run the existing phone Kanban dialog scenario as mobile-parity evidence. + +### Out of scope + +- Changing archive, cascade, descendant-count, settings, or navigation APIs. +- Prefetching or caching descendant counts before an archive request. +- Changing bulk, forced-dialog, explicit inline, coarse-pointer row, delete, or + confirmation-preference behavior. +- Changing confirmation copy, localization catalogs, geometry, or styling. + +## Technical approach + +### Fine-pointer classification gate + +In `apps/web/components/task/task-archive-confirmation.tsx`, remove the +fine-pointer `ArchiveClassifyingPopover` presentation. While the shared +`useSubtaskCountState` result is `idle` or `loading`, the standard fine-pointer +branch returns no confirmation shell. The controlled `open` state and current +request remain intact, so the existing resolved-zero popover, positive-count +dialog, and error dialog mount normally once classification settles. Removing +the provisional branch also removes its loader-only import and markup. + +A non-rendering pending controller retains dismissal behavior without +reintroducing a temporary confirmation. Escape closes the request and restores +focus to the configured trigger; a new pointer interaction closes it without +blocking that target. It also closes the request when live data removes the +originating anchor. Closing disables descendant classification, so a late +response cannot mount a confirmation after dismissal or anchor removal. + +The existing coarse-pointer pending branch already renders no popup and is the +nearest implementation precedent. `forceDialog`, bulk, inline, preference +bypass, mutation-pending, and final confirmation branches remain unchanged. + +### Component regression + +Update `apps/web/components/task/task-archive-confirmation.test.tsx` so a +deferred descendant-count promise proves the full state transition. Before the +promise resolves, neither `task-archive-confirm-popover` nor an alert dialog or +archive action is present. Resolving it with a positive count renders the full +cascade dialog and never exposes the popover. Existing resolved-zero, error, +forced-dialog, callback, and cleanup-content tests continue to cover the other +branches. Separate pending-dismissal cases cover Escape with trigger focus +restoration and outside pointer intent. + +### Browser regression and mobile parity + +Extend `apps/web/e2e/tests/kanban/cascade-subtasks-toggle.spec.ts` with a +fine-pointer parent-task scenario that holds the exact +`GET /api/v1/tasks/:id/subtask-count` response behind a deterministic promise +gate. After the request is observed, assert that no provisional confirmation is +visible; dismiss it with Escape, release the response, and prove no late surface +mounts. Reopen Archive, then assert that only the cascade alert dialog appears. +Release and unregister the route in `finally` so failures cannot leak fixture +state. + +Phone Kanban remains on its contained forced-dialog branch, with the full +dialog as the single scroll owner and its existing stacked 44px actions, +viewport insets, focus return, and no document overflow. No mobile markup or +interaction changes; the existing +`apps/web/e2e/tests/kanban/mobile-card-archive-confirmation.spec.ts` is the +mobile-parity check. + +## Tests + +| Acceptance criterion | Evidence | +| ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `AC-TASKS-CONFIRMATION-SURFACE-002.4` | Deferred-promise regressions in `task-archive-confirmation.test.tsx`: no pending shell, then only the positive-count dialog; Escape restores trigger focus and outside pointer intent dismisses. | +| `AC-TASKS-CONFIRMATION-SURFACE-002.1`, `.3`, `.4` | Desktop `cascade-subtasks-toggle.spec.ts`: delayed classification never exposes the popover and resolves to the cascade dialog; existing zero-count coverage retains the anchored popover. | +| `AC-UI-TASK-CLEANUP-CONFIRMATION-001.7` | Existing component cases retain archive callbacks, failure fallback, cascade choice, and mutation-pending behavior. | +| `AC-UI-TASK-CLEANUP-CONFIRMATION-001.8` | Existing `mobile-card-archive-confirmation.spec.ts` retains the contained phone dialog and archive outcome. | + +## E2E tests + +- Chromium: extend `apps/web/e2e/tests/kanban/cascade-subtasks-toggle.spec.ts` + with the delayed parent-task classification scenario. +- Mobile Chrome: re-run + `apps/web/e2e/tests/kanban/mobile-card-archive-confirmation.spec.ts`; no new + mobile scenario is needed because the changed branch is fine-pointer-only and + the existing spec exercises the intentionally different phone composition. + +## Work orders + +- [x] [Task 01: Stabilize archive confirmation selection](task-01-stabilize-archive-confirmation-selection.md) — done + +## Verification results + +- RED component regression: failed on the provisional + `task-archive-confirm-popover` before the production change. +- RED Chromium regression: failed with the same provisional popover while the + intercepted descendant-count request remained pending. +- Review-remediation RED component cases: 3 failed because the hidden pending + request did not retain Escape, outside-pointer dismissal, or the prior + connected-anchor lifecycle guard. +- Review-remediation RED Chromium regression: failed because a late positive + result mounted the cascade dialog after Escape dismissal. +- Focused component suite: 13 passed. +- Touched-file ESLint: passed with no findings. +- Web typecheck: passed. +- Mobile Chrome confirmation E2E: 2 passed. +- Chromium cascade archive/delete E2E: 6 passed, including the delayed + classification case and a fresh desktop PR screenshot. + +## Risks + +- A slow descendant-count request gives no confirmation shell until the final + presentation is known. The request remains active and normally targets the + local backend; adding another loading overlay would recreate the shell-swap + defect. +- Over-broadly changing unresolved classification could regress forced mobile, + bulk, or inline callers. Keep the correction inside the standard + fine-pointer branch and retain focused coverage for the existing routes. +- The E2E regression must wait until the intercepted request is observed before + asserting absence, or it could pass before React enters the pending state. diff --git a/docs/plans/archive-confirmation-stable-surface/task-01-stabilize-archive-confirmation-selection.md b/docs/plans/archive-confirmation-stable-surface/task-01-stabilize-archive-confirmation-selection.md new file mode 100644 index 0000000000..8ec5e4e390 --- /dev/null +++ b/docs/plans/archive-confirmation-stable-surface/task-01-stabilize-archive-confirmation-selection.md @@ -0,0 +1,128 @@ +--- +id: "01-stabilize-archive-confirmation-selection" +title: "Stabilize archive confirmation selection" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-CONFIRMATION-SURFACE-002 + - REQ-UI-TASK-CLEANUP-CONFIRMATION-001 +acceptance_criteria: + - AC-TASKS-CONFIRMATION-SURFACE-002.1 + - AC-TASKS-CONFIRMATION-SURFACE-002.3 + - AC-TASKS-CONFIRMATION-SURFACE-002.4 + - AC-UI-TASK-CLEANUP-CONFIRMATION-001.7 + - AC-UI-TASK-CLEANUP-CONFIRMATION-001.8 +system_design: + - ../../specs/ui/system-design/confirmation-warning-hierarchy.md +--- + +# Task 01: Stabilize Archive Confirmation Selection + +## Summary + +Make descendant classification choose the standard fine-pointer archive shell +before any confirmation UI mounts. Add component and desktop browser +regressions for the delayed positive-count path while preserving every existing +final confirmation route and the contained phone dialog. + +## In scope + +- Remove the provisional fine-pointer loading popover from + `TaskArchiveConfirmation` without changing its controlled request state. +- Preserve Escape and outside-pointer dismissal while the pending request has + no rendered confirmation shell. +- Preserve dismissal when live data removes the originating anchor while the + pending request has no rendered confirmation shell. +- Add a RED component regression that holds descendant classification pending, + then resolves it to a positive count and observes only the cascade dialog. +- Add a RED desktop Kanban E2E regression that deterministically holds the + subtask-count response and proves the provisional popover never appears. +- Run the existing zero-descendant desktop and contained phone confirmation + scenarios after the correction. + +## Out of scope + +- Backend, archive API, user-settings, cascade, navigation, or descendant-count + changes. +- New caching, prefetching, loading copy, animations, or confirmation surfaces. +- Changes to bulk, inline, coarse-pointer row, delete, or mobile presentation. + +## Acceptance + +- While a standard fine-pointer descendant classification is unresolved, no + archive confirmation shell or actionable archive control is rendered; a + positive result mounts only the existing full cascade dialog. +- Escape restores focus to the configured trigger, outside pointer intent + dismisses without being blocked, and a late result cannot mount a surface + after either dismissal. +- Removing the originating anchor dismisses the pending request, and a late + positive result cannot mount a stale dialog. +- Resolved zero counts still mount the anchored popover, classification errors + still fail safe to the full dialog, and callbacks/focus/preference behavior + remain unchanged. +- The focused component, desktop E2E, existing mobile E2E, typecheck, and + touched-file lint commands pass. + +## Verification + +```bash +cd apps && pnpm --filter @kandev/web test -- --run components/task/task-archive-confirmation.test.tsx +cd apps/web && pnpm exec eslint components/task/task-archive-confirmation.tsx components/task/task-archive-confirmation.test.tsx e2e/tests/kanban/cascade-subtasks-toggle.spec.ts +cd apps/web && pnpm run typecheck +cd apps/web && pnpm e2e:run --project mobile-chrome tests/kanban/mobile-card-archive-confirmation.spec.ts +cd apps/web && CAPTURE_PR_ASSETS=1 pnpm e2e:run --no-build tests/kanban/cascade-subtasks-toggle.spec.ts +``` + +The component and desktop E2E regressions must be run and observed failing for +the provisional popover before the production correction, then rerun green. + +## Files likely touched + +- `apps/web/components/task/task-archive-confirmation.tsx` +- `apps/web/components/task/task-archive-confirmation.test.tsx` +- `apps/web/e2e/tests/kanban/cascade-subtasks-toggle.spec.ts` + +## Dependencies + +None. + +## Risks + +- The pending branch is shared by several presentation modes; keep the change + after the existing force-dialog/bulk decision and inside the standard + fine-pointer path. +- A browser absence assertion can race ahead of the intercepted request; the + test must wait for the route handler to observe the request before asserting. + +## Parallelism + +`sequential` + +## Inputs + +- `AC-TASKS-CONFIRMATION-SURFACE-002.4` and the classification-gated surface + selection section in the UI confirmation system design. +- `apps/web/components/task/task-archive-confirmation.tsx` and its component + test suite. +- `apps/web/e2e/tests/kanban/cascade-subtasks-toggle.spec.ts` and the existing + held-route patterns in task E2E coverage. +- The coarse-pointer unresolved branch, which already avoids mounting a + temporary popup. + +## Results + +- Added deferred descendant-count component and Chromium regressions. Before + the implementation, both failed because the fine-pointer loading popover was + visible while classification remained pending. +- Removed the provisional fine-pointer loading popover and its loader import. + A non-rendering controller preserves Escape, trigger-focus, and outside + pointer dismissal while the request waits, then renders the existing + resolved-zero popover or safe dialog only if it remains open. +- Review-remediation RED cases exposed the missing hidden-state dismissal at + both component and Chromium boundaries before the controller was added. A + later RED component case exposed the missing connected-anchor lifecycle guard. +- Component suite: 13 passed. Touched-file ESLint and web typecheck passed. +- Existing Mobile Chrome confirmation spec: 2 passed. Chromium cascade spec: + 6 passed, with a fresh desktop capture of the final cascade dialog. diff --git a/docs/specs/ui/requirements/confirmation-warning-hierarchy.md b/docs/specs/ui/requirements/confirmation-warning-hierarchy.md index 73ab075105..8d0bfe02f8 100644 --- a/docs/specs/ui/requirements/confirmation-warning-hierarchy.md +++ b/docs/specs/ui/requirements/confirmation-warning-hierarchy.md @@ -2,7 +2,7 @@ status: active system: ui created: 2026-08-24 -updated: 2026-09-05 +updated: 2026-09-07 owners: - kandev --- @@ -76,6 +76,16 @@ pointer inline confirmation behavior. and compact desktop/tablet viewport containment. Coarse-pointer inline confirmation may continue to expand its row and shall retain its existing 44px action geometry and zero-overflow behavior. +- **AC-TASKS-CONFIRMATION-SURFACE-002.4:** When a fine-pointer archive action + needs descendant classification before choosing its confirmation surface, no + provisional confirmation popover or archive action shall appear while that + classification is pending. After classification, exactly one final surface + shall appear: the anchored popover for zero descendants, or the full dialog + for one or more descendants. An unavailable classification shall fail safe to + the full dialog without first showing the popover. While no surface is + visible, Escape or a new pointer interaction shall dismiss the pending + request; keyboard dismissal shall restore trigger focus, and a late + classification result shall not mount a confirmation after dismissal. ### REQ-UI-TASK-CLEANUP-CONFIRMATION-001: Scannable task cleanup confirmation @@ -113,9 +123,9 @@ repository resources, so that I can understand the consequence before acting. system's semantic destructive action treatment without a competing primary treatment. Archive shall retain its non-destructive action treatment. - **AC-UI-TASK-CLEANUP-CONFIRMATION-001.7:** Cancel, dismiss, focus, Escape, - Enter default-action behavior, loading state, cascade selection, in-flight - warning visibility, and single/bulk archive and delete callbacks shall remain - unchanged. + Enter default-action behavior, archive/delete mutation loading state, cascade + selection, in-flight warning visibility, and single/bulk archive and delete + callbacks shall remain unchanged. - **AC-UI-TASK-CLEANUP-CONFIRMATION-001.8:** When archive is initiated from a phone Kanban card, the card shall use the contained full task-cleanup alert dialog even when the task has no descendants. The dialog shall be centered @@ -154,6 +164,8 @@ repository resources, so that I can understand the consequence before acting. dimensions. - Replacing the centered task alerts with a drawer, sheet, or new navigation flow. +- Changing the descendant-count API or introducing background prefetching for + archive confirmation. - Masking sidebar row layout changes with fixed/minimum heights or negative offsets. - Adding animation or redesigning any confirmation surface beyond the shared diff --git a/docs/specs/ui/system-design/confirmation-warning-hierarchy.md b/docs/specs/ui/system-design/confirmation-warning-hierarchy.md index e39326ebd9..5a09eca7ec 100644 --- a/docs/specs/ui/system-design/confirmation-warning-hierarchy.md +++ b/docs/specs/ui/system-design/confirmation-warning-hierarchy.md @@ -1,11 +1,11 @@ --- status: current system: ui -updated: 2026-09-05 requirements: - REQ-TASKS-CONFIRMATION-WARNING-001 - REQ-TASKS-CONFIRMATION-SURFACE-002 - REQ-UI-TASK-CLEANUP-CONFIRMATION-001 +updated: 2026-09-07 --- # Task Confirmation Surface System Design @@ -23,7 +23,7 @@ the task runtime contract. | Requirement | Design section | | -------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `REQ-TASKS-CONFIRMATION-WARNING-001` | [Components and responsibilities](#components-and-responsibilities) and [Mobile and desktop containment](#mobile-and-desktop-containment) | -| `REQ-TASKS-CONFIRMATION-SURFACE-002` | [Popover width contract](#popover-width-contract), [Fine-pointer mounting](#fine-pointer-mounting), and [Mobile and desktop containment](#mobile-and-desktop-containment) | +| `REQ-TASKS-CONFIRMATION-SURFACE-002` | [Classification-gated surface selection](#classification-gated-surface-selection), [Popover width contract](#popover-width-contract), [Fine-pointer mounting](#fine-pointer-mounting), and [Mobile and desktop containment](#mobile-and-desktop-containment) | | `REQ-UI-TASK-CLEANUP-CONFIRMATION-001` | [Task cleanup content model](#task-cleanup-content-model), [Full-dialog composition](#full-dialog-composition), [Compact archive surfaces](#compact-archive-surfaces), [Source-specific archive routing](#source-specific-archive-routing), and [Mobile and desktop containment](#mobile-and-desktop-containment) | ## Components and responsibilities @@ -120,6 +120,36 @@ existing row-owned surface. These callers keep their established focus, containment, and action contracts; only the mobile Kanban card changes its surface selection. +### Classification-gated surface selection + +`TaskArchiveConfirmation` keeps descendant classification as the authoritative +input for choosing between the compact confirmation and the cascade dialog. A +standard fine-pointer caller does not mount a provisional popover while the +classification is idle or loading. A resolved zero count mounts the anchored +popover; a positive count mounts the full cascade dialog; and an error mounts +the same full dialog as the fail-safe path. No archive action is exposed before +that choice is known. + +The controlled archive request remains open while classification is pending, so +the final surface can mount without another user action. This avoids moving +focus and assistive-technology context from a temporary popover into a dialog. +It also preserves the existing final surfaces and their callbacks instead of +adding cascade controls to the compact popover. + +A non-rendering pending-dismissal controller retains the lifecycle previously +owned by the provisional popover. Escape closes the controlled request and +returns focus to the configured trigger; a new pointer interaction closes it +without preventing the new target's interaction. If live data removes the +originating anchor, the controller closes the request before classification can +select a final surface. Closing disables the classification hook, whose cleanup +ignores any late response, so a dismissed request cannot surface later. The +controller mounts no role, action, or visual confirmation shell. + +Callers whose final presentation is already known keep their current behavior. +Forced mobile Kanban and bulk operations may mount the full dialog immediately +with its action disabled until classification settles. Explicit inline and +coarse-pointer row confirmations retain their established loading treatment. + ### Popover width contract `ActionConfirmPopover` gains a small width/size contract whose default remains @@ -181,10 +211,11 @@ dialog computes the localized task outcome and cleanup model during render, then passes the model to the shared task-local renderer. Archive/delete callbacks and dialog state remain untouched. The source adapter supplies the existing `presentation` value and, for mobile Kanban only, maps it to -`forceDialog`; pointer classification continues to select the fine-pointer -popover or the explicitly inline row surface. This changes rendering only, -not archive commands, descendant classification, preference state, or -callbacks. +`forceDialog`. For a standard fine-pointer request, descendant classification +settles before the selected popover or dialog mounts. Pointer classification +continues to select the fine-pointer popover or the explicitly inline row +surface after that gate. This changes presentation timing only, not archive +commands, descendant classification, preference state, or callbacks. The delete dialog resets discard selection when it closes. On confirm, it sends the cascade and discard choices through the existing callback. A typed dirty @@ -193,13 +224,16 @@ Other deletion errors keep their existing error handling. ## Failure and recovery -If localized text is longer than the available width, the shared surface-text -contract wraps it inside the body. If content is taller than the dynamic -viewport, the body scrolls without moving the title or actions. +A failed descendant classification selects the full dialog without first +mounting a compact confirmation. If localized text is longer than the available +width, the shared surface-text contract wraps it inside the body. If content is +taller than the dynamic viewport, the body scrolls without moving the title or +actions. If deletion returns `task_delete_dirty_worktree`, the task remains visible. The localized message tells the user to reopen the confirmation and select discard. -Unknown executor types retain the generic running session effect. +Unknown executor types retain the generic running session effect. If no task +activity is present, no warning mounts, as before. ## Persistence @@ -211,10 +245,19 @@ typed HTTP 409 handling are defined in ## Observability Existing component tests continue to cover warning visibility and cleanup -content. Delete-dialog tests cover selection reset, disabled actions, single and -bulk callback values, cascade behavior, and typed conflict feedback. Rendered -desktop and phone checks cover viewport bounds, scroll ownership, action reach, -and document overflow. +content. Cleanup-summary tests cover every executor, bulk grouping, ordering, +effects, and supporting notes. Delete-dialog tests cover selection reset, +disabled actions, single and bulk callback values, cascade behavior, and typed +conflict feedback. Dialog and compact-surface tests assert equivalent structured +content, semantic action variants, unchanged callbacks, and that a deferred +positive descendant result does not expose a provisional fine-pointer popover. +Pending-dismissal component cases cover Escape, trigger focus restoration, and +outside pointer intent. +Rendered desktop and phone checks cover viewport bounds, scroll ownership, +action reach, and document overflow. The desktop cascade E2E holds the +descendant-count response, dismisses the invisible request, proves a late +result cannot mount a confirmation, then reopens Archive and observes only the +cascade dialog. ## Mobile and desktop containment @@ -223,6 +266,9 @@ records the row `getBoundingClientRect().height` before opening Archive, while the archive popover is visible, and after Cancel; all three values must remain stable within subpixel precision. It also verifies the archive popover is strictly wider than 256px and remains inside the viewport at compact widths. +For a parent task, a delayed descendant-count response leaves the confirmation +shell absent until classification settles; after release, only the full cascade +dialog is rendered and the anchored popover never becomes visible. The phone Kanban check enters through a card overflow menu and expects the mobile presentation to use the full alert dialog even for a zero-descendant