feat(web): show and move a task's workflow step from the kanban preview (#3329)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -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)}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
@@ -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)}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -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 };
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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: () => <div data-testid="preview-session-tabs" />,
|
||||
}));
|
||||
|
||||
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<ComponentProps<typeof TaskPreviewPanel>> = {}) {
|
||||
return render(
|
||||
<TaskPreviewPanel
|
||||
task={TASK}
|
||||
onClose={vi.fn()}
|
||||
workflowSteps={STEPS}
|
||||
currentStepId={STEP_A_ID}
|
||||
taskWorkflowId={WORKFLOW_ID}
|
||||
onMoveStep={vi.fn()}
|
||||
{...overrides}
|
||||
/>,
|
||||
);
|
||||
}
|
||||
|
||||
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(
|
||||
<TaskPreviewPanel
|
||||
task={OTHER_TASK}
|
||||
onClose={vi.fn()}
|
||||
workflowSteps={STEPS}
|
||||
currentStepId={STEP_A_ID}
|
||||
taskWorkflowId={WORKFLOW_ID}
|
||||
onMoveStep={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
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<boolean>((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(
|
||||
<TaskPreviewPanel
|
||||
task={OTHER_TASK}
|
||||
onClose={vi.fn()}
|
||||
workflowSteps={STEPS}
|
||||
currentStepId={STEP_A_ID}
|
||||
taskWorkflowId={WORKFLOW_ID}
|
||||
onMoveStep={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
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(<TaskPreviewPanel task={TASK} onClose={vi.fn()} workflowSteps={[]} />);
|
||||
|
||||
expect(onDisclosureOpenChange).toHaveBeenLastCalledWith(false);
|
||||
});
|
||||
});
|
||||
@@ -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<boolean>;
|
||||
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<boolean>;
|
||||
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 (
|
||||
<div className="flex items-center justify-between border-b px-4 py-3">
|
||||
<div className="flex min-w-0 flex-1 items-center">
|
||||
<h2 className="min-w-[88px] flex-1 truncate text-sm font-semibold">
|
||||
{task?.title ?? t("task:taskChat")}
|
||||
</h2>
|
||||
{handleMoveStep && (
|
||||
<div
|
||||
// Keyed by task id so a task switch remounts this subtree instead of
|
||||
// reusing it: the disclosure's open state is local to it, and nothing
|
||||
// else ties that state (or a still-settling move promise's closure)
|
||||
// to which task is being previewed.
|
||||
key={task?.id ?? "none"}
|
||||
// `w-full` on the trigger button counteracts a browser quirk:
|
||||
// a <button> 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"
|
||||
>
|
||||
<MinimalWorkflowStepper
|
||||
sortedSteps={workflowSteps}
|
||||
currentIndex={currentIndex}
|
||||
taskId={task?.id ?? null}
|
||||
workflowId={taskWorkflowId}
|
||||
isArchived={isArchived}
|
||||
movingToStepId={movingToStepId}
|
||||
onMove={handleMoveStep}
|
||||
onDisclosureOpenChange={onDisclosureOpenChange}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
<div className="flex items-center gap-1">
|
||||
{onMaximize && task && (
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="icon"
|
||||
className="h-8 w-8 cursor-pointer"
|
||||
onClick={() => onMaximize(task)}
|
||||
title={t("common:openFullPage")}
|
||||
>
|
||||
<IconArrowsMaximize className="h-4 w-4" />
|
||||
<span className="sr-only">{t("common:openFullPage")}</span>
|
||||
</Button>
|
||||
)}
|
||||
<Button variant="ghost" size="icon" className="h-8 w-8 cursor-pointer" onClick={onClose}>
|
||||
<IconX className="h-4 w-4" />
|
||||
<span className="sr-only">{t("task:closePreview")}</span>
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
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 */}
|
||||
<div className="flex items-center justify-between border-b px-4 py-3">
|
||||
<h2 className="text-sm font-semibold truncate">{task?.title ?? t("task:taskChat")}</h2>
|
||||
<div className="flex items-center gap-1">
|
||||
{onMaximize && task && (
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="icon"
|
||||
className="h-8 w-8 cursor-pointer"
|
||||
onClick={() => onMaximize(task)}
|
||||
title={t("common:openFullPage")}
|
||||
>
|
||||
<IconArrowsMaximize className="h-4 w-4" />
|
||||
<span className="sr-only">{t("common:openFullPage")}</span>
|
||||
</Button>
|
||||
)}
|
||||
<Button variant="ghost" size="icon" className="h-8 w-8 cursor-pointer" onClick={onClose}>
|
||||
<IconX className="h-4 w-4" />
|
||||
<span className="sr-only">{t("task:closePreview")}</span>
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
<PreviewPanelHeader
|
||||
task={task}
|
||||
onClose={onClose}
|
||||
onMaximize={onMaximize}
|
||||
workflowSteps={workflowSteps}
|
||||
currentStepId={currentStepId}
|
||||
taskWorkflowId={taskWorkflowId}
|
||||
isArchived={isArchived}
|
||||
movingToStepId={movingToStepId}
|
||||
onMoveStep={onMoveStep}
|
||||
onDisclosureOpenChange={onDisclosureOpenChange}
|
||||
/>
|
||||
|
||||
{moveError !== null && <TaskMoveErrorBanner error={moveError} />}
|
||||
|
||||
{/* Content */}
|
||||
<div className="flex-1 min-h-0 flex flex-col">
|
||||
|
||||
@@ -48,6 +48,8 @@ type MinimalWorkflowStepperProps = {
|
||||
workflowId?: string | null;
|
||||
movingToStepId: string | null;
|
||||
onMove: (stepId: string) => Promise<boolean>;
|
||||
/** 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<boolean>;
|
||||
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 = (
|
||||
<CompactWorkflowTrigger
|
||||
current={current}
|
||||
@@ -376,6 +391,7 @@ function CompactWorkflowStepDisclosure({
|
||||
className="w-72 max-w-[calc(100vw-1rem)] p-2"
|
||||
onOpenAutoFocus={controls.handleOpenAutoFocus}
|
||||
onCloseAutoFocus={controls.handleCloseAutoFocus}
|
||||
onEscapeKeyDown={(event) => 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"
|
||||
>
|
||||
<StepCircleIndicator isCurrent isCompleted={false} />
|
||||
<span className="truncate text-xs font-medium leading-none text-foreground">
|
||||
<StepCircleIndicator isCurrent={currentIndex >= 0} isCompleted={false} />
|
||||
<span className="min-w-0 truncate text-xs font-medium leading-none text-foreground">
|
||||
{current.name}
|
||||
</span>
|
||||
</div>
|
||||
@@ -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 (
|
||||
<span className="relative flex items-center justify-center shrink-0">
|
||||
<span
|
||||
data-marker-state={state}
|
||||
className="relative flex items-center justify-center shrink-0"
|
||||
>
|
||||
<span className="absolute h-3.5 w-3.5 rounded-full border-2 border-primary/40" />
|
||||
<span className="h-2 w-2 rounded-full bg-primary" />
|
||||
</span>
|
||||
@@ -548,13 +572,16 @@ export function StepCircleIndicator({
|
||||
}
|
||||
if (isCompleted) {
|
||||
return (
|
||||
<span className="relative flex items-center justify-center shrink-0">
|
||||
<span
|
||||
data-marker-state={state}
|
||||
className="relative flex items-center justify-center shrink-0"
|
||||
>
|
||||
<span className="h-2 w-2 rounded-full bg-muted-foreground/60" />
|
||||
</span>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<span className="relative flex items-center justify-center shrink-0">
|
||||
<span data-marker-state={state} className="relative flex items-center justify-center shrink-0">
|
||||
<span className="h-2 w-2 rounded-full border border-muted-foreground/40" />
|
||||
</span>
|
||||
);
|
||||
|
||||
@@ -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(<WorkflowStepper steps={TIEBREAK_STEPS} currentStepId={null} />);
|
||||
|
||||
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(<WorkflowStepper steps={STEPS} currentStepId="b" />);
|
||||
@@ -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(<WorkflowStepper steps={STEPS} currentStepId={null} />);
|
||||
|
||||
// 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(<WorkflowStepper steps={STEPS} currentStepId="a" />);
|
||||
|
||||
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(
|
||||
|
||||
@@ -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<string | null>(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<boolean> => {
|
||||
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<HTMLDivElement>(null);
|
||||
const isCollapsed = useToolbarCollapsed(containerRef);
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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<boolean>;
|
||||
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<unknown>(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,
|
||||
};
|
||||
}
|
||||
@@ -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<string, boolean> },
|
||||
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<ReturnType<typeof moveTask>>);
|
||||
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<ReturnType<typeof moveTask>>);
|
||||
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<boolean>;
|
||||
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<ReturnType<typeof moveTask>>);
|
||||
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<boolean>;
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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<boolean>;
|
||||
};
|
||||
|
||||
/**
|
||||
* 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<string | null>(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<boolean> => {
|
||||
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 };
|
||||
}
|
||||
@@ -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<string, { steps: StoreStep[] }> };
|
||||
};
|
||||
|
||||
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"]);
|
||||
});
|
||||
});
|
||||
@@ -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]);
|
||||
}
|
||||
@@ -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"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,12 +1,6 @@
|
||||
type Step = { id: string };
|
||||
type StepTask = { workflowStepId: string | null };
|
||||
|
||||
export function sortWorkflowStepsByPosition<T extends Step & { position: number }>(
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -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"]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,7 @@
|
||||
type Step = { id: string };
|
||||
|
||||
export function sortWorkflowStepsByPosition<T extends Step & { position: number }>(
|
||||
steps: T[],
|
||||
): T[] {
|
||||
return [...steps].sort((a, b) => a.position - b.position || a.id.localeCompare(b.id));
|
||||
}
|
||||
@@ -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.
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
"product": 16384,
|
||||
"requirement": 20480,
|
||||
"system-design": 32768,
|
||||
"system-index": 12288,
|
||||
"system-index": 16384,
|
||||
"template": 8192
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 `<current>/<total>`. 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.
|
||||
@@ -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`.
|
||||
Reference in New Issue
Block a user