fix(web): stabilize archive confirmation surface (#3460)
Co-authored-by: Kandev Agent <agent@kandev.dev>
This commit is contained in:
João Salavisa
@@ -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({
|
||||
<button ref={anchorRef} type="button" data-testid="archive-anchor">
|
||||
Archive source
|
||||
</button>
|
||||
<button type="button" data-testid="outside-action">
|
||||
Outside action
|
||||
</button>
|
||||
<TaskArchiveConfirmation
|
||||
open
|
||||
onOpenChange={onOpenChange}
|
||||
@@ -52,6 +59,43 @@ function ConfirmationHarness({
|
||||
);
|
||||
}
|
||||
|
||||
function AnchorLifecycleHarness({
|
||||
showAnchor,
|
||||
onOpenChange,
|
||||
}: {
|
||||
showAnchor: boolean;
|
||||
onOpenChange: (open: boolean) => void;
|
||||
}) {
|
||||
const [open, setOpen] = useState(true);
|
||||
const anchorRef = useRef<HTMLButtonElement>(null);
|
||||
const handleOpenChange = useCallback(
|
||||
(nextOpen: boolean) => {
|
||||
setOpen(nextOpen);
|
||||
onOpenChange(nextOpen);
|
||||
},
|
||||
[onOpenChange],
|
||||
);
|
||||
|
||||
return (
|
||||
<>
|
||||
{showAnchor ? (
|
||||
<button ref={anchorRef} type="button" data-testid="archive-anchor">
|
||||
Archive source
|
||||
</button>
|
||||
) : null}
|
||||
<TaskArchiveConfirmation
|
||||
open={open}
|
||||
onOpenChange={handleOpenChange}
|
||||
anchorRef={anchorRef}
|
||||
taskId="task-1"
|
||||
taskTitle="Task One"
|
||||
executorType="worktree"
|
||||
onConfirm={vi.fn()}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
function renderConfirmation(onConfirm = vi.fn(), onOpenChange = vi.fn(), forceDialog = false) {
|
||||
return render(
|
||||
<StateProvider>
|
||||
@@ -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) => (
|
||||
<StateProvider>
|
||||
<AnchorLifecycleHarness showAnchor={showAnchor} onOpenChange={onOpenChange} />
|
||||
</StateProvider>
|
||||
);
|
||||
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 () => {
|
||||
|
||||
@@ -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<HTMLElement | null>;
|
||||
focusReturnRef?: RefObject<HTMLElement | null>;
|
||||
focusBoundaryRef?: RefObject<HTMLElement | null>;
|
||||
onOpenChange: (open: boolean) => void;
|
||||
confirmTestId: string;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
return (
|
||||
<ActionConfirmPopover
|
||||
open
|
||||
size="wide"
|
||||
anchorRef={anchorRef}
|
||||
focusReturnRef={focusReturnRef}
|
||||
focusBoundaryRef={focusBoundaryRef ?? anchorRef}
|
||||
title={t("task:archiveTaskTitle")}
|
||||
description={
|
||||
<span className="flex items-center gap-2">
|
||||
<IconLoader className="h-4 w-4 animate-spin" />
|
||||
{t("common:loading")}
|
||||
</span>
|
||||
}
|
||||
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 (
|
||||
<ArchiveClassifyingPopover
|
||||
<PendingArchiveDismissal
|
||||
anchorRef={anchorRef}
|
||||
focusReturnRef={focusReturnRef}
|
||||
focusBoundaryRef={focusBoundaryRef}
|
||||
onOpenChange={dialogProps.onOpenChange}
|
||||
confirmTestId={dialogProps.confirmTestId ?? DEFAULT_CONFIRM_TEST_ID}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<void>((resolve) => {
|
||||
releaseResponse = resolve;
|
||||
});
|
||||
let markRequestObserved: () => void = () => {};
|
||||
const requestObserved = new Promise<void>((resolve) => {
|
||||
markRequestObserved = resolve;
|
||||
});
|
||||
let requestStarted = false;
|
||||
let markHandlerSettled: () => void = () => {};
|
||||
const handlerSettled = new Promise<void>((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,
|
||||
|
||||
@@ -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.
|
||||
+128
@@ -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.
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user