From 7f24cce795b2bd355ad02bc45599b29422c3f54c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 26 Aug 2026 13:11:37 +0000 Subject: [PATCH] feat: expand compact workflow step navigation (#3054) --- .agents/skills/mobile-parity/SKILL.md | 2 +- apps/web/AGENTS.md | 2 +- .../task/workflow-step-disclosure.tsx | 567 ++++++++++++++++++ .../components/task/workflow-stepper.test.tsx | 181 +++++- apps/web/components/task/workflow-stepper.tsx | 131 +--- apps/web/e2e/helpers/animations.ts | 11 + .../task-topbar-workflow-stepper.spec.ts | 144 +++++ .../plugins/bitbucket-packaged-plugin.spec.ts | 13 +- .../tests/review/submodule-review-helpers.ts | 11 +- .../compact-workflow-step-navigation/plan.md | 121 ++++ .../task-01-compact-step-disclosure.md | 126 ++++ docs/specs/ui/README.md | 4 +- .../compact-workflow-step-navigation.md | 50 ++ .../compact-workflow-step-navigation.md | 114 ++++ 14 files changed, 1336 insertions(+), 141 deletions(-) create mode 100644 apps/web/components/task/workflow-step-disclosure.tsx create mode 100644 apps/web/e2e/helpers/animations.ts create mode 100644 apps/web/e2e/tests/layout/task-topbar-workflow-stepper.spec.ts create mode 100644 docs/plans/compact-workflow-step-navigation/plan.md create mode 100644 docs/plans/compact-workflow-step-navigation/task-01-compact-step-disclosure.md create mode 100644 docs/specs/ui/requirements/compact-workflow-step-navigation.md create mode 100644 docs/specs/ui/system-design/compact-workflow-step-navigation.md diff --git a/.agents/skills/mobile-parity/SKILL.md b/.agents/skills/mobile-parity/SKILL.md index 9cab3b6f5f..cb8c54cff3 100644 --- a/.agents/skills/mobile-parity/SKILL.md +++ b/.agents/skills/mobile-parity/SKILL.md @@ -64,7 +64,7 @@ When a task changes composition, navigation, overlays, touch behavior, scrolling 3. Implement responsive UI. - Reuse domain hooks, state, view-model derivation, filtering/selection, and action handlers across viewports; keep responsive wrappers focused on presentation. Branch composition when the desktop interaction model depends on width or a fine pointer. Do not mount a heavyweight desktop workbench and merely hide or squeeze it on phones. - Use `useResponsiveBreakpoint` and existing `@kandev/ui` or mobile primitives. Reuse current Dropdown/ContextMenu primitives for contextual actions; use `Drawer` or an existing picker shell for structured phone navigation and choices. Use `useTouchDrawer` when a hover disclosure needs a coarse-pointer alternative. - - Keep touch targets large enough for touch use, generally at least 44px in the active dimension. + - Keep coarse-pointer and mobile touch targets large enough for touch use, generally at least 44px in the active dimension. This is an active hit-area rule, not a universal desktop visual-size rule: fine-pointer desktop controls should retain the surrounding design-system density, and touch-sized classes such as `h-11` must not become the shared desktop button size. - Use dynamic viewport units and an explicit internal scroll region for viewport-bound/full-height or potentially overflowing surfaces. Ensure bottom-fixed controls and tall drawers clear safe-area insets; short drawers can retain the shared primitive's intrinsic sizing. Keep document-level horizontal overflow at zero. - If mobile substitutes an unsupported desktop view, derive an effective mobile view without overwriting the user's saved desktop preference. - Use semantic controls, visible labels or accessible names, focus return, and existing design-system components. diff --git a/apps/web/AGENTS.md b/apps/web/AGENTS.md index 1f86bff141..2fdb6b146e 100644 --- a/apps/web/AGENTS.md +++ b/apps/web/AGENTS.md @@ -28,7 +28,7 @@ import { Dialog } from "@kandev/ui/dialog"; - Use `hooks/use-responsive-breakpoint.ts` for application layout decisions. Its mobile boundary matches the sidebar's 768px `md` boundary, and it also models tablet, compact desktop, full desktop, and pointer precision; do not substitute the UI package's generic `useIsMobile` hook. Tablet is a coarse-pointer fallback between `md` and `lg`, so no fine-pointer width reports it — a spec that needs the tablet layout has to emulate touch. - When a Tailwind visibility class gates the same surface the hook picks, both must use the same boundary. A `sm:` class paired with a hook-driven mobile branch leaves 640-767px in a state neither side renders. -- Use `useTouchDrawer` when a hover/popover disclosure needs a coarse-pointer `Drawer` alternative. Width-based phone composition and pointer-based disclosure behavior are related but not interchangeable. +- Use `useTouchDrawer` when a hover/popover disclosure needs a coarse-pointer `Drawer` alternative. Width-based phone composition and pointer-based disclosure behavior are related but not interchangeable. Apply the 44px minimum to coarse-pointer hit areas, touch rows, and mobile controls only; keep fine-pointer desktop controls at the surrounding design-system density and do not reuse a touch-sized `h-11` class as the shared visual button size. - Existing Radix DropdownMenu and ContextMenu surfaces receive inset, safe-area-aware bottom-sheet treatment below 640px in `app/globals.css`. Reuse those primitives for contextual actions and add focused coverage for long or nested menus instead of creating a parallel mobile menu. - Mobile capability parity does not require desktop layout parity. Load `/mobile-parity` for the Kandev surface decision guide, mobile design contract, and verification requirements. diff --git a/apps/web/components/task/workflow-step-disclosure.tsx b/apps/web/components/task/workflow-step-disclosure.tsx new file mode 100644 index 0000000000..3fb3a370dc --- /dev/null +++ b/apps/web/components/task/workflow-step-disclosure.tsx @@ -0,0 +1,567 @@ +"use client"; + +import { + forwardRef, + useEffect, + useRef, + useState, + type ComponentPropsWithoutRef, + type FocusEvent, + type RefObject, +} from "react"; +import { cn } from "@kandev/ui/lib/utils"; +import { Button } from "@kandev/ui/button"; +import { + Drawer, + DrawerContent, + DrawerDescription, + DrawerHeader, + DrawerTitle, + DrawerTrigger, +} from "@kandev/ui/drawer"; +import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover"; +import { IconArrowRight, IconChevronDown } from "@tabler/icons-react"; +import { StepCapabilityIcons } from "@/components/step-capability-icons"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; +import type { KanbanStepEvents } from "@/lib/state/slices/kanban/types"; +import { useTranslation } from "react-i18next"; + +export type WorkflowStepperStep = { + id: string; + name: string; + color: string; + position: number; + events?: KanbanStepEvents; + allow_manual_move?: boolean; + prompt?: string; + is_start_step?: boolean; + agent_profile_id?: string; +}; + +type Step = WorkflowStepperStep; + +type MinimalWorkflowStepperProps = { + sortedSteps: Step[]; + currentIndex: number; + isArchived?: boolean; + taskId?: string | null; + workflowId?: string | null; + movingToStepId: string | null; + onMove: (stepId: string) => Promise; +}; + +export function MinimalWorkflowStepper({ + sortedSteps, + currentIndex, + isArchived, + taskId, + workflowId, + movingToStepId, + onMove, +}: MinimalWorkflowStepperProps) { + const { t } = useTranslation(); + + if (isArchived) { + return ( + + {t("task:filterDimensionArchived")} + + ); + } + + const current = currentIndex >= 0 ? sortedSteps[currentIndex] : sortedSteps[0]; + if (!current) return null; + + if (!taskId || !workflowId) { + return ( + + ); + } + + return ( + + ); +} + +type CompactWorkflowDisclosureControls = { + open: boolean; + setOpen: (open: boolean) => void; + triggerRef: RefObject; + setTriggerRef: (node: HTMLButtonElement | null) => void; + contentRef: RefObject; + openDisclosure: () => void; + openDisclosureFromFocus: () => void; + scheduleClose: () => void; + cancelScheduledClose: () => void; + handleTriggerFocus: () => void; + handleTriggerBlur: (event: FocusEvent) => void; + handleContentFocus: () => void; + handleContentBlur: (event: FocusEvent) => void; + handleOpenAutoFocus: (event: Event) => void; + handleCloseAutoFocus: (event: Event) => void; +}; + +function isElementWithin( + target: EventTarget | null, + ref: RefObject, +): boolean { + return target instanceof Node && ref.current?.contains(target) === true; +} + +function useCompactDisclosureCloseTimer( + triggerRef: RefObject, + contentRef: RefObject, + setOpen: (open: boolean) => void, +) { + const closeTimerRef = useRef(null); + const cancelScheduledClose = () => { + if (closeTimerRef.current !== null) { + window.clearTimeout(closeTimerRef.current); + closeTimerRef.current = null; + } + }; + const scheduleClose = () => { + cancelScheduledClose(); + closeTimerRef.current = window.setTimeout(() => { + closeTimerRef.current = null; + const activeElement = document.activeElement; + const focusIsInsideDisclosure = + isElementWithin(activeElement, triggerRef) || isElementWithin(activeElement, contentRef); + if (!focusIsInsideDisclosure) setOpen(false); + }, 100); + }; + useEffect( + () => () => { + if (closeTimerRef.current !== null) window.clearTimeout(closeTimerRef.current); + }, + [], + ); + return { cancelScheduledClose, scheduleClose }; +} + +function useCompactWorkflowDisclosure(): CompactWorkflowDisclosureControls { + const [open, setOpen] = useState(false); + const triggerRef = useRef(null); + const setTriggerRef = (node: HTMLButtonElement | null) => { + triggerRef.current = node; + }; + const contentRef = useRef(null); + const suppressFocusOpenRef = useRef(false); + const openedFromFocusRef = useRef(false); + const contentHasFocusRef = useRef(false); + const { cancelScheduledClose, scheduleClose } = useCompactDisclosureCloseTimer( + triggerRef, + contentRef, + setOpen, + ); + const openDisclosure = () => { + openedFromFocusRef.current = false; + cancelScheduledClose(); + setOpen(true); + }; + const openDisclosureFromFocus = () => { + openedFromFocusRef.current = !open; + cancelScheduledClose(); + setOpen(true); + }; + const handleTriggerFocus = () => { + if (suppressFocusOpenRef.current) { + suppressFocusOpenRef.current = false; + return; + } + openDisclosureFromFocus(); + }; + const handleTriggerBlur = (event: FocusEvent) => { + suppressFocusOpenRef.current = false; + if (isElementWithin(event.relatedTarget, contentRef)) { + cancelScheduledClose(); + return; + } + scheduleClose(); + }; + const handleContentBlur = (event: FocusEvent) => { + const relatedTarget = event.relatedTarget; + if (isElementWithin(relatedTarget, contentRef)) { + contentHasFocusRef.current = true; + cancelScheduledClose(); + return; + } + contentHasFocusRef.current = false; + if (isElementWithin(relatedTarget, triggerRef)) { + cancelScheduledClose(); + return; + } + scheduleClose(); + }; + const handleContentFocus = () => { + contentHasFocusRef.current = true; + cancelScheduledClose(); + }; + const handleCloseAutoFocus = (event: Event) => { + event.preventDefault(); + if (contentHasFocusRef.current) { + contentHasFocusRef.current = false; + suppressFocusOpenRef.current = true; + triggerRef.current?.focus(); + } + }; + const handleOpenAutoFocus = (event: Event) => { + if (!openedFromFocusRef.current) event.preventDefault(); + openedFromFocusRef.current = false; + }; + return { + open, + setOpen, + triggerRef, + setTriggerRef, + contentRef, + openDisclosure, + openDisclosureFromFocus, + scheduleClose, + cancelScheduledClose, + handleTriggerFocus, + handleTriggerBlur, + handleContentFocus, + handleContentBlur, + handleOpenAutoFocus, + handleCloseAutoFocus, + }; +} + +type CompactWorkflowTriggerProps = ComponentPropsWithoutRef<"button"> & { + current: Step; + currentIndex: number; + total: number; + usesTouchDrawer: boolean; + controls: CompactWorkflowDisclosureControls; +}; + +const CompactWorkflowTrigger = forwardRef( + function CompactWorkflowTrigger( + { current, currentIndex, total, usesTouchDrawer, controls, className, ...buttonProps }, + ref, + ) { + const { t } = useTranslation(); + const currentDisplayIndex = currentIndex >= 0 ? currentIndex : 0; + return ( + + ); + }, +); + +function CompactWorkflowStepDisclosure({ + sortedSteps, + current, + currentIndex, + isArchived, + taskId, + workflowId, + movingToStepId, + onMove, +}: { + sortedSteps: Step[]; + current: Step; + currentIndex: number; + isArchived?: boolean; + taskId: string; + workflowId: string; + movingToStepId: string | null; + onMove: (stepId: string) => Promise; +}) { + const { t } = useTranslation(); + const usesTouchDrawer = useTouchDrawer(); + const controls = useCompactWorkflowDisclosure(); + const trigger = ( + + ); + const handleDisclosureMove = async (stepId: string) => { + const moved = await onMove(stepId); + if (moved) controls.setOpen(false); + return moved; + }; + const content = ( + + ); + + if (usesTouchDrawer) { + return ( + + {trigger} + + + {t("task:moveTo")} + + {t("task:stepCount", { count: sortedSteps.length })} + + + {content} + + + ); + } + + return ( + + {trigger} + + {content} + + + ); +} + +function MinimalStepIndicator({ + current, + currentIndex, + total, +}: { + current: Step; + currentIndex: number; + total: number; +}) { + return ( +
+ +
+ ); +} + +function MinimalStepContents({ + current, + currentIndex, + total, +}: { + current: Step; + currentIndex: number; + total: number; +}) { + const displayIndex = currentIndex >= 0 ? currentIndex : 0; + return ( + <> +
= 0 ? "step" : undefined} + className="flex min-w-0 items-center gap-1.5 text-xs" + > + + + {current.name} + +
+ {total > 1 && ( + + {displayIndex + 1}/{total} + + )} + + ); +} + +function StepDisclosureBody({ + sortedSteps, + currentIndex, + isArchived, + taskId, + workflowId, + movingToStepId, + onMove, +}: { + sortedSteps: Step[]; + currentIndex: number; + isArchived?: boolean; + taskId: string; + workflowId: string; + movingToStepId: string | null; + onMove: (stepId: string) => Promise; +}) { + const { t } = useTranslation(); + return ( +
+ {sortedSteps.map((step, index) => { + const isCurrent = index === currentIndex; + const isCompleted = !isArchived && currentIndex >= 0 && index < currentIndex; + const isAdjacent = + currentIndex >= 0 && (index === currentIndex - 1 || index === currentIndex + 1); + const canMove = canMoveToStep({ + isArchived, + isCurrent, + taskId, + workflowId, + isAdjacent, + allowManualMove: step.allow_manual_move, + }); + return ( +
+
+ + + {step.name} + + +
+ {isCurrent ? ( + + {t("task:currentStep")} + + ) : ( + canMove && ( + + ) + )} +
+ ); + })} +
+ ); +} + +export function canMoveToStep(params: { + isArchived: boolean | undefined; + isCurrent: boolean; + taskId: string | null | undefined; + workflowId: string | null | undefined; + isAdjacent: boolean; + allowManualMove: boolean | undefined; +}): boolean { + if (params.isArchived || params.isCurrent || !params.taskId || !params.workflowId) return false; + return params.isAdjacent || !!params.allowManualMove; +} + +export function StepCircleIndicator({ + isCurrent, + isCompleted, +}: { + isCurrent: boolean; + isCompleted: boolean; +}) { + if (isCurrent) { + return ( + + + + + ); + } + if (isCompleted) { + return ( + + + + ); + } + return ( + + + + ); +} + +export function getStepLabelClass(isCurrent: boolean, isCompleted: boolean): string { + if (isCurrent) return "text-foreground font-medium"; + if (isCompleted) return "text-muted-foreground"; + return "text-muted-foreground/60"; +} diff --git a/apps/web/components/task/workflow-stepper.test.tsx b/apps/web/components/task/workflow-stepper.test.tsx index 25ef00b331..fbf949d772 100644 --- a/apps/web/components/task/workflow-stepper.test.tsx +++ b/apps/web/components/task/workflow-stepper.test.tsx @@ -1,10 +1,67 @@ -import { cleanup, render, screen } from "@testing-library/react"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { moveTask } from "@/lib/api"; import { WorkflowStepper, type WorkflowStepperStep } from "./workflow-stepper"; +const mocks = vi.hoisted(() => ({ touchDrawer: false })); + +vi.mock("@/lib/api", () => ({ + moveTask: vi.fn(), +})); + +vi.mock("@/hooks/use-compact-task-chrome", () => ({ + useTouchDrawer: () => mocks.touchDrawer, +})); + +vi.mock("@kandev/ui/drawer", async () => { + const React = await vi.importActual("react"); + const DrawerContext = React.createContext<{ + open: boolean; + onOpenChange: (open: boolean) => void; + } | null>(null); + + return { + Drawer: ({ + children, + open, + onOpenChange, + }: { + children: React.ReactNode; + open: boolean; + onOpenChange: (open: boolean) => void; + }) => ( + {children} + ), + DrawerTrigger: ({ children }: { children: React.ReactElement }) => { + const context = React.useContext(DrawerContext); + return React.cloneElement(children as React.ReactElement>, { + onClick: () => context?.onOpenChange(true), + }); + }, + DrawerContent: ({ children, ...props }: React.HTMLAttributes) => { + const context = React.useContext(DrawerContext); + return context?.open ? ( +
+ {children} +
+ ) : null; + }, + DrawerHeader: ({ children, ...props }: React.HTMLAttributes) => ( +
{children}
+ ), + DrawerTitle: ({ children, ...props }: React.HTMLAttributes) => ( +

{children}

+ ), + DrawerDescription: ({ children, ...props }: React.HTMLAttributes) => ( +

{children}

+ ), + }; +}); + afterEach(() => { cleanup(); vi.clearAllMocks(); + mocks.touchDrawer = false; }); // useToolbarCollapsed is mocked because the test DOM can't measure offsetWidth. @@ -32,6 +89,18 @@ const STEPS: WorkflowStepperStep[] = [ { id: "c", name: "Review", color: "#333", position: 2 }, ]; +const DISCLOSURE_STEPS: WorkflowStepperStep[] = [ + ...STEPS, + { id: "d", name: "Done", color: "#444", position: 3, allow_manual_move: false }, +]; +const TASK_ID = "task-1"; +const WORKFLOW_ID = "workflow-1"; +const DISCLOSURE_TEST_ID = "workflow-step-disclosure"; +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"; + describe("WorkflowStepper", () => { it("renders every step when there is room (not collapsed)", () => { collapsedMock.mockReturnValue(false); @@ -62,7 +131,106 @@ describe("WorkflowStepper", () => { // Position indicator reflects the current step out of the total. expect(screen.getByText("2/3")).toBeTruthy(); }); +}); +describe("WorkflowStepper compact disclosure", () => { + it("opens every ordered step for fine-pointer users and marks only eligible targets", () => { + collapsedMock.mockReturnValue(true); + render( + , + ); + + const trigger = screen.getByRole("button", { name: TRIGGER_LABEL }); + expect(trigger.getAttribute("aria-expanded")).toBe("false"); + + fireEvent.mouseEnter(trigger); + + 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( + "step", + ); + expect(screen.getByTestId("workflow-step-disclosure-row-c")).toBeTruthy(); + expect(screen.getByTestId("workflow-step-disclosure-row-d")).toBeTruthy(); + expect(screen.getByTestId(MOVE_A_TEST_ID)).toBeTruthy(); + expect(screen.queryByTestId("workflow-step-disclosure-move-b")).toBeNull(); + expect(screen.getByTestId(MOVE_C_TEST_ID)).toBeTruthy(); + expect(screen.queryByTestId(MOVE_D_TEST_ID)).toBeNull(); + }); + + it("moves an eligible target with the existing payload and closes after success", async () => { + collapsedMock.mockReturnValue(true); + vi.mocked(moveTask).mockResolvedValue({} as Awaited>); + render( + , + ); + + fireEvent.mouseEnter(screen.getByRole("button", { name: TRIGGER_LABEL })); + fireEvent.click(screen.getByTestId(MOVE_C_TEST_ID)); + + await waitFor(() => + expect(moveTask).toHaveBeenCalledWith(TASK_ID, { + workflow_id: WORKFLOW_ID, + workflow_step_id: "c", + position: 0, + }), + ); + await waitFor(() => expect(screen.queryByTestId(DISCLOSURE_TEST_ID)).toBeNull()); + }); + + it("keeps the disclosure open and re-enables the target after a failed move", async () => { + collapsedMock.mockReturnValue(true); + vi.mocked(moveTask).mockRejectedValue(new Error("network error")); + render( + , + ); + + fireEvent.mouseEnter(screen.getByRole("button", { name: TRIGGER_LABEL })); + const moveButton = screen.getByTestId(MOVE_C_TEST_ID); + fireEvent.click(moveButton); + + await waitFor(() => expect(moveButton.hasAttribute("disabled")).toBe(false)); + expect(screen.getByTestId(DISCLOSURE_TEST_ID)).toBeTruthy(); + }); + + it("uses the same step choices in a coarse-pointer drawer", () => { + collapsedMock.mockReturnValue(true); + mocks.touchDrawer = true; + render( + , + ); + + fireEvent.click(screen.getByRole("button", { name: TRIGGER_LABEL })); + + expect(screen.getByRole("dialog")).toBeTruthy(); + expect(screen.getByTestId(DISCLOSURE_TEST_ID)).toBeTruthy(); + expect(screen.getByTestId(MOVE_A_TEST_ID)).toBeTruthy(); + expect(screen.getByTestId(MOVE_C_TEST_ID)).toBeTruthy(); + expect(screen.queryByTestId(MOVE_D_TEST_ID)).toBeNull(); + }); +}); + +describe("WorkflowStepper fallback states", () => { it("falls back to the first step when collapsed with no current step", () => { collapsedMock.mockReturnValue(true); render(); @@ -74,12 +242,21 @@ describe("WorkflowStepper", () => { it("shows the archived badge instead of a step when collapsed and archived", () => { collapsedMock.mockReturnValue(true); - render(); + render( + , + ); expect(screen.getByText("Archived")).toBeTruthy(); // Archived badge carries the minimal test id for collapsed-mode detection. expect(screen.getByTestId("workflow-stepper-minimal")).toBeTruthy(); expect(screen.queryByTestId("workflow-step-Work")).toBeNull(); + expect(screen.queryByRole("button")).toBeNull(); }); it("renders nothing when there are no steps", () => { diff --git a/apps/web/components/task/workflow-stepper.tsx b/apps/web/components/task/workflow-stepper.tsx index 2f53e31319..0862f0ad98 100644 --- a/apps/web/components/task/workflow-stepper.tsx +++ b/apps/web/components/task/workflow-stepper.tsx @@ -12,20 +12,16 @@ 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 type { KanbanStepEvents } from "@/lib/state/slices/kanban/types"; import { useTranslation } from "react-i18next"; +import { + MinimalWorkflowStepper, + StepCircleIndicator, + canMoveToStep, + getStepLabelClass, + type WorkflowStepperStep, +} from "./workflow-step-disclosure"; -type Step = { - id: string; - name: string; - color: string; - position: number; - events?: KanbanStepEvents; - allow_manual_move?: boolean; - prompt?: string; - is_start_step?: boolean; - agent_profile_id?: string; -}; +type Step = WorkflowStepperStep; const PLAN_CONTEXT_PATH = "plan:context"; @@ -86,8 +82,8 @@ const WorkflowStepper = memo(function WorkflowStepper({ ); const handleMove = useCallback( - async (stepId: string) => { - if (!taskId || !workflowId) return; + async (stepId: string): Promise => { + if (!taskId || !workflowId) return false; disablePlanMode(); setMovingToStepId(stepId); try { @@ -96,8 +92,10 @@ const WorkflowStepper = memo(function WorkflowStepper({ workflow_step_id: stepId, position: 0, }); + return true; } catch (err) { console.error("[WorkflowStepper] Failed to move task:", err); + return false; } finally { setMovingToStepId(null); } @@ -122,6 +120,10 @@ const WorkflowStepper = memo(function WorkflowStepper({ sortedSteps={sortedSteps} currentIndex={currentIndex} isArchived={isArchived} + taskId={taskId} + workflowId={workflowId} + movingToStepId={movingToStepId} + onMove={handleMove} /> ) : ( <> @@ -154,68 +156,6 @@ const WorkflowStepper = memo(function WorkflowStepper({ ); }); -/** Minimal stepper: current step only (or archived badge), keeping the per-step test id + aria-current. */ -function MinimalWorkflowStepper({ - sortedSteps, - currentIndex, - isArchived, -}: { - sortedSteps: Step[]; - currentIndex: number; - isArchived?: boolean; -}) { - const { t } = useTranslation(); - if (isArchived) { - return ( - - {t("task:filterDimensionArchived")} - - ); - } - - const current = currentIndex >= 0 ? sortedSteps[currentIndex] : sortedSteps[0]; - if (!current) return null; - - return ( -
-
= 0 ? "step" : undefined} - className="flex min-w-0 items-center gap-1.5 text-xs" - > - - - {current.name} - -
- {sortedSteps.length > 1 && ( - - {(currentIndex >= 0 ? currentIndex : 0) + 1}/{sortedSteps.length} - - )} -
- ); -} - -/** Check if a step can be moved to */ -function canMoveToStep(params: { - isArchived: boolean | undefined; - isCurrent: boolean; - taskId: string | null | undefined; - workflowId: string | null | undefined; - isAdjacent: boolean; - allowManualMove: boolean | undefined; -}): boolean { - if (params.isArchived || params.isCurrent || !params.taskId || !params.workflowId) return false; - return params.isAdjacent || !!params.allowManualMove; -} - /** Individual step in the workflow stepper */ function WorkflowStepItem({ step, @@ -328,42 +268,5 @@ function StepHoverContent({ ); } -/** Circle indicator for step state */ -function StepCircleIndicator({ - isCurrent, - isCompleted, -}: { - isCurrent: boolean; - isCompleted: boolean; -}) { - if (isCurrent) { - return ( - - - - - ); - } - if (isCompleted) { - return ( - - - - ); - } - return ( - - - - ); -} - -/** Get CSS class for step label based on state */ -function getStepLabelClass(isCurrent: boolean, isCompleted: boolean): string { - if (isCurrent) return "text-foreground font-medium"; - if (isCompleted) return "text-muted-foreground"; - return "text-muted-foreground/60"; -} - export { WorkflowStepper }; -export type { Step as WorkflowStepperStep }; +export type { WorkflowStepperStep } from "./workflow-step-disclosure"; diff --git a/apps/web/e2e/helpers/animations.ts b/apps/web/e2e/helpers/animations.ts new file mode 100644 index 0000000000..e5c16655ff --- /dev/null +++ b/apps/web/e2e/helpers/animations.ts @@ -0,0 +1,11 @@ +import type { Locator } from "@playwright/test"; + +export async function waitForFiniteAnimations(surface: Locator): Promise { + await surface.evaluate(async (element) => { + const animations = element.getAnimations({ subtree: true }).filter((animation) => { + const iterations = animation.effect?.getComputedTiming().iterations; + return typeof iterations === "number" && Number.isFinite(iterations); + }); + await Promise.all(animations.map((animation) => animation.finished.catch(() => undefined))); + }); +} diff --git a/apps/web/e2e/tests/layout/task-topbar-workflow-stepper.spec.ts b/apps/web/e2e/tests/layout/task-topbar-workflow-stepper.spec.ts new file mode 100644 index 0000000000..9bdaf7c687 --- /dev/null +++ b/apps/web/e2e/tests/layout/task-topbar-workflow-stepper.spec.ts @@ -0,0 +1,144 @@ +import { test, expect } from "../../fixtures/test-base"; +import { waitForFiniteAnimations } from "../../helpers/animations"; +import { SessionPage } from "../../pages/session-page"; + +const COMPACT_TASK_TITLE = `Compact workflow navigation ${"W".repeat(90)}`; + +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("compact workflow step test requires an adjacent target"); + return target; +} + +test.describe("Compact task topbar workflow stepper", () => { + test("opens ordered steps on hover and moves the task", async ({ + testPage, + apiClient, + seedData, + }) => { + const task = await apiClient.seedTask(seedData.workspaceId, COMPACT_TASK_TITLE, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + const targetStep = adjacentStep(seedData.steps, seedData.startStepId); + + await testPage.setViewportSize({ width: 900, height: 800 }); + await testPage.goto(`/t/${task.task_id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + + const trigger = testPage.getByTestId("workflow-stepper-minimal"); + await expect(trigger).toBeVisible(); + await expect(trigger).toHaveAttribute("aria-haspopup", "dialog"); + await trigger.hover(); + + const disclosure = testPage.getByTestId("workflow-step-disclosure"); + const disclosureSurface = testPage.getByRole("dialog", { name: "Move to" }); + await expect(disclosure).toBeVisible(); + await expect(disclosureSurface).toBeVisible(); + await testPage.mouse.move(0, 0); + await expect(disclosureSurface).toBeHidden(); + await trigger.focus(); + await expect(disclosureSurface).toBeVisible(); + await testPage.keyboard.press("Escape"); + await expect(disclosureSurface).toBeHidden(); + await expect(trigger).toBeFocused(); + await testPage.keyboard.press("Tab"); + await trigger.focus(); + await expect(disclosureSurface).toBeVisible(); + await expect(disclosure.locator('[data-testid^="workflow-step-disclosure-row-"]')).toHaveCount( + seedData.steps.length, + ); + + const moveButton = testPage.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).toBeLessThan(40); + + let moveButtonFocused = false; + for (let tabCount = 0; tabCount < seedData.steps.length + 2; tabCount += 1) { + if (await moveButton.evaluate((element) => element === document.activeElement)) { + moveButtonFocused = true; + break; + } + await testPage.keyboard.press("Tab"); + } + expect(moveButtonFocused).toBe(true); + await testPage.keyboard.press("Enter"); + + await expect + .poll(async () => (await apiClient.getTask(task.task_id)).workflow_step_id, { + timeout: 15_000, + }) + .toBe(targetStep.id); + }); + + test("opens the same steps in a contained tablet touch drawer", async ({ + tabletTestPage, + apiClient, + seedData, + }) => { + const task = await apiClient.seedTask(seedData.workspaceId, COMPACT_TASK_TITLE, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + const targetStep = adjacentStep(seedData.steps, seedData.startStepId); + + await tabletTestPage.goto(`/t/${task.task_id}`); + const session = new SessionPage(tabletTestPage); + await session.waitForLoad(); + + const trigger = tabletTestPage.getByTestId("workflow-stepper-minimal"); + await expect(trigger).toBeVisible(); + const triggerBox = await trigger.boundingBox(); + expect(triggerBox).not.toBeNull(); + if (!triggerBox) return; + expect(triggerBox.height).toBeGreaterThanOrEqual(44); + await trigger.tap(); + + const drawer = tabletTestPage.getByRole("dialog", { name: "Move to" }); + await expect(drawer).toBeVisible(); + await waitForFiniteAnimations(drawer); + const drawerBox = await drawer.boundingBox(); + expect(drawerBox).not.toBeNull(); + if (!drawerBox) return; + const viewport = await tabletTestPage.evaluate(() => ({ + height: innerHeight, + width: innerWidth, + })); + expect(drawerBox.x).toBeGreaterThanOrEqual(0); + expect(drawerBox.y).toBeGreaterThanOrEqual(0); + expect(drawerBox.x + drawerBox.width).toBeLessThanOrEqual(viewport.width); + expect(drawerBox.y + drawerBox.height).toBeLessThanOrEqual(viewport.height); + expect( + await tabletTestPage.evaluate(() => document.documentElement.scrollWidth), + ).toBeLessThanOrEqual(await tabletTestPage.evaluate(() => window.innerWidth)); + + const targetRow = tabletTestPage.getByTestId(`workflow-step-disclosure-row-${targetStep.id}`); + const targetRowBox = await targetRow.boundingBox(); + expect(targetRowBox).not.toBeNull(); + if (!targetRowBox) return; + expect(targetRowBox.height).toBeGreaterThanOrEqual(44); + + const targetButton = tabletTestPage.getByTestId( + `workflow-step-disclosure-move-${targetStep.id}`, + ); + const targetButtonBox = await targetButton.boundingBox(); + expect(targetButtonBox).not.toBeNull(); + if (!targetButtonBox) return; + expect(targetButtonBox.height).toBeGreaterThanOrEqual(44); + await targetButton.tap(); + await expect + .poll(async () => (await apiClient.getTask(task.task_id)).workflow_step_id, { + timeout: 15_000, + }) + .toBe(targetStep.id); + }); +}); diff --git a/apps/web/e2e/tests/plugins/bitbucket-packaged-plugin.spec.ts b/apps/web/e2e/tests/plugins/bitbucket-packaged-plugin.spec.ts index f879b5cfa5..247ded2ff3 100644 --- a/apps/web/e2e/tests/plugins/bitbucket-packaged-plugin.spec.ts +++ b/apps/web/e2e/tests/plugins/bitbucket-packaged-plugin.spec.ts @@ -1,6 +1,7 @@ import path from "node:path"; import type { Page } from "@playwright/test"; import { expect, test } from "../../fixtures/test-base"; +import { waitForFiniteAnimations } from "../../helpers/animations"; import { PrAssetCapture } from "../../helpers/pr-asset-capture"; const PLUGIN_ID = "kandev-plugin-bitbucket"; @@ -176,16 +177,6 @@ async function expectNoHorizontalOverflow(page: Page): Promise { .toBe(true); } -async function waitForFiniteAnimations(page: Page): Promise { - await page.evaluate(async () => { - const animations = document.getAnimations().filter((animation) => { - const iterations = animation.effect?.getComputedTiming().iterations; - return typeof iterations === "number" && Number.isFinite(iterations); - }); - await Promise.all(animations.map((animation) => animation.finished.catch(() => undefined))); - }); -} - async function expectContainedInViewport(page: Page, selector: string): Promise { const element = page.locator(selector); await expect @@ -318,7 +309,7 @@ test.describe("Bitbucket packaged plugin", () => { expect((await filterButton.boundingBox())?.height).toBeGreaterThanOrEqual(44); await filterButton.click(); await expect(testPage.getByRole("heading", { name: "Bitbucket filters" })).toBeVisible(); - await waitForFiniteAnimations(testPage); + await waitForFiniteAnimations(testPage.locator("body")); await expectContainedInViewport(testPage, ".bb-filter-sheet"); } else { const configureButton = workbench.getByRole("button", { name: "Configure Bitbucket" }); diff --git a/apps/web/e2e/tests/review/submodule-review-helpers.ts b/apps/web/e2e/tests/review/submodule-review-helpers.ts index 990d7bcf57..39f639268a 100644 --- a/apps/web/e2e/tests/review/submodule-review-helpers.ts +++ b/apps/web/e2e/tests/review/submodule-review-helpers.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { execFileSync } from "node:child_process"; import { expect, type Locator } from "@playwright/test"; import type { ApiClient } from "../../helpers/api-client"; +import { waitForFiniteAnimations } from "../../helpers/animations"; import { dwell } from "../../helpers/causal-waits"; import type { SeedData } from "../../fixtures/test-base"; import { makeGitEnv } from "../../helpers/git-helper"; @@ -12,16 +13,6 @@ const GIT_PROTOCOL_ARGS = ["-c", "protocol.file.allow=always"]; const GIT_INDEX_LOCK_ATTEMPTS = 3; const GIT_INDEX_LOCK_RETRY_MS = 300; -async function waitForFiniteAnimations(surface: Locator): Promise { - await surface.evaluate(async (element) => { - const animations = element.getAnimations({ subtree: true }).filter((animation) => { - const iterations = animation.effect?.getComputedTiming().iterations; - return typeof iterations === "number" && Number.isFinite(iterations); - }); - await Promise.all(animations.map((animation) => animation.finished.catch(() => undefined))); - }); -} - export async function expectStickyReviewHeaderClearance( review: Locator, pointer: "mouse" | "touch", diff --git a/docs/plans/compact-workflow-step-navigation/plan.md b/docs/plans/compact-workflow-step-navigation/plan.md new file mode 100644 index 0000000000..57d5aa7fe0 --- /dev/null +++ b/docs/plans/compact-workflow-step-navigation/plan.md @@ -0,0 +1,121 @@ +--- +created: 2026-08-26 +status: complete +requirements: + - REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001 +system_design: + - ../../specs/ui/system-design/compact-workflow-step-navigation.md +legacy_specs: [] +--- + +# Implementation Plan: Compact Workflow Step Navigation + +## Overview + +Add an interactive disclosure to the compact task top-bar stepper. Reuse the existing movement policy and task-move callback. + +Implement the component behavior and its browser proof in one vertical work order. This order keeps the responsive contract and movement result in one TDD cycle. + +## Scope + +### In scope + +- Make the compact stepper a semantic and accessible trigger for active tasks. +- Show every workflow step in workflow order from the compact presentation. +- Permit moves only to targets that the full stepper already permits. +- Use an interactive Popover for fine pointers and an inset drawer for coarse pointers. The fine-pointer trigger and dialog expose matching ARIA semantics, and the move buttons remain keyboard tabbable. +- Give the coarse-pointer trigger a 44px hit area and visible disclosure cue. +- Preserve the existing phone movement path and full-stepper behavior. +- Cover the compact desktop and tablet paths with component and E2E tests. + +### Out of scope + +- Backend task-move changes. +- Cross-workflow movement from the compact stepper. +- A new phone top-bar control. +- New move notifications or error copy. +- Public documentation changes. + +## Technical approach + +### Stepper model and disclosure + +Update `apps/web/components/task/workflow-stepper.tsx`. Keep `WorkflowStepper` as the owner of sorted steps, the current index, and move state. + +Replace the static active-task branch in `MinimalWorkflowStepper` with a disclosure trigger. Keep the archived branch static. + +Extract one disclosure body that renders each step from a shared row model. The model uses `canMoveToStep` for every target. + +Reuse `StepCircleIndicator`, `task:currentStep`, `task:moveHere`, `task:moving`, and `task:stepOf`. Do not add untranslated copy. + +Use `useTouchDrawer` to select the surface. Fine pointers use a controlled `Popover` with hover and focus behavior, dialog semantics, keyboard focus handling, and compact desktop-sized movement buttons. Coarse pointers use `Drawer` with internal overflow, safe-area padding, a 44px trigger hit area, 44px action hit areas, and a visible disclosure cue. Apply touch sizing only through coarse-pointer styles. + +Keep the move request in `handleMove`. Return its success result so the temporary surface closes only after a successful request. + +### Component tests + +Extend `apps/web/components/task/workflow-stepper.test.tsx`. Mock width collapse and pointer mode independently. + +Add tests for these results: + +- fine-pointer hover or focus shows all ordered steps +- eligible targets show movement controls and the current step does not +- selection calls `moveTask` with the existing payload +- coarse-pointer activation opens the drawer with the same targets +- archived compact state stays static +- expanded mode does not mount the compact disclosure + +### Browser tests + +Add `apps/web/e2e/tests/layout/task-topbar-workflow-stepper.spec.ts`. Use a long task title and a 900px viewport to force the measured compact state. + +The desktop scenario hovers and focuses the compact trigger, verifies the Popover dialog semantics, tabs to a step-specific move button, activates it with Enter, and polls the task API for the new step. + +The tablet scenario uses `tabletTestPage`. It verifies the trigger hit area, opens the touch drawer, checks viewport containment, selects a target, and polls the same API result. + +Keep `apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts` unchanged. Its current Pixel 5 move scenario remains the phone parity proof. + +## Tests + +| Acceptance criterion | Evidence | +| ---------------------------------------------- | ------------------------------------------------------------- | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.1` | Compact component test and desktop E2E trigger assertion | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.2` | Fine-pointer component test and desktop Popover E2E | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.3` | Touch-drawer component test and tablet trigger geometry E2E | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.4` | Eligibility and current-step component tests | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.5` | `moveTask` payload component test and API-poll E2E assertions | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.6` | Keyboard desktop E2E with Tab and Enter, plus tablet geometry | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.7` | Archived compact-state component test | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.8` | Existing expanded-mode component test | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.9` | Existing Pixel 5 mobile task-drawer E2E scenario | +| `AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.10` | Desktop compact button and coarse-pointer hit-area E2E | + +## E2E tests + +- `apps/web/e2e/tests/layout/task-topbar-workflow-stepper.spec.ts`, Chromium project: compact hover disclosure and task movement. +- `apps/web/e2e/tests/layout/task-topbar-workflow-stepper.spec.ts`, Chromium project with `tabletTestPage`: touch drawer, geometry, and task movement. +- `apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts`, mobile-chrome project: existing phone task movement through **Move to**. + +## Work orders + +- [x] [Task 01: Add compact workflow step disclosure](task-01-compact-step-disclosure.md) + +## Verification results + +Completed 2026-08-26. + +- Component tests: 9 passed in `components/task/workflow-stepper.test.tsx`. +- Focused ESLint: passed with no warnings for the changed component, test, and E2E files. +- TypeScript typecheck: passed. +- i18n checks: passed with no new violations. +- Chromium E2E: compact Popover dialog semantics, keyboard Tab and Enter movement, Escape dismissal, focus return, and tablet touch-drawer scenarios passed. +- Mobile-chrome E2E: the existing phone **Move to** scenario passed. +- Desktop density: the fine-pointer move button measured below 40px, while the coarse-pointer move button and disclosure trigger remained at least 44px. +- Tablet geometry: the disclosure trigger and touch rows measured at least 44px, the touch drawer stayed inside the viewport, and the document did not gain horizontal overflow. + +## Risks + +- A nested disclosure can close its parent before a movement button receives the pointer. The design uses one Popover surface and no nested hover roots. +- The compact state depends on rendered width. The E2E fixture must prove that `workflow-stepper-minimal` is visible before it opens the disclosure. +- Tablet pointer detection depends on browser media queries. The E2E uses the existing `tabletTestPage` touch context. +- A failed move must not leave movement controls disabled or close the disclosure. diff --git a/docs/plans/compact-workflow-step-navigation/task-01-compact-step-disclosure.md b/docs/plans/compact-workflow-step-navigation/task-01-compact-step-disclosure.md new file mode 100644 index 0000000000..6fa844dab3 --- /dev/null +++ b/docs/plans/compact-workflow-step-navigation/task-01-compact-step-disclosure.md @@ -0,0 +1,126 @@ +--- +id: "01-compact-step-disclosure" +title: "Add compact workflow step disclosure" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001 +acceptance_criteria: + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.1 + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.2 + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.3 + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.4 + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.5 + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.6 + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.7 + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.8 + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.9 + - AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.10 +system_design: + - ../../specs/ui/system-design/compact-workflow-step-navigation.md +--- + +# Task 01: Add Compact Workflow Step Disclosure + +## Summary + +Add the ordered workflow-step disclosure to the compact task top bar. Prove desktop hover, keyboard movement, tablet touch, and task movement through one TDD cycle. + +## In scope + +- Add the shared disclosure body and responsive surface selection. +- Reuse current-step state, target eligibility, plan-mode cleanup, and the task-move API. +- Add focused component tests for interaction, accessibility, movement, and archived behavior. +- Add desktop and tablet E2E coverage for the compact top bar, including keyboard activation and trigger geometry. +- Run the existing phone task-move E2E scenario as mobile parity proof. + +## Out of scope + +- Backend changes. +- Cross-workflow movement. +- A new phone task top-bar entry point. +- New user-facing copy. +- Changes to the full-stepper layout. + +## Acceptance + +- The compact trigger shows all ordered steps through hover, focus, or coarse-pointer activation. Fine-pointer movement controls are reachable by keyboard and retain compact desktop sizing; coarse-pointer activation has a 44px trigger and action hit area with a visible cue. +- The disclosure identifies the current step and moves the task only to eligible targets. +- The Popover and drawer stay usable, contained, and accessible. Archived and full-stepper states retain their existing behavior. + +## Verification + +Run these commands from `apps/web` after the workspace dependencies are installed: + +```bash +pnpm exec vitest run components/task/workflow-stepper.test.tsx +pnpm run i18n:check +pnpm e2e:run --host --project chromium -- tests/layout/task-topbar-workflow-stepper.spec.ts --workers=1 +pnpm e2e:run --host --no-build --project mobile-chrome -- tests/task/mobile-sidebar-task-actions.spec.ts --grep "moves a task to another step from the mobile task drawer" --workers=1 +pnpm exec tsc --noEmit +``` + +## Files likely touched + +- `apps/web/components/task/workflow-stepper.tsx` +- `apps/web/components/task/workflow-step-disclosure.tsx` +- `apps/web/components/task/workflow-stepper.test.tsx` +- `apps/web/e2e/helpers/animations.ts` +- `apps/web/e2e/tests/layout/task-topbar-workflow-stepper.spec.ts` +- `apps/web/e2e/tests/plugins/bitbucket-packaged-plugin.spec.ts` +- `apps/web/e2e/tests/review/submodule-review-helpers.ts` +- `docs/plans/compact-workflow-step-navigation/plan.md` +- `docs/plans/compact-workflow-step-navigation/task-01-compact-step-disclosure.md` + +## Dependencies + +None. + +## Risks + +- Hover content must stay open while the pointer moves from the trigger to a movement control. +- A move error must clear the target state without closing the disclosure. +- Width-based collapse must remain stable while the disclosure portal mounts. +- Coarse-pointer rows and action controls must meet the 44px target size without making the desktop Popover controls visually oversized. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001` and all related acceptance criteria. +- `docs/specs/ui/system-design/compact-workflow-step-navigation.md`. +- `apps/web/components/task/workflow-stepper.tsx` and its component tests. +- `apps/web/components/task/changes-panel-header.tsx` for the hover-card and touch-drawer precedent. +- `apps/web/e2e/fixtures/test-base.ts` for `tabletTestPage`. +- `apps/web/e2e/tests/task/mobile-sidebar-task-actions.spec.ts` for phone movement proof. + +## Results + +Completed 2026-08-26. + +- Added a semantic compact-step trigger for active tasks and preserved the static archived presentation. +- Reused the existing step eligibility policy and task-move payload, including plan-mode cleanup and move-state handling. +- Added a shared ordered disclosure body with current-step identification and 44px target rows. +- Selected a fine-pointer Popover or coarse-pointer drawer through the existing responsive pointer hook. The Popover exposes dialog semantics and keeps movement controls keyboard tabbable; the coarse trigger exposes a 44px hit area and disclosure cue. +- Added component coverage for expanded and compact states, ordered disclosure content, eligibility, movement payload, coarse-pointer behavior, archived state, and empty workflows. +- Added Chromium coverage for hover, Popover dialog semantics, keyboard Tab and Enter movement, Escape dismissal, focus return, tablet trigger geometry, and touch-drawer containment. +- Re-ran the existing Pixel 5 mobile task-drawer movement scenario successfully. +- Applied PR review remediation: defined the disclosure-owned step type, added failed-move recovery coverage, centralized finite-animation waiting for the affected E2E helpers, replaced the non-interactive hover surface with a keyboard-accessible Popover, and added coarse-pointer trigger geometry and cue coverage. +- Applied desktop density correction: fine-pointer move buttons use compact `h-7` sizing, while coarse-pointer action controls retain the 44px touch hit area. + +Verification commands passed: + +```text +pnpm exec vitest run components/task/workflow-stepper.test.tsx +pnpm run i18n:check +pnpm run i18n:ratchet -- --files components/task/workflow-stepper.tsx components/task/workflow-step-disclosure.tsx components/task/workflow-stepper.test.tsx e2e/tests/layout/task-topbar-workflow-stepper.spec.ts +pnpm exec eslint components/task/workflow-stepper.tsx components/task/workflow-step-disclosure.tsx components/task/workflow-stepper.test.tsx e2e/tests/layout/task-topbar-workflow-stepper.spec.ts +pnpm e2e:run --host --no-build --project chromium -- tests/layout/task-topbar-workflow-stepper.spec.ts --workers=1 +pnpm e2e:run --host --no-build --project mobile-chrome -- tests/task/mobile-sidebar-task-actions.spec.ts --grep "moves a task to another step from the mobile task drawer" --workers=1 +pnpm exec eslint components/task/workflow-stepper.tsx components/task/workflow-step-disclosure.tsx components/task/workflow-stepper.test.tsx e2e/helpers/animations.ts e2e/tests/layout/task-topbar-workflow-stepper.spec.ts e2e/tests/plugins/bitbucket-packaged-plugin.spec.ts e2e/tests/review/submodule-review-helpers.ts +pnpm e2e:run --host --no-build --project chromium -- tests/layout/task-topbar-workflow-stepper.spec.ts --workers=1 +``` diff --git a/docs/specs/ui/README.md b/docs/specs/ui/README.md index 8dd5e09d56..2540085449 100644 --- a/docs/specs/ui/README.md +++ b/docs/specs/ui/README.md @@ -55,6 +55,7 @@ contract that other capabilities can reuse. - [Clarification Shared Context](requirements/clarification-context.md) - [Clarification submit feedback](requirements/clarification-submit-feedback.md) - [Command-panel Sidebar Task Reveal](requirements/command-panel-sidebar-task-reveal.md) +- [Compact Workflow Step Navigation](requirements/compact-workflow-step-navigation.md) - [Comment Markdown Rendering](requirements/comment-markdown.md) - [Composer Suggestion Overlays](requirements/composer-suggestion-overlays.md) - [Context Compaction Count](requirements/context-compaction-count.md) @@ -142,12 +143,11 @@ contract that other capabilities can reuse. ### System design - - - [Agent Todo List Panel](system-design/agent-todo-list-panel.md) - [App Status Bar](system-design/app-status-bar.md) - [Changes File Row Containment](system-design/changes-file-row-containment.md) - [Composer Suggestion Overlays](system-design/composer-suggestion-overlays.md) +- [Compact Workflow Step Navigation](system-design/compact-workflow-step-navigation.md) - [Per-workflow column visibility on the kanban board System Design Part 1](system-design/board-step-visibility-filter-01.md) - [Per-workflow column visibility on the kanban board System Design Part 2](system-design/board-step-visibility-filter-02.md) - [Task PR Automation Controls System Design Part 1](system-design/ci-pr-automation-01.md) diff --git a/docs/specs/ui/requirements/compact-workflow-step-navigation.md b/docs/specs/ui/requirements/compact-workflow-step-navigation.md new file mode 100644 index 0000000000..81c65c8138 --- /dev/null +++ b/docs/specs/ui/requirements/compact-workflow-step-navigation.md @@ -0,0 +1,50 @@ +--- +status: draft +system: ui +created: 2026-08-26 +owners: + - kandev +--- + +# Compact Workflow Step Navigation Requirements + +## Overview + +The task top bar replaces the full workflow stepper when its center area is too narrow. The compact form currently hides all other steps. + +The UI system owns this responsive disclosure. The task system continues to own workflow order, move eligibility, and task transitions. + +## Terminology + +- **Compact stepper:** The current step name, state marker, and position count that replace the full stepper. +- **Step disclosure:** The temporary surface that shows all steps from the compact stepper. +- **Eligible step:** A workflow step that the existing task-move policy permits as a manual target. + +## Requirements + +### REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001: Compact workflow step navigation + +**Intent:** Users need to inspect and select workflow steps when the task top bar cannot show the full stepper. + +**User story:** As a task user, I want the compact stepper to show all steps, so that I can move the task without leaving it. + +#### Acceptance criteria + +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.1:** When the full stepper cannot fit, the system shall show the current step name and its position in the workflow. +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.2:** When a fine-pointer user hovers or focuses the compact stepper, the system shall show every workflow step in workflow order in an interactive dialog surface. The surface shall expose its move controls to keyboard users. +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.3:** When a coarse-pointer user activates the compact stepper, the system shall show the same step choices in a touch surface. The trigger shall provide a minimum 44px hit area and a visible disclosure cue. +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.4:** When the disclosure is open, the system shall identify the current step and enable each eligible target. +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.5:** When a user selects an eligible target, the system shall use the existing task-move behavior and eligibility rules. +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.6:** When the disclosure opens, it shall remain inside the viewport and provide keyboard navigation, including Tab to an eligible step-specific move control and Enter activation, Escape dismissal, and focus return. The trigger and dialog surface shall expose matching ARIA semantics. +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.7:** When a task is archived, the compact presentation shall remain a static archived indicator and shall not offer task movement. +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.8:** When the task top bar has sufficient space, the existing full-stepper presentation and move behavior shall remain unchanged. +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.9:** When a phone does not show the task top bar, the mobile task drawer shall retain its existing **Move to** path. +- **AC-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001.10:** Fine-pointer movement controls shall retain compact desktop sizing. The 44px minimum shall apply only to coarse-pointer touch hit areas and mobile controls. + +## Out of scope + +- Changes to workflow order, task-move permissions, or backend task transitions. +- A new workflow control in the phone task top bar. +- Changes to the expanded stepper layout. +- Cross-workflow task movement from the compact stepper. +- New task-move notifications or error messages. diff --git a/docs/specs/ui/system-design/compact-workflow-step-navigation.md b/docs/specs/ui/system-design/compact-workflow-step-navigation.md new file mode 100644 index 0000000000..6bf0128c98 --- /dev/null +++ b/docs/specs/ui/system-design/compact-workflow-step-navigation.md @@ -0,0 +1,114 @@ +--- +status: draft +system: ui +requirements: + - REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001 +--- + +# Compact Workflow Step Navigation System Design + +## Purpose and boundaries + +The UI system owns the responsive stepper and its temporary disclosure. The task system owns workflow data and the task-move API. + +This design changes only the compact branch in `WorkflowStepper`. It does not change backend contracts or the full-stepper layout. + +## Requirement mapping + +| Requirement | Design section | +| --------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `REQ-UI-COMPACT-WORKFLOW-STEP-NAVIGATION-001` | [Components and responsibilities](#components-and-responsibilities), [Control flow](#control-flow), [Responsive interaction](#responsive-interaction), [Accessibility and geometry](#accessibility-and-geometry) | + +## Components and responsibilities + +- `WorkflowStepper` sorts the steps, finds the current index, measures available width, and owns task-move state. +- `useToolbarCollapsed` continues to select the full or compact presentation from the center area's measured width. +- The compact presentation becomes an interactive trigger for active tasks. It keeps the current step marker, name, and position count. On coarse pointers it also exposes a 44px hit area and a visible disclosure cue. +- A shared disclosure body shows all sorted steps. It marks the current step and shows movement controls for eligible targets. +- `canMoveToStep` remains the single UI policy for target eligibility. Adjacent steps and steps with `allow_manual_move` remain eligible. +- `useTouchDrawer` selects the disclosure surface from pointer precision. This selection does not duplicate task state or move logic. +- The archived presentation remains a static badge and does not mount the disclosure. + +## Data and contracts + +The design uses the existing `WorkflowStepperStep` values. It does not add a new store field, API field, or persisted preference. + +The disclosure receives these derived values: + +- the steps sorted by `position` +- the current step index +- the target step that has a move in progress +- the result of `canMoveToStep` for each step +- the existing `handleMove` callback + +The move callback continues to call `moveTask` with `workflow_id`, `workflow_step_id`, and position `0`. It also retains plan-mode cleanup. + +## Control flow + +1. `WorkflowStepper` measures the center area and selects the compact presentation when the full stepper overflows. +2. The compact trigger shows the current step and its position count. +3. The user opens the disclosure through hover, focus, or touch activation. +4. The disclosure shows all steps in workflow order. +5. The UI enables only eligible targets and leaves the current step unavailable. +6. The user selects a target. `handleMove` disables plan mode and sends the existing move request. +7. A successful request closes the temporary surface. Live task state then updates the current step. +8. A failed request restores the target control and keeps the surface available for another action. + +## Responsive interaction + +### Fine pointer + +The compact trigger uses a controlled interactive `Popover`. Hover or keyboard focus opens a vertical list below the trigger. The trigger declares `aria-haspopup="dialog"`, and the Popover content exposes the matching dialog role and accessible name. Its movement buttons remain tabbable, so keyboard users can tab to a step-specific action and activate it with Enter. + +The hover and close delays match the full stepper. Users can move the pointer from the trigger into the content. Pointer opening does not steal focus, while keyboard opening moves focus into the interactive content. Escape closes the Popover and returns focus to the trigger. + +### Coarse pointer + +The compact trigger opens an inset `Drawer` through `useTouchDrawer`. The drawer uses the same list and movement callback. + +This drawer is the tablet fallback because the desktop top bar still renders at tablet widths. Phone task pages use the existing task drawer path. + +### Mobile design contract + +- **Desktop outcome:** The compact trigger shows all steps and supports eligible task moves. +- **Mobile entry point:** The phone task drawer keeps **Task actions** > **Move to**. The tablet compact trigger opens the new drawer. +- **Nearest shipped exemplar:** `BranchHoverCard` provides the hover-card and touch-drawer split. `mobile-sidebar-task-actions.spec.ts` provides phone movement proof. +- **Hierarchy:** The current step stays in the top bar. The temporary surface lists all steps and their movement actions. +- **Presentation rationale:** Step selection is a short, temporary choice. A hover card or drawer is more direct than a new route. +- **Scroll owner:** The disclosure list owns vertical overflow. The document does not gain horizontal overflow. +- **Shared logic:** Both surfaces use the same sorted steps, eligibility policy, move state, and callback. +- **Mobile proof:** The existing Pixel 5 move scenario remains part of the focused commands. + +## Accessibility and geometry + +The compact trigger is a semantic button. Its accessible name includes the current step, step number, and total step count. On coarse pointers its active area is at least 44px high and includes a visible chevron cue. + +The current row uses `aria-current="step"`. Eligible movement controls are direct keyboard-tab stops and use existing translated labels. The fine-pointer disclosure uses Popover dialog semantics instead of a hover-card surface, because the move controls are interactive. + +Fine-pointer movement controls use the surrounding compact desktop button density. The 44px minimum applies to the coarse-pointer drawer's action hit areas and rows, not to the desktop button's visual height. + +The Popover stays within the viewport through Radix collision handling. The drawer uses internal scrolling and bottom safe-area padding. + +The disclosure supports Escape dismissal and focus return. Touch rows use a minimum active height of 44px. + +## Failure and recovery + +The existing move request remains the authority for transition errors. A request error clears the move-in-progress state and keeps the disclosure usable. + +If the current step is absent, the compact trigger keeps the existing first-step fallback. It does not mark that fallback as the current step. + +If live workflow data changes while the disclosure is open, React derives the list from the latest sorted steps. Removed steps disappear without persisted selection state. + +## Test strategy + +Component tests cover compact content, eligibility, fine-pointer disclosure, touch-drawer selection, archived behavior, and move callbacks. + +Desktop E2E coverage forces the measured compact state at a narrow top-bar width. It opens all steps, verifies the dialog role, tabs to a step-specific move control, activates it with Enter, and moves the task through the UI. + +Tablet E2E coverage uses `tabletTestPage`. It verifies the compact trigger hit area, opens the touch drawer, checks containment, and selects an eligible step. + +The existing Pixel 5 task-drawer scenario proves that phone users retain step movement without the desktop top bar. + +## Related decisions + +No architecture decision record applies. This design reuses the current responsive overlay and task-move contracts.