diff --git a/apps/web/components/kanban-card.tsx b/apps/web/components/kanban-card.tsx index 04abca24f0..88a340b38a 100644 --- a/apps/web/components/kanban-card.tsx +++ b/apps/web/components/kanban-card.tsx @@ -494,6 +494,7 @@ export function dispatchKanbanCardClick( function KanbanCardFrame({ task, + presentation, repositoryChips, draggable, menu, @@ -510,6 +511,7 @@ function KanbanCardFrame({ }: Pick< KanbanCardProps, | "task" + | "presentation" | "repositoryChips" | "isSelected" | "isMultiSelectMode" @@ -571,6 +573,7 @@ function KanbanCardFrame({ taskId={task.id} executorType={task.primaryExecutorType} isArchiving={isArchiving} + forceDialog={presentation === "mobile"} onOpenChange={menu.setShowArchiveConfirm} onConfirm={({ cascade }) => onArchive?.(task, { cascade })} /> @@ -634,6 +637,7 @@ export function KanbanCard({ <> void; + focusReturnRef?: RefObject; taskTitle?: string; isBulkOperation?: boolean; count?: number; @@ -106,6 +107,7 @@ function isArchiveActionDisabled( export function TaskArchiveConfirmDialog({ open, onOpenChange, + focusReturnRef, taskTitle, isBulkOperation, count, @@ -133,6 +135,12 @@ export function TaskArchiveConfirmDialog({ : getCleanupSummary(executorType); const [cascade, setCascade] = useState(false); + const confirmedRef = useRef(false); + const restoreFocus = () => { + if (confirmedRef.current) return; + const focusReturnTarget = focusReturnRef?.current; + if (focusReturnTarget?.isConnected) focusReturnTarget.focus(); + }; const requiresConfirmation = useArchiveConfirmationMode( open, confirmTaskArchive, @@ -152,7 +160,10 @@ export function TaskArchiveConfirmDialog({ const taskIsInFlight = computeTaskIsInFlight(isInFlight, storeInFlight); const handleOpenChange = (next: boolean) => { - if (!next) setCascade(false); + if (!next) { + setCascade(false); + restoreFocus(); + } onOpenChange(next); }; @@ -160,7 +171,16 @@ export function TaskArchiveConfirmDialog({ return ( - + { + event.preventDefault(); + restoreFocus(); + confirmedRef.current = false; + }} + > {title} @@ -202,6 +222,7 @@ export function TaskArchiveConfirmDialog({ data-testid={confirmTestId} onClick={() => { if (archiveDisabled) return; + confirmedRef.current = true; onConfirm({ cascade }); handleOpenChange(false); }} diff --git a/apps/web/components/task/task-archive-confirmation.test.tsx b/apps/web/components/task/task-archive-confirmation.test.tsx index 375d420636..b0756e914a 100644 --- a/apps/web/components/task/task-archive-confirmation.test.tsx +++ b/apps/web/components/task/task-archive-confirmation.test.tsx @@ -25,9 +25,11 @@ afterEach(cleanup); function ConfirmationHarness({ onConfirm, onOpenChange, + forceDialog = false, }: { onConfirm: () => void; onOpenChange: (open: boolean) => void; + forceDialog?: boolean; }) { const anchorRef = useRef(null); return ( @@ -44,15 +46,20 @@ function ConfirmationHarness({ executorType="worktree" onConfirm={onConfirm} confirmTestId={CONFIRM_TEST_ID} + forceDialog={forceDialog} /> ); } -function renderConfirmation(onConfirm = vi.fn(), onOpenChange = vi.fn()) { +function renderConfirmation(onConfirm = vi.fn(), onOpenChange = vi.fn(), forceDialog = false) { return render( - + , ); } @@ -99,6 +106,43 @@ describe("TaskArchiveConfirmation classification", () => { expect(screen.getByTestId(CONFIRM_TEST_ID).className).toContain("h-11"); }); + // @covers AC-UI-TASK-CLEANUP-CONFIRMATION-001.8 + it("uses the contained dialog when a coarse-pointer caller forces it", async () => { + getSubtaskCountMock.mockResolvedValue({ count: 0 }); + const onConfirm = vi.fn(); + const onOpenChange = vi.fn(); + + renderConfirmation(onConfirm, onOpenChange, true); + + const dialog = await screen.findByRole("alertdialog", { name: /Archive task/ }); + expect(dialog).toBeTruthy(); + expect(screen.queryByTestId(INLINE_CONFIRMATION_TEST_ID)).toBeNull(); + expect(screen.getByTestId("task-confirmation-outcome").textContent).toContain("Task One"); + + const archive = screen.getByTestId(CONFIRM_TEST_ID); + expect(archive.className).toContain("min-h-11"); + expect(archive.className).toContain("w-full"); + expect(archive.getAttribute("data-variant")).toBe("default"); + + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect(onOpenChange).toHaveBeenCalledWith(false); + + fireEvent.click(archive); + await waitFor(() => expect(onConfirm).toHaveBeenCalledWith({ cascade: false })); + expect(onOpenChange).toHaveBeenCalledWith(false); + }); + + it("keeps the forced dialog archive action disabled while classification is pending", async () => { + getSubtaskCountMock.mockReturnValue(new Promise(() => undefined)); + + renderConfirmation(vi.fn(), vi.fn(), true); + + const dialog = await screen.findByRole("alertdialog", { name: /Archive task/ }); + expect(dialog).toBeTruthy(); + expect(screen.queryByTestId(INLINE_CONFIRMATION_TEST_ID)).toBeNull(); + expect(screen.getByTestId(CONFIRM_TEST_ID).hasAttribute("disabled")).toBe(true); + }); + it("uses the same semantic cleanup effect list and supporting notes as the dialog", async () => { getSubtaskCountMock.mockResolvedValue({ count: 0 }); diff --git a/apps/web/components/task/task-archive-confirmation.tsx b/apps/web/components/task/task-archive-confirmation.tsx index c16f814bb8..f38ef30eb5 100644 --- a/apps/web/components/task/task-archive-confirmation.tsx +++ b/apps/web/components/task/task-archive-confirmation.tsx @@ -39,6 +39,8 @@ export type TaskArchiveConfirmationProps = { confirmTestId?: string; /** Render a simple confirmation inside an existing action surface. */ inline?: boolean; + /** Use the contained dialog even when classification resolves without descendants. */ + forceDialog?: boolean; }; function ArchiveDescription({ @@ -199,6 +201,7 @@ function ArchiveClassifyingPopover({ type ArchiveDialogProps = Pick< TaskArchiveConfirmationProps, | "onOpenChange" + | "focusReturnRef" | "taskTitle" | "isBulkOperation" | "count" @@ -220,6 +223,7 @@ function ArchiveDialog({ subtaskClassification, ...props }: ArchiveDialogProps) type ArchiveConfirmationContentProps = ArchiveDialogProps & { confirmTaskArchive: boolean; + forceDialog: boolean; isFinePointer: boolean; taskIsInFlight: boolean; anchorRef: RefObject; @@ -230,6 +234,7 @@ type ArchiveConfirmationContentProps = ArchiveDialogProps & { function ArchiveConfirmationContent({ confirmTaskArchive, + forceDialog, isFinePointer, taskIsInFlight, anchorRef, @@ -241,13 +246,20 @@ function ArchiveConfirmationContent({ }: ArchiveConfirmationContentProps) { const { t } = useTranslation(); const shouldUseDialog = + forceDialog || !confirmTaskArchive || dialogProps.isBulkOperation || subtaskClassification.status === "error" || subtaskClassification.total > 0; if (shouldUseDialog) { - return ; + return ( + + ); } if (subtaskClassification.status !== "resolved") { @@ -321,6 +333,7 @@ export function TaskArchiveConfirmation({ onConfirm, confirmTestId = DEFAULT_CONFIRM_TEST_ID, inline = false, + forceDialog = false, }: TaskArchiveConfirmationProps) { const { isFinePointer } = useResponsiveBreakpoint(); const confirmTaskArchive = useAppStore((state) => state.userSettings?.confirmTaskArchive ?? true); @@ -333,6 +346,7 @@ export function TaskArchiveConfirmation({ return ( { + const normalized = channel / 255; + return normalized <= 0.03928 ? normalized / 12.92 : ((normalized + 0.055) / 1.055) ** 2.4; + }; + return 0.2126 * linearize(red) + 0.7152 * linearize(green) + 0.0722 * linearize(blue); +} + +function contrastRatio(foreground: Rgb, background: Rgb): number { + const foregroundLuminance = relativeLuminance(foreground); + const backgroundLuminance = relativeLuminance(background); + const lighter = Math.max(foregroundLuminance, backgroundLuminance); + const darker = Math.min(foregroundLuminance, backgroundLuminance); + return (lighter + 0.05) / (darker + 0.05); +} + +async function openArchiveMenu(testPage: Page, taskId: string): Promise { + const mobile = new MobileKanbanPage(testPage); + const card = mobile.taskCard(taskId); + const archiveTrigger = card.getByRole("button", { name: "More options" }); + await archiveTrigger.tap(); + + const menu = testPage.locator('[data-slot="dropdown-menu-content"]:visible').last(); + await expect(menu).toBeVisible(); + const archiveItem = menu.getByRole("menuitem", { name: "Archive", exact: true }); + await expect(archiveItem).toBeVisible(); + await archiveItem.tap(); + return archiveTrigger; +} + +test.describe("Mobile Kanban card archive confirmation", () => { + test("uses a contained dark dialog and archives the card after confirmation", async ({ + testPage, + apiClient, + seedData, + prCapture, + }) => { + await testPage.addInitScript(() => localStorage.setItem("theme", "dark")); + const task = await apiClient.createTask(seedData.workspaceId, TASK_TITLE, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + await apiClient.createTask(seedData.workspaceId, "Mobile Kanban archive neighbor", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + + const mobile = new MobileKanbanPage(testPage); + await mobile.goto(); + await expect(testPage.locator("html")).toHaveClass(/(^|\s)dark(\s|$)/); + + const card = mobile.taskCard(task.id); + await expect(card).toBeVisible(); + const startUrl = testPage.url(); + const beforeBox = await card.boundingBox(); + if (!beforeBox) throw new Error("mobile archive task card has no layout box"); + + const archiveTrigger = await openArchiveMenu(testPage, task.id); + + const dialog = testPage.getByRole("alertdialog", { name: /Archive task/ }); + await expect(dialog).toBeVisible(); + await expect(dialog).toContainText(TASK_TITLE); + await expect(dialog.getByTestId("task-confirmation-outcome")).toContainText(TASK_TITLE); + await expect(dialog.getByRole("button", { name: "Cancel", exact: true })).toBeVisible(); + await expect(dialog.getByRole("button", { name: "Archive", exact: true })).toBeVisible(); + await expect(testPage.getByTestId(INLINE_CONFIRMATION_TEST_ID)).toHaveCount(0); + + await waitForFiniteAnimations(dialog); + await prCapture.screenshot("after-dark", { + caption: + "After: mobile Kanban uses the contained archive alert dialog without changing card layout.", + }); + const [dialogBox, viewport, surfaceColors] = await Promise.all([ + dialog.boundingBox(), + testPage.evaluate(() => ({ width: window.innerWidth, height: window.innerHeight })), + dialog.evaluate((element) => { + const styles = getComputedStyle(element); + return { backgroundColor: styles.backgroundColor, foregroundColor: styles.color }; + }), + ]); + if (!dialogBox) throw new Error("mobile archive dialog has no layout box"); + expect(dialogBox.x).toBeGreaterThanOrEqual(12); + expect(viewport.width - (dialogBox.x + dialogBox.width)).toBeGreaterThanOrEqual(12); + expect(dialogBox.y).toBeGreaterThanOrEqual(0); + expect(dialogBox.y + dialogBox.height).toBeLessThanOrEqual(viewport.height); + const dialogCenterY = dialogBox.y + dialogBox.height / 2; + expect(Math.abs(dialogCenterY - viewport.height / 2)).toBeLessThanOrEqual(8); + expect(surfaceColors.backgroundColor).not.toBe("rgba(0, 0, 0, 0)"); + expect(surfaceColors.backgroundColor).toMatch(/^rgb/); + expect(surfaceColors.foregroundColor).toMatch(/^rgb/); + expect(relativeLuminance(parseRgb(surfaceColors.backgroundColor))).toBeLessThan(0.1); + expect( + contrastRatio( + parseRgb(surfaceColors.foregroundColor), + parseRgb(surfaceColors.backgroundColor), + ), + ).toBeGreaterThanOrEqual(4.5); + + for (const actionName of ["Cancel", "Archive"]) { + const actionBox = await dialog + .getByRole("button", { name: actionName, exact: true }) + .boundingBox(); + if (!actionBox) throw new Error(`${actionName} action has no layout box`); + expect(actionBox.height).toBeGreaterThanOrEqual(44); + } + const afterOpenBox = await card.boundingBox(); + if (!afterOpenBox) + throw new Error("mobile archive task card disappeared while dialog was open"); + expect(afterOpenBox.height).toBeCloseTo(beforeBox.height, 1); + expect(testPage.url()).toBe(startUrl); + await assertNoDocumentHorizontalOverflow(testPage, "mobile Kanban archive dialog"); + + await dialog.getByRole("button", { name: "Cancel", exact: true }).tap(); + await expect(dialog).not.toBeVisible(); + await expect(archiveTrigger).toBeFocused(); + await expect(card).toBeVisible(); + expect(testPage.url()).toBe(startUrl); + + await openArchiveMenu(testPage, task.id); + const reopenedDialog = testPage.getByRole("alertdialog", { name: /Archive task/ }); + await expect(reopenedDialog).toBeVisible(); + const archiveResponse = waitForHttp( + testPage, + "POST", + new RegExp(`/api/v1/tasks/${task.id}/archive$`), + ); + await reopenedDialog.getByRole("button", { name: "Archive", exact: true }).tap(); + await archiveResponse; + await expect(card).not.toBeVisible(); + }); + + test("bypasses the dialog when archive confirmation is disabled", async ({ + testPage, + apiClient, + seedData, + }) => { + const task = await apiClient.createTask(seedData.workspaceId, "Mobile archive bypass", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + await apiClient.saveUserSettings({ confirm_task_archive: false }); + + const mobile = new MobileKanbanPage(testPage); + await mobile.goto(); + const card = mobile.taskCard(task.id); + await expect(card).toBeVisible(); + + const archiveResponse = waitForHttp( + testPage, + "POST", + new RegExp(`/api/v1/tasks/${task.id}/archive$`), + ); + await openArchiveMenu(testPage, task.id); + await archiveResponse; + await expect(card).not.toBeVisible(); + await expect(testPage.getByRole("alertdialog")).toHaveCount(0); + await expect(testPage.getByTestId(INLINE_CONFIRMATION_TEST_ID)).toHaveCount(0); + }); +}); diff --git a/docs/specs/ui/requirements/confirmation-warning-hierarchy.md b/docs/specs/ui/requirements/confirmation-warning-hierarchy.md index 0d0367c7f9..9502242d9a 100644 --- a/docs/specs/ui/requirements/confirmation-warning-hierarchy.md +++ b/docs/specs/ui/requirements/confirmation-warning-hierarchy.md @@ -115,6 +115,18 @@ repository resources, so that I can understand the consequence before acting. Enter default-action behavior, 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 + and inset, use the existing theme tokens, keep its content internally + scrollable, and expose stacked full-width actions at least 44 CSS px high; + no inline confirmation shall be inserted between cards or change the card's + height, board URL, or selected task. Fine-pointer desktop and compact-desktop + Kanban shall retain their anchored archive popover. Coarse-pointer tablet + Kanban shall retain its existing routing, including its current inline + zero-descendant surface. Task-switcher/task-row surfaces shall retain their + intentional row-owned coarse-pointer inline confirmation, and a disabled + archive-confirmation preference shall continue to bypass the surface. ## Out of scope diff --git a/docs/specs/ui/system-design/confirmation-warning-hierarchy.md b/docs/specs/ui/system-design/confirmation-warning-hierarchy.md index e974e32851..2506d198c8 100644 --- a/docs/specs/ui/system-design/confirmation-warning-hierarchy.md +++ b/docs/specs/ui/system-design/confirmation-warning-hierarchy.md @@ -20,11 +20,11 @@ existing components and runtime contracts. ## Requirement mapping -| 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-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), and [Mobile and desktop containment](#mobile-and-desktop-containment) | +| 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-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 @@ -32,8 +32,9 @@ existing components and runtime contracts. markup and style owner. - `TaskArchiveConfirmDialog` and `TaskDeleteConfirmDialog` render it inside the existing full confirmation dialog. -- `TaskArchiveConfirmation` renders the same component through - `ArchiveDescription` for the desktop popover and phone inline branch. +- `TaskArchiveConfirmation` routes the card-owned request to the existing + dialog, desktop popover, or explicitly inline branch while reusing the same + cleanup content and callbacks. - Consumers continue to decide whether a warning mounts. The shared component does not inspect task state or alter callbacks. @@ -93,6 +94,24 @@ touch density, callbacks, and focus-return behavior. Only the internal copy hierarchy changes: direct archive outcome, ordered effects, then supporting notes and the existing still-working warning. +### Source-specific archive routing + +Archive presentation is selected by the source surface, not by coarse-pointer +classification alone. The mobile Kanban card opts into the existing full +`TaskArchiveConfirmDialog` through `TaskArchiveConfirmation`'s internal +`forceDialog` presentation prop. This keeps the confirmation portaled over the +board, so a virtualized card row does not grow when a zero-descendant task is +being archived. + +Fine-pointer desktop and compact-desktop Kanban retain the anchored +`ActionConfirmPopover`. Coarse-pointer tablet Kanban retains its existing +coarse-pointer routing, including the current inline zero-descendant surface. +The task-switcher/task-row adapter may explicitly own the coarse-pointer inline +confirmation, and command-panel callers that request `inline` retain their +existing row-owned surface. These callers keep their established focus, +containment, and action contracts; only the mobile Kanban card changes its +surface selection. + ### Popover width contract `ActionConfirmPopover` gains a small width/size contract whose default remains @@ -137,15 +156,23 @@ No API, WebSocket, state, or persisted-data contract changes are required. Localization catalogs change only for task confirmation presentation; executor cleanup semantics remain unchanged. +`forceDialog` is an optional internal presentation prop with a default of +`false`. It is passed from the card-owned source to the existing routing +component and does not change archive commands, descendant classification, +preference state, mutation state, or callbacks. + ## Control flow The existing task-level `foregroundActivity` projection and explicit `isInFlight` props continue to determine whether the warning is rendered. The 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 context-menu adapter only -chooses the mounting branch described above based on the existing responsive -pointer classification. +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. ## Failure and recovery @@ -177,8 +204,15 @@ 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. -The phone check keeps the existing coarse-pointer sidebar inline flow. It -expects the inline confirmation to remain intentionally row-owned, keeps +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 +task. It records the card height before and during the dialog, verifies the +centered/inset surface, stacked actions, cleanup copy, dark theme tokens, and +zero document horizontal overflow, then cancels and repeats the existing +archive callback flow. The card remains unchanged while the dialog is open. + +The phone task-switcher check keeps the existing coarse-pointer sidebar inline +flow. It expects that confirmation to remain intentionally row-owned, keeps actions at or above 44px, and asserts zero document horizontal overflow. The task-delete phone check enters through the real task drawer action menu and