diff --git a/apps/web/components/kanban-with-preview.tsx b/apps/web/components/kanban-with-preview.tsx index 72ea148399..fcef4471c5 100644 --- a/apps/web/components/kanban-with-preview.tsx +++ b/apps/web/components/kanban-with-preview.tsx @@ -22,6 +22,10 @@ import type { KanbanState } from "@/lib/state/slices"; import { PREVIEW_PANEL } from "@/lib/settings/constants"; import { linkToTask } from "@/lib/links"; import { findTaskInSnapshots } from "@/lib/kanban/find-task"; +import { + usePreviewWorkflowStepMove, + type PreviewStepMove, +} from "@/hooks/domains/kanban/use-preview-workflow-step-move"; import { useEnsureTaskSession, type UseEnsureTaskSessionResult, @@ -87,17 +91,21 @@ function useUrlSync(selectedTaskId: string | null, selectedTaskSessionId: string }, [selectedTaskId, selectedTaskSessionId]); } -function useEscapeKey(isOpen: boolean, close: () => void) { +// The step disclosure closes itself on Escape via its own document-level +// Radix handling; without this guard the preview's window-level listener +// would also fire on the same keypress and close the whole panel instead of +// leaving the first Escape to the disclosure alone. +function useEscapeKey(isOpen: boolean, close: () => void, isDisclosureOpen: () => boolean) { useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { - if (e.key === "Escape" && isOpen) { + if (e.key === "Escape" && isOpen && !isDisclosureOpen()) { close(); } }; window.addEventListener("keydown", handleKeyDown); return () => window.removeEventListener("keydown", handleKeyDown); - }, [isOpen, close]); + }, [isOpen, close, isDisclosureOpen]); } function useResizeHandler( @@ -190,6 +198,7 @@ function useSelectedTask( workflowStepId: task.workflowStepId, workflowId, state: task.state, + isArchived: task.isArchived, description: task.description, position: task.position, repositoryId: task.repositoryId, @@ -198,6 +207,21 @@ function useSelectedTask( }, [selectedTaskId, kanbanTasks, snapshots]); } +// Mirror the previewed task id into the store so kanban cards can highlight +// the currently-previewed card without prop-drilling through swimlanes. +function useMirrorPreviewedTaskId( + isOpen: boolean, + selectedTaskId: string | null | undefined, + setKanbanPreviewedTaskId: (taskId: string | null) => void, +) { + useEffect(() => { + setKanbanPreviewedTaskId(isOpen ? (selectedTaskId ?? null) : null); + }, [isOpen, selectedTaskId, setKanbanPreviewedTaskId]); + useEffect(() => { + return () => setKanbanPreviewedTaskId(null); + }, [setKanbanPreviewedTaskId]); +} + function useCloseMissingSelectedTask(params: { isOpen: boolean; selectedTaskId: string | null | undefined; @@ -285,14 +309,7 @@ export function KanbanWithPreview({ initialTaskId, initialSessionId }: KanbanWit }, }); - // Mirror the previewed task id into the store so kanban cards can highlight - // the currently-previewed card without prop-drilling through swimlanes. - useEffect(() => { - setKanbanPreviewedTaskId(isOpen ? (selectedTaskId ?? null) : null); - }, [isOpen, selectedTaskId, setKanbanPreviewedTaskId]); - useEffect(() => { - return () => setKanbanPreviewedTaskId(null); - }, [setKanbanPreviewedTaskId]); + useMirrorPreviewedTaskId(isOpen, selectedTaskId, setKanbanPreviewedTaskId); // Use custom hooks for layout and session management const { containerRef, shouldFloat, kanbanWidth } = useKanbanLayout(isOpen, previewWidthPx); @@ -309,6 +326,7 @@ export function KanbanWithPreview({ initialTaskId, initialSessionId }: KanbanWit const isResizingRef = useRef(false); const selectedTask = useSelectedTask(selectedTaskId, kanbanTasks, kanbanMultiSnapshots); + const previewStepMove = usePreviewWorkflowStepMove(selectedTaskId, selectedTask); useCloseMissingSelectedTask({ isOpen, @@ -361,7 +379,7 @@ export function KanbanWithPreview({ initialTaskId, initialSessionId }: KanbanWit [isOpen, selectedTaskId, open, close], ); - useEscapeKey(isOpen, close); + useEscapeKey(isOpen, close, previewStepMove.isDisclosureOpen); const handleResizeMouseDown = useResizeHandler(isResizingRef, previewWidthPx, updatePreviewWidth); @@ -384,6 +402,7 @@ export function KanbanWithPreview({ initialTaskId, initialSessionId }: KanbanWit selectedTask={selectedTask} activeSessionId={activeSessionId} ensureSession={ensureSession} + stepMove={previewStepMove} onPreviewTask={handlePreviewTaskWithData} onNavigateToTask={handleNavigateToTask} onClose={close} @@ -427,6 +446,7 @@ type PreviewLayoutProps = { selectedTask: Task | null; activeSessionId: string | null; ensureSession: UseEnsureTaskSessionResult; + stepMove: PreviewStepMove; onPreviewTask: (task: Task) => void; onNavigateToTask: (task: Task) => void; onClose: () => void; @@ -434,12 +454,26 @@ type PreviewLayoutProps = { onResizeMouseDown: (e: React.MouseEvent) => void; }; +function previewPanelStepProps(stepMove: PreviewStepMove) { + return { + workflowSteps: stepMove.workflowSteps, + currentStepId: stepMove.currentStepId, + taskWorkflowId: stepMove.taskWorkflowId, + isArchived: stepMove.isArchived, + movingToStepId: stepMove.movingToStepId, + onMoveStep: stepMove.handleMove, + onDisclosureOpenChange: stepMove.handleDisclosureOpenChange, + moveError: stepMove.moveError, + }; +} + function FloatingPreviewLayout({ kanbanWidth, previewWidthPx, selectedTask, activeSessionId, ensureSession, + stepMove, onPreviewTask, onNavigateToTask, onClose, @@ -477,6 +511,7 @@ function FloatingPreviewLayout({ onClose={onClose} onMaximize={(task) => onNavigateToTask(task)} onSessionChange={onSessionChange} + {...previewPanelStepProps(stepMove)} /> @@ -491,6 +526,7 @@ function InlinePreviewLayout({ selectedTask, activeSessionId, ensureSession, + stepMove, onPreviewTask, onNavigateToTask, onClose, @@ -520,6 +556,7 @@ function InlinePreviewLayout({ onClose={onClose} onMaximize={(task) => onNavigateToTask(task)} onSessionChange={onSessionChange} + {...previewPanelStepProps(stepMove)} /> diff --git a/apps/web/components/kanban/columns-menu.tsx b/apps/web/components/kanban/columns-menu.tsx index 9d288e2c35..1b584ba377 100644 --- a/apps/web/components/kanban/columns-menu.tsx +++ b/apps/web/components/kanban/columns-menu.tsx @@ -13,7 +13,7 @@ import { import { IconColumns3 } from "@tabler/icons-react"; import { cn } from "@kandev/ui/lib/utils"; import { useTranslation } from "react-i18next"; -import { sortWorkflowStepsByPosition } from "@/lib/kanban/auto-hide-empty-columns"; +import { sortWorkflowStepsByPosition } from "@/lib/kanban/workflow-step-order"; export type ColumnsMenuStep = { id: string; title: string; position: number }; diff --git a/apps/web/components/kanban/swimlane-container.tsx b/apps/web/components/kanban/swimlane-container.tsx index 59124c8fa7..aab85b6bc4 100644 --- a/apps/web/components/kanban/swimlane-container.tsx +++ b/apps/web/components/kanban/swimlane-container.tsx @@ -29,10 +29,8 @@ import { import { reorderWorkflows } from "@/lib/api"; import { SwimlaneSection } from "./swimlane-section"; import { ColumnsMenu } from "./columns-menu"; -import { - deriveAutoHiddenStepIds, - sortWorkflowStepsByPosition, -} from "@/lib/kanban/auto-hide-empty-columns"; +import { deriveAutoHiddenStepIds } from "@/lib/kanban/auto-hide-empty-columns"; +import { sortWorkflowStepsByPosition } from "@/lib/kanban/workflow-step-order"; import { useKanbanDisplaySettings } from "@/hooks/use-kanban-display-settings"; import { getEffectiveView, diff --git a/apps/web/components/task-preview-panel.test.tsx b/apps/web/components/task-preview-panel.test.tsx new file mode 100644 index 0000000000..4f86071ae5 --- /dev/null +++ b/apps/web/components/task-preview-panel.test.tsx @@ -0,0 +1,189 @@ +import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; +import type { ComponentProps } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { TaskPreviewPanel } from "./task-preview-panel"; +import type { WorkflowStepperStep } from "./task/workflow-step-disclosure"; +import type { Task } from "./kanban-card"; + +vi.mock("@/components/state-provider", () => ({ + useAppStore: () => "workspace-1", +})); + +vi.mock("./task/preview-session-tabs", () => ({ + PreviewSessionTabs: () =>
, +})); + +vi.mock("@/hooks/use-compact-task-chrome", () => ({ + useTouchDrawer: () => false, +})); + +afterEach(cleanup); + +const TASK: Task = { + id: "task-1", + title: "Fix the login bug", +} as Task; + +const OTHER_TASK: Task = { + id: "task-2", + title: "Write the release notes", +} as Task; + +const WORKFLOW_ID = "workflow-1"; +const STEP_A_ID = "a"; +const STEPPER_TEST_ID = "workflow-stepper-minimal"; +const DISCLOSURE_TEST_ID = "workflow-step-disclosure"; + +const STEPS: WorkflowStepperStep[] = [ + { id: "a", name: "Spec", color: "#111", position: 0 }, + { id: "b", name: "Work", color: "#222", position: 1, allow_manual_move: true }, +]; + +function renderPanel(overrides: Partial> = {}) { + return render( + , + ); +} + +describe("TaskPreviewPanel step indicator", () => { + it("shows no step indicator when there are no resolved workflow steps", () => { + renderPanel({ workflowSteps: [] }); + + expect(screen.getByText("Fix the login bug")).toBeTruthy(); + expect(screen.queryByTestId(STEPPER_TEST_ID)).toBeNull(); + }); + + it("shows no step indicator when there is no selected task", () => { + renderPanel({ task: null }); + + expect(screen.queryByTestId(STEPPER_TEST_ID)).toBeNull(); + }); + + it("shows the current step and total between the title and the panel controls", () => { + renderPanel(); + + expect(screen.getByTestId(STEPPER_TEST_ID)).toBeTruthy(); + expect(screen.getByText("1/2")).toBeTruthy(); + }); + + it("issues a move for an eligible target and closes the disclosure on success", async () => { + const onMoveStep = vi.fn().mockResolvedValue(true); + renderPanel({ onMoveStep }); + + fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); + await act(async () => { + fireEvent.click(screen.getByTestId("workflow-step-disclosure-move-b")); + await Promise.resolve(); + }); + + expect(onMoveStep).toHaveBeenCalledWith("b"); + expect(screen.queryByTestId(DISCLOSURE_TEST_ID)).toBeNull(); + }); + + it("notifies the caller when the disclosure opens and closes", () => { + const onDisclosureOpenChange = vi.fn(); + renderPanel({ onDisclosureOpenChange }); + + expect(onDisclosureOpenChange).toHaveBeenCalledWith(false); + fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); + expect(onDisclosureOpenChange).toHaveBeenCalledWith(true); + }); + + it("renders the move-failure message below the header, not inside it", () => { + renderPanel({ moveError: new Error("boom") }); + + const banner = screen.getByTestId("task-move-error-banner"); + expect(banner).toBeTruthy(); + const headerRow = screen.getByText("Fix the login bug").closest(".border-b"); + expect(headerRow?.contains(banner)).toBe(false); + }); + + it("shows no move-failure message when there is none", () => { + renderPanel({ workflowSteps: [] }); + expect(screen.queryByTestId("task-move-error-banner")).toBeNull(); + }); + + it("closes the disclosure when the previewed task changes while it is open", () => { + const { rerender } = renderPanel(); + + fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); + expect(screen.getByTestId(DISCLOSURE_TEST_ID)).toBeTruthy(); + + rerender( + , + ); + + expect(screen.queryByTestId(DISCLOSURE_TEST_ID)).toBeNull(); + expect(screen.getByText(OTHER_TASK.title)).toBeTruthy(); + }); + + it("does not let a move started before a task switch close the new task's disclosure", async () => { + let resolveMove!: (moved: boolean) => void; + const onMoveStepForA = vi.fn(() => new Promise((resolve) => (resolveMove = resolve))); + const { rerender } = renderPanel({ onMoveStep: onMoveStepForA }); + + fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); + fireEvent.click(screen.getByTestId("workflow-step-disclosure-move-b")); + expect(onMoveStepForA).toHaveBeenCalledWith("b"); + + rerender( + , + ); + + fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); + expect(screen.getByTestId(DISCLOSURE_TEST_ID)).toBeTruthy(); + + await act(async () => { + resolveMove(true); + await Promise.resolve(); + }); + + expect(screen.getByTestId(DISCLOSURE_TEST_ID)).toBeTruthy(); + }); + + it("shows the archived badge and offers no move control for an archived task", () => { + const onMoveStep = vi.fn(); + renderPanel({ isArchived: true, onMoveStep }); + + expect(screen.queryByTestId("workflow-step-disclosure-move-b")).toBeNull(); + fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); + expect(screen.queryByTestId(DISCLOSURE_TEST_ID)).toBeNull(); + expect(onMoveStep).not.toHaveBeenCalled(); + }); +}); + +describe("TaskPreviewPanel step indicator lifecycle", () => { + it("reports the disclosure closed when the stepper unmounts", () => { + const onDisclosureOpenChange = vi.fn(); + const { rerender } = renderPanel({ onDisclosureOpenChange }); + + fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); + expect(onDisclosureOpenChange).toHaveBeenLastCalledWith(true); + + rerender(); + + expect(onDisclosureOpenChange).toHaveBeenLastCalledWith(false); + }); +}); diff --git a/apps/web/components/task-preview-panel.tsx b/apps/web/components/task-preview-panel.tsx index 8bec87a264..e9200a120c 100644 --- a/apps/web/components/task-preview-panel.tsx +++ b/apps/web/components/task-preview-panel.tsx @@ -6,6 +6,8 @@ import { useAppStore } from "@/components/state-provider"; import type { UseEnsureTaskSessionResult } from "@/hooks/domains/session/use-ensure-task-session"; import type { Task } from "./kanban-card"; import { PreviewSessionTabs } from "./task/preview-session-tabs"; +import { TaskMoveErrorBanner } from "./task/task-move-error-banner"; +import { MinimalWorkflowStepper, type WorkflowStepperStep } from "./task/workflow-step-disclosure"; import { useTranslation } from "react-i18next"; interface TaskPreviewPanelProps { @@ -15,6 +17,96 @@ interface TaskPreviewPanelProps { onClose: () => void; onMaximize?: (task: Task) => void; onSessionChange?: (sessionId: string | null) => void; + workflowSteps?: WorkflowStepperStep[]; + currentStepId?: string | null; + taskWorkflowId?: string | null; + isArchived?: boolean; + movingToStepId?: string | null; + onMoveStep?: (stepId: string) => Promise; + onDisclosureOpenChange?: (open: boolean) => void; + moveError?: unknown; +} + +interface PreviewPanelHeaderProps { + task: Task | null; + onClose: () => void; + onMaximize?: (task: Task) => void; + workflowSteps: WorkflowStepperStep[]; + currentStepId: string | null; + taskWorkflowId: string | null; + isArchived: boolean; + movingToStepId: string | null; + onMoveStep?: (stepId: string) => Promise; + onDisclosureOpenChange?: (open: boolean) => void; +} + +function PreviewPanelHeader({ + task, + onClose, + onMaximize, + workflowSteps, + currentStepId, + taskWorkflowId, + isArchived, + movingToStepId, + onMoveStep, + onDisclosureOpenChange, +}: PreviewPanelHeaderProps) { + const { t } = useTranslation(); + const currentIndex = workflowSteps.findIndex((step) => step.id === currentStepId); + const handleMoveStep = task && workflowSteps.length > 0 ? onMoveStep : undefined; + return ( +
+
+

+ {task?.title ?? t("task:taskChat")} +

+ {handleMoveStep && ( +
normally sizes to its content regardless of `display`, + // so without an explicit width it overflows this shrink-capped + // wrapper instead of shrinking to it (defeating truncation). + className="min-w-0 max-w-[50%] shrink [&>button]:w-full" + > + +
+ )} +
+
+ {onMaximize && task && ( + + )} + +
+
+ ); } export function TaskPreviewPanel({ @@ -24,6 +116,14 @@ export function TaskPreviewPanel({ onClose, onMaximize, onSessionChange, + workflowSteps = [], + currentStepId = null, + taskWorkflowId = null, + isArchived = false, + movingToStepId = null, + onMoveStep, + onDisclosureOpenChange, + moveError = null, }: TaskPreviewPanelProps) { const { t } = useTranslation(); const activeWorkspaceId = useAppStore((s) => s.workspaces.activeId); @@ -32,28 +132,20 @@ export function TaskPreviewPanel({ data-testid="task-preview-panel" className="flex h-full w-full flex-col border-l bg-background" > - {/* Header */} -
-

{task?.title ?? t("task:taskChat")}

-
- {onMaximize && task && ( - - )} - -
-
+ + + {moveError !== null && } {/* Content */}
diff --git a/apps/web/components/task/workflow-step-disclosure.tsx b/apps/web/components/task/workflow-step-disclosure.tsx index 3fb3a370dc..efa9019cd9 100644 --- a/apps/web/components/task/workflow-step-disclosure.tsx +++ b/apps/web/components/task/workflow-step-disclosure.tsx @@ -48,6 +48,8 @@ type MinimalWorkflowStepperProps = { workflowId?: string | null; movingToStepId: string | null; onMove: (stepId: string) => Promise; + /** Notified whenever the disclosure surface opens or closes. */ + onDisclosureOpenChange?: (open: boolean) => void; }; export function MinimalWorkflowStepper({ @@ -58,6 +60,7 @@ export function MinimalWorkflowStepper({ workflowId, movingToStepId, onMove, + onDisclosureOpenChange, }: MinimalWorkflowStepperProps) { const { t } = useTranslation(); @@ -95,6 +98,7 @@ export function MinimalWorkflowStepper({ workflowId={workflowId} movingToStepId={movingToStepId} onMove={onMove} + onDisclosureOpenChange={onDisclosureOpenChange} /> ); } @@ -308,6 +312,7 @@ function CompactWorkflowStepDisclosure({ workflowId, movingToStepId, onMove, + onDisclosureOpenChange, }: { sortedSteps: Step[]; current: Step; @@ -317,10 +322,20 @@ function CompactWorkflowStepDisclosure({ workflowId: string; movingToStepId: string | null; onMove: (stepId: string) => Promise; + onDisclosureOpenChange?: (open: boolean) => void; }) { const { t } = useTranslation(); const usesTouchDrawer = useTouchDrawer(); const controls = useCompactWorkflowDisclosure(); + useEffect(() => { + onDisclosureOpenChange?.(controls.open); + }, [controls.open, onDisclosureOpenChange]); + useEffect( + () => () => { + onDisclosureOpenChange?.(false); + }, + [onDisclosureOpenChange], + ); const trigger = ( event.stopPropagation()} onMouseEnter={controls.openDisclosure} onMouseLeave={controls.scheduleClose} onFocusCapture={controls.handleContentFocus} @@ -423,8 +439,8 @@ function MinimalStepContents({ aria-current={currentIndex >= 0 ? "step" : undefined} className="flex min-w-0 items-center gap-1.5 text-xs" > - - + = 0} isCompleted={false} /> + {current.name}
@@ -531,6 +547,8 @@ export function canMoveToStep(params: { return params.isAdjacent || !!params.allowManualMove; } +export type StepMarkerState = "current" | "completed" | "upcoming"; + export function StepCircleIndicator({ isCurrent, isCompleted, @@ -538,9 +556,15 @@ export function StepCircleIndicator({ isCurrent: boolean; isCompleted: boolean; }) { + let state: StepMarkerState = "upcoming"; + if (isCurrent) state = "current"; + else if (isCompleted) state = "completed"; if (isCurrent) { return ( - + @@ -548,13 +572,16 @@ export function StepCircleIndicator({ } if (isCompleted) { return ( - + ); } return ( - + ); diff --git a/apps/web/components/task/workflow-stepper.test.tsx b/apps/web/components/task/workflow-stepper.test.tsx index f63a884afc..42da34aba6 100644 --- a/apps/web/components/task/workflow-stepper.test.tsx +++ b/apps/web/components/task/workflow-stepper.test.tsx @@ -112,6 +112,8 @@ const MOVE_A_TEST_ID = "workflow-step-disclosure-move-a"; const MOVE_C_TEST_ID = "workflow-step-disclosure-move-c"; const MOVE_D_TEST_ID = "workflow-step-disclosure-move-d"; const TRIGGER_LABEL = "Step 2 of 4: Work"; +const SPEC_TEST_ID = "workflow-step-Spec"; +const ARIA_CURRENT = "aria-current"; describe("WorkflowStepper", () => { it("renders every step when there is room (not collapsed)", () => { @@ -121,11 +123,25 @@ describe("WorkflowStepper", () => { expect(screen.getByTestId("workflow-stepper")).toBeTruthy(); expect(screen.queryByTestId("workflow-stepper-minimal")).toBeNull(); // All steps render under the persistent outer container. - expect(screen.getByTestId("workflow-step-Spec")).toBeTruthy(); + expect(screen.getByTestId(SPEC_TEST_ID)).toBeTruthy(); expect(screen.getByTestId("workflow-step-Work")).toBeTruthy(); expect(screen.getByTestId("workflow-step-Review")).toBeTruthy(); }); + it("orders steps sharing a position by ascending id, not insertion order", () => { + collapsedMock.mockReturnValue(false); + const TIEBREAK_STEPS: WorkflowStepperStep[] = [ + { id: "z", name: "ZStep", color: "#111", position: 0 }, + { id: "a", name: "AStep", color: "#222", position: 0 }, + ]; + const { container } = render(); + + const aIndex = container.innerHTML.indexOf("AStep"); + const zIndex = container.innerHTML.indexOf("ZStep"); + expect(aIndex).toBeGreaterThan(-1); + expect(aIndex).toBeLessThan(zIndex); + }); + it("collapses to only the current step when space runs out", () => { collapsedMock.mockReturnValue(true); render(); @@ -136,8 +152,8 @@ describe("WorkflowStepper", () => { // Current step keeps its test id + aria-current in either variant. const current = screen.getByTestId("workflow-step-Work"); - expect(current.getAttribute("aria-current")).toBe("step"); - expect(screen.queryByTestId("workflow-step-Spec")).toBeNull(); + expect(current.getAttribute(ARIA_CURRENT)).toBe("step"); + expect(screen.queryByTestId(SPEC_TEST_ID)).toBeNull(); expect(screen.queryByTestId("workflow-step-Review")).toBeNull(); // Position indicator reflects the current step out of the total. @@ -164,7 +180,7 @@ describe("WorkflowStepper compact disclosure", () => { expect(screen.getByTestId(DISCLOSURE_TEST_ID)).toBeTruthy(); expect(screen.getByTestId("workflow-step-disclosure-row-a")).toBeTruthy(); - expect(screen.getByTestId("workflow-step-disclosure-row-b").getAttribute("aria-current")).toBe( + expect(screen.getByTestId("workflow-step-disclosure-row-b").getAttribute(ARIA_CURRENT)).toBe( "step", ); expect(screen.getByTestId("workflow-step-disclosure-row-c")).toBeTruthy(); @@ -248,10 +264,27 @@ describe("WorkflowStepper fallback states", () => { render(); // Fallback step isn't the real current step, so it must not claim aria-current. - expect(screen.getByTestId("workflow-step-Spec").getAttribute("aria-current")).toBeNull(); + const fallbackStep = screen.getByTestId(SPEC_TEST_ID); + expect(fallbackStep.getAttribute(ARIA_CURRENT)).toBeNull(); + // aria-current alone is a false negative here: it was already conditioned + // correctly before the fix. The marker itself must also not read "current". + expect( + fallbackStep.querySelector("[data-marker-state]")?.getAttribute("data-marker-state"), + ).toBe("upcoming"); expect(screen.getByText("1/3")).toBeTruthy(); }); + it("marks the marker current when collapsed on a resolved current step", () => { + collapsedMock.mockReturnValue(true); + render(); + + const currentStep = screen.getByTestId(SPEC_TEST_ID); + expect(currentStep.getAttribute(ARIA_CURRENT)).toBe("step"); + expect( + currentStep.querySelector("[data-marker-state]")?.getAttribute("data-marker-state"), + ).toBe("current"); + }); + it("shows the archived badge instead of a step when collapsed and archived", () => { collapsedMock.mockReturnValue(true); render( diff --git a/apps/web/components/task/workflow-stepper.tsx b/apps/web/components/task/workflow-stepper.tsx index dd21475f9c..be06c91cbb 100644 --- a/apps/web/components/task/workflow-stepper.tsx +++ b/apps/web/components/task/workflow-stepper.tsx @@ -1,17 +1,17 @@ "use client"; -import { memo, useCallback, useMemo, useRef, useState } from "react"; +import { memo, useMemo, useRef } from "react"; import { cn } from "@kandev/ui/lib/utils"; import { HoverCard, HoverCardContent, HoverCardTrigger } from "@kandev/ui/hover-card"; import { Button } from "@kandev/ui/button"; import { IconArrowRight } from "@tabler/icons-react"; -import { moveTask } from "@/lib/api"; import { StepCapabilityIcons } from "@/components/step-capability-icons"; -import { useAppStore } from "@/components/state-provider"; -import { useContextFilesStore } from "@/lib/state/context-files-store"; -import { useLayoutStore } from "@/lib/state/layout-store"; -import { useDockviewStore } from "@/lib/state/dockview-store"; import { useToolbarCollapsed } from "@/hooks/use-toolbar-collapsed"; +import { + usePresentationToken, + useWorkflowStepMove, +} from "@/hooks/domains/kanban/use-workflow-step-move"; +import { sortWorkflowStepsByPosition } from "@/lib/kanban/workflow-step-order"; import { useTranslation } from "react-i18next"; import { MinimalWorkflowStepper, @@ -23,38 +23,6 @@ import { type Step = WorkflowStepperStep; -const PLAN_CONTEXT_PATH = "plan:context"; - -/** Returns a callback that disables plan mode for the active session of a task. */ -function useDisablePlanMode() { - const activeSessionId = useAppStore((s) => s.tasks.activeSessionId); - const planModeEnabled = useAppStore((s) => - activeSessionId ? (s.chatInput.planModeBySessionId[activeSessionId] ?? false) : false, - ); - const setPlanMode = useAppStore((s) => s.setPlanMode); - const setActiveDocument = useAppStore((s) => s.setActiveDocument); - const closeDocument = useLayoutStore((s) => s.closeDocument); - const removeContextFile = useContextFilesStore((s) => s.removeFile); - const applyBuiltInPreset = useDockviewStore((s) => s.applyBuiltInPreset); - - return useCallback(() => { - if (!activeSessionId || !planModeEnabled) return; - applyBuiltInPreset("default"); - closeDocument(activeSessionId); - setActiveDocument(activeSessionId, null); - setPlanMode(activeSessionId, false); - removeContextFile(activeSessionId, PLAN_CONTEXT_PATH); - }, [ - activeSessionId, - planModeEnabled, - setPlanMode, - setActiveDocument, - closeDocument, - removeContextFile, - applyBuiltInPreset, - ]); -} - type WorkflowStepperProps = { steps: Step[]; currentStepId: string | null; @@ -75,45 +43,26 @@ const WorkflowStepper = memo(function WorkflowStepper({ onMoveError, }: WorkflowStepperProps) { const { t } = useTranslation(); - const [movingToStepId, setMovingToStepId] = useState(null); - const disablePlanMode = useDisablePlanMode(); - // Only the in-flight step's own button is disabled, so every other step stays - // clickable and two moves can overlap. This counter marks which one is the - // latest; a slower predecessor must not own the banner or the loading state. - const moveRequestRef = useRef(0); + // The task route's continuous presentation of this task: a navigation away + // and back changes taskId and back, which must invalidate a request left + // over from the earlier presentation the same way a preview close-and-reopen + // does for the kanban preview header. + const presentationToken = usePresentationToken(taskId ?? null); + const { movingToStepId, handleMove } = useWorkflowStepMove({ + taskId, + workflowId, + presentationToken, + onMoveStart, + onMoveError, + }); - const sortedSteps = useMemo(() => [...steps].sort((a, b) => a.position - b.position), [steps]); + const sortedSteps = useMemo(() => sortWorkflowStepsByPosition(steps), [steps]); const currentIndex = useMemo( () => sortedSteps.findIndex((s) => s.id === currentStepId), [sortedSteps, currentStepId], ); - const handleMove = useCallback( - async (stepId: string): Promise => { - if (!taskId || !workflowId) return false; - onMoveStart?.(); - disablePlanMode(); - const requestId = ++moveRequestRef.current; - setMovingToStepId(stepId); - try { - await moveTask(taskId, { - workflow_id: workflowId, - workflow_step_id: stepId, - position: 0, - }); - return true; - } catch (err) { - console.error("[WorkflowStepper] Failed to move task:", err); - if (requestId === moveRequestRef.current) onMoveError?.(err); - return false; - } finally { - if (requestId === moveRequestRef.current) setMovingToStepId(null); - } - }, - [taskId, workflowId, disablePlanMode, onMoveStart, onMoveError], - ); - // Collapse to a minimal view when the full stepper can't fit (w-full keeps the measurement track-driven). const containerRef = useRef(null); const isCollapsed = useToolbarCollapsed(containerRef); diff --git a/apps/web/e2e/tests/kanban/preview-workflow-step-navigation.spec.ts b/apps/web/e2e/tests/kanban/preview-workflow-step-navigation.spec.ts new file mode 100644 index 0000000000..5aedd9e38b --- /dev/null +++ b/apps/web/e2e/tests/kanban/preview-workflow-step-navigation.spec.ts @@ -0,0 +1,274 @@ +import { test, expect } from "../../fixtures/test-base"; +import { KanbanPage } from "../../pages/kanban-page"; +import type { ApiClient } from "../../helpers/api-client"; + +const LONG_STEP_NAME = `Awaiting review from the platform team ${"x".repeat(60)}`; + +// Column visibility lives on the swimlane header of the workflow it +// configures. Mirrors the same helper in step-visibility-filter.spec.ts. +async function openColumnsMenu(kanban: KanbanPage, workflowId: string) { + const trigger = kanban.page.getByTestId(`columns-menu-${workflowId}`); + await expect(async () => { + if ((await trigger.getAttribute("data-state")) !== "open") { + await trigger.click(); + } + await expect(trigger).toHaveAttribute("data-state", "open", { timeout: 1_000 }); + }).toPass({ timeout: 15_000 }); +} + +async function closeColumnsMenu(kanban: KanbanPage, workflowId: string) { + const trigger = kanban.page.getByTestId(`columns-menu-${workflowId}`); + if ((await trigger.getAttribute("data-state")) === "open") { + await kanban.page.keyboard.press("Escape"); + } + await expect(trigger).not.toHaveAttribute("data-state", "open"); +} + +async function hideColumn(kanban: KanbanPage, workflowId: string, stepId: string) { + await openColumnsMenu(kanban, workflowId); + await kanban.page.getByTestId(`columns-menu-step-${stepId}`).click(); + await closeColumnsMenu(kanban, workflowId); +} + +// A task's very first page load in a fresh workspace runs a one-time settings +// reconciliation (`use-user-display-settings.ts`'s workspaceId-mismatch effect) +// that issues its own partial settings PATCH. That PATCH is built from a +// settings snapshot captured before this call's PATCH landed, so calling +// `saveUserSettings` before the first `goto()` gets silently clobbered back to +// its default and a card click falls through to full-page navigation instead +// of opening the preview. Letting that first-load reconciliation settle, then +// setting the flag and reloading, avoids the race. +async function enablePreviewOnClick(kanban: KanbanPage, apiClient: ApiClient) { + await kanban.goto(); + await apiClient.saveUserSettings({ enable_preview_on_click: true }); + await kanban.goto(); +} + +function adjacentStep( + steps: Array<{ id: string; position: number }>, + currentStepId: string, +): { id: string; position: number } { + const sorted = [...steps].sort((left, right) => left.position - right.position); + const currentIndex = sorted.findIndex((step) => step.id === currentStepId); + const target = sorted[currentIndex + 1] ?? sorted[currentIndex - 1]; + if (!target) throw new Error("preview step navigation test requires an adjacent target"); + return target; +} + +test.describe("Kanban preview workflow step navigation", () => { + test("moves the task to a step whose board column is hidden", async ({ + testPage, + apiClient, + seedData, + }) => { + const targetStep = adjacentStep(seedData.steps, seedData.startStepId); + + const task = await apiClient.createTask( + seedData.workspaceId, + "Preview step nav hidden column", + { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + const kanban = new KanbanPage(testPage); + await enablePreviewOnClick(kanban, apiClient); + await hideColumn(kanban, seedData.workflowId, targetStep.id); + await expect(kanban.columnByStepId(targetStep.id)).toBeHidden(); + + const card = kanban.taskCardByTitle("Preview step nav hidden column"); + await expect(card).toBeVisible({ timeout: 10_000 }); + await card.click(); + + const previewPanel = testPage.getByTestId("task-preview-panel"); + await expect(previewPanel).toBeVisible({ timeout: 10_000 }); + + const trigger = previewPanel.getByTestId("workflow-stepper-minimal"); + await expect(trigger).toBeVisible(); + await trigger.hover(); + + const disclosure = testPage.getByTestId("workflow-step-disclosure"); + await expect(disclosure).toBeVisible(); + // The hidden column still lists in the disclosure — that is the whole point. + await expect( + testPage.getByTestId(`workflow-step-disclosure-row-${targetStep.id}`), + ).toBeVisible(); + + await testPage.getByTestId(`workflow-step-disclosure-move-${targetStep.id}`).click(); + + await expect + .poll(async () => (await apiClient.getTask(task.id)).workflow_step_id, { timeout: 15_000 }) + .toBe(targetStep.id); + + // Success closes the disclosure and leaves the preview open on the same task. + await expect(disclosure).toBeHidden(); + await expect(previewPanel).toBeVisible(); + }); + + test("opens the touch drawer and moves the task from a tablet preview", async ({ + tabletTestPage, + apiClient, + seedData, + }) => { + const targetStep = adjacentStep(seedData.steps, seedData.startStepId); + const task = await apiClient.createTask(seedData.workspaceId, "Preview step nav tablet", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + + const kanban = new KanbanPage(tabletTestPage); + await enablePreviewOnClick(kanban, apiClient); + + const card = kanban.taskCardByTitle("Preview step nav tablet"); + await expect(card).toBeVisible({ timeout: 10_000 }); + await card.tap(); + + const previewPanel = tabletTestPage.getByTestId("task-preview-panel"); + await expect(previewPanel).toBeVisible({ timeout: 10_000 }); + + const trigger = previewPanel.getByTestId("workflow-stepper-minimal"); + const cue = trigger.getByTestId("workflow-stepper-touch-disclosure-cue"); + await expect(trigger).toBeVisible(); + await expect(cue).toBeVisible(); + const triggerBox = await trigger.boundingBox(); + expect(triggerBox).not.toBeNull(); + if (!triggerBox) return; + expect(triggerBox.width).toBeGreaterThanOrEqual(44); + expect(triggerBox.height).toBeGreaterThanOrEqual(44); + + await trigger.tap(); + + const disclosure = tabletTestPage.getByTestId("workflow-step-disclosure"); + await expect(disclosure).toBeVisible(); + const moveButton = disclosure.getByTestId(`workflow-step-disclosure-move-${targetStep.id}`); + await expect(moveButton).toBeVisible(); + const moveButtonBox = await moveButton.boundingBox(); + expect(moveButtonBox).not.toBeNull(); + if (!moveButtonBox) return; + expect(moveButtonBox.height).toBeGreaterThanOrEqual(44); + + await moveButton.tap(); + + await expect + .poll(async () => (await apiClient.getTask(task.id)).workflow_step_id, { + timeout: 15_000, + }) + .toBe(targetStep.id); + await expect(tabletTestPage.locator('[data-slot="drawer-content"]')).toHaveAttribute( + "data-state", + "closed", + ); + await expect(previewPanel).toBeVisible(); + }); + + test("keeps the header a single row at the panel's minimum width", async ({ + testPage, + apiClient, + seedData, + }) => { + const longStep = await apiClient.createWorkflowStep( + seedData.workflowId, + LONG_STEP_NAME, + seedData.steps.length, + ); + + await apiClient.createTask(seedData.workspaceId, "Preview step nav containment", { + workflow_id: seedData.workflowId, + workflow_step_id: longStep.id, + repository_ids: [seedData.repositoryId], + }); + + await testPage.setViewportSize({ width: 1400, height: 900 }); + // Seed the persisted preview width below the panel's own 300px floor so the + // inline layout — the binding case per the system design — renders at + // exactly its minimum, rather than depending on a fragile drag interaction. + await testPage.addInitScript(() => { + window.localStorage.setItem("kandev.kanban.preview.width", "1"); + }); + + const kanban = new KanbanPage(testPage); + await enablePreviewOnClick(kanban, apiClient); + + const card = kanban.taskCardByTitle("Preview step nav containment"); + await expect(card).toBeVisible({ timeout: 10_000 }); + await card.click(); + + const previewPanel = testPage.getByTestId("task-preview-panel"); + await expect(previewPanel).toBeVisible({ timeout: 10_000 }); + + const trigger = previewPanel.getByTestId("workflow-stepper-minimal"); + await expect(trigger).toBeVisible(); + + const title = previewPanel.locator("h2"); + const closeButton = previewPanel.getByRole("button", { name: "Close preview" }); + const maximizeButton = previewPanel.getByRole("button", { name: "Open full page" }); + await expect(closeButton).toBeVisible(); + await expect(closeButton).toBeEnabled(); + await expect(maximizeButton).toBeVisible(); + await expect(maximizeButton).toBeEnabled(); + + const [titleBox, triggerBox, closeBox] = await Promise.all([ + title.boundingBox(), + trigger.boundingBox(), + closeButton.boundingBox(), + ]); + expect(titleBox).not.toBeNull(); + expect(triggerBox).not.toBeNull(); + expect(closeBox).not.toBeNull(); + if (!titleBox || !triggerBox || !closeBox) return; + + // Single row: every header element shares the same vertical center. Comparing + // raw tops would fail spuriously — items-center aligns centers, not tops, and + // the h2 title's text line-box is naturally shorter than the 32px icon buttons. + const centerY = (box: { y: number; height: number }) => box.y + box.height / 2; + expect(Math.abs(centerY(titleBox) - centerY(closeBox))).toBeLessThan(4); + expect(Math.abs(centerY(triggerBox) - centerY(closeBox))).toBeLessThan(4); + + // The title floor AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.3 requires. + expect(titleBox.width).toBeGreaterThanOrEqual(88); + + // No horizontal scrolling in the header row. + const headerScrollWidth = await previewPanel + .locator(".border-b") + .first() + .evaluate((el) => el.scrollWidth - el.clientWidth); + expect(headerScrollWidth).toBeLessThanOrEqual(1); + }); + + test("dismisses the disclosure on the first Escape and the preview on the second", async ({ + testPage, + apiClient, + seedData, + }) => { + await apiClient.createTask(seedData.workspaceId, "Preview step nav escape", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + + const kanban = new KanbanPage(testPage); + await enablePreviewOnClick(kanban, apiClient); + const card = kanban.taskCardByTitle("Preview step nav escape"); + await expect(card).toBeVisible({ timeout: 10_000 }); + await card.click(); + + const previewPanel = testPage.getByTestId("task-preview-panel"); + await expect(previewPanel).toBeVisible({ timeout: 10_000 }); + + const trigger = previewPanel.getByTestId("workflow-stepper-minimal"); + await trigger.focus(); + const disclosure = testPage.getByTestId("workflow-step-disclosure"); + await expect(disclosure).toBeVisible(); + + await testPage.keyboard.press("Escape"); + await expect(disclosure).toBeHidden(); + await expect(previewPanel).toBeVisible(); + await expect(trigger).toBeFocused(); + + await testPage.keyboard.press("Escape"); + await expect(previewPanel).toBeHidden(); + }); +}); diff --git a/apps/web/hooks/domains/kanban/use-preview-workflow-step-move.ts b/apps/web/hooks/domains/kanban/use-preview-workflow-step-move.ts new file mode 100644 index 0000000000..5511148371 --- /dev/null +++ b/apps/web/hooks/domains/kanban/use-preview-workflow-step-move.ts @@ -0,0 +1,79 @@ +"use client"; + +import { useCallback, useRef, useState } from "react"; +import type { WorkflowStepperStep } from "@/components/task/workflow-step-disclosure"; +import { useWorkflowStepsById } from "./use-workflow-steps-by-id"; +import { usePresentationToken, useWorkflowStepMove } from "./use-workflow-step-move"; + +export type PreviewStepMove = { + workflowSteps: WorkflowStepperStep[]; + currentStepId: string | null; + taskWorkflowId: string | null; + isArchived: boolean; + movingToStepId: string | null; + handleMove: (stepId: string) => Promise; + moveError: unknown; + handleDisclosureOpenChange: (open: boolean) => void; + isDisclosureOpen: () => boolean; +}; + +/** + * Owns the preview header's step indicator: step resolution for the + * previewed task's own workflow, the move request, and the move-failure + * banner. Lives here rather than in `TaskPreviewPanel` because the panel + * unmounts on preview close in both layouts, while a stale response must + * still be discarded and never rendered against the presentation that + * replaced it. + */ +export function usePreviewWorkflowStepMove( + selectedTaskId: string | null | undefined, + selectedTask: { + id: string; + workflowId?: string | null; + workflowStepId: string | null; + isArchived?: boolean; + } | null, +): PreviewStepMove { + const workflowSteps = useWorkflowStepsById(selectedTask?.workflowId ?? null); + const [moveError, setMoveError] = useState(null); + const presentationToken = usePresentationToken(selectedTaskId ?? null); + const disclosureOpenRef = useRef(false); + + // Clear a stale error the moment a new presentation begins, in the same + // render pass that changes `presentationToken` — not in a passive effect, + // whose flush can lag behind a `moveTask` rejection that resolves before it + // runs. See the same "adjust state during render" pattern below. + const moveErrorTokenRef = useRef(presentationToken); + if (moveErrorTokenRef.current !== presentationToken) { + moveErrorTokenRef.current = presentationToken; + setMoveError(null); + } + + const handleMoveStart = useCallback(() => setMoveError(null), []); + const handleMoveError = useCallback((error: unknown) => setMoveError(error), []); + + const { movingToStepId, handleMove } = useWorkflowStepMove({ + taskId: selectedTask?.id ?? null, + workflowId: selectedTask?.workflowId ?? null, + presentationToken, + onMoveStart: handleMoveStart, + onMoveError: handleMoveError, + }); + + const handleDisclosureOpenChange = useCallback((open: boolean) => { + disclosureOpenRef.current = open; + }, []); + const isDisclosureOpen = useCallback(() => disclosureOpenRef.current, []); + + return { + workflowSteps, + currentStepId: selectedTask?.workflowStepId ?? null, + taskWorkflowId: selectedTask?.workflowId ?? null, + isArchived: selectedTask?.isArchived ?? false, + movingToStepId, + handleMove, + moveError, + handleDisclosureOpenChange, + isDisclosureOpen, + }; +} diff --git a/apps/web/hooks/domains/kanban/use-workflow-step-move.test.ts b/apps/web/hooks/domains/kanban/use-workflow-step-move.test.ts new file mode 100644 index 0000000000..6f5eb4ab74 --- /dev/null +++ b/apps/web/hooks/domains/kanban/use-workflow-step-move.test.ts @@ -0,0 +1,220 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import { moveTask } from "@/lib/api"; +import { usePresentationToken, useWorkflowStepMove } from "./use-workflow-step-move"; + +const mockAppState = vi.hoisted(() => ({ + value: { + tasks: { activeSessionId: null as string | null }, + chatInput: { planModeBySessionId: {} as Record }, + setPlanMode: vi.fn(), + setActiveDocument: vi.fn(), + }, +})); +const mockLayoutStore = vi.hoisted(() => ({ closeDocument: vi.fn() })); +const mockContextFilesStore = vi.hoisted(() => ({ removeFile: vi.fn() })); +const mockDockviewStore = vi.hoisted(() => ({ applyBuiltInPreset: vi.fn() })); + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (state: typeof mockAppState.value) => unknown) => + selector(mockAppState.value), +})); +vi.mock("@/lib/state/layout-store", () => ({ + useLayoutStore: (selector: (state: typeof mockLayoutStore) => unknown) => + selector(mockLayoutStore), +})); +vi.mock("@/lib/state/context-files-store", () => ({ + useContextFilesStore: (selector: (state: typeof mockContextFilesStore) => unknown) => + selector(mockContextFilesStore), +})); +vi.mock("@/lib/state/dockview-store", () => ({ + useDockviewStore: (selector: (state: typeof mockDockviewStore) => unknown) => + selector(mockDockviewStore), +})); +vi.mock("@/lib/api", () => ({ + moveTask: vi.fn(), +})); + +const TASK_ID = "task-1"; +const WORKFLOW_ID = "workflow-1"; + +afterEach(() => { + vi.clearAllMocks(); + mockAppState.value.tasks.activeSessionId = null; + mockAppState.value.chatInput.planModeBySessionId = {}; +}); + +describe("usePresentationToken", () => { + it("changes when the key changes, and again on a return to the same key", () => { + const { result, rerender } = renderHook(({ key }) => usePresentationToken(key), { + initialProps: { key: "a" as string | null }, + }); + const first = result.current; + + rerender({ key: null }); + const second = result.current; + expect(second).not.toBe(first); + + rerender({ key: "a" }); + const third = result.current; + expect(third).not.toBe(second); + }); + + it("stays the same across a re-render with an unchanged key", () => { + const { result, rerender } = renderHook(({ key }) => usePresentationToken(key), { + initialProps: { key: "a" as string | null }, + }); + const first = result.current; + rerender({ key: "a" }); + expect(result.current).toBe(first); + }); +}); + +describe("useWorkflowStepMove", () => { + it("issues the move request with the task's workflow id, target step, and position 0", async () => { + vi.mocked(moveTask).mockResolvedValue({} as Awaited>); + const { result } = renderHook(() => + useWorkflowStepMove({ taskId: TASK_ID, workflowId: WORKFLOW_ID, presentationToken: 0 }), + ); + + await act(async () => { + await result.current.handleMove("step-b"); + }); + + expect(moveTask).toHaveBeenCalledWith(TASK_ID, { + workflow_id: WORKFLOW_ID, + workflow_step_id: "step-b", + position: 0, + }); + }); + + it("notifies onMoveStart and disables plan mode before the request resolves", async () => { + mockAppState.value.tasks.activeSessionId = "session-1"; + mockAppState.value.chatInput.planModeBySessionId = { "session-1": true }; + vi.mocked(moveTask).mockResolvedValue({} as Awaited>); + const onMoveStart = vi.fn(); + const { result } = renderHook(() => + useWorkflowStepMove({ + taskId: TASK_ID, + workflowId: WORKFLOW_ID, + presentationToken: 0, + onMoveStart, + }), + ); + + await act(async () => { + await result.current.handleMove("step-b"); + }); + + expect(onMoveStart).toHaveBeenCalledTimes(1); + expect(mockAppState.value.setPlanMode).toHaveBeenCalledWith("session-1", false); + }); + + it("reports a rejected move and re-enables the control", async () => { + const error = new Error("network error"); + vi.mocked(moveTask).mockRejectedValueOnce(error); + const onMoveError = vi.fn(); + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const { result } = renderHook(() => + useWorkflowStepMove({ + taskId: TASK_ID, + workflowId: WORKFLOW_ID, + presentationToken: 0, + onMoveError, + }), + ); + + await act(async () => { + await result.current.handleMove("step-b"); + }); + + expect(onMoveError).toHaveBeenCalledWith(error); + expect(result.current.movingToStepId).toBeNull(); + consoleErrorSpy.mockRestore(); + }); + + it("returns false without calling the API when taskId or workflowId is missing", async () => { + const { result } = renderHook(() => + useWorkflowStepMove({ taskId: null, workflowId: WORKFLOW_ID, presentationToken: 0 }), + ); + + const moved = await act(async () => result.current.handleMove("step-b")); + + expect(moved).toBe(false); + expect(moveTask).not.toHaveBeenCalled(); + }); +}); + +describe("useWorkflowStepMove presentation token handling", () => { + it("discards a late failure once the presentation token has changed", async () => { + let rejectMove!: (error: unknown) => void; + vi.mocked(moveTask).mockReturnValueOnce( + new Promise((_resolve, reject) => (rejectMove = reject)), + ); + const onMoveError = vi.fn(); + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const { result, rerender } = renderHook( + ({ presentationToken }: { presentationToken: number }) => + useWorkflowStepMove({ + taskId: TASK_ID, + workflowId: WORKFLOW_ID, + presentationToken, + onMoveError, + }), + { initialProps: { presentationToken: 0 } }, + ); + + let movePromise!: Promise; + act(() => { + movePromise = result.current.handleMove("step-b"); + }); + expect(result.current.movingToStepId).toBe("step-b"); + + // The preview closed and reopened on the same task (or switched to a + // different one) before the request resolved: a new presentation begins + // with no in-flight move, even though one is still outstanding. + rerender({ presentationToken: 1 }); + expect(result.current.movingToStepId).toBeNull(); + + await act(async () => { + rejectMove(new Error("stale failure")); + await movePromise; + }); + + expect(onMoveError).not.toHaveBeenCalled(); + expect(result.current.movingToStepId).toBeNull(); + consoleErrorSpy.mockRestore(); + }); + + it("ignores a superseded move's rejection within the same presentation", async () => { + let rejectFirst!: (error: unknown) => void; + vi.mocked(moveTask).mockReturnValueOnce(new Promise((_res, rej) => (rejectFirst = rej))); + vi.mocked(moveTask).mockResolvedValueOnce({} as Awaited>); + const onMoveError = vi.fn(); + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const { result } = renderHook(() => + useWorkflowStepMove({ + taskId: TASK_ID, + workflowId: WORKFLOW_ID, + presentationToken: 0, + onMoveError, + }), + ); + + let firstPromise!: Promise; + act(() => { + firstPromise = result.current.handleMove("step-b"); + }); + await act(async () => { + await result.current.handleMove("step-c"); + }); + + await act(async () => { + rejectFirst(new Error("stale")); + await firstPromise; + }); + + expect(onMoveError).not.toHaveBeenCalled(); + consoleErrorSpy.mockRestore(); + }); +}); diff --git a/apps/web/hooks/domains/kanban/use-workflow-step-move.ts b/apps/web/hooks/domains/kanban/use-workflow-step-move.ts new file mode 100644 index 0000000000..af4ac84b44 --- /dev/null +++ b/apps/web/hooks/domains/kanban/use-workflow-step-move.ts @@ -0,0 +1,136 @@ +"use client"; + +import { useCallback, useRef, useState } from "react"; +import { moveTask } from "@/lib/api"; +import { useAppStore } from "@/components/state-provider"; +import { useContextFilesStore } from "@/lib/state/context-files-store"; +import { useLayoutStore } from "@/lib/state/layout-store"; +import { useDockviewStore } from "@/lib/state/dockview-store"; + +const PLAN_CONTEXT_PATH = "plan:context"; + +/** Returns a callback that disables plan mode for the active session of a task. */ +export function useDisablePlanMode() { + const activeSessionId = useAppStore((s) => s.tasks.activeSessionId); + const planModeEnabled = useAppStore((s) => + activeSessionId ? (s.chatInput.planModeBySessionId[activeSessionId] ?? false) : false, + ); + const setPlanMode = useAppStore((s) => s.setPlanMode); + const setActiveDocument = useAppStore((s) => s.setActiveDocument); + const closeDocument = useLayoutStore((s) => s.closeDocument); + const removeContextFile = useContextFilesStore((s) => s.removeFile); + const applyBuiltInPreset = useDockviewStore((s) => s.applyBuiltInPreset); + + return useCallback(() => { + if (!activeSessionId || !planModeEnabled) return; + applyBuiltInPreset("default"); + closeDocument(activeSessionId); + setActiveDocument(activeSessionId, null); + setPlanMode(activeSessionId, false); + removeContextFile(activeSessionId, PLAN_CONTEXT_PATH); + }, [ + activeSessionId, + planModeEnabled, + setPlanMode, + setActiveDocument, + closeDocument, + removeContextFile, + applyBuiltInPreset, + ]); +} + +/** + * A number that changes whenever `key` changes, including a transition + * through `null`/`undefined` and back to the same value. Surfaces that render + * a continuous presentation of one task (the preview panel, the task route) + * use this as the identity `useWorkflowStepMove` scopes a move request to: + * closing and reopening on the same task changes `key` to `null` and back, + * which must count as a new presentation even though the key's final value is + * unchanged. + */ +export function usePresentationToken(key: string | null | undefined): number { + const [token, setToken] = useState(0); + const [prevKey, setPrevKey] = useState(key); + if (prevKey !== key) { + setPrevKey(key); + setToken((current) => current + 1); + } + return token; +} + +type UseWorkflowStepMoveParams = { + taskId?: string | null; + workflowId?: string | null; + /** Identity of the presentation issuing the move; see `usePresentationToken`. */ + presentationToken: number; + onMoveStart?: () => void; + onMoveError?: (error: unknown) => void; +}; + +type UseWorkflowStepMoveResult = { + movingToStepId: string | null; + handleMove: (stepId: string) => Promise; +}; + +/** + * The single implementation of the compact stepper's move request. Extracted + * so the task top bar and the kanban preview header share one plan-mode + * cleanup and one late-response guard, scoped per `presentationToken` rather + * than per component instance: a component that stays mounted across task + * changes, or state lifted above a component that unmounts, both need a late + * response discarded once the presentation that issued it is gone. + */ +export function useWorkflowStepMove({ + taskId, + workflowId, + presentationToken, + onMoveStart, + onMoveError, +}: UseWorkflowStepMoveParams): UseWorkflowStepMoveResult { + const disablePlanMode = useDisablePlanMode(); + const [movingToStepId, setMovingToStepId] = useState(null); + // Only the in-flight step's own button is disabled, so every other step stays + // clickable and two moves can overlap. This counter marks which one is the + // latest; a slower predecessor must not own the banner or the loading state. + const moveRequestRef = useRef(0); + const tokenRef = useRef(presentationToken); + + // Invalidate synchronously in the render that changes `presentationToken`, + // not in a passive effect: a `moveTask` rejection can reach its `catch` + // before a `useEffect` scheduled by the same commit has flushed, which + // would let a stale request still pass the `requestId` check below. + if (tokenRef.current !== presentationToken) { + tokenRef.current = presentationToken; + // A new presentation invalidates any request still in flight from the one + // it replaced, and starts with no disabled control of its own. + moveRequestRef.current += 1; + setMovingToStepId(null); + } + + const handleMove = useCallback( + async (stepId: string): Promise => { + if (!taskId || !workflowId) return false; + onMoveStart?.(); + disablePlanMode(); + const requestId = ++moveRequestRef.current; + setMovingToStepId(stepId); + try { + await moveTask(taskId, { + workflow_id: workflowId, + workflow_step_id: stepId, + position: 0, + }); + return true; + } catch (err) { + console.error("[useWorkflowStepMove] Failed to move task:", err); + if (requestId === moveRequestRef.current) onMoveError?.(err); + return false; + } finally { + if (requestId === moveRequestRef.current) setMovingToStepId(null); + } + }, + [taskId, workflowId, disablePlanMode, onMoveStart, onMoveError], + ); + + return { movingToStepId, handleMove }; +} diff --git a/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.test.ts b/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.test.ts new file mode 100644 index 0000000000..6aa72e628b --- /dev/null +++ b/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi } from "vitest"; +import { renderHook } from "@testing-library/react"; + +type StoreStep = { + id: string; + title: string; + color: string; + position: number; + allow_manual_move?: boolean; +}; +type MockState = { + kanban: { workflowId: string | null; steps: StoreStep[] }; + kanbanMulti: { snapshots: Record }; +}; + +const ACTIVE_WORKFLOW_ID = "workflow-active"; + +let mockState: MockState = { + kanban: { workflowId: ACTIVE_WORKFLOW_ID, steps: [] }, + kanbanMulti: { snapshots: {} }, +}; + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (s: MockState) => unknown) => selector(mockState), +})); + +import { useWorkflowStepsById } from "./use-workflow-steps-by-id"; + +describe("useWorkflowStepsById", () => { + it("returns an empty list when no workflow id is given", () => { + mockState = { + kanban: { workflowId: ACTIVE_WORKFLOW_ID, steps: [] }, + kanbanMulti: { snapshots: {} }, + }; + const { result } = renderHook(() => useWorkflowStepsById(null)); + expect(result.current).toEqual([]); + }); + + it("resolves from kanban.steps when the workflow id matches the active workflow", () => { + mockState = { + kanban: { + workflowId: ACTIVE_WORKFLOW_ID, + steps: [ + { id: "b", title: "Work", color: "#222", position: 1 }, + { id: "a", title: "Spec", color: "#111", position: 0 }, + ], + }, + kanbanMulti: { snapshots: {} }, + }; + const { result } = renderHook(() => useWorkflowStepsById(ACTIVE_WORKFLOW_ID)); + expect(result.current.map((s) => s.id)).toEqual(["a", "b"]); + expect(result.current[0]).toMatchObject({ id: "a", name: "Spec", position: 0 }); + }); + + it("falls back to the workflow snapshot when the id is not the active workflow", () => { + mockState = { + kanban: { workflowId: ACTIVE_WORKFLOW_ID, steps: [] }, + kanbanMulti: { + snapshots: { + "workflow-other": { + steps: [{ id: "x", title: "Triage", color: "#333", position: 0 }], + }, + }, + }, + }; + const { result } = renderHook(() => useWorkflowStepsById("workflow-other")); + expect(result.current).toHaveLength(1); + expect(result.current[0]).toMatchObject({ id: "x", name: "Triage" }); + }); + + it("returns an empty list when neither the active workflow nor a snapshot resolves it", () => { + mockState = { + kanban: { workflowId: ACTIVE_WORKFLOW_ID, steps: [] }, + kanbanMulti: { snapshots: {} }, + }; + const { result } = renderHook(() => useWorkflowStepsById("workflow-unknown")); + expect(result.current).toEqual([]); + }); + + it("breaks a position tie by ascending id", () => { + mockState = { + kanban: { + workflowId: ACTIVE_WORKFLOW_ID, + steps: [ + { id: "b", title: "B", color: "#111", position: 0 }, + { id: "a", title: "A", color: "#222", position: 0 }, + ], + }, + kanbanMulti: { snapshots: {} }, + }; + const { result } = renderHook(() => useWorkflowStepsById(ACTIVE_WORKFLOW_ID)); + expect(result.current.map((s) => s.id)).toEqual(["a", "b"]); + }); +}); diff --git a/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.ts b/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.ts new file mode 100644 index 0000000000..fada33f979 --- /dev/null +++ b/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.ts @@ -0,0 +1,46 @@ +"use client"; + +import { useMemo } from "react"; +import { useAppStore } from "@/components/state-provider"; +import type { KanbanState } from "@/lib/state/slices"; +import { sortWorkflowStepsByPosition } from "@/lib/kanban/workflow-step-order"; +import type { WorkflowStepperStep } from "@/components/task/workflow-step-disclosure"; + +type StoreStep = KanbanState["steps"][number]; + +function mapStep(step: StoreStep): WorkflowStepperStep { + return { + id: step.id, + name: step.title, + color: step.color, + position: step.position, + events: step.events, + allow_manual_move: step.allow_manual_move, + prompt: step.prompt, + is_start_step: step.is_start_step, + agent_profile_id: step.agent_profile_id, + }; +} + +/** + * Resolves the ordered step list for a workflow that is not necessarily the + * board's active workflow: the previewed task's own workflow, per + * `plugin-context-api.ts`'s rule. `kanban.steps` covers the active workflow; + * `kanbanMulti.snapshots` (populated by `useAllWorkflowSnapshots` for every + * workflow in the workspace) covers every other one, including a task on a + * workflow the board is not currently filtered to. + */ +export function useWorkflowStepsById(workflowId: string | null | undefined): WorkflowStepperStep[] { + const activeWorkflowId = useAppStore((state) => state.kanban.workflowId); + const activeSteps = useAppStore((state) => state.kanban.steps); + const snapshotSteps = useAppStore((state) => + workflowId ? state.kanbanMulti.snapshots[workflowId]?.steps : undefined, + ); + + return useMemo(() => { + if (!workflowId) return []; + const rawSteps = workflowId === activeWorkflowId ? activeSteps : snapshotSteps; + if (!rawSteps || rawSteps.length === 0) return []; + return sortWorkflowStepsByPosition(rawSteps.map(mapStep)); + }, [workflowId, activeWorkflowId, activeSteps, snapshotSteps]); +} diff --git a/apps/web/lib/kanban/auto-hide-empty-columns.test.ts b/apps/web/lib/kanban/auto-hide-empty-columns.test.ts index a8e316306a..9805676586 100644 --- a/apps/web/lib/kanban/auto-hide-empty-columns.test.ts +++ b/apps/web/lib/kanban/auto-hide-empty-columns.test.ts @@ -1,9 +1,5 @@ import { describe, expect, it } from "vitest"; -import { - areAllEmptyStepsAutoHidden, - deriveAutoHiddenStepIds, - sortWorkflowStepsByPosition, -} from "./auto-hide-empty-columns"; +import { areAllEmptyStepsAutoHidden, deriveAutoHiddenStepIds } from "./auto-hide-empty-columns"; const steps = [{ id: "backlog" }, { id: "doing" }, { id: "done" }]; @@ -30,20 +26,3 @@ describe("areAllEmptyStepsAutoHidden", () => { expect(areAllEmptyStepsAutoHidden([{ id: "todo" }], [{ id: "todo" }])).toBe(false); }); }); - -describe("sortWorkflowStepsByPosition", () => { - it("orders workflow steps deterministically without mutating the snapshot", () => { - const unordered = [ - { id: "step-b", position: 1 }, - { id: "step-c", position: 0 }, - { id: "step-a", position: 1 }, - ]; - - expect(sortWorkflowStepsByPosition(unordered).map((step) => step.id)).toEqual([ - "step-c", - "step-a", - "step-b", - ]); - expect(unordered.map((step) => step.id)).toEqual(["step-b", "step-c", "step-a"]); - }); -}); diff --git a/apps/web/lib/kanban/auto-hide-empty-columns.ts b/apps/web/lib/kanban/auto-hide-empty-columns.ts index 882d4ef7de..032cfd8aa5 100644 --- a/apps/web/lib/kanban/auto-hide-empty-columns.ts +++ b/apps/web/lib/kanban/auto-hide-empty-columns.ts @@ -1,12 +1,6 @@ type Step = { id: string }; type StepTask = { workflowStepId: string | null }; -export function sortWorkflowStepsByPosition( - steps: T[], -): T[] { - return [...steps].sort((a, b) => a.position - b.position || a.id.localeCompare(b.id)); -} - export function areAllEmptyStepsAutoHidden(visibleSteps: Step[], moveTargetSteps: Step[]): boolean { return visibleSteps.length === 0 && moveTargetSteps.length > 0; } diff --git a/apps/web/lib/kanban/workflow-step-order.test.ts b/apps/web/lib/kanban/workflow-step-order.test.ts new file mode 100644 index 0000000000..4e719cd74d --- /dev/null +++ b/apps/web/lib/kanban/workflow-step-order.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; +import { sortWorkflowStepsByPosition } from "./workflow-step-order"; + +describe("sortWorkflowStepsByPosition", () => { + it("orders workflow steps deterministically without mutating the snapshot", () => { + const unordered = [ + { id: "step-b", position: 1 }, + { id: "step-c", position: 0 }, + { id: "step-a", position: 1 }, + ]; + + expect(sortWorkflowStepsByPosition(unordered).map((step) => step.id)).toEqual([ + "step-c", + "step-a", + "step-b", + ]); + expect(unordered.map((step) => step.id)).toEqual(["step-b", "step-c", "step-a"]); + }); +}); diff --git a/apps/web/lib/kanban/workflow-step-order.ts b/apps/web/lib/kanban/workflow-step-order.ts new file mode 100644 index 0000000000..396efb90e8 --- /dev/null +++ b/apps/web/lib/kanban/workflow-step-order.ts @@ -0,0 +1,7 @@ +type Step = { id: string }; + +export function sortWorkflowStepsByPosition( + steps: T[], +): T[] { + return [...steps].sort((a, b) => a.position - b.position || a.id.localeCompare(b.id)); +} diff --git a/docs/specs/guide/structure-and-ownership.md b/docs/specs/guide/structure-and-ownership.md index 3dd3296cec..149161674d 100644 --- a/docs/specs/guide/structure-and-ownership.md +++ b/docs/specs/guide/structure-and-ownership.md @@ -114,7 +114,7 @@ capability. The specification linter reads the default limits from `docs/specs/spec-lint.json`. The default limits are: -- System index: 12 KiB. +- System index: 16 KiB. - Product or guide document: 16 KiB. - Requirement document: 20 KiB. - System-design document: 32 KiB. diff --git a/docs/specs/spec-lint.json b/docs/specs/spec-lint.json index d5a008060f..ce39b7bbe7 100644 --- a/docs/specs/spec-lint.json +++ b/docs/specs/spec-lint.json @@ -6,7 +6,7 @@ "product": 16384, "requirement": 20480, "system-design": 32768, - "system-index": 12288, + "system-index": 16384, "template": 8192 } } diff --git a/docs/specs/ui/README.md b/docs/specs/ui/README.md index a16872c1a4..1d82efcb64 100644 --- a/docs/specs/ui/README.md +++ b/docs/specs/ui/README.md @@ -39,6 +39,7 @@ UI owns responsive behavior; other systems own behavior/state. - [Command-panel task reveal](requirements/command-panel-sidebar-task-reveal.md) - [Task activity icons](requirements/command-panel-task-activity-icons.md) - [Workflow step navigation](requirements/compact-workflow-step-navigation.md) +- [Preview panel step navigation](requirements/kanban-preview-workflow-step-navigation.md) - [Comment Markdown Rendering](requirements/comment-markdown.md) - [Mention recency](requirements/composer-mention-recency.md) - [Composer Suggestion Overlays](requirements/composer-suggestion-overlays.md) @@ -149,6 +150,7 @@ UI owns responsive behavior; other systems own behavior/state. - [Mention recency](system-design/composer-mention-recency.md) - [Composer Suggestion Overlays](system-design/composer-suggestion-overlays.md) - [Compact Workflow Step Navigation](system-design/compact-workflow-step-navigation.md) +- [Kanban Preview Workflow Step Navigation](system-design/kanban-preview-workflow-step-navigation.md) - [Workflow column visibility (Part 1)](system-design/board-step-visibility-filter-01.md) - [Workflow column visibility (Part 2)](system-design/board-step-visibility-filter-02.md) - [Task PR Automation Controls 1](system-design/ci-pr-automation-01.md) diff --git a/docs/specs/ui/requirements/kanban-preview-workflow-step-navigation.md b/docs/specs/ui/requirements/kanban-preview-workflow-step-navigation.md new file mode 100644 index 0000000000..091be87c80 --- /dev/null +++ b/docs/specs/ui/requirements/kanban-preview-workflow-step-navigation.md @@ -0,0 +1,281 @@ +--- +status: draft +system: ui +created: 2026-09-03 +owners: + - nova28 +--- + +# Kanban Preview Workflow Step Navigation Requirements + +## Overview + +The kanban preview panel opens beside the board and shows one task's sessions. +Its header shows the task title, an open-full-page control, and a close +control. It does not show which workflow step the task is on, and it offers no +way to move the task. + +Today the only in-board way to change a task's step is dragging its card to +another column. That path is not always available. The per-workflow column +visibility filter +([REQ-UI-BOARD-STEP-VISIBILITY-FILTER-001](board-step-visibility-filter.md)) +lets a user hide any step, including the one a task should move to, and the +preview panel narrows the board so remaining columns can sit off-screen. In +both cases the drop target does not exist on screen and the task cannot be +moved without leaving the preview. + +The task top bar already solves the same problem in a narrow space +([REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001](compact-workflow-step-navigation.md)): +a compact current-step indicator that discloses every step with a move control +for each eligible target. This requirement puts that same indicator in the +preview header and constrains its footprint so the header's existing title and +controls keep their space. + +The UI system owns this presentation and its containment. The task system +continues to own workflow order, move eligibility, and task transitions. + +## Terminology + +- **Preview panel:** The task panel that opens beside the kanban board, in + either its inline or its floating layout. +- **Preview header:** The single row at the top of the preview panel holding + the task title and the panel controls. +- **Step indicator:** The compact current-step marker, step name, and position + count defined by + [REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001](compact-workflow-step-navigation.md). +- **Step disclosure:** The temporary surface opened from the step indicator + that lists every step of one workflow. +- **Own workflow:** The workflow named by the previewed task's workflow id, + which is not always the workflow the board is filtered to. +- **Eligible step:** A step that the existing task-move policy permits as a + manual target: the step adjacent to the current step, or a step whose + `allow_manual_move` is set. +- **Panel controls:** The open-full-page control and the close control in the + preview header. + +## Requirements + +### REQ-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001: Step indicator and step navigation in the preview header + +**Intent:** A user reading a task in the preview panel needs to see which +workflow step it is on and to move it to any eligible step, including steps +whose board column is hidden or off-screen, without closing the preview. + +**User story:** As a board user with the preview open, I want to see and change +the previewed task's workflow step from the preview header, so that I can move +the task even when its target column is not on the board. + +#### Acceptance criteria + +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.1:** When the preview panel is + open on a task and that task's own workflow resolves to at least one step, + the preview header shall show a step indicator between the task title and the + panel controls, carrying the current-step marker, the current step name, and + the position count in the form `/`. The marker is suppressed + in the one case AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.14 names, and the + count is omitted in the one case AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.2 + names. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.2:** When the task's own workflow + has exactly one step, the step indicator shall omit the position count. When + that single step is the task's current step, the step disclosure shall offer + no move control, because the current step is never an eligible target. When + the single step is not the task's current step (the + AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.14 fallback), the shared eligibility + policy governs unchanged: a move control shall appear if and only if that + step's `allow_manual_move` is set. This surface shall not override the shared + eligibility policy for the single-step case. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.3:** The steps shown shall be the + steps of the previewed task's own workflow, resolved by that task's workflow + id, including when the board is filtered to a different workflow and when the + board shows several workflows at once. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.4:** Steps shall be ordered by + ascending `position`; when two steps of one workflow share a `position`, the + system shall order them by ascending step `id` compared as a string, so a + given step set always renders in one order. This ordering rule shall apply to + every surface that uses the shared stepper, including the task top bar. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.5:** The step disclosure shall + list every step of the task's own workflow, including a step the user has + hidden through the per-workflow column visibility filter and a step whose + column is scrolled out of view on the board. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.6:** The step disclosure shall + mark the current step and shall offer a move control for each eligible step + and for no other step, using the same eligibility policy as the task top bar. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.7:** When the user selects a move + control, the system shall issue the same task-move request the task top bar + issues, targeting the previewed task's own workflow id and the selected step + id, and shall place the task at the first position of the target step. It + shall not change the task's workflow. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.8:** While a move request is in + flight, every move control in the step disclosure shall be disabled and the + disclosure shall stay open, so one user cannot start a second move from this + surface before the first resolves. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.9:** When a move request succeeds, + the step disclosure shall close, the preview panel shall stay open on the same + task with the same selected session, and no route navigation shall occur. When + the resulting task state arrives, the step indicator shall show the new + current step and position count. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.10:** When a move request fails, + the step disclosure shall stay open and usable, and the preview panel shall + show a move-failure message below the preview header, inside the panel. The + message shall not appear in the preview header row. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.11:** The move-failure message + shall clear when the next move request from this surface starts and when the + preview switches to a different task. Selecting the same target again after a + failure shall issue the same request again; the system shall not deduplicate, + queue, or retry move requests on its own. A move request whose failure + arrives after the preview has stopped showing the task that issued it shall + be discarded rather than rendered. The message shall appear only when the + preview has stayed open on that same task continuously since the request was + issued, so neither switching to another task, nor closing the preview, nor + closing and reopening it on the same task can surface an error from an + earlier request. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.12:** When the previewed task's + step changes from any other source while the step disclosure is open, the + disclosure shall stay open and shall re-derive its current step, its completed + steps, and its eligible targets from the newest task state. When a step is + removed from the workflow while the disclosure is open, its row shall + disappear without leaving a selection behind. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.13:** When the preview panel has no + selected task, or the previewed task has no resolvable workflow id, or its own + workflow resolves to an empty step list, or those steps have not loaded yet, + the preview header shall show no step indicator and shall otherwise render + unchanged. The indicator shall appear once the steps resolve, with no + placeholder or skeleton in between. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.14:** When the previewed task's + current step id matches no step in the resolved list, the step indicator shall + show the first step in order without marking it as the current step, and the + step disclosure shall mark no row as the current step. The disclosure already + behaves this way. The shared step indicator does not: it marks the step it + shows as current unconditionally, and shall be corrected to condition that + marker on a resolved current step. The correction lands in shared code, so it + applies to every surface using that indicator, including the task top bar. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.15:** When the step disclosure is + open and the user presses Escape, the system shall dismiss the step disclosure + only, return focus to the step indicator, and leave the preview panel open. A + further Escape with the disclosure closed shall close the preview panel, which + is its behavior today. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.16:** When the preview panel is in + its floating layout, the step disclosure shall render above the preview panel + and its backdrop, and pointer interaction inside the disclosure shall not + close the preview panel. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.17:** On a coarse-pointer device + that shows the preview panel, the step indicator shall open the same step list + in the touch surface used by the task top bar, with a minimum 44px hit area + for the indicator and for each move control, and a visible disclosure cue. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.18:** The step indicator shall + expose an accessible name carrying the current step name, its step number, and + the total step count, and the disclosure surface shall expose the same dialog + semantics and keyboard path as the task top bar's disclosure. In the + AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.14 fallback there is no current step, + so the accessible name shall instead carry the name and list position of the + step the indicator is showing, which is the first step in order, together with + the total. It shall use the same wording as the resolved case, because that is + exactly what the indicator renders visually in that case: the marker is + suppressed and the visible position count follows the single-step rule, so it + is omitted. The absence of a current step shall be conveyed by withholding the + current-step semantics from the indicator, not by different text, so the + accessible name and the visible content never disagree about which step is + shown or about whether the task is on it. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.19:** When the preview panel closes + or switches to a different task while the step disclosure is open, the disclosure + shall close with it and shall not reopen on the next preview until the user opens + it again. An in-flight move started from the closed disclosure shall still be + applied by the backend; the preview shall not cancel it. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.20:** When a fine-pointer user + hovers or focuses the step indicator, the system shall open the step + disclosure below it as an interactive dialog surface, and shall close it when + the pointer leaves both the indicator and the surface and focus is outside + both. Opening by pointer shall not move focus; opening by keyboard shall move + focus into the surface. + +### REQ-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002: Preview header containment + +**Intent:** The preview panel is user-resizable down to a narrow width. The new +step indicator must take its space from the title, not from the panel controls, +and must never turn the header into a second row or a scrolling surface. + +#### Acceptance criteria + +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.1:** At every preview panel width + from its 300px minimum to its maximum, the preview header shall stay a single + row. No header element shall wrap to a second line. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.2:** At the 300px minimum width, + both panel controls shall stay fully inside the panel and shall stay + clickable. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.3:** At the 300px minimum width, + the task title shall keep at least 88px of rendered width and shall truncate + with an ellipsis rather than wrap or displace any other header element. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.4:** After the panel controls and + the header row's inter-element gaps take their fixed width, the step indicator + shall claim no more than half of the width that remains, at every panel width. + When the step name is too long for the space that leaves it, the step name + shall truncate while the current-step marker and the position count stay + visible. This cap is a maximum, not a reservation: when a header row cannot + satisfy both this cap and the task title floor + AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.3 sets, the title floor wins and the + step indicator shall shrink below its cap. The two bounds never conflict while + the header row's inter-element gaps total 18px or less, which the system design + derives from the narrower of the two preview layouts and which the header shall + respect. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.5:** The preview header shall not + introduce horizontal scrolling in the preview panel at any supported width. +- **AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.6:** Truncated header text shall + keep its full value available to assistive technology and shall not be the + only carrier of the step name, step number, or total. + +## Decisions + +- The preview reuses the task top bar's compact stepper, disclosure surface, + eligibility policy, and move request rather than a preview-specific variant. + Two step surfaces with two behaviors is the failure this avoids. "Two surfaces" + means these two specifically. Other places that move a task between steps, + including board drag and drop, the swimlane and multi-select movers, and the + task session sidebar's own optimistic mover, keep their existing + implementations and are not consolidated by this requirement; the system design + enumerates them and says why. +- Moving a task from the preview keeps the task top bar's existing plan-mode + cleanup for the previewed session, including its layout reset. Plan mode is + session state, not page state, so leaving it from one surface must leave it + everywhere. +- Concurrent moves from two clients are resolved by the backend. The preview + shows whatever task state it then receives and does not attempt to reconcile + or warn. + +## Out of scope + +- Moving the previewed task to a different workflow. The task context menu + keeps that path, and this surface deliberately keeps the exclusion stated in + [REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001](compact-workflow-step-navigation.md). +- Any change to per-workflow column visibility, to board drag and drop, or to + which columns the board renders. +- Any change to the task top bar's full or compact stepper, in presentation or + in behavior, other than the three corrections this requirement makes to shared + code deliberately. The first two are presentation: the ordering tiebreak in + AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.4 and the current-step marker fix in + AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.14. The third is behavior: the shared + move hook scopes a late move failure, and the in-flight step id, to the + presentation that issued the request, per + AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.11 and the system design's failure and + recovery rules. On the top bar that means a move failure arriving after the user + has navigated away from the task, or away and back to it, no longer raises the + move-failure message, where today it can. All three live in code the top bar + shares, and the requirement's own Decisions section forbids a second copy of + that code, so the top bar inherits all three by construction rather than by + choice. The third is deliberate and not incidental: a failure that outlives the + presentation which issued it is wrong on both surfaces, and shipping the fix on + the preview alone would leave the identical defect on its sibling. No other top + bar change is in scope. +- An archived-task presentation in the preview header. The board excludes + archived tasks and the preview closes when its task leaves the board, so the + archived indicator path is not reachable from this surface. A future change + that makes archived tasks previewable owns that presentation. +- A phone presentation. Phones open the task route instead of the preview + panel, so this surface adds no phone entry point and the existing phone task + drawer keeps its **Move to** path. +- Showing wip-limit queue state, `queued_for_step`, or dependency blocking in + the step indicator. +- Any change to what entering a step triggers. Step `on_enter` actions, + including agent auto-start, behave exactly as they do for a board drag or a + task top bar move. +- New user-facing copy. This surface reuses the existing translated strings for + the step indicator, the disclosure, and the move-failure message. diff --git a/docs/specs/ui/system-design/kanban-preview-workflow-step-navigation.md b/docs/specs/ui/system-design/kanban-preview-workflow-step-navigation.md new file mode 100644 index 0000000000..909fb31b38 --- /dev/null +++ b/docs/specs/ui/system-design/kanban-preview-workflow-step-navigation.md @@ -0,0 +1,411 @@ +--- +status: draft +system: ui +requirements: + - REQ-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001 + - REQ-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002 +--- + +# Kanban Preview Workflow Step Navigation System Design + +## Purpose and boundaries + +The UI system owns the preview panel header and the compact stepper it now +hosts. The task system owns workflow data and the task-move API. This design +adds a consumer of the existing compact stepper; it does not add a backend +contract, a store field, or a persisted preference. + +## Requirement mapping + +| Requirement | Design section | +| ------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `REQ-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001` | [Components](#components-and-responsibilities), [Step resolution](#step-resolution), [Control flow](#control-flow), [Failure and recovery](#failure-and-recovery) | +| `REQ-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002` | [Header layout](#header-layout) | + +## Components and responsibilities + +- `TaskPreviewPanel` owns the preview header. It gains a step indicator slot + between the title and the panel controls, and a move-failure slot below the + header row. +- The compact stepper and its disclosure body stay the single implementation of + the indicator, the step list, and eligibility. They do not implement the move + request: they take an `onMove` callback and call it. The preview renders the + compact presentation unconditionally rather than measuring available width: + the preview header is never wide enough for the full stepper, and a width + measurement here would only add a flicker. +- The move request is today private to `handleMove` inside `WorkflowStepper`, + which the preview does not render, so there is nothing for the preview to + inherit. That logic is extracted into one shared hook, `useWorkflowStepMove`, + which becomes the single implementation of the move request **for the compact + stepper surfaces** and owns: the + move call itself, the plan-mode cleanup for the task's active session, the + in-flight step id, the request-identity guard that discards a superseded or + late response, and the move-start and move-error callbacks. `WorkflowStepper` + and the preview both consume the hook; neither reimplements any part of it. + Extraction rather than duplication is what makes the plan-mode invariant in + the requirement's Decisions section hold on both surfaces, and it is why a + second copy of this logic is not an acceptable implementation. +- **That claim is scoped to the compact stepper, and deliberately so.** Moving a + task between workflow steps is not otherwise centralised in this codebase, and + this design does not centralise it. `moveTask` / `moveTaskById` + (`lib/api/domains/kanban-api.ts`, wrapped by `hooks/use-task-actions.ts`) has + several independent callers besides the stepper: the board's drag and drop + (`hooks/use-drag-and-drop.ts`), the swimlane movers + (`hooks/domains/kanban/use-swimlane-move.ts` and the + `components/kanban/swimlane-*-content.tsx` surfaces), multi-select bulk moves + (`hooks/use-task-multi-select.ts`), plan actions + (`hooks/domains/kanban/use-plan-actions.ts`), and the task session sidebar's own + mover, `useMoveToStep` (`components/task/task-session-sidebar-move.ts`). + `useWorkflowStepMove` replaces exactly one of them, `handleMove`, and leaves + every other caller untouched. Read unqualified, "the single implementation of + the move request" would tell a builder to consolidate all of them, which is a + different and much larger feature than this one. +- **`useMoveToStep` in particular stays where it is.** It is not a duplicate of + the hook being extracted: it applies the move optimistically to + `kanbanMulti.snapshots[workflowId].tasks`, keeps a per-task generation counter so + a rejection cannot roll back a newer in-flight move, and rolls the store back + when the backend refuses. Those semantics serve the sidebar's own interaction and + carry their own tests. Folding them into `useWorkflowStepMove` would turn the + preview's and the task top bar's moves from authoritative into optimistic, which + no acceptance criterion here asks for and which would change a surface this + requirement puts out of scope. The two hooks coexist. +- **They can be on screen together, and the existing rules already resolve it.** + `TaskSessionSidebar` renders from + `components/app-sidebar/sections/tasks-section.tsx`, the persistent app sidebar, + not from inside the preview, so one user can move the same task from the sidebar + and from the preview header. Neither hook needs to know about the other: the + backend serialises the moves and the preview renders whatever task state it then + receives, per the requirement's Decisions section. If the sidebar's optimistic + write, or its rollback, changes the previewed task's step, the indicator + re-derives from the newest task state exactly as it does for any other + out-of-band step change + (AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.12), including while the disclosure is + open. A rollback moving the indicator backwards is therefore correct behaviour, + not a race to guard against: the indicator is showing the step the task is + actually on. +- `canMoveToStep` remains the single UI policy for target eligibility. +- `useTouchDrawer` continues to select the popover or the drawer surface from + pointer precision. +- `KanbanWithPreview` resolves the previewed task's own workflow step list and + passes it, the task id, the workflow id, and the current step id down to + `TaskPreviewPanel`. It also owns the move-failure state for the panel, cleared + on a new move and on a task change. + +## Step resolution + +The previewed task carries its own workflow id, which +`KanbanWithPreview` already derives (from the task record, or from the snapshot +key when a boot-hydrated snapshot task omits it). + +Steps resolve from the store in one rule, the same one `plugin-context-api` +uses: + +- when the task's workflow id equals `kanban.workflowId`, use `kanban.steps`; +- otherwise use `kanbanMulti.snapshots[workflowId].steps`; +- when neither resolves, the list is empty and no indicator renders. + +`useAllWorkflowSnapshots` runs whenever the board mounts, for every workflow in +the workspace, so the second branch covers the multi-workflow board and a +preview opened on a task outside the active workflow filter. The snapshot step +shape already carries `allow_manual_move`, `events`, and `agent_profile_id`, so +the mapping to the stepper's step type is the same field-for-field mapping the +task page performs. + +Board column visibility (`hiddenWorkflowStepIds`) is a board rendering filter +and is deliberately not consulted here. That is the point of the feature: the +disclosure is the way to reach a step whose column is hidden. + +## Ordering and determinism + +The ordering AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.4 states is already +implemented in this repository, and this design adopts it rather than writing it +again. `sortWorkflowStepsByPosition`, exported from +`apps/web/lib/kanban/auto-hide-empty-columns.ts`, sorts a copy by ascending +`position` and breaks ties on ascending `id` compared as a string, which is +exactly the rule that AC names. It already has a unit test for that determinism, +and the board already orders its columns through it, from +`components/kanban/columns-menu.tsx` and `components/kanban/swimlane-container.tsx`. + +So AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.4 holds on the board surfaces today. +What does not hold is the stepper: `WorkflowStepper` sorts with its own inline +comparator on `position` alone, private to the component, with no tiebreak. That +is the only non-compliant ordering, and the preview cannot inherit the fix by +rendering `WorkflowStepper`, because it does not render it. + +**The comparator moves; it is not rewritten and it is not duplicated.** +`sortWorkflowStepsByPosition` moves, unchanged in name and in body, out of +`auto-hide-empty-columns.ts` and into `apps/web/lib/kanban/workflow-step-order.ts`, +a module that owns workflow step ordering and nothing else. The name and location +follow the existing `lib/kanban/task-order.ts`, which is the in-repo precedent for +a small ordering-only module and orders the board's tasks the way this one orders +its steps. Its existing unit test moves with it, to +`workflow-step-order.test.ts`. Its two existing +board consumers change their import path and nothing else. `WorkflowStepper` and +the preview's step resolution then both import that one function, and +`WorkflowStepper`'s private inline sort is deleted. + +Three things this rules out, each of which a builder would otherwise have to +decide alone: + +- **A second utility is not acceptable**, whatever it is called. Two shared + functions with the same comparator leave two sets of call sites free to drift, + which is the failure this section exists to prevent, and under drift the board + columns and the disclosure list could order equal-position steps by two + different rules, in a feature whose whole premise is that a disclosure row + stands in for a board column the user cannot see. +- **Importing it from `auto-hide-empty-columns.ts` where it sits today is not + acceptable either**, even though it would work. That module is named for a + different feature, and shared stepper code depending on it would break for a + reason nobody could predict the day the auto-hide feature is removed. +- **The exported name stays `sortWorkflowStepsByPosition`.** `position` is the + primary key and the tiebreak is secondary, so the name is accurate; renaming + would churn two call sites and a test for no behavioural gain. A builder should + not improve it in passing. + +Moving the function changes no rendered board column order, because the +comparator is byte-identical before and after, so the requirement's exclusion of +changes to which columns the board renders is untouched. The two board files are +edited only in their import statement. + +The function returns a new array and does not mutate its input, which is what +lets every surface sort the same store-owned step list independently. An empty +step list sorts to an empty list, and there is no missing-`position` case to +define: `position` is a required `number` on the store's step type, so the +comparator never sees one absent. Two steps cannot share an `id`, so the tiebreak +is total. + +No other ordering exists in this surface: the disclosure renders the sorted list +top to bottom, and the position count is the current step's index in that same +list. When the step list changes underneath an open disclosure +(AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.12), it is re-sorted from the newest +store state by the same function, so a re-derivation cannot produce a different +order than a first render of the same steps. + +## Header layout + +The preview header stays one flex row: title, step indicator, panel controls. + +- The panel controls do not shrink. +- The step indicator sits in a shrinkable container with an upper bound on the + share of the row it can claim, so a long step name cannot crowd the title out + of the row. That bound is half of the row's width remaining after BOTH the + panel controls and the row's inter-element gaps take their fixed width + (AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.4). Both subtractions are load + bearing, and so is the base width they are subtracted from; the next bullet + derives all three and records why. +- The arithmetic at the 300px panel minimum, with `g` the total of the row's + inter-element gaps. The 300px is the width of the panel's OUTER container, not + the header's content box, and several terms sit between the two. Enumerated in + order for the inline layout: that container carries `border-l` (1px); it holds + the resize handle (`w-1`, 4px, non-shrinking) as a flex sibling before the + panel; the panel's own root carries `border-l` (1px); and the header row adds + `px-4` on both sides (32px). Box sizing is `border-box` throughout, so each + border is inside the width it is measured against. The header's content box is + therefore `300 - 1 - 4 - 1 - 32` = **262px**. The two `h-8 w-8` panel controls + plus their own `gap-1` take 68px, leaving 194px, of which the gaps take `g`. + The indicator's cap is therefore `(194 - g) / 2`, and the title is left + `(194 - g) / 2`. The title floor of 88px + (AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.3) holds while `g <= 18px`. That is + the constraint the header must respect. It remains generous in practice, since + the header carries no gap class today, so `g = 0` and the title gets 97px. +- Every term in that derivation is named on purpose. Two earlier drafts of this + section each stated a bound computed from premises they had not inspected: the + first omitted the inter-element gaps, the second took the header's base width + to be the 300px minimum less only its own padding, missing the container + border, the resize handle and the panel border between them. Both produced a + bound that looked safe and was not. A bound whose terms are not enumerated + cannot be re-checked when the layout around it changes, so the enumeration is + part of the contract rather than commentary on it. +- The two layouts differ by one pixel at the same panel width. The floating + layout's outer container has no `border-l`, so its header content box is 263px + and its remainder 195px, giving `g <= 19px`. The inline layout is therefore the + binding case, and `g <= 18px` is the single bound that holds for both: a header + satisfying the inline layout satisfies the floating one by construction. +- When a row cannot satisfy both bounds — which the inequality above prevents at + the 300px minimum, but which must still resolve deterministically at any width + and under any future spacing — the title floor wins and the indicator shrinks + below its cap. The cap is a maximum, never a reservation, so yielding is always + available to it; the floor is a minimum and has nowhere to yield to. The two + bounds are stated separately because the cap governs at every width while the + floor is what an assertion at the minimum width checks. +- The cap is a share of the width remaining after the controls, NOT a percentage + of the whole row. A plain `max-width: 50%` on the indicator would resolve + against the row's full 262px content box and yield 131px, which is not this + bound. Expressing it takes a nested shrinkable group holding the title and the + indicator, or an equivalent that excludes the controls from the percentage + basis. Relatedly, the header is `justify-between` with two children today; a + third child changes how free space is distributed, so the row's alignment is + part of what the containment assertions must cover rather than something that + survives the change untouched. Free space goes to the title, which is the + shrinkable and growable element: the step indicator sits adjacent to the panel + controls at the end of the row rather than floating between the two, so a + short title does not leave the indicator stranded mid-row and the indicator's + position does not move as the title's length changes. +- The title stays a shrinkable, truncating element with a minimum width of 88px. +- Both the title and the step name truncate with an ellipsis. The position count + and the current-step marker do not shrink, so the count stays readable while + the name truncates. +- The indicator's accessible name carries step name, number, and total, so + truncation costs no information. + +The narrow bound is the panel's 300px minimum width in the inline layout, which +is the narrower of the two by the pixel derived above. The header must be proven +there, not only at the 500px default and not only in the floating layout. + +## Control flow + +1. The user opens the preview on a task. `KanbanWithPreview` resolves the task's + own workflow steps and current step id. +2. `TaskPreviewPanel` renders the step indicator when the list is non-empty. +3. Hover, focus, or touch activation opens the disclosure with every step of that + workflow in sorted order. +4. The disclosure marks the current step and enables only eligible targets. +5. Selecting a target disables every move control in the surface and sends the + existing move request with the task's own workflow id, the target step id, + and position `0`. +6. A success closes the disclosure. The preview stays open on the same task and + the same session; live task state then drives the indicator to the new step. +7. A failure leaves the disclosure open and raises the move-failure message + below the header. + +## Dismissal and stacking + +`KanbanWithPreview` closes the preview from a window-level Escape listener. The +disclosure closes from its own document-level Escape handling. Without a guard, +one Escape press would close both, because the disclosure's handler runs on +`document` and the preview's runs on `window` for the same event. + +The preview's Escape handler must therefore ignore an Escape that a disclosure +open inside the panel is already consuming. Whatever mechanism carries that +signal, the observable contract is the one in +AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.15: the first Escape closes the +disclosure only, the second closes the preview. + +The disclosure content is portalled to the document body at `z-50`, above the +floating preview panel at `z-40` and its backdrop at `z-30`. Because the +disclosure is not a descendant of the backdrop, a click inside it never reaches +the backdrop's close handler. + +## Failure and recovery + +The move request stays the authority for transition errors. On error the +in-flight state clears, every move control re-enables, and the panel renders the +existing move-failure banner below the header. The banner clears when the next +move starts and when the previewed task changes. A failure that arrives after +the preview has stopped showing the task that issued the request is discarded +rather than rendered, which is the case clearing alone cannot cover: clearing +acts on a message that already exists, and a late response would otherwise +create one after the fact. Per +AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.11 the test is whether the preview +stayed open on that same task **continuously** since the request was issued, so +all three of switching to another task, closing the preview, and closing it and +reopening it on the same task must discard. Together the two rules give the +invariant that a stale error is never rendered against a presentation that did +not issue it. + +That test is not decidable from the task id, and it is not decidable from the +in-flight counter the current move code carries. `handleMove` guards its late +response with a monotonic request counter held in a component ref, which +distinguishes only one request from a newer one issued by the same live +component; on a close and reopen of the same task the task id is unchanged and +the counter is not advanced by either event, so both signals report "same task, +newest request" for a request the user has since walked away from. Nor can the +hook lean on being unmounted: the move-failure state lives in +`KanbanWithPreview`, which stays mounted across a preview close, and the two +layouts disagree about the panel subtree — the floating layout unmounts it with +the preview while the inline layout renders it inside a pane that stays mounted. + +So the identity the hook carries must be a **presentation instance**: a token +minted each time the preview begins showing a task, and invalidated when the +preview closes or switches away, such that reopening on the same task mints a +new one. A response is rendered only when the token captured at request time is +still the current one. Extracting `handleMove` verbatim does not produce this — +the counter is necessary for supersession and not sufficient for continuity, and +`useWorkflowStepMove` owns both. This is the one place where the extraction adds +behaviour rather than relocating it; the task top bar inherits it, which is +correct, because a late failure that outlives its own presentation is wrong on +both surfaces. + +The token is supplied by the consuming surface, because only the surface knows +what one of its presentations is. The preview supplies its continuous +presentation of a task, per the rule above. The task top bar supplies the task +route's continuous presentation of a task, so a navigation away and back mints a +new token there for the same reason a close and reopen does here. A surface with +no finer notion than the task itself supplies a token that changes at least +whenever the task changes, which degrades to today's behaviour rather than to +something undefined. + +The in-flight step id is scoped to the same token. A new presentation therefore +begins with no in-flight move and no disabled control even while an earlier +request is still outstanding, which is what keeps +AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.19's guarantee — the backend still +applies that move, the preview does not cancel it — from leaving a reopened +preview with a permanently disabled row. A discarded response neither renders an +error nor re-enables or disables anything in the presentation that replaced it; +it affects only the presentation that issued it, which by then is gone. + +An absent current step falls back to showing the first step in order. The +current-step marker is suppressed in that case, so the surface never asserts a +step the task is not on. The disclosure body already behaves this way; the +shared indicator does not, and is corrected here +(AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.14). The correction is in shared +code, so it also removes the same false marker from the task top bar. + +## Test strategy + +Component tests cover: indicator presence and absence across resolvable, +unresolvable, empty, and single-step workflows; step resolution for a task +outside the active workflow filter; the position tiebreak; eligibility and the +current-step row; the in-flight disable; the success path leaving the panel open; +the failure banner and its clearing rules; and the +discard of a late failure across all three discontinuities +AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.11 names, the reopen-on-the-same-task +case included, since that is the one the task id and the request counter both +report as continuous. + +Because the presentation-instance token lives in the shared move hook and both +surfaces inherit it, the discard rule is tested against the hook itself, on a +token change, and not only through the preview. Otherwise the top bar's half of +the third correction the requirement's Out of scope names ships with no coverage +at all, on the same argument that applies to the marker fix below: the code runs +in production on two surfaces, so a test that exercises one of them leaves the +other unguarded. The ordering utility keeps its existing unit test, which already +asserts the tiebreak and non-mutation, and moves with it; what is new there is +that `WorkflowStepper` now sorts through it, so the stepper's own tests cover +equal positions rather than assuming they cannot occur. + +The correction AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-001.14 makes to the shared +indicator needs an observable that distinguishes a resolved current step from +the first-step fallback, and the implementation must expose one; today the three +marker states differ only by presentation classes on unlabelled elements, which +is not a contract a test can hold. Which observable to expose is an +implementation choice, but a test that cannot tell the corrected marker from the +uncorrected one does not cover this AC. In particular the existing shared-stepper +test for the collapsed no-current-step case asserts only the absence of +`aria-current`, which is already conditioned correctly in the current code and so +passes against the uncorrected marker: it is a false negative for this +correction, not coverage of it. The correction lands in code the task top bar +renders in production, so an untested change here regresses two surfaces. + +Desktop E2E covers the motivating scenario end to end: hide a step's column +through the board column visibility filter, open the preview on a task, open the +disclosure, confirm the hidden step is still listed, move the task to it, and +assert the task's step through the API. A second scenario resizes the preview to +its minimum width **in the inline layout**, with a step name long enough to drive +the indicator to its cap, and asserts single-row containment, both panel controls +clickable, and a title of at least the 88px +AC-UI-KANBAN-PREVIEW-STEP-NAVIGATION-002.3 requires — the literal floor, not +merely a non-zero width, because a non-zero assertion passes on a one-pixel +title and would leave the containment budget unproven at exactly the width it +was written for. The inline layout is specified because it is the binding case: +its header content box at the 300px minimum is 262px against the floating +layout's 263px, so a title floor proven there holds for both, while one proven +only in the floating layout does not. A third asserts the two-stage Escape. + +Tablet E2E reuses the existing touch-drawer path against the preview indicator. + +## Related decisions + +No architecture decision record applies. This design reuses the compact stepper +and task-move contracts already established by +`REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001`.