From 99c37aa09668ca87aba48bfe336dc72d86a2a911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:57:24 +0000 Subject: [PATCH] fix(web): polish archive confirmation warning surfaces (#2995) --- .../action-confirm-popover.test.tsx | 14 ++ .../confirmation/action-confirm-popover.tsx | 10 +- .../task/task-archive-confirm-dialog.test.tsx | 17 ++ .../task/task-archive-confirmation.tsx | 2 + .../task/task-confirm-dialog.test-helpers.ts | 11 ++ .../task/task-delete-confirm-dialog.test.tsx | 18 ++ .../task/task-still-working-warning.tsx | 4 +- .../task/task-switcher-context-menu.tsx | 12 +- .../archive-confirmation-preference.spec.ts | 176 ++++++++++++++++++ .../task/mobile-sidebar-task-actions.spec.ts | 75 ++++++++ .../task-confirmation-warning-polish/plan.md | 131 +++++++++++++ .../task-01-compact-warning.md | 143 ++++++++++++++ docs/specs/ui/README.md | 2 + .../confirmation-warning-hierarchy.md | 82 ++++++++ .../confirmation-warning-hierarchy.md | 125 +++++++++++++ 15 files changed, 814 insertions(+), 8 deletions(-) create mode 100644 apps/web/components/task/task-confirm-dialog.test-helpers.ts create mode 100644 docs/plans/task-confirmation-warning-polish/plan.md create mode 100644 docs/plans/task-confirmation-warning-polish/task-01-compact-warning.md create mode 100644 docs/specs/ui/requirements/confirmation-warning-hierarchy.md create mode 100644 docs/specs/ui/system-design/confirmation-warning-hierarchy.md diff --git a/apps/web/components/confirmation/action-confirm-popover.test.tsx b/apps/web/components/confirmation/action-confirm-popover.test.tsx index 70998b15db..795ac0e8dd 100644 --- a/apps/web/components/confirmation/action-confirm-popover.test.tsx +++ b/apps/web/components/confirmation/action-confirm-popover.test.tsx @@ -9,9 +9,11 @@ const deleteTitle = "Delete item?"; function Harness({ onConfirm = vi.fn(), confirmDisabled = false, + size, }: { onConfirm?: () => void | Promise; confirmDisabled?: boolean; + size?: "wide"; }) { const [open, setOpen] = useState(true); const [showAnchor, setShowAnchor] = useState(true); @@ -34,6 +36,7 @@ function Harness({ cancelLabel="Cancel" confirmLabel="Delete" confirmDisabled={confirmDisabled} + size={size} onOpenChange={setOpen} onConfirm={onConfirm} /> @@ -55,6 +58,17 @@ describe("ActionConfirmPopover", () => { ).toContain("min-h-11"); }); + it("supports a wider bounded surface without changing the default width", async () => { + render(); + expect(screen.getByRole("dialog").className).toContain("w-64"); + + cleanup(); + render(); + const wideDialog = screen.getByRole("dialog"); + expect(wideDialog.className).toContain("w-72"); + expect(wideDialog.className).toContain("max-w-[calc(100vw-1rem)]"); + }); + it("closes before invoking an unresolved confirmation and returns focus on cancel", async () => { const onConfirm = vi.fn(() => new Promise(() => {})); render(); diff --git a/apps/web/components/confirmation/action-confirm-popover.tsx b/apps/web/components/confirmation/action-confirm-popover.tsx index 4c1ec97cc4..ad4ed7a61b 100644 --- a/apps/web/components/confirmation/action-confirm-popover.tsx +++ b/apps/web/components/confirmation/action-confirm-popover.tsx @@ -2,6 +2,7 @@ import { useId, useLayoutEffect, useRef, type ReactNode, type RefObject } from "react"; import { Button } from "@kandev/ui/button"; +import { cn } from "@/lib/utils"; import { Popover, PopoverAnchor, @@ -11,8 +12,11 @@ import { PopoverTitle, } from "@kandev/ui/popover"; +export type ActionConfirmPopoverSize = "default" | "wide"; + export type ActionConfirmPopoverProps = { open: boolean; + size?: ActionConfirmPopoverSize; disabled?: boolean; anchorRef: RefObject; focusReturnRef?: RefObject; @@ -40,6 +44,7 @@ export type ActionConfirmPopoverProps = { */ export function ActionConfirmPopover({ open, + size = "default", disabled = false, anchorRef, focusReturnRef, @@ -104,6 +109,7 @@ export function ActionConfirmPopover({ {/* Radix accepts a null current value at runtime while its public type omits it. */} } /> { event.preventDefault(); cancelRef.current?.focus(); diff --git a/apps/web/components/task/task-archive-confirm-dialog.test.tsx b/apps/web/components/task/task-archive-confirm-dialog.test.tsx index 2241a07e57..6d115e65d7 100644 --- a/apps/web/components/task/task-archive-confirm-dialog.test.tsx +++ b/apps/web/components/task/task-archive-confirm-dialog.test.tsx @@ -3,6 +3,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { render, screen, cleanup, fireEvent, waitFor } from "@testing-library/react"; import { StateProvider, useAppStore } from "@/components/state-provider"; import { defaultState } from "@/lib/state/default-state"; +import { expectCompactWarning } from "./task-confirm-dialog.test-helpers"; const mockGetSubtaskCount = vi.fn(); @@ -264,6 +265,22 @@ describe("TaskArchiveConfirmDialog cleanup copy", () => { }); describe("TaskArchiveConfirmDialog still-working guard", () => { + it("keeps the in-flight warning visually subordinate to confirmation copy", () => { + renderWithTasks( + {}} + taskTitle="My task" + taskId="task-1" + executorType="worktree" + onConfirm={() => {}} + />, + [{ id: "task-1", foregroundActivity: "generating" }], + ); + + expectCompactWarning(screen.getByTestId(WARNING_TESTID)); + }); + it("warns when the task is generating", () => { renderWithTasks( ({ })); import { TaskDeleteConfirmDialog } from "./task-delete-confirm-dialog"; +import { expectCompactWarning } from "./task-confirm-dialog.test-helpers"; type SeedTask = { id: string; foregroundActivity?: "generating" | "background" | null }; @@ -190,6 +191,23 @@ describe("TaskDeleteConfirmDialog executor cleanup copy", () => { }); describe("TaskDeleteConfirmDialog still-working guard", () => { + it("keeps the shared in-flight warning visually subordinate", () => { + mockGetSubtaskCount.mockResolvedValue({ count: 0 }); + renderDialog( + {}} + taskTitle="My task" + taskId="task-1" + executorType="worktree" + onConfirm={() => {}} + />, + [{ id: "task-1", foregroundActivity: "generating" }], + ); + + expectCompactWarning(screen.getByTestId(WARNING_TESTID)); + }); + it("warns when the task is generating", () => { mockGetSubtaskCount.mockResolvedValue({ count: 0 }); renderDialog( diff --git a/apps/web/components/task/task-still-working-warning.tsx b/apps/web/components/task/task-still-working-warning.tsx index 7d5f6a9afb..d037e0872e 100644 --- a/apps/web/components/task/task-still-working-warning.tsx +++ b/apps/web/components/task/task-still-working-warning.tsx @@ -9,9 +9,9 @@ export function StillWorkingWarning({ count }: { count?: number }) {
- + {t("task:stillWorkingWarning", { subject })}
); diff --git a/apps/web/components/task/task-switcher-context-menu.tsx b/apps/web/components/task/task-switcher-context-menu.tsx index 6ecafa7828..85ed2c4d95 100644 --- a/apps/web/components/task/task-switcher-context-menu.tsx +++ b/apps/web/components/task/task-switcher-context-menu.tsx @@ -23,6 +23,7 @@ import { } from "@/components/task/task-move-context-menu"; import { TaskNestContextMenuItems } from "@/components/task/task-nest-context-menu"; import { useTaskWorkflowMove } from "@/hooks/use-task-workflow-move"; +import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; import { TaskColorMenu } from "./task-switcher-color-menu"; import { TaskPluginLinkMenu, @@ -168,22 +169,23 @@ export function TaskItemWithContextMenu(props: ContextMenuProps) { setMenuKey((k) => k + 1); }; const { handleOpenChange, triggerProps } = useMenuTouchDragCancel(setContextOpen); + const { isFinePointer } = useResponsiveBreakpoint(); const archive = useTaskSwitcherArchiveConfirmation({ task: menuProps.task, onArchiveTask: menuProps.onArchiveTask, isArchiving: menuProps.isArchiving, closeMenu, }); + const archiveConfirmation = archive.archiveOpen ? archive.archiveConfirmation : undefined; + const inlineArchiveConfirmation = isFinePointer ? undefined : archiveConfirmation; + const portaledArchiveConfirmation = isFinePointer ? archiveConfirmation : undefined; return (
- {cloneWithMenuOpen( - children, - contextOpen, - archive.archiveOpen ? archive.archiveConfirmation : undefined, - )} + {cloneWithMenuOpen(children, contextOpen, inlineArchiveConfirmation)} + {portaledArchiveConfirmation}
{ + test("keeps the fine-pointer archive popover wide and the sidebar row stable", async ({ + testPage, + apiClient, + seedData, + prCapture, + }, testInfo) => { + await testPage.setViewportSize({ width: 900, height: 900 }); + const title = "Fine pointer archive geometry target"; + const task = await apiClient.seedTask(seedData.workspaceId, title, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + + await testPage.goto(`/t/${task.task_id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + + const taskRow = session.sidebarTaskItem(title); + await expect(taskRow).toBeVisible({ timeout: 15_000 }); + const heightBefore = await taskRow.evaluate( + (element) => element.getBoundingClientRect().height, + ); + await prCapture.screenshot("before-archive-popover", { + caption: "Fine-pointer sidebar row before archive confirmation", + }); + + await session.openSidebarMenuAndClick(title, "Archive"); + const popover = testPage.getByTestId("task-archive-confirm-popover"); + await expect(popover).toBeVisible(); + await prCapture.screenshot("archive-popover-open", { + caption: "Fine-pointer archive confirmation popover", + }); + + const openRowMetrics = await taskRow.evaluate((element) => { + const rowBox = element.getBoundingClientRect(); + return { rowHeight: rowBox.height }; + }); + const openSurfaceMetrics = await popover.evaluate((element) => { + const surfaceBox = element.getBoundingClientRect(); + return { + surface: { + x: surfaceBox.x, + y: surfaceBox.y, + width: surfaceBox.width, + height: surfaceBox.height, + }, + viewport: { width: window.innerWidth, height: window.innerHeight }, + documentWidth: document.documentElement.scrollWidth, + viewportWidth: document.documentElement.clientWidth, + }; + }); + const openMetrics = { + rowHeight: openRowMetrics.rowHeight, + ...openSurfaceMetrics, + }; + + expect(openMetrics.rowHeight).toBeCloseTo(heightBefore, 3); + expect(openMetrics.surface.width).toBeGreaterThan(256); + expect(openMetrics.surface.x).toBeGreaterThanOrEqual(0); + expect(openMetrics.surface.x + openMetrics.surface.width).toBeLessThanOrEqual( + openMetrics.viewport.width, + ); + expect(openMetrics.surface.y).toBeGreaterThanOrEqual(0); + expect(openMetrics.surface.y + openMetrics.surface.height).toBeLessThanOrEqual( + openMetrics.viewport.height, + ); + expect(openMetrics.documentWidth).toBeLessThanOrEqual(openMetrics.viewportWidth); + + await popover.getByRole("button", { name: "Cancel", exact: true }).click(); + await expect(popover).toBeHidden(); + const heightAfter = await taskRow.evaluate((element) => element.getBoundingClientRect().height); + expect(heightAfter).toBeCloseTo(heightBefore, 3); + await testInfo.attach("archive-popover-geometry.json", { + body: Buffer.from( + JSON.stringify( + { + heightBefore, + heightOpen: openMetrics.rowHeight, + heightAfter, + popover: openMetrics.surface, + viewport: openMetrics.viewport, + documentWidth: openMetrics.documentWidth, + viewportWidth: openMetrics.viewportWidth, + }, + null, + 2, + ), + ), + contentType: "application/json", + }); + }); + + test("keeps the full archive warning subordinate to dialog copy", async ({ + testPage, + apiClient, + seedData, + prCapture, + }) => { + test.setTimeout(120_000); + const title = "Full archive warning geometry target"; + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description: "/background 60s", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + await apiClient.seedTask(seedData.workspaceId, "Full archive warning child", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + parent_id: task.id, + }); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await waitForActiveSessionForegroundActivity(testPage, "generating"); + await prCapture.screenshot("before-full-archive-dialog", { + caption: "In-flight task before opening full archive confirmation", + }); + + const taskRow = session.sidebarTaskItem(title); + await expect(taskRow).toBeVisible({ timeout: 15_000 }); + const taskActions = taskRow.getByRole("button", { name: "Task actions" }); + await expect(taskActions).toBeVisible(); + await taskActions.click({ force: true }); + const archiveMenuItem = testPage.getByRole("menuitem", { name: "Archive", exact: true }); + await expect(archiveMenuItem).toBeVisible(); + await archiveMenuItem.click(); + const dialog = testPage.getByRole("alertdialog"); + await expect(dialog).toBeVisible(); + const warning = dialog.getByTestId("still-working-warning"); + await expect(warning).toBeVisible(); + await prCapture.screenshot("full-archive-dialog-warning", { + caption: "Compact still-working warning in full archive dialog", + }); + + const hierarchy = await dialog.evaluate((dialogElement) => { + const warning = dialogElement.querySelector( + '[data-testid="still-working-warning"]', + ); + const description = dialogElement.querySelector( + '[data-slot="alert-dialog-description"]', + ); + if (!warning || !description) throw new Error("dialog hierarchy targets missing"); + const warningStyle = getComputedStyle(warning); + const descriptionStyle = getComputedStyle(description); + const warningBox = warning.getBoundingClientRect(); + const dialogBox = dialogElement.getBoundingClientRect(); + return { + warningFontSize: warningStyle.fontSize, + warningLineHeight: warningStyle.lineHeight, + descriptionFontSize: descriptionStyle.fontSize, + warning: { left: warningBox.left, right: warningBox.right }, + dialog: { left: dialogBox.left, right: dialogBox.right }, + documentWidth: document.documentElement.scrollWidth, + viewportWidth: document.documentElement.clientWidth, + }; + }); + expect(Number.parseFloat(hierarchy.warningFontSize)).toBeLessThan( + Number.parseFloat(hierarchy.descriptionFontSize), + ); + expect(hierarchy.warningLineHeight).toBe("20px"); + expect(hierarchy.warning.left).toBeGreaterThanOrEqual(hierarchy.dialog.left); + expect(hierarchy.warning.right).toBeLessThanOrEqual(hierarchy.dialog.right); + expect(hierarchy.documentWidth).toBeLessThanOrEqual(hierarchy.viewportWidth); + + await dialog.getByRole("button", { name: "Cancel", exact: true }).click(); + await expect(dialog).toBeHidden(); + }); + test("disabling confirmation archives immediately from the desktop sidebar", async ({ testPage, apiClient, diff --git a/apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts b/apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts index 2cf49ccb60..5d9acadc2d 100644 --- a/apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts +++ b/apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { test, expect } from "../../fixtures/test-base"; import { activeSessionId, seedSecondaryClarificationTask } from "../../helpers/clarification"; import { makeGitEnv } from "../../helpers/git-helper"; +import { waitForActiveSessionForegroundActivity } from "../../helpers/session-store"; import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions"; import { SessionPage } from "../../pages/session-page"; @@ -265,6 +266,80 @@ test.describe("Mobile sidebar task actions", () => { expect(pageWidth.scroll).toBeLessThanOrEqual(pageWidth.client); }); + test("keeps the in-flight warning compact and reachable on a phone", async ({ + testPage, + apiClient, + seedData, + prCapture, + }) => { + test.setTimeout(120_000); + const title = "Mobile in-flight archive warning"; + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description: "/background 30s", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await waitForActiveSessionForegroundActivity(testPage, "generating"); + await testPage.getByTestId("mobile-session-menu").tap(); + + const drawer = testPage.getByRole("dialog", { name: "Tasks" }); + const taskRow = drawer.getByTestId("sidebar-task-item").filter({ hasText: title }); + const heightBefore = await taskRow.evaluate( + (element) => element.getBoundingClientRect().height, + ); + await prCapture.screenshot("before-mobile-in-flight-archive", { + caption: "Phone task row before in-flight archive confirmation", + }); + await taskRow.getByRole("button", { name: "Task actions" }).tap(); + await testPage.getByRole("menuitem", { name: "Archive", exact: true }).tap(); + + const confirmation = taskRow.getByTestId("task-archive-inline-confirmation"); + const warning = confirmation.getByTestId("still-working-warning"); + await expect(confirmation).toBeVisible(); + await expect(warning).toBeVisible(); + await prCapture.screenshot("mobile-in-flight-archive-warning", { + caption: "Compact in-flight warning in phone archive confirmation", + }); + + const warningMetrics = await warning.evaluate((element) => { + const style = getComputedStyle(element); + return { fontSize: style.fontSize, lineHeight: style.lineHeight }; + }); + expect(warningMetrics).toEqual({ fontSize: "12px", lineHeight: "20px" }); + await expect(warning).toHaveClass(/text-pretty/); + const heightOpen = await taskRow.evaluate((element) => element.getBoundingClientRect().height); + expect(heightOpen).toBeGreaterThan(heightBefore); + + for (const button of [ + confirmation.getByRole("button", { name: "Cancel" }), + confirmation.getByTestId("archive-task-confirm"), + ]) { + const box = await button.boundingBox(); + if (!box) throw new Error("in-flight inline archive action has no layout box"); + expect(box.width).toBeGreaterThanOrEqual(44); + expect(box.height).toBeGreaterThanOrEqual(44); + await expect(button).toBeInViewport(); + } + + const pageWidth = await testPage.evaluate(() => ({ + scroll: document.documentElement.scrollWidth, + client: document.documentElement.clientWidth, + })); + expect(pageWidth.scroll).toBeLessThanOrEqual(pageWidth.client); + await confirmation.getByRole("button", { name: "Cancel" }).tap(); + await expect(confirmation).toBeHidden(); + }); + test("keeps the tablet task switcher as a left-side sheet", async ({ testPage, apiClient, diff --git a/docs/plans/task-confirmation-warning-polish/plan.md b/docs/plans/task-confirmation-warning-polish/plan.md new file mode 100644 index 0000000000..fc9a7fd9f3 --- /dev/null +++ b/docs/plans/task-confirmation-warning-polish/plan.md @@ -0,0 +1,131 @@ +--- +created: 2026-08-24 +status: complete +requirements: + - ../../specs/ui/requirements/confirmation-warning-hierarchy.md +system_design: + - ../../specs/ui/system-design/confirmation-warning-hierarchy.md +legacy_specs: [] +--- + +# Implementation Plan: Task Confirmation Warning Polish + +## Overview + +Reduce the visual weight of the shared still-working warning so archive and +delete confirmations keep title and primary cleanup copy as the dominant +hierarchy. Give the fine-pointer archive popover modest extra width and remove +its source sidebar-row layout jitter. Deliver shared style/placement changes, +regression coverage, and focused desktop and phone rendered evidence without +changing text or behavior. + +## Scope + +### In scope + +- Compact shared warning typography, wrapping, spacing, and icon scale. +- Archive-only fine-pointer popover width opt-in with viewport containment. +- Fine-pointer archive mounting that keeps the originating sidebar row height + stable while preserving coarse-pointer inline expansion. +- Regression assertions for the shared warning contract in archive and delete + dialog tests. +- Regression assertions for archive width and fine-pointer sidebar row-height + stability. +- Focused production-build desktop and phone checks, screenshots, computed + hierarchy, containment, action reachability, and zero document horizontal + overflow. + +### Out of scope + +- Warning copy, translations, in-flight detection, archive/delete callbacks, + global confirmation-popover width, dialog geometry, focus, Escape, safe-area, + action sizes, or animation. + +## Technical approach + +Change `apps/web/components/task/task-still-working-warning.tsx` for the shared +visual contract. Keep the existing semantic classes and markup identity; use +compact body typography with `text-pretty`, a readable explicit line height, +and proportionally smaller padding, gap, and icon. + +Add a width/size contract to +`apps/web/components/confirmation/action-confirm-popover.tsx` with `w-64` as +the default. Pass the wider archive-only variant from +`TaskArchiveConfirmation`, with a viewport-aware max width. Keep title/body +pretty-wrapped and action geometry unchanged. + +At `TaskItemWithContextMenu`, use `useResponsiveBreakpoint` to keep the +fine-pointer archive confirmation outside the `TaskItem` action slot while +retaining the same anchor/focus refs and shared confirmation node. Continue +injecting the node into `TaskItem` for coarse-pointer inline confirmation. +This targets the observed `flex-wrap` plus `basis-full` extra-line cause, not +the row's dimensions. + +Add a style-hierarchy regression to the existing archive and delete warning +tests. Run it against current main first so the old `text-sm`, `p-3`, and 16px +icon contract fails, then make the shared style change and rerun the focused +tests green. No catalog changes are expected, but the affected-file i18n +ratchet/check remains part of verification. + +Use the real desktop sidebar archive flow and the phone +sidebar-task-actions inline confirmation coverage. Rebuild the production web +assets before Playwright. Capture before and after screenshots of the full +desktop dialog/popover and phone inline confirmation with the warning visible, +and inspect computed font size/line height, warning-to-description hierarchy, +popover width and viewport bounds, source-row height before/open/cancel, +button reachability, and document `scrollWidth`. + +## Tests + +| Acceptance criterion | Evidence | +| --- | --- | +| `AC-TASKS-CONFIRMATION-WARNING-001.1` | Archive and delete component warning tests assert compact class contract and existing warning text. | +| `AC-TASKS-CONFIRMATION-WARNING-001.2` | Existing archive/delete activity tests assert localized warning presence and `role=alert`; shared component keeps test ID and semantics. | +| `AC-TASKS-CONFIRMATION-WARNING-001.3` | Desktop card-action confirmation E2E plus phone inline confirmation E2E and rendered geometry checks. | +| `AC-TASKS-CONFIRMATION-SURFACE-002.1` | Action-confirm-popover width-contract test plus desktop sidebar computed width/bounds assertion; non-archive default remains `w-64`. | +| `AC-TASKS-CONFIRMATION-SURFACE-002.2` | Desktop sidebar E2E records exact source-row height before/open/cancel and fails on the current extra flex-line behavior. | +| `AC-TASKS-CONFIRMATION-SURFACE-002.3` | Desktop anchor/focus/callback checks plus phone inline 44px and zero-overflow checks. | + +## E2E tests + +- Desktop: `tests/task/archive-confirmation-preference.spec.ts` with a real + sidebar task row, fine-pointer archive popover width/bounds, and + before/open/cancel row-height measurements. Retain the existing + `tests/kanban/card-menu-delete-archive.spec.ts` archive/delete behavior + coverage and add full-dialog warning hierarchy checks where the seeded + in-flight task can open it. +- Phone: `tests/task/mobile-sidebar-task-actions.spec.ts` existing inline + confirmation scenario, extended only if needed to expose the warning state; + preserve 44px actions, in-viewport checks, and zero document overflow. +- Manual rendered evidence: capture `.pr-assets` before/after screenshots for + desktop full dialog and phone inline confirmation. Do not commit screenshot + binaries to the feature branch. + +## Work orders + +- [x] [Task 01: Compact shared confirmation warning and stable archive surface](task-01-compact-warning.md) + +## Verification results + +- RED: the new archive/delete compactness assertions and wide-popover contract + failed against the unchanged implementation (3 failed, 38 passed); the + desktop row-height regression measured 54.203125px before vs 62.203125px + while the old fine-pointer popover was open. +- GREEN: focused Vitest passed 5 files and 59 tests; affected ESLint passed; + web typecheck passed; `i18n:ratchet` passed with 0 new violations and 644 + guard entries; `make build-web` passed. +- GREEN rendered checks against rebuilt assets: desktop archive geometry/full + dialog 2/2, phone inline archive flows 2/2, and card archive/delete behavior + 5/5. Desktop assertions verified stable row height before/open/cancel, + popover width above 256px, viewport containment, and zero document overflow. + Phone assertions verified compact 12px/20px warning type, intentional inline + row expansion, 44px-or-larger in-viewport actions, and zero overflow. +- Fresh compressed media is captured under `apps/web/.pr-assets/`; screenshot + binaries remain uncommitted and are published through the PR media ref. + +## Risks + +- Shared styling affects delete as well as archive. Preserve the semantic + yellow treatment and run both dialog suites. +- Longer locale strings may wrap differently. `text-pretty`, viewport checks, + and the pseudo-locale build must remain clean. diff --git a/docs/plans/task-confirmation-warning-polish/task-01-compact-warning.md b/docs/plans/task-confirmation-warning-polish/task-01-compact-warning.md new file mode 100644 index 0000000000..6f8bac199c --- /dev/null +++ b/docs/plans/task-confirmation-warning-polish/task-01-compact-warning.md @@ -0,0 +1,143 @@ +--- +id: "01-compact-warning" +title: "Compact warning and stable archive surface" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - ../../specs/ui/requirements/confirmation-warning-hierarchy.md +acceptance_criteria: + - AC-TASKS-CONFIRMATION-WARNING-001.1 + - AC-TASKS-CONFIRMATION-WARNING-001.2 + - AC-TASKS-CONFIRMATION-WARNING-001.3 + - AC-TASKS-CONFIRMATION-SURFACE-002.1 + - AC-TASKS-CONFIRMATION-SURFACE-002.2 + - AC-TASKS-CONFIRMATION-SURFACE-002.3 +system_design: + - ../../specs/ui/system-design/confirmation-warning-hierarchy.md +--- + +# Task 01: Compact warning and stable archive surface + +## Summary + +Reduce shared still-working warning density in archive and delete confirmation +surfaces while preserving text, localization, alert semantics, in-flight +visibility, and action behavior. Widen only the fine-pointer archive popover +and remove its originating sidebar row's extra layout line while preserving +coarse-pointer inline expansion. Prove each contract with failing assertions +before production changes, then verify rendered desktop and phone states +against a rebuilt frontend. + +## In scope + +- Update the shared warning container and icon classes. +- Add an archive-only wider `ActionConfirmPopover` width contract, retaining + the `w-64` default for unrelated consumers and constraining it to the + viewport. +- Mount the fine-pointer archive confirmation outside `TaskItem`'s inline + action slot at the context-menu adapter boundary; keep coarse-pointer + confirmation row-owned. +- Add or update compactness assertions in archive and delete warning tests. +- Add a desktop sidebar RED/GREEN assertion for archive popover width, + viewport containment, and exact source-row height before/open/cancel. +- Run focused unit, lint/typecheck/i18n, production-build, desktop, and phone + rendered checks. +- Capture and inspect before/after PR screenshots and computed geometry evidence. + +## Out of scope + +- Warning copy or locale values. +- Global confirmation-popover width changes. +- Archive/delete state, callbacks, detection, dialog composition, action + dimensions, focus, Escape, safe-area, or animation. + +## Acceptance + +- The warning uses compact body typography with intentional line height and + pretty wrapping; spacing and icon scale are optically reduced in proportion. +- Archive and delete tests retain generating/background/idle behavior coverage, + `role=alert`, localized text, and shared warning identity. +- The archive popover is wider than 256px only for the archive variant, stays + inside the viewport at compact widths, and keeps title/body hierarchy. +- Fine-pointer sidebar row height is stable before opening, while open, and + after cancel; coarse-pointer inline confirmation remains intentionally + expanding. +- Desktop full-dialog and archive popover/inline paths remain contained; phone + actions stay reachable at 44px or larger and document horizontal overflow is + zero. + +## Verification + +```bash +(cd apps && pnpm install --frozen-lockfile) +(cd apps && pnpm --filter @kandev/web test -- --run components/task/task-archive-confirm-dialog.test.tsx components/task/task-delete-confirm-dialog.test.tsx components/task/task-archive-confirmation.test.tsx) +(cd apps && pnpm --filter @kandev/web test -- --run components/confirmation/action-confirm-popover.test.tsx components/task/task-switcher-context-menu.test.tsx) +(cd apps && pnpm --filter @kandev/web exec eslint components/task/task-still-working-warning.tsx components/task/task-archive-confirm-dialog.test.tsx components/task/task-delete-confirm-dialog.test.tsx components/task/task-archive-confirmation.test.tsx) +(cd apps && pnpm --filter @kandev/web exec eslint components/confirmation/action-confirm-popover.tsx components/task/task-archive-confirmation.tsx components/task/task-switcher-context-menu.tsx) +(cd apps/web && pnpm run typecheck) +(cd apps/web && pnpm run i18n:ratchet) +(make build-web) +(cd apps/web && pnpm e2e:run --project chromium tests/task/archive-confirmation-preference.spec.ts tests/kanban/card-menu-delete-archive.spec.ts) +(cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-sidebar-task-actions.spec.ts) +``` + +The compactness regression must fail before the shared class change and pass +after it. The final rendered checks must use the production web build and +record screenshot paths plus computed hierarchy, viewport containment, action +reachability, and document overflow results. + +## Files likely touched + +- `apps/web/components/task/task-still-working-warning.tsx` +- `apps/web/components/confirmation/action-confirm-popover.tsx` +- `apps/web/components/task/task-archive-confirmation.tsx` +- `apps/web/components/task/task-switcher-context-menu.tsx` +- `apps/web/components/task/task-archive-confirm-dialog.test.tsx` +- `apps/web/components/task/task-delete-confirm-dialog.test.tsx` +- `apps/web/components/confirmation/action-confirm-popover.test.tsx` +- `apps/web/components/task/task-switcher-context-menu.test.tsx` +- `apps/web/e2e/tests/task/archive-confirmation-preference.spec.ts` +- `apps/web/e2e/tests/kanban/card-menu-delete-archive.spec.ts` if a focused + desktop warning assertion is needed +- `apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts` if the existing + phone scenario needs a warning-state assertion +- `apps/web/.pr-assets/` only as uncommitted capture output + +## Dependencies + +None. + +## Risks + +- Shared change intentionally affects delete warning density too; check both + dialog variants and all supported locale wrapping. +- Do not use stale Vite assets for E2E. Rebuild after every frontend edit. + +## Parallelism + +`sequential` + +## Inputs + +- `docs/specs/ui/requirements/confirmation-warning-hierarchy.md` +- `docs/specs/ui/system-design/confirmation-warning-hierarchy.md` +- `apps/web/components/task/task-still-working-warning.tsx` +- Existing archive/delete warning tests and phone exemplar in + `apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts` + +## Results + +- RED compactness/width assertions failed against current main before the + production style and width changes. The old fine-pointer flow also grew the + source row from 54.203125px to 62.203125px while open. +- GREEN focused unit coverage: 5 files, 59 tests. Affected ESLint, web + typecheck, i18n ratchet, and `make build-web` passed. +- GREEN rendered coverage: desktop sidebar geometry and full dialog 2/2, + phone idle/in-flight inline archive 2/2, and card archive/delete behavior + 5/5. Width, containment, computed hierarchy, row stability, touch targets, + and zero-overflow assertions passed. +- Captured and inspected fresh compressed desktop and phone before/after media + in `apps/web/.pr-assets/`; media is intentionally not committed to the PR + branch. diff --git a/docs/specs/ui/README.md b/docs/specs/ui/README.md index fe48bb522b..3e14e4408f 100644 --- a/docs/specs/ui/README.md +++ b/docs/specs/ui/README.md @@ -125,6 +125,7 @@ contract that other capabilities can reuse. - [Slash Command Composer Selection](requirements/slash-command-composer.md) - [Subagent Observability](requirements/subagent-observability.md) - [Nested Submodule Review](requirements/submodule-review.md) +- [Task Confirmation Warning Hierarchy](requirements/confirmation-warning-hierarchy.md) - [Task Layout Profiles](requirements/task-layout-profiles.md) - [Task Listing Display Preferences](requirements/task-listing-display-preferences.md) - [Task transcript history visibility](requirements/task-prompt-transcript-visibility.md) @@ -161,6 +162,7 @@ contract that other capabilities can reuse. - [Persistent status motion](system-design/persistent-status-motion.md) - [Prompt History Panel](system-design/prompt-history-panel.md) - [Quick Chat and Terminal Tabs](system-design/quick-terminal.md) +- [Task Confirmation Warning Hierarchy](system-design/confirmation-warning-hierarchy.md) - [Resizable Markdown Table Columns](system-design/resizable-markdown-tables.md) - [Task Layout Profiles](system-design/task-layout-profiles.md) - [Terminal Rendering](system-design/terminal-rendering.md) diff --git a/docs/specs/ui/requirements/confirmation-warning-hierarchy.md b/docs/specs/ui/requirements/confirmation-warning-hierarchy.md new file mode 100644 index 0000000000..4f00e21a9a --- /dev/null +++ b/docs/specs/ui/requirements/confirmation-warning-hierarchy.md @@ -0,0 +1,82 @@ +--- +status: active +system: ui +created: 2026-08-24 +owners: + - kandev +--- + +# Task Confirmation Warning Hierarchy Requirements + +## Overview + +Task archive and delete confirmations reuse one in-flight caution. Its visual +treatment must read as supporting warning copy, not compete with the +confirmation title or primary cleanup description. The fine-pointer archive +popover must also give its copy enough room without changing unrelated +confirmation surfaces or changing the originating sidebar row's geometry. + +## Terminology + +- **Still-working warning:** The localized caution shown when the task or one + of the selected tasks has generating or background activity. + +## Requirements + +### REQ-TASKS-CONFIRMATION-WARNING-001: Compact still-working warning + +**Intent:** Make in-flight caution easy to scan while preserving its semantic +meaning and the existing archive and delete workflows. + +#### Acceptance criteria + +- **AC-TASKS-CONFIRMATION-WARNING-001.1:** When an archive or delete + confirmation renders for an in-flight task, the still-working warning shall + use compact secondary typography with readable line height and deliberate + short-text wrapping, and its icon, spacing, and padding shall remain optically + balanced with that typography. +- **AC-TASKS-CONFIRMATION-WARNING-001.2:** When the warning renders in a full + dialog, desktop archive popover, archive inline confirmation, or shared delete + dialog, it shall preserve its localized text, `role=alert`, restrained yellow + semantic treatment, and existing in-flight visibility conditions. +- **AC-TASKS-CONFIRMATION-WARNING-001.3:** At desktop and phone confirmation + widths, the warning shall remain contained without document-level horizontal + overflow, and existing confirmation actions shall remain reachable with their + current touch-target geometry. + +### REQ-TASKS-CONFIRMATION-SURFACE-002: Stable fine-pointer archive surface + +**Intent:** Give the fine-pointer archive confirmation readable width while +keeping the source sidebar row stable and preserving the intentional coarse +pointer inline confirmation behavior. + +#### Acceptance criteria + +- **AC-TASKS-CONFIRMATION-SURFACE-002.1:** The fine-pointer archive popover + shall use an archive-only width opt-in wider than the existing 256px default + (targeting a modest `w-72` contract after rendered inspection), with a + viewport-aware maximum width. Unrelated `ActionConfirmPopover` consumers + shall retain the existing `w-64` default. +- **AC-TASKS-CONFIRMATION-SURFACE-002.2:** Opening and cancelling the + fine-pointer archive popover shall leave the originating sidebar task row's + `getBoundingClientRect().height` stable within normal subpixel precision. + The implementation shall remove the source of any extra flex line rather + than mask it with fixed or minimum row height, negative margins, or a + tolerance-only assertion. +- **AC-TASKS-CONFIRMATION-SURFACE-002.3:** The fine-pointer popover shall + preserve its existing anchor, focus-return boundary, confirmation callbacks, + 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. + +## Out of scope + +- Rewriting or re-translating warning text. +- Widening all confirmation popovers globally. +- Changing in-flight detection, archive behavior, delete behavior, API + contracts, focus handling, Escape handling, safe-area handling, or action + dimensions. +- Masking sidebar row layout changes with fixed/minimum heights or negative + offsets. +- Adding animation or redesigning any confirmation surface beyond the shared + warning density. diff --git a/docs/specs/ui/system-design/confirmation-warning-hierarchy.md b/docs/specs/ui/system-design/confirmation-warning-hierarchy.md new file mode 100644 index 0000000000..5f63174cca --- /dev/null +++ b/docs/specs/ui/system-design/confirmation-warning-hierarchy.md @@ -0,0 +1,125 @@ +--- +status: current +system: ui +requirements: + - REQ-TASKS-CONFIRMATION-WARNING-001 + - REQ-TASKS-CONFIRMATION-SURFACE-002 +--- + +# Task Confirmation Warning Hierarchy System Design + +## Purpose and boundaries + +This design owns the presentation contract for the shared still-working warning +and the fine-pointer archive confirmation surface used by task archive and +delete workflows. It changes density, archive-only width, and mounting location +only; the task in-flight signal and destructive-action behavior remain owned by +their 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) | + +## Components and responsibilities + +- `apps/web/components/task/task-still-working-warning.tsx` remains the single + 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. +- Consumers continue to decide whether a warning mounts. The shared component + does not inspect task state or alter callbacks. + +### Popover width contract + +`ActionConfirmPopover` gains a small width/size contract whose default remains +the current `w-64` surface. The archive confirmation opts into a modest wider +variant, targeting `w-72`, with a viewport-aware maximum width such as +`max-w-[calc(100vw-1rem)]`. The class contract stays local to the shared +popover primitive, so watcher and other confirmation consumers do not widen +implicitly. The archive body keeps `text-pretty` wrapping and the existing +title/action hierarchy. + +### Fine-pointer mounting + +At the `TaskItemWithContextMenu` adapter boundary, `useResponsiveBreakpoint` +determines where the already-created archive confirmation node mounts: + +- Fine-pointer confirmation mounts as a sibling of the cloned task row inside + the existing anchor wrapper. It does not pass `archiveConfirmation` into + `TaskItem`, so `TaskItem` does not add its `flex-wrap` row branch or the + `basis-full` action slot for a portaled popover. +- Coarse-pointer confirmation continues to pass through `TaskItem`'s existing + inline action slot. Its intentional row expansion and mobile action geometry + remain unchanged. + +Both branches reuse the same `useTaskSwitcherArchiveConfirmation` node, +callbacks, anchor ref, and focus-return ref. No business logic or duplicate +confirmation markup is introduced. The change removes the layout cause of +fine-pointer row growth instead of compensating for it with row dimensions. + +## Data and contracts + +The component preserves `data-testid="still-working-warning"`, `role="alert"`, +the translated `task:stillWorkingWarning` and subject keys, and the existing +yellow border/background/text classes. The compact style contract is: + +- warning container: `gap-1.5`, `p-2.5`, `text-xs`, `leading-5`, and + `text-pretty`; +- warning icon: `h-3.5 w-3.5`, `mt-0.5`, and `shrink-0`; +- existing rounded border, yellow semantic colors, and dark-mode contrast stay + unchanged. + +No API, WebSocket, state, localization catalog, or persisted-data contract +changes are required. + +## Control flow + +The existing task-level `foregroundActivity` projection and explicit +`isInFlight` props continue to determine whether the warning is rendered. Once +mounted, the shared component formats the same localized subject and warning +text, then returns the compact alert markup. 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. + +## Failure and recovery + +No new failure path exists. If localized text is longer than the available +width, `text-pretty` and the explicit line height allow natural wrapping inside +the existing flex container. If no task activity is present, no warning mounts, +as before. + +## Persistence + +None. This is a client-side presentation-only change. + +## Observability + +Existing component tests continue to assert warning presence and absence for +generating, background, and idle activity. A compactness regression asserts the +shared class contract. A focused archive-popover regression asserts the wider +archive-only class contract. Rendered desktop and phone checks inspect +computed type hierarchy, popover width and viewport bounds, source-row height +before/open/cancel, action reachability, and document overflow. + +## Mobile and desktop containment + +The desktop check uses the real sidebar task row with a fine pointer. It +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. + +The phone check keeps the existing coarse-pointer sidebar inline flow. It +expects the inline confirmation to remain intentionally row-owned, keeps +actions at or above 44px, and asserts zero document horizontal overflow. + +## Related decisions + +- [ADR 0049: Fine-grained foreground-idle busy signal](../../../decisions/0049-fine-grained-foreground-idle-busy-signal.md) +- [Mobile task navigation](../requirements/mobile-task-navigation.md)