From 982cdb63eb338f38599912c6a3545229972d5282 Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 11 Sep 2026 01:01:59 +0800 Subject: [PATCH] feat(web): add task actions menu to preview panel and detail top bar (#3395) Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio --- .../confirmation/action-confirm-popover.tsx | 7 +- .../components/gitlab/task-mr-link-dialog.tsx | 12 +- .../components/kanban-card-edit-submenu.tsx | 5 +- .../kanban-card-menu-items.test.tsx | 155 +++++- .../web/components/kanban-card-menu-items.tsx | 42 +- apps/web/components/kanban-card.tsx | 36 +- apps/web/components/kanban-with-preview.tsx | 59 ++- .../components/task-create-dialog-types.ts | 5 + .../components/task-create-dialog.test.tsx | 63 ++- apps/web/components/task-create-dialog.tsx | 6 + ...task-preview-panel-step-indicator.test.tsx | 201 ++++++++ .../components/task-preview-panel.test.tsx | 476 +++++++++++------ apps/web/components/task-preview-panel.tsx | 134 ++++- .../task/task-actions-menu-dialogs.test.tsx | 210 ++++++++ .../task/task-actions-menu-dialogs.tsx | 229 +++++++++ .../task/task-actions-menu-trigger.tsx | 65 +++ .../task/task-archive-confirm-dialog.tsx | 12 +- .../task/task-archive-confirmation.test.tsx | 49 ++ .../task/task-archive-confirmation.tsx | 8 + .../task/task-delete-confirm-dialog.tsx | 19 +- .../task/task-detach-confirm-dialog.test.tsx | 12 + .../task/task-detach-confirm-dialog.tsx | 6 + .../task/task-external-link-dialog.tsx | 12 +- .../task/task-github-issue-dialog.tsx | 12 +- .../components/task/task-github-pr-dialog.tsx | 12 +- ...sk-page-content-helpers-board-row.test.tsx | 91 ++++ .../task/task-page-content-helpers.ts | 40 ++ apps/web/components/task/task-page-inner.tsx | 11 + .../task/task-top-bar-actions-menu.test.tsx | 486 ++++++++++++++++++ .../task/task-top-bar-actions-menu.tsx | 173 +++++++ .../web/components/task/task-top-bar.test.tsx | 163 +++++- apps/web/components/task/task-top-bar.tsx | 28 + .../kanban/task-actions-menu-detail.spec.ts | 56 ++ .../kanban/task-actions-menu-preview.spec.ts | 143 ++++++ .../domains/kanban/use-kanban-actions.ts | 2 +- ...se-task-actions-menu-move-targets.test.tsx | 121 +++++ .../use-task-actions-menu-move-targets.ts | 79 +++ apps/web/hooks/use-task-actions-menu.ts | 319 ++++++++++++ apps/web/hooks/use-task-actions.test.ts | 55 +- apps/web/hooks/use-task-actions.ts | 38 +- apps/web/hooks/use-task-menu-dialog-state.ts | 53 ++ apps/web/lib/dialog-focus-return.ts | 18 + .../kanban/task-actions-menu-entries.test.ts | 178 +++++++ .../lib/kanban/task-actions-menu-entries.ts | 66 +++ docs/specs/tasks/README.md | 4 + .../task-actions-menu-concurrency.md | 151 ++++++ .../task-actions-menu-outcomes.md | 95 ++++ .../tasks/requirements/task-actions-menu.md | 302 +++++++++++ .../tasks/system-design/task-actions-menu.md | 362 +++++++++++++ 49 files changed, 4637 insertions(+), 244 deletions(-) create mode 100644 apps/web/components/task-preview-panel-step-indicator.test.tsx create mode 100644 apps/web/components/task/task-actions-menu-dialogs.test.tsx create mode 100644 apps/web/components/task/task-actions-menu-dialogs.tsx create mode 100644 apps/web/components/task/task-actions-menu-trigger.tsx create mode 100644 apps/web/components/task/task-page-content-helpers-board-row.test.tsx create mode 100644 apps/web/components/task/task-top-bar-actions-menu.test.tsx create mode 100644 apps/web/components/task/task-top-bar-actions-menu.tsx create mode 100644 apps/web/e2e/tests/kanban/task-actions-menu-detail.spec.ts create mode 100644 apps/web/e2e/tests/kanban/task-actions-menu-preview.spec.ts create mode 100644 apps/web/hooks/use-task-actions-menu-move-targets.test.tsx create mode 100644 apps/web/hooks/use-task-actions-menu-move-targets.ts create mode 100644 apps/web/hooks/use-task-actions-menu.ts create mode 100644 apps/web/hooks/use-task-menu-dialog-state.ts create mode 100644 apps/web/lib/dialog-focus-return.ts create mode 100644 apps/web/lib/kanban/task-actions-menu-entries.test.ts create mode 100644 apps/web/lib/kanban/task-actions-menu-entries.ts create mode 100644 docs/specs/tasks/requirements/task-actions-menu-concurrency.md create mode 100644 docs/specs/tasks/requirements/task-actions-menu-outcomes.md create mode 100644 docs/specs/tasks/requirements/task-actions-menu.md create mode 100644 docs/specs/tasks/system-design/task-actions-menu.md diff --git a/apps/web/components/confirmation/action-confirm-popover.tsx b/apps/web/components/confirmation/action-confirm-popover.tsx index 0cd755a3c9..197731f8c3 100644 --- a/apps/web/components/confirmation/action-confirm-popover.tsx +++ b/apps/web/components/confirmation/action-confirm-popover.tsx @@ -21,6 +21,7 @@ export type ActionConfirmPopoverProps = { anchorRef: RefObject; focusReturnRef?: RefObject; focusBoundaryRef?: RefObject; + restoreFocusOnConfirm?: boolean; title: ReactNode; description?: ReactNode; cancelLabel: ReactNode; @@ -53,6 +54,7 @@ export function ActionConfirmPopover({ anchorRef, focusReturnRef, focusBoundaryRef, + restoreFocusOnConfirm = false, title, description, cancelLabel, @@ -129,6 +131,7 @@ export function ActionConfirmPopover({ cancelRef={cancelRef} focusReturnRef={focusReturnRef} focusBoundaryRef={focusBoundaryRef} + restoreFocusOnConfirm={restoreFocusOnConfirm} confirmedRef={confirmedRef} anchorRef={anchorRef} onCancel={() => handleOpenChange(false)} @@ -155,6 +158,7 @@ type ActionConfirmPopoverContentProps = { cancelRef: RefObject; focusReturnRef?: RefObject; focusBoundaryRef?: RefObject; + restoreFocusOnConfirm: boolean; confirmedRef: { current: boolean }; anchorRef: RefObject; onCancel: () => void; @@ -178,6 +182,7 @@ function ActionConfirmPopoverContent({ cancelRef, focusReturnRef, focusBoundaryRef, + restoreFocusOnConfirm, confirmedRef, anchorRef, onCancel, @@ -209,7 +214,7 @@ function ActionConfirmPopoverContent({ }} onCloseAutoFocus={(event) => { event.preventDefault(); - if (!confirmedRef.current) { + if (!confirmedRef.current || restoreFocusOnConfirm) { const focusReturnTarget = focusReturnRef?.current ?? null; if (isConnected(focusReturnTarget)) focusReturnTarget.focus(); else if (isConnected(anchorRef.current)) anchorRef.current.focus(); diff --git a/apps/web/components/gitlab/task-mr-link-dialog.tsx b/apps/web/components/gitlab/task-mr-link-dialog.tsx index ab9dac07ac..54bf2f7854 100644 --- a/apps/web/components/gitlab/task-mr-link-dialog.tsx +++ b/apps/web/components/gitlab/task-mr-link-dialog.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useState, type RefObject } from "react"; import { Button } from "@kandev/ui/button"; import { Dialog, @@ -16,6 +16,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@ import { useAppStore } from "@/components/state-provider"; import { useToast } from "@/components/toast-provider"; import { createTaskMR } from "@/lib/api/domains/gitlab-api"; +import { createFocusReturnHandler } from "@/lib/dialog-focus-return"; import type { Repository } from "@/lib/types/http"; import { useTranslation } from "react-i18next"; @@ -111,6 +112,7 @@ export function TaskMRLinkDialog({ workspaceId, taskRepositories, repositories, + focusReturnRef, }: { open: boolean; onOpenChange: (open: boolean) => void; @@ -118,6 +120,9 @@ export function TaskMRLinkDialog({ workspaceId: string; taskRepositories: TaskRepositoryLink[]; repositories: Repository[]; + /** Element to return keyboard focus to on close (AC-TASKS-TASK-ACTIONS-MENU-001.12). + * Omitted callers keep Radix's default restore-to-previously-focused-element behavior. */ + focusReturnRef?: RefObject; }) { const { t } = useTranslation(); const options = useRepositoryOptions(repositories, taskRepositories); @@ -168,7 +173,10 @@ export function TaskMRLinkDialog({ return ( - + {t("gitlab:linkGitlabMergeRequest")} {t("gitlab:pasteAMergeRequestUrlFrom")} diff --git a/apps/web/components/kanban-card-edit-submenu.tsx b/apps/web/components/kanban-card-edit-submenu.tsx index 2a3c998b9e..2d88324dd8 100644 --- a/apps/web/components/kanban-card-edit-submenu.tsx +++ b/apps/web/components/kanban-card-edit-submenu.tsx @@ -20,12 +20,15 @@ export function buildEditMenuEntry({ onEdit, disabled, context, + forceFlat, }: { onEdit?: () => void; disabled?: boolean; context: PluginTaskMenuContext; + /** Skip the plugin `edit`-group lookup and always return the flat item. */ + forceFlat?: boolean; }): KanbanCardMenuEntry { - const pluginActions = visiblePluginMenuActions("edit", context); + const pluginActions = forceFlat ? [] : visiblePluginMenuActions("edit", context); const icon = ; diff --git a/apps/web/components/kanban-card-menu-items.test.tsx b/apps/web/components/kanban-card-menu-items.test.tsx index fba3328207..9b3e0fde5f 100644 --- a/apps/web/components/kanban-card-menu-items.test.tsx +++ b/apps/web/components/kanban-card-menu-items.test.tsx @@ -1,12 +1,16 @@ import type { ReactNode } from "react"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { render, screen, fireEvent } from "@testing-library/react"; +import { act, render, renderHook, screen, fireEvent } from "@testing-library/react"; import { IconFlag } from "@tabler/icons-react"; +import type { StoreApi } from "zustand"; import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from "@kandev/ui/dropdown-menu"; import { pluginRegistry } from "@/lib/plugins/registry"; +import { StateProvider, useAppStoreApi } from "@/components/state-provider"; +import type { AppState } from "@/lib/state/store"; import { buildKanbanCardMenuEntries, KanbanCardDropdownMenuItems, + useKanbanCardMoveTargets, type KanbanCardMenuEntry, } from "./kanban-card-menu-items"; @@ -413,3 +417,152 @@ describe("buildKanbanCardMenuEntries — priority action", () => { expect(onSelectPriority).toHaveBeenCalledWith("critical"); }); }); + +describe("useKanbanCardMoveTargets — explicit steps (AC-TASKS-TASK-ACTIONS-MENU-002.3b)", () => { + const WORKFLOW_ID = "wf-1"; + const TASK_ID = "task-1"; + const HIDDEN_STEP_ID = "step-hidden"; + + function wrapper({ children }: { children: React.ReactNode }) { + return ( + + {children} + + ); + } + + it("uses the caller's explicit steps as-is for the current workflow's entry", () => { + const explicitSteps = [{ id: HIDDEN_STEP_ID, title: "Hidden step", color: "gray" }]; + const { result } = renderHook(() => useKanbanCardMoveTargets(TASK_ID, explicitSteps), { + wrapper, + }); + + const stepIds = result.current.stepsByWorkflowId[WORKFLOW_ID].map((step) => step.id); + expect(stepIds).toEqual([HIDDEN_STEP_ID]); + }); +}); + +describe("useKanbanCardMoveTargets — card hot path has no kanban.tasks/hiddenWorkflowStepIds subscription", () => { + const WORKFLOW_ID = "wf-1"; + const TASK_ID = "task-1"; + const CARD_STEPS = [{ id: "step-a", title: "Todo", color: "blue" }]; + + let capturedStore: StoreApi | null = null; + + function StoreCapture() { + capturedStore = useAppStoreApi(); + return null; + } + + function wrapper({ children }: { children: React.ReactNode }) { + return ( + + + {children} + + ); + } + + // The card's own render path already knows its workflow's steps (it passes + // `steps` explicitly), so this hook must not force every card to re-render + // on unrelated `kanban.tasks`/`hiddenWorkflowStepIds` writes just to serve + // the preview/detail surfaces' fallback (system design: card hook keeps its + // original subscription set). + it("does not recompute when kanban.tasks or hiddenWorkflowStepIds change while the caller passes explicit steps", () => { + capturedStore = null; + let renderCount = 0; + const { result } = renderHook( + () => { + renderCount++; + return useKanbanCardMoveTargets(TASK_ID, CARD_STEPS); + }, + { wrapper }, + ); + + expect(result.current.currentWorkflowId).toBe(WORKFLOW_ID); + const countAfterMount = renderCount; + + act(() => { + capturedStore!.setState((state) => ({ + kanban: { + ...state.kanban, + tasks: [ + ...state.kanban.tasks, + { + id: "other-task", + workflowId: WORKFLOW_ID, + workflowStepId: "step-a", + title: "Other", + position: 1, + }, + ], + }, + })); + }); + expect(renderCount).toBe(countAfterMount); + + act(() => { + capturedStore!.setState((state) => ({ + userSettings: { + ...state.userSettings, + hiddenWorkflowStepIds: { [WORKFLOW_ID]: ["step-a"] }, + }, + })); + }); + expect(renderCount).toBe(countAfterMount); + }); +}); diff --git a/apps/web/components/kanban-card-menu-items.tsx b/apps/web/components/kanban-card-menu-items.tsx index 2716809732..a6a6f60d3d 100644 --- a/apps/web/components/kanban-card-menu-items.tsx +++ b/apps/web/components/kanban-card-menu-items.tsx @@ -40,6 +40,7 @@ import { } from "@/lib/tasks/task-priority"; import type { TaskPriority } from "@/lib/types/http"; import { cn } from "@/lib/utils"; +import { sortWorkflowStepsByPosition } from "@/lib/kanban/workflow-step-order"; import { buildLinkSubmenu } from "./kanban-card-link-submenu"; import type { PluginIcon, PluginTaskMenuContext } from "@/lib/plugins/types"; import { buildEditMenuEntry } from "./kanban-card-edit-submenu"; @@ -91,7 +92,7 @@ export type KanbanPluginLinkAction = { onSelect: () => void; }; -type BuildKanbanCardMenuEntriesArgs = { +export type BuildKanbanCardMenuEntriesArgs = { currentWorkflowId?: string | null; currentStepId?: string | null; workflows: TaskMoveWorkflow[]; @@ -119,6 +120,12 @@ type BuildKanbanCardMenuEntriesArgs = { onSendToWorkflow?: (workflowId: string, stepId: string) => void; /** Defaults to an empty-id context (no visible plugin actions match it in practice). */ pluginMenuContext?: PluginTaskMenuContext; + /** + * Forces the flat Edit item regardless of registered plugin `edit`-group + * actions. Group `edit` is a card-only plugin contract; surfaces outside + * the card set this so they never present the submenu form. + */ + forceFlatEdit?: boolean; }; const EMPTY_PLUGIN_MENU_CONTEXT: PluginTaskMenuContext = { @@ -129,7 +136,7 @@ const EMPTY_PLUGIN_MENU_CONTEXT: PluginTaskMenuContext = { presentation: "desktop", }; -function resolvePluginMenuContext(context?: PluginTaskMenuContext): PluginTaskMenuContext { +export function resolvePluginMenuContext(context?: PluginTaskMenuContext): PluginTaskMenuContext { return context ?? EMPTY_PLUGIN_MENU_CONTEXT; } @@ -345,6 +352,7 @@ export function buildKanbanCardMenuEntries({ onMoveToStep, onSendToWorkflow, pluginMenuContext, + forceFlatEdit, }: BuildKanbanCardMenuEntriesArgs): KanbanCardMenuEntry[] { const visibleWorkflows = workflows.filter((workflow) => !workflow.hidden); const currentSteps = currentWorkflowId ? (stepsByWorkflowId[currentWorkflowId] ?? []) : []; @@ -354,6 +362,7 @@ export function buildKanbanCardMenuEntries({ onEdit, disabled: isProcessing, context: resolvePluginMenuContext(pluginMenuContext), + forceFlat: forceFlatEdit, }), ]; @@ -411,7 +420,7 @@ export function buildKanbanCardMenuEntries({ return entries; } -function buildArchiveEntry({ +export function buildArchiveEntry({ isArchiving, isProcessing, onArchive, @@ -434,7 +443,7 @@ function buildArchiveEntry({ }; } -function buildDeleteEntry({ +export function buildDeleteEntry({ isDeleting, isProcessing, onDelete, @@ -482,6 +491,16 @@ function buildDetachEntry({ }; } +/** + * The card's hot render path: a caller with column context already passes a + * `steps` list pre-filtered for this workflow, and its `currentWorkflowId` + * always resolves from `kanbanMulti.snapshots`. Kept to exactly these two + * subscriptions (system design: no `kanban.tasks`/`hiddenWorkflowStepIds` + * reads here) so unrelated writes to either slice never re-render every + * visible card. Callers without column context (preview panel, detail top + * bar) use `useTaskActionsMenuMoveTargets` instead, which layers the + * flat-list and hidden-step fallbacks this hook intentionally omits. + */ export function useKanbanCardMoveTargets( taskId: string, steps?: WorkflowStep[], @@ -513,15 +532,12 @@ export function useKanbanCardMoveTargets( const stepsByWorkflowId = useMemo>(() => { const result: Record = {}; for (const [workflowId, snapshotSteps] of Object.entries(snapshotStepsByWorkflowId)) { - result[workflowId] = snapshotSteps - .slice() - .sort((a, b) => a.position - b.position) - .map((step) => ({ - id: step.id, - title: step.title, - color: step.color, - events: step.events, - })); + result[workflowId] = sortWorkflowStepsByPosition(snapshotSteps).map((step) => ({ + id: step.id, + title: step.title, + color: step.color, + events: step.events, + })); } if (currentWorkflowId && steps) { result[currentWorkflowId] = steps.map((step) => ({ diff --git a/apps/web/components/kanban-card.tsx b/apps/web/components/kanban-card.tsx index 2ccf8fa8c2..4d9f261407 100644 --- a/apps/web/components/kanban-card.tsx +++ b/apps/web/components/kanban-card.tsx @@ -1,6 +1,6 @@ "use client"; -import { useRef, useState } from "react"; +import { useRef } from "react"; import { useDraggable } from "@dnd-kit/core"; import { KanbanCardContextMenu } from "@/components/kanban-card-context-menu"; import { KanbanCardShell } from "@/components/kanban-card-content"; @@ -24,6 +24,7 @@ import { useTaskMultiSelectStore } from "@/hooks/use-task-multi-select"; import type { TaskActionOptions } from "@/hooks/use-task-actions"; import { useDetachTask } from "@/hooks/use-detach-task"; import { useUpdateTaskPriority } from "@/hooks/use-update-task-priority"; +import { useTaskMenuDialogState } from "@/hooks/use-task-menu-dialog-state"; import { type ForegroundActivity, type TaskPendingAction, @@ -230,7 +231,7 @@ function externalLinkHandlers( /** Link-dialog openers shared by both the dropdown and context menu builds. */ function buildLinkDialogHandlers( externalLinkAvailability: KanbanExternalLinkAvailability, - dialogs: ReturnType, + dialogs: ReturnType, ) { return { onLinkPullRequest: () => dialogs.setShowPRDialog(true), @@ -256,35 +257,6 @@ export function buildPluginMenuContext( }; } -/** Every confirm/link-dialog open flag the card menus and their dialogs share. */ -function useKanbanCardDialogState() { - const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); - const [showArchiveConfirm, setShowArchiveConfirm] = useState(false); - const [showDetachConfirm, setShowDetachConfirm] = useState(false); - const [showPRDialog, setShowPRDialog] = useState(false); - const [showIssueDialog, setShowIssueDialog] = useState(false); - const [showMRDialog, setShowMRDialog] = useState(false); - const [externalLinkProvider, setExternalLinkProvider] = useState( - null, - ); - return { - showDeleteConfirm, - setShowDeleteConfirm, - showArchiveConfirm, - setShowArchiveConfirm, - showDetachConfirm, - setShowDetachConfirm, - showPRDialog, - setShowPRDialog, - showIssueDialog, - setShowIssueDialog, - showMRDialog, - setShowMRDialog, - externalLinkProvider, - setExternalLinkProvider, - }; -} - function useKanbanCardMenus({ task, workspaceId, @@ -322,7 +294,7 @@ function useKanbanCardMenus({ // just disabled doesn't linger as a stale entry. usePluginRegistry(); const moveMenu = useKanbanCardMoveMenuActions({ task, steps, isSelected, selectedIds, onMove }); - const dialogs = useKanbanCardDialogState(); + const dialogs = useTaskMenuDialogState(); const { detachTask, detachingTaskId } = useDetachTask(); const updateTaskPriority = useUpdateTaskPriority(); const detachAnchorRef = useRef(null); diff --git a/apps/web/components/kanban-with-preview.tsx b/apps/web/components/kanban-with-preview.tsx index fcef4471c5..ef8c6e8787 100644 --- a/apps/web/components/kanban-with-preview.tsx +++ b/apps/web/components/kanban-with-preview.tsx @@ -98,7 +98,15 @@ function useUrlSync(selectedTaskId: string | null, selectedTaskSessionId: string function useEscapeKey(isOpen: boolean, close: () => void, isDisclosureOpen: () => boolean) { useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { - if (e.key === "Escape" && isOpen && !isDisclosureOpen()) { + // `e.defaultPrevented` is the authoritative signal that an open actions + // menu already consumed this exact keypress (its `onEscapeKeyDown` + // calls `preventDefault()` during document-capture, which always runs + // before this window-bubble listener sees the same event). Do not gate + // on menu-open state read at listener-fire time instead: Radix's + // Escape handling can synchronously re-render and re-attach this very + // listener mid-dispatch, so that state can already read "closed" for + // the SAME keypress that closed it (AC-TASKS-TASK-ACTIONS-MENU-001.11). + if (e.key === "Escape" && isOpen && !isDisclosureOpen() && !e.defaultPrevented) { close(); } }; @@ -108,6 +116,21 @@ function useEscapeKey(isOpen: boolean, close: () => void, isDisclosureOpen: () = }, [isOpen, close, isDisclosureOpen]); } +/** Mirrors 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 useResizeHandler( isResizingRef: React.RefObject, previewWidthPx: number, @@ -202,26 +225,15 @@ function useSelectedTask( description: task.description, position: task.position, repositoryId: task.repositoryId, + repositories: task.repositories, primarySessionId: task.primarySessionId, + parentTaskId: task.parentTaskId, + primaryExecutorType: task.primaryExecutorType, + workspaceMode: task.workspaceMode, }; }, [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; @@ -325,6 +337,13 @@ export function KanbanWithPreview({ initialTaskId, initialSessionId }: KanbanWit // Track resize state const isResizingRef = useRef(false); + // Gates the preview's own Escape-to-close so a first Escape only closes an + // open actions menu (AC-TASKS-TASK-ACTIONS-MENU-001.11): this reads the + // pre-keypress state, since the window `keydown` listener below runs after + // Radix's own Escape handling has requested the menu close for this same + // keypress but before that state update has re-rendered. + const [actionsMenuOpen, setActionsMenuOpen] = useState(false); + const selectedTask = useSelectedTask(selectedTaskId, kanbanTasks, kanbanMultiSnapshots); const previewStepMove = usePreviewWorkflowStepMove(selectedTaskId, selectedTask); @@ -379,7 +398,7 @@ export function KanbanWithPreview({ initialTaskId, initialSessionId }: KanbanWit [isOpen, selectedTaskId, open, close], ); - useEscapeKey(isOpen, close, previewStepMove.isDisclosureOpen); + useEscapeKey(isOpen && !actionsMenuOpen, close, previewStepMove.isDisclosureOpen); const handleResizeMouseDown = useResizeHandler(isResizingRef, previewWidthPx, updatePreviewWidth); @@ -408,6 +427,7 @@ export function KanbanWithPreview({ initialTaskId, initialSessionId }: KanbanWit onClose={close} onSessionChange={setUserSelectedSessionId} onResizeMouseDown={handleResizeMouseDown} + onActionsMenuOpenChange={setActionsMenuOpen} /> ); } @@ -452,6 +472,7 @@ type PreviewLayoutProps = { onClose: () => void; onSessionChange: (sessionId: string | null) => void; onResizeMouseDown: (e: React.MouseEvent) => void; + onActionsMenuOpenChange: (open: boolean) => void; }; function previewPanelStepProps(stepMove: PreviewStepMove) { @@ -479,6 +500,7 @@ function FloatingPreviewLayout({ onClose, onSessionChange, onResizeMouseDown, + onActionsMenuOpenChange, }: PreviewLayoutProps) { const { t } = useTranslation(); return ( @@ -512,6 +534,7 @@ function FloatingPreviewLayout({ onMaximize={(task) => onNavigateToTask(task)} onSessionChange={onSessionChange} {...previewPanelStepProps(stepMove)} + onActionsMenuOpenChange={onActionsMenuOpenChange} /> @@ -532,6 +555,7 @@ function InlinePreviewLayout({ onClose, onSessionChange, onResizeMouseDown, + onActionsMenuOpenChange, }: PreviewLayoutProps & { isOpen: boolean }) { return (
@@ -557,6 +581,7 @@ function InlinePreviewLayout({ onMaximize={(task) => onNavigateToTask(task)} onSessionChange={onSessionChange} {...previewPanelStepProps(stepMove)} + onActionsMenuOpenChange={onActionsMenuOpenChange} />
diff --git a/apps/web/components/task-create-dialog-types.ts b/apps/web/components/task-create-dialog-types.ts index f7acc6f5a6..60780ea1a9 100644 --- a/apps/web/components/task-create-dialog-types.ts +++ b/apps/web/components/task-create-dialog-types.ts @@ -1,4 +1,5 @@ import type React from "react"; +import type { RefObject } from "react"; import type { LocalRepository, Repository, @@ -85,6 +86,10 @@ export interface TaskCreateDialogProps { extraFormSlot?: React.ReactNode; bottomSlot?: React.ReactNode; submitBlockedReason?: string | null; + /** Element to return keyboard focus to on close, confirmed or cancelled. + * Omitted callers keep Radix's default restore-to-previously-focused-element + * behavior. */ + focusReturnRef?: RefObject; } export type DialogPromptEnhance = { diff --git a/apps/web/components/task-create-dialog.test.tsx b/apps/web/components/task-create-dialog.test.tsx index f52fce606e..8f09d59e94 100644 --- a/apps/web/components/task-create-dialog.test.tsx +++ b/apps/web/components/task-create-dialog.test.tsx @@ -1,4 +1,11 @@ -import { createRef, type ReactNode, useEffect, useImperativeHandle, useRef } from "react"; +import { + createRef, + type ComponentProps, + type ReactNode, + useEffect, + useImperativeHandle, + useRef, +} from "react"; import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -16,20 +23,26 @@ const ENHANCE_PROMPT_BUTTON_TEST_ID = "enhance-prompt-button"; type EscapeEvent = { preventDefault: () => void }; +type CloseAutoFocusEvent = { preventDefault: () => void }; + let allowProgrammaticSet = true; let mockFs: DialogFormState; let dialogEscapeHandler: ((event: EscapeEvent) => void) | undefined; +let dialogCloseAutoFocusHandler: ((event: CloseAutoFocusEvent) => void) | undefined; vi.mock("@kandev/ui/dialog", () => ({ Dialog: ({ children }: { children: ReactNode }) =>
{children}
, DialogContent: ({ children, onEscapeKeyDown, + onCloseAutoFocus, }: { children: ReactNode; onEscapeKeyDown?: (event: EscapeEvent) => void; + onCloseAutoFocus?: (event: CloseAutoFocusEvent) => void; }) => { dialogEscapeHandler = onEscapeKeyDown; + dialogCloseAutoFocusHandler = onCloseAutoFocus; return
{children}
; }, DialogHeader: ({ children }: { children: ReactNode }) =>
{children}
, @@ -351,7 +364,10 @@ function buildMockFs(initialDescription = ORIGINAL_PROMPT): DialogFormState { }; } -function renderDialog(mode: "create" | "edit" | "session" = "create") { +function renderDialog( + mode: "create" | "edit" | "session" = "create", + extraProps: Partial> = {}, +) { return render( , ); } @@ -375,9 +392,51 @@ beforeEach(() => { toastMock.mockReset(); setHasDescriptionMock.mockReset(); dialogEscapeHandler = undefined; + dialogCloseAutoFocusHandler = undefined; mockFs = buildMockFs(); }); +describe("TaskCreateDialog focus return (AC-TASKS-TASK-ACTIONS-MENU-001.12)", () => { + it("returns focus to the given focusReturnRef target on close, overriding Radix's default", () => { + const target = document.createElement("button"); + document.body.appendChild(target); + target.focus = vi.fn(); + const focusReturnRef = { current: target }; + + renderDialog("edit", { focusReturnRef }); + expect(dialogCloseAutoFocusHandler).toBeTypeOf("function"); + + const event = { preventDefault: vi.fn() }; + dialogCloseAutoFocusHandler?.(event); + + expect(event.preventDefault).toHaveBeenCalled(); + expect(target.focus).toHaveBeenCalled(); + + document.body.removeChild(target); + }); + + it("leaves Radix's default close-focus behavior alone when the target has left the document", () => { + const target = document.createElement("button"); + target.focus = vi.fn(); + const focusReturnRef = { current: target }; + + renderDialog("edit", { focusReturnRef }); + const event = { preventDefault: vi.fn() }; + dialogCloseAutoFocusHandler?.(event); + + expect(event.preventDefault).not.toHaveBeenCalled(); + expect(target.focus).not.toHaveBeenCalled(); + }); + + it("leaves Radix's default close-focus behavior alone when no focusReturnRef is given", () => { + renderDialog("edit"); + const event = { preventDefault: vi.fn() }; + dialogCloseAutoFocusHandler?.(event); + + expect(event.preventDefault).not.toHaveBeenCalled(); + }); +}); + describe("TaskCreateDialog Escape dismissal", () => { it("prevents Escape from dismissing create mode", () => { renderDialog(); diff --git a/apps/web/components/task-create-dialog.tsx b/apps/web/components/task-create-dialog.tsx index e6f3016bb6..039f9abcf3 100644 --- a/apps/web/components/task-create-dialog.tsx +++ b/apps/web/components/task-create-dialog.tsx @@ -274,6 +274,12 @@ export function TaskCreateDialog(props: TaskCreateDialogProps) { onEscapeKeyDown={(event) => { if (setup.isCreateMode) event.preventDefault(); }} + onCloseAutoFocus={(event) => { + const target = props.focusReturnRef?.current; + if (!target || !document.contains(target)) return; + event.preventDefault(); + target.focus(); + }} data-testid="create-task-dialog" data-webkit-safe-motion="true" showCloseButton={false} diff --git a/apps/web/components/task-preview-panel-step-indicator.test.tsx b/apps/web/components/task-preview-panel-step-indicator.test.tsx new file mode 100644 index 0000000000..44bcf89720 --- /dev/null +++ b/apps/web/components/task-preview-panel-step-indicator.test.tsx @@ -0,0 +1,201 @@ +import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; +import type { ComponentProps, ReactNode } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { StateProvider } from "@/components/state-provider"; +import { ToastProvider } from "@/components/toast-provider"; +import { TaskPreviewPanel } from "./task-preview-panel"; +import type { WorkflowStepperStep } from "./task/workflow-step-disclosure"; +import type { Task } from "./kanban-card"; + +vi.mock("./task/preview-session-tabs", () => ({ + PreviewSessionTabs: () =>
, +})); + +vi.mock("@/hooks/use-compact-task-chrome", () => ({ + useTouchDrawer: () => false, +})); + +afterEach(cleanup); + +const TASK: Task = { + id: "task-1", + title: "Fix the login bug", +} as Task; + +const OTHER_TASK: Task = { + id: "task-2", + title: "Write the release notes", +} as Task; + +const WORKFLOW_ID = "workflow-1"; +const STEP_A_ID = "a"; +const STEPPER_TEST_ID = "workflow-stepper-minimal"; +const DISCLOSURE_TEST_ID = "workflow-step-disclosure"; + +const STEPS: WorkflowStepperStep[] = [ + { id: "a", name: "Spec", color: "#111", position: 0 }, + { id: "b", name: "Work", color: "#222", position: 1, allow_manual_move: true }, +]; + +function withProviders(ui: ReactNode) { + return ( + + {ui} + + ); +} + +function renderPanel(overrides: Partial> = {}) { + return render( + withProviders( + , + ), + ); +} + +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( + withProviders( + , + ), + ); + + expect(screen.queryByTestId(DISCLOSURE_TEST_ID)).toBeNull(); + expect(screen.getByText(OTHER_TASK.title)).toBeTruthy(); + }); + + it("does not let a move started before a task switch close the new task's disclosure", async () => { + let resolveMove!: (moved: boolean) => void; + const onMoveStepForA = vi.fn(() => new Promise((resolve) => (resolveMove = resolve))); + const { rerender } = renderPanel({ onMoveStep: onMoveStepForA }); + + fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); + fireEvent.click(screen.getByTestId("workflow-step-disclosure-move-b")); + expect(onMoveStepForA).toHaveBeenCalledWith("b"); + + rerender( + withProviders( + , + ), + ); + + 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(withProviders()); + + expect(onDisclosureOpenChange).toHaveBeenLastCalledWith(false); + }); +}); diff --git a/apps/web/components/task-preview-panel.test.tsx b/apps/web/components/task-preview-panel.test.tsx index 4f86071ae5..86aa79812c 100644 --- a/apps/web/components/task-preview-panel.test.tsx +++ b/apps/web/components/task-preview-panel.test.tsx @@ -1,189 +1,367 @@ -import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; -import type { ComponentProps } from "react"; +import { act, cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { StateProvider } from "@/components/state-provider"; +import { ToastProvider } from "@/components/toast-provider"; 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", -})); +const detachTaskMock = vi.hoisted(() => vi.fn().mockResolvedValue({ id: "task-1" })); +const getSubtaskCountMock = vi.hoisted(() => vi.fn().mockResolvedValue({ count: 0 })); +const archiveTaskMock = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); +const deleteTaskMock = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); +vi.mock("@/lib/api/domains/kanban-api", async () => { + const actual = await vi.importActual( + "@/lib/api/domains/kanban-api", + ); + return { + ...actual, + detachTask: detachTaskMock, + getSubtaskCount: getSubtaskCountMock, + archiveTask: archiveTaskMock, + deleteTask: deleteTaskMock, + }; +}); + +afterEach(() => { + cleanup(); + vi.useRealTimers(); + detachTaskMock.mockClear(); + getSubtaskCountMock.mockClear(); + getSubtaskCountMock.mockResolvedValue({ count: 0 }); + archiveTaskMock.mockClear(); + archiveTaskMock.mockResolvedValue(undefined); + deleteTaskMock.mockClear(); + deleteTaskMock.mockResolvedValue(undefined); +}); vi.mock("./task/preview-session-tabs", () => ({ PreviewSessionTabs: () =>
, })); -vi.mock("@/hooks/use-compact-task-chrome", () => ({ - useTouchDrawer: () => false, -})); - -afterEach(cleanup); +const TRIGGER_TEST_ID = "task-preview-actions-menu"; +const DETACH_MENU_ITEM_NAME = "Detach from parent"; +const DETACH_CONFIRM_POPOVER_TEST_ID = "detach-task-confirm-popover"; const TASK: Task = { id: "task-1", - title: "Fix the login bug", -} as Task; + title: "Fix the sidebar", + workflowStepId: "step-1", +}; -const OTHER_TASK: Task = { - id: "task-2", - title: "Write the release notes", -} as Task; +const CHILD_TASK: Task = { + ...TASK, + parentTaskId: "task-parent", +}; -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> = {}) { +function renderPanel(ui: React.ReactNode) { return render( - , + + {ui} + , ); } -describe("TaskPreviewPanel step indicator", () => { - it("shows no step indicator when there are no resolved workflow steps", () => { - renderPanel({ workflowSteps: [] }); +function rerenderPanel(rerender: (ui: React.ReactNode) => void, ui: React.ReactNode) { + rerender( + + {ui} + , + ); +} - expect(screen.getByText("Fix the login bug")).toBeTruthy(); - expect(screen.queryByTestId(STEPPER_TEST_ID)).toBeNull(); +function getTrigger() { + return screen.getByTestId(TRIGGER_TEST_ID); +} + +function openMenu() { + const trigger = getTrigger(); + fireEvent.pointerDown(trigger, { button: 0, pointerId: 1 }); + fireEvent.click(trigger); + return trigger; +} + +function expectMenuOpen(open: boolean) { + expect(getTrigger().getAttribute("aria-expanded")).toBe(String(open)); +} + +describe("TaskPreviewPanel actions menu trigger", () => { + it("renders no trigger when the panel has no subject task", () => { + renderPanel(); + + expect(screen.queryByTestId(TRIGGER_TEST_ID)).toBeNull(); }); - it("shows no step indicator when there is no selected task", () => { - renderPanel({ task: null }); + it("renders the trigger before Maximize, with the More options accessible name", () => { + renderPanel(); - expect(screen.queryByTestId(STEPPER_TEST_ID)).toBeNull(); + const trigger = getTrigger(); + expect(trigger.getAttribute("aria-label")).toBe("More options"); + expect(trigger.getAttribute("aria-haspopup")).toBe("menu"); + + // AC-TASKS-TASK-ACTIONS-MENU-001.1: before Maximize, which is itself + // before Close. + const controls = screen.getAllByRole("button"); + const maximizeIndex = controls.findIndex((el) => el.title === "Open full page"); + const triggerIndex = controls.indexOf(trigger); + expect(triggerIndex).toBeGreaterThanOrEqual(0); + expect(triggerIndex).toBeLessThan(maximizeIndex); }); - it("shows the current step and total between the title and the panel controls", () => { - renderPanel(); + it("opens a menu offering Detach from parent for a subject task with a parent", () => { + renderPanel(); - expect(screen.getByTestId(STEPPER_TEST_ID)).toBeTruthy(); - expect(screen.getByText("1/2")).toBeTruthy(); + openMenu(); + + expect(screen.getByRole("menuitem", { name: DETACH_MENU_ITEM_NAME })).toBeTruthy(); }); - it("issues a move for an eligible target and closes the disclosure on success", async () => { - const onMoveStep = vi.fn().mockResolvedValue(true); - renderPanel({ onMoveStep }); + it("offers no Detach from parent when the subject task has none", () => { + renderPanel(); - fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); + openMenu(); + + expect(screen.queryByRole("menuitem", { name: DETACH_MENU_ITEM_NAME })).toBeNull(); + }); + + it("requests detach for the subject task and closes the confirmation once it completes (AC-TASKS-TASK-ACTIONS-MENU-003.10)", async () => { + vi.useFakeTimers(); + renderPanel(); + + openMenu(); + fireEvent.click(screen.getByRole("menuitem", { name: DETACH_MENU_ITEM_NAME })); await act(async () => { - fireEvent.click(screen.getByTestId("workflow-step-disclosure-move-b")); - await Promise.resolve(); + vi.advanceTimersByTime(300); }); + expect(screen.getByTestId(DETACH_CONFIRM_POPOVER_TEST_ID)).toBeTruthy(); - expect(onMoveStep).toHaveBeenCalledWith("b"); - expect(screen.queryByTestId(DISCLOSURE_TEST_ID)).toBeNull(); - }); + fireEvent.click(screen.getByTestId("detach-task-confirm")); + vi.useRealTimers(); - it("notifies the caller when the disclosure opens and closes", () => { - const onDisclosureOpenChange = vi.fn(); - renderPanel({ onDisclosureOpenChange }); - - expect(onDisclosureOpenChange).toHaveBeenCalledWith(false); - fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); - expect(onDisclosureOpenChange).toHaveBeenCalledWith(true); - }); - - it("renders the move-failure message below the header, not inside it", () => { - renderPanel({ moveError: new Error("boom") }); - - const banner = screen.getByTestId("task-move-error-banner"); - expect(banner).toBeTruthy(); - const headerRow = screen.getByText("Fix the login bug").closest(".border-b"); - expect(headerRow?.contains(banner)).toBe(false); - }); - - it("shows no move-failure message when there is none", () => { - renderPanel({ workflowSteps: [] }); - expect(screen.queryByTestId("task-move-error-banner")).toBeNull(); - }); - - it("closes the disclosure when the previewed task changes while it is open", () => { - const { rerender } = renderPanel(); - - fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); - expect(screen.getByTestId(DISCLOSURE_TEST_ID)).toBeTruthy(); - - rerender( - , - ); - - expect(screen.queryByTestId(DISCLOSURE_TEST_ID)).toBeNull(); - expect(screen.getByText(OTHER_TASK.title)).toBeTruthy(); - }); - - it("does not let a move started before a task switch close the new task's disclosure", async () => { - let resolveMove!: (moved: boolean) => void; - const onMoveStepForA = vi.fn(() => new Promise((resolve) => (resolveMove = resolve))); - const { rerender } = renderPanel({ onMoveStep: onMoveStepForA }); - - fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); - fireEvent.click(screen.getByTestId("workflow-step-disclosure-move-b")); - expect(onMoveStepForA).toHaveBeenCalledWith("b"); - - rerender( - , - ); - - fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); - expect(screen.getByTestId(DISCLOSURE_TEST_ID)).toBeTruthy(); - - await act(async () => { - resolveMove(true); - await Promise.resolve(); - }); - - expect(screen.getByTestId(DISCLOSURE_TEST_ID)).toBeTruthy(); - }); - - it("shows the archived badge and offers no move control for an archived task", () => { - const onMoveStep = vi.fn(); - renderPanel({ isArchived: true, onMoveStep }); - - expect(screen.queryByTestId("workflow-step-disclosure-move-b")).toBeNull(); - fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); - expect(screen.queryByTestId(DISCLOSURE_TEST_ID)).toBeNull(); - expect(onMoveStep).not.toHaveBeenCalled(); + await waitFor(() => expect(detachTaskMock).toHaveBeenCalledWith(CHILD_TASK.id)); + await waitFor(() => expect(screen.queryByTestId(DETACH_CONFIRM_POPOVER_TEST_ID)).toBeNull()); }); }); -describe("TaskPreviewPanel step indicator lifecycle", () => { - it("reports the disclosure closed when the stepper unmounts", () => { - const onDisclosureOpenChange = vi.fn(); - const { rerender } = renderPanel({ onDisclosureOpenChange }); +describe("TaskPreviewPanel actions menu — subject identity change (AC-TASKS-TASK-ACTIONS-MENU-004.5a)", () => { + const OTHER_TASK: Task = { id: "task-2", title: "Other task", workflowStepId: "step-1" }; - fireEvent.mouseEnter(screen.getByTestId(STEPPER_TEST_ID)); - expect(onDisclosureOpenChange).toHaveBeenLastCalledWith(true); + it("closes an open menu, without re-targeting it, when the subject task's identifier changes", () => { + const onActionsMenuOpenChange = vi.fn(); + const { rerender } = renderPanel( + , + ); + openMenu(); + expectMenuOpen(true); + onActionsMenuOpenChange.mockClear(); - rerender(); + rerenderPanel( + rerender, + , + ); - expect(onDisclosureOpenChange).toHaveBeenLastCalledWith(false); + expectMenuOpen(false); + expect(onActionsMenuOpenChange).toHaveBeenCalledWith(false); + }); + + it("leaves an open menu open across a re-render that keeps the same subject identifier", () => { + const { rerender } = renderPanel(); + openMenu(); + expectMenuOpen(true); + + // A field-only update to the same task (new object, same id) must not + // close the menu. + rerenderPanel( + rerender, + , + ); + + expectMenuOpen(true); + }); +}); + +describe("TaskPreviewPanel actions menu — focus return to trigger (AC-TASKS-TASK-ACTIONS-MENU-001.12)", () => { + it("returns focus to the trigger once a cancelled Delete confirmation closes", async () => { + renderPanel(); + + const trigger = openMenu(); + fireEvent.click(screen.getByRole("menuitem", { name: "Delete" })); + expect(await screen.findByRole("alertdialog")).toBeTruthy(); + + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + + await waitFor(() => expect(screen.queryByRole("alertdialog")).toBeNull()); + await waitFor(() => expect(document.activeElement).toBe(trigger)); + }); + + it("returns focus to the trigger once a cancelled Detach confirmation closes", async () => { + renderPanel(); + + const trigger = openMenu(); + fireEvent.click(screen.getByRole("menuitem", { name: DETACH_MENU_ITEM_NAME })); + const popover = await screen.findByTestId(DETACH_CONFIRM_POPOVER_TEST_ID); + + fireEvent.click(within(popover).getByRole("button", { name: "Cancel" })); + + await waitFor(() => expect(screen.queryByTestId(DETACH_CONFIRM_POPOVER_TEST_ID)).toBeNull()); + await waitFor(() => expect(document.activeElement).toBe(trigger)); + }); + + it("returns focus to the trigger once a cancelled Archive confirmation closes for a task with subtasks", async () => { + getSubtaskCountMock.mockResolvedValue({ count: 2 }); + vi.useFakeTimers(); + renderPanel(); + + const trigger = openMenu(); + fireEvent.click(screen.getByRole("menuitem", { name: "Archive" })); + await act(async () => { + vi.advanceTimersByTime(300); + }); + vi.useRealTimers(); + + // The subtask count resolves to >0, so this is the AlertDialog cascade + // branch (`shouldUseDialog`), not the popover. + const dialog = await screen.findByRole("alertdialog"); + fireEvent.click(within(dialog).getByRole("button", { name: "Cancel" })); + + await waitFor(() => expect(screen.queryByRole("alertdialog")).toBeNull()); + await waitFor(() => expect(document.activeElement).toBe(trigger)); + }); +}); + +describe("TaskPreviewPanel actions menu — terminal activation closes the menu (AC-TASKS-TASK-ACTIONS-MENU-004.2)", () => { + it("closes the dropdown menu itself as soon as a terminal entry (Archive) is activated", () => { + renderPanel(); + + openMenu(); + expectMenuOpen(true); + fireEvent.click(screen.getByRole("menuitem", { name: "Archive" })); + + expectMenuOpen(false); + }); + + it("closes the dropdown menu itself as soon as a terminal entry (Detach from parent) is activated", () => { + renderPanel(); + + openMenu(); + expectMenuOpen(true); + fireEvent.click(screen.getByRole("menuitem", { name: DETACH_MENU_ITEM_NAME })); + + expectMenuOpen(false); + }); +}); + +describe("TaskPreviewPanel actions menu — closes on Archive/Delete success (AC-TASKS-TASK-ACTIONS-MENU-003.3)", () => { + it("calls onClose once the Archive request resolves, independent of board/snapshot cache state", async () => { + vi.useFakeTimers(); + const onClose = vi.fn(); + renderPanel(); + + openMenu(); + fireEvent.click(screen.getByRole("menuitem", { name: "Archive" })); + await act(async () => { + vi.advanceTimersByTime(300); + }); + vi.useRealTimers(); + + const confirmation = await screen.findByTestId("task-archive-confirm-popover"); + fireEvent.click(within(confirmation).getByTestId("archive-task-confirm")); + + await waitFor(() => expect(archiveTaskMock).toHaveBeenCalledWith("task-1", { cascade: false })); + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + }); + + it("calls onClose once the Delete request resolves", async () => { + const onClose = vi.fn(); + renderPanel(); + + openMenu(); + fireEvent.click(screen.getByRole("menuitem", { name: "Delete" })); + + const dialog = await screen.findByRole("alertdialog"); + fireEvent.click(within(dialog).getByTestId("delete-discard-worktree-checkbox")); + fireEvent.click(within(dialog).getByRole("button", { name: "Delete" })); + + await waitFor(() => + expect(deleteTaskMock).toHaveBeenCalledWith("task-1", { + cascade: false, + discardWorktreeChanges: true, + }), + ); + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + }); + + it("does not call onClose when the Archive request rejects", async () => { + archiveTaskMock.mockRejectedValueOnce(new Error("network error")); + vi.useFakeTimers(); + const onClose = vi.fn(); + renderPanel(); + + openMenu(); + fireEvent.click(screen.getByRole("menuitem", { name: "Archive" })); + await act(async () => { + vi.advanceTimersByTime(300); + }); + vi.useRealTimers(); + + const confirmation = await screen.findByTestId("task-archive-confirm-popover"); + fireEvent.click(within(confirmation).getByTestId("archive-task-confirm")); + + await waitFor(() => expect(archiveTaskMock).toHaveBeenCalledTimes(1)); + expect(onClose).not.toHaveBeenCalled(); + }); +}); + +describe("TaskPreviewPanel actions menu — confirmation retargeting (AC-TASKS-TASK-ACTIONS-MENU-004.5a)", () => { + const OTHER_CHILD_TASK: Task = { + id: "task-3", + title: "Other child task", + workflowStepId: "step-1", + parentTaskId: "task-other-parent", + }; + + function clickDetach() { + openMenu(); + fireEvent.click(screen.getByRole("menuitem", { name: DETACH_MENU_ITEM_NAME })); + } + + it("drops a detach confirmation requested for a subject the panel has since swapped away from", async () => { + vi.useFakeTimers(); + const { rerender } = renderPanel(); + + clickDetach(); + + // Still inside the 300ms open-delay when the subject swaps out. + await act(async () => { + vi.advanceTimersByTime(100); + }); + rerenderPanel(rerender, ); + await act(async () => { + vi.advanceTimersByTime(300); + }); + + expect(screen.queryByTestId(DETACH_CONFIRM_POPOVER_TEST_ID)).toBeNull(); + }); + + it("closes an already-open detach confirmation when the subject swaps away from it", async () => { + vi.useFakeTimers(); + const { rerender } = renderPanel(); + + clickDetach(); + await act(async () => { + vi.advanceTimersByTime(300); + }); + expect(screen.getByTestId(DETACH_CONFIRM_POPOVER_TEST_ID)).toBeTruthy(); + + rerenderPanel(rerender, ); + + expect(screen.queryByTestId(DETACH_CONFIRM_POPOVER_TEST_ID)).toBeNull(); }); }); diff --git a/apps/web/components/task-preview-panel.tsx b/apps/web/components/task-preview-panel.tsx index 38a792b369..bb911e524b 100644 --- a/apps/web/components/task-preview-panel.tsx +++ b/apps/web/components/task-preview-panel.tsx @@ -1,5 +1,6 @@ "use client"; +import { useEffect, useRef, useState } from "react"; import { IconArrowsMaximize, IconX } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; import { useAppStore } from "@/components/state-provider"; @@ -8,6 +9,10 @@ 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 { TaskActionsMenuTrigger } from "./task/task-actions-menu-trigger"; +import { TaskActionsMenuDialogs } from "./task/task-actions-menu-dialogs"; +import { useTaskActionsMenu, type TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; +import { useTaskCRUD } from "@/hooks/use-task-crud"; import { useTranslation } from "react-i18next"; interface TaskPreviewPanelProps { @@ -25,6 +30,87 @@ interface TaskPreviewPanelProps { onMoveStep?: (stepId: string) => Promise; onDisclosureOpenChange?: (open: boolean) => void; moveError?: unknown; + /** Lets the enclosing surface skip its own Escape-close while this menu is open. */ + onActionsMenuOpenChange?: (open: boolean) => void; +} + +function buildBoardRow(task: Task | null): TaskActionsMenuBoardRow | null { + if (!task) return null; + return { + id: task.id, + title: task.title, + description: task.description, + workflowStepId: task.workflowStepId, + state: task.state, + priority: task.priority, + repositoryId: task.repositoryId, + repositories: task.repositories, + parentTaskId: task.parentTaskId, + primaryExecutorType: task.primaryExecutorType, + workspaceMode: task.workspaceMode, + }; +} + +/** + * Menu open state lives here (not inside the Radix primitive alone) so a + * subject-identity change can force it closed without retargeting + * (AC-TASKS-TASK-ACTIONS-MENU-004.5a), and so the panel's own Escape handler + * can tell whether an open menu already owns the keypress + * (AC-TASKS-TASK-ACTIONS-MENU-001.11). + */ +function useActionsMenuOpenState( + taskId: string | null, + onActionsMenuOpenChange?: (open: boolean) => void, +) { + const [open, setOpenState] = useState(false); + const setOpen = (next: boolean) => { + setOpenState(next); + onActionsMenuOpenChange?.(next); + }; + + const prevTaskIdRef = useRef(taskId); + useEffect(() => { + if (prevTaskIdRef.current !== taskId) { + prevTaskIdRef.current = taskId; + setOpen(false); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [taskId]); + + return { open, setOpen }; +} + +function usePreviewActionsMenu( + task: Task | null, + taskId: string | null, + taskTitle: string, + workspaceId: string | null, + onClose: () => void, +) { + const taskCRUD = useTaskCRUD(); + const boardRow = buildBoardRow(task); + const isArchiving = task ? taskCRUD.archivingTaskId === task.id : false; + const isDeleting = task ? taskCRUD.deletingTaskId === task.id : false; + + const menu = useTaskActionsMenu({ + taskId, + taskTitle, + workspaceId, + // The board excludes archived tasks, so the preview panel never holds one. + isArchived: false, + boardRow, + isArchiving, + isDeleting, + // `useTaskCRUD` only prunes `kanban.tasks`, so a task also present in + // `kanbanMulti.snapshots` (multi-workflow board view) would otherwise + // keep resolving via that snapshot until a WS lifecycle event catches + // up, leaving the preview open on an archived/deleted task. Close it + // directly on HTTP success instead of depending on cache completeness. + onArchive: (opts) => (task ? taskCRUD.handleArchive(task, opts).then(onClose) : undefined), + onDelete: (opts) => (task ? taskCRUD.handleDelete(task, opts).then(onClose) : undefined), + }); + + return { menu, boardRow, isArchiving, isDeleting }; } interface PreviewPanelHeaderProps { @@ -38,6 +124,10 @@ interface PreviewPanelHeaderProps { movingToStepId: string | null; onMoveStep?: (stepId: string) => Promise; onDisclosureOpenChange?: (open: boolean) => void; + menuEntries: ReturnType["entries"]; + triggerRef: ReturnType["triggerRef"]; + actionsMenuOpen: boolean; + setActionsMenuOpen: (open: boolean) => void; } function PreviewPanelHeader({ @@ -51,6 +141,10 @@ function PreviewPanelHeader({ movingToStepId, onMoveStep, onDisclosureOpenChange, + menuEntries, + triggerRef, + actionsMenuOpen, + setActionsMenuOpen, }: PreviewPanelHeaderProps) { const { t } = useTranslation(); const currentIndex = workflowSteps.findIndex((step) => step.id === currentStepId); @@ -88,6 +182,15 @@ function PreviewPanelHeader({ )}
+ {task && ( + + )} {onMaximize && task && (
+
); } diff --git a/apps/web/components/task/task-actions-menu-dialogs.test.tsx b/apps/web/components/task/task-actions-menu-dialogs.test.tsx new file mode 100644 index 0000000000..d30f23d629 --- /dev/null +++ b/apps/web/components/task/task-actions-menu-dialogs.test.tsx @@ -0,0 +1,210 @@ +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { StateProvider } from "@/components/state-provider"; +import { ToastProvider } from "@/components/toast-provider"; +import { useTaskActionsMenu, type TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; +import { TaskActionsMenuDialogs } from "./task-actions-menu-dialogs"; + +const capturedEditDialogProps: Array<{ open: boolean; focusReturnRef?: { current: unknown } }> = []; +type CapturedLinkDialogProps = { open: boolean; focusReturnRef?: { current: unknown } }; +const capturedLinkDialogProps: Record = { + pr: [], + issue: [], + mr: [], + external: [], +}; + +vi.mock("@/components/task-create-dialog", () => ({ + TaskCreateDialog: (props: { open: boolean; focusReturnRef?: { current: unknown } }) => { + capturedEditDialogProps.push({ open: props.open, focusReturnRef: props.focusReturnRef }); + return props.open ?
: null; + }, +})); + +vi.mock("@/components/task/task-github-pr-dialog", () => ({ + TaskGitHubPRDialog: (props: CapturedLinkDialogProps) => { + capturedLinkDialogProps.pr.push(props); + return props.open ?
: null; + }, +})); + +vi.mock("@/components/task/task-github-issue-dialog", () => ({ + TaskGitHubIssueDialog: (props: CapturedLinkDialogProps) => { + capturedLinkDialogProps.issue.push(props); + return props.open ?
: null; + }, +})); + +vi.mock("@/components/gitlab/task-mr-link-dialog", () => ({ + TaskMRLinkDialog: (props: CapturedLinkDialogProps) => { + capturedLinkDialogProps.mr.push(props); + return props.open ?
: null; + }, +})); + +vi.mock("@/components/task/task-external-link-dialog", () => ({ + TaskExternalLinkDialog: (props: CapturedLinkDialogProps) => { + capturedLinkDialogProps.external.push(props); + return props.open ?
: null; + }, +})); + +afterEach(() => { + cleanup(); + capturedEditDialogProps.length = 0; + for (const key of Object.keys(capturedLinkDialogProps)) capturedLinkDialogProps[key].length = 0; +}); + +const TASK_ID = "task-1"; + +const BOARD_ROW: TaskActionsMenuBoardRow = { + id: TASK_ID, + title: "Fix the sidebar", + workflowStepId: "step-1", + priority: "high", +}; + +function Harness({ boardRow }: { boardRow: TaskActionsMenuBoardRow | null }) { + const menu = useTaskActionsMenu({ + taskId: TASK_ID, + taskTitle: "Fix the sidebar", + workspaceId: "ws-1", + isArchived: false, + boardRow, + onArchive: () => undefined, + onDelete: () => undefined, + }); + return ( + <> + + + + + + + + {menu.entries.map((entry) => entry.key).join(",")} + + + ); +} + +function renderHarness(boardRow: TaskActionsMenuBoardRow | null) { + return render( + + + + + , + ); +} + +function rerenderHarness( + rerender: (ui: React.ReactNode) => void, + boardRow: TaskActionsMenuBoardRow | null, +) { + rerender( + + + + + , + ); +} + +describe("TaskActionsMenuDialogs — Edit dialog stays mounted through a board-row loss (AC-TASKS-TASK-ACTIONS-MENU-001.12)", () => { + it("opens the Edit dialog for a resolved board row", () => { + renderHarness(BOARD_ROW); + fireEvent.click(screen.getByTestId("open-edit")); + + expect(screen.getByTestId("edit-dialog-open")).toBeTruthy(); + }); + + it("renders no Edit dialog at all when the board row has never resolved", () => { + renderHarness(null); + + expect(capturedEditDialogProps).toHaveLength(0); + }); + + it("closes the Edit dialog via its own open prop, not an abrupt unmount, when the board row disappears while it is open", () => { + const { rerender } = renderHarness(BOARD_ROW); + fireEvent.click(screen.getByTestId("open-edit")); + expect(screen.getByTestId("edit-dialog-open")).toBeTruthy(); + capturedEditDialogProps.length = 0; + + rerenderHarness(rerender, null); + + // The dialog closed via `open` flipping to false in this same render, + // not by the wrapper disappearing from the tree: a real unmount would + // never give TaskCreateDialog the chance to run its own close-focus + // transition. + expect(screen.queryByTestId("edit-dialog-open")).toBeNull(); + expect(capturedEditDialogProps).toHaveLength(1); + expect(capturedEditDialogProps[0]?.open).toBe(false); + }); + + it("passes a focusReturnRef pointing at the trigger to the Edit dialog", () => { + renderHarness(BOARD_ROW); + fireEvent.click(screen.getByTestId("open-edit")); + + expect(capturedEditDialogProps.at(-1)?.focusReturnRef).toBeTruthy(); + }); +}); + +describe("TaskActionsMenuDialogs — card action parity", () => { + it("includes Priority for a resolved task", () => { + renderHarness(BOARD_ROW); + + expect(screen.getByTestId("menu-entry-keys").textContent?.split(",")).toContain("priority"); + }); +}); + +describe("TaskActionsMenuDialogs — Link dialogs get a focusReturnRef (AC-TASKS-TASK-ACTIONS-MENU-001.12)", () => { + it("passes a focusReturnRef pointing at the trigger to the GitHub PR dialog", () => { + renderHarness(BOARD_ROW); + fireEvent.click(screen.getByTestId("open-pr")); + + expect(screen.getByTestId("pr-dialog-open")).toBeTruthy(); + expect(capturedLinkDialogProps.pr.at(-1)?.focusReturnRef).toBeTruthy(); + }); + + it("passes a focusReturnRef pointing at the trigger to the GitHub Issue dialog", () => { + renderHarness(BOARD_ROW); + fireEvent.click(screen.getByTestId("open-issue")); + + expect(screen.getByTestId("issue-dialog-open")).toBeTruthy(); + expect(capturedLinkDialogProps.issue.at(-1)?.focusReturnRef).toBeTruthy(); + }); + + it("passes a focusReturnRef pointing at the trigger to the GitLab MR dialog", () => { + renderHarness(BOARD_ROW); + fireEvent.click(screen.getByTestId("open-mr")); + + expect(screen.getByTestId("mr-dialog-open")).toBeTruthy(); + expect(capturedLinkDialogProps.mr.at(-1)?.focusReturnRef).toBeTruthy(); + }); + + it("passes a focusReturnRef pointing at the trigger to the external link dialog", () => { + renderHarness(BOARD_ROW); + fireEvent.click(screen.getByTestId("open-external")); + + expect(screen.getByTestId("external-dialog-open")).toBeTruthy(); + expect(capturedLinkDialogProps.external.at(-1)?.focusReturnRef).toBeTruthy(); + }); +}); diff --git a/apps/web/components/task/task-actions-menu-dialogs.tsx b/apps/web/components/task/task-actions-menu-dialogs.tsx new file mode 100644 index 0000000000..83f01dcdcd --- /dev/null +++ b/apps/web/components/task/task-actions-menu-dialogs.tsx @@ -0,0 +1,229 @@ +"use client"; + +import { useRef, type RefObject } from "react"; +import { useActiveWorkspaceRepositories } from "@/components/kanban-card-repositories"; +import { TaskArchiveConfirmation } from "@/components/task/task-archive-confirmation"; +import { TaskCreateDialog } from "@/components/task-create-dialog"; +import { TaskDeleteConfirmDialog } from "@/components/task/task-delete-confirm-dialog"; +import { TaskDetachConfirmationSurface } from "@/components/task/task-detach-confirm-dialog"; +import { TaskExternalLinkDialog } from "@/components/task/task-external-link-dialog"; +import { TaskGitHubIssueDialog } from "@/components/task/task-github-issue-dialog"; +import { TaskGitHubPRDialog } from "@/components/task/task-github-pr-dialog"; +import { TaskMRLinkDialog } from "@/components/gitlab/task-mr-link-dialog"; +import type { Task } from "@/components/kanban-card"; +import type { useTaskActionsMenu, TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; +import { hydrateEditedTask } from "@/hooks/domains/kanban/use-kanban-actions"; +import { useAppStoreApi } from "@/components/state-provider"; + +type TaskActionsMenuState = ReturnType; + +function buildLinkDialogTask( + taskId: string, + taskTitle: string, + boardRow: TaskActionsMenuBoardRow | null, +): Task { + return { + id: taskId, + title: taskTitle, + workflowStepId: boardRow?.workflowStepId ?? "", + repositories: boardRow?.repositories, + }; +} + +type TaskActionsMenuDialogsProps = { + taskId: string | null; + taskTitle: string; + workspaceId: string | null; + boardRow: TaskActionsMenuBoardRow | null; + isArchiving?: boolean; + isDeleting?: boolean; + menu: TaskActionsMenuState; + /** The subject's own executor type, independent of `boardRow`: falls back + * to this for the Archive/Delete confirmations' executor-specific cleanup + * copy when the board row is unresolvable (e.g. an archived task). */ + subjectExecutorType?: string | null; +}; + +type TaskActionsMenuEditDialogProps = { + taskId: string; + taskTitle: string; + workspaceId: string | null; + boardRow: TaskActionsMenuBoardRow; + /** Whether the dialog should be visually open. Distinct from + * `menu.showEditDialog`: this also folds in board-row availability, so a + * board row lost mid-edit closes the dialog through its own close + * transition (and focus-return) rather than an abrupt unmount. */ + open: boolean; + focusReturnRef: RefObject; + menu: TaskActionsMenuState; +}; + +function TaskActionsMenuEditDialog({ + taskId, + taskTitle, + workspaceId, + boardRow, + open, + focusReturnRef, + menu, +}: TaskActionsMenuEditDialogProps) { + const store = useAppStoreApi(); + const editSteps = menu.stepsByWorkflowId[menu.currentWorkflowId ?? ""] ?? []; + + return ( + hydrateEditedTask(store, task, store.getState().kanban)} + /> + ); +} + +/** + * Tracks the most recent non-null board row seen for the current subject. + * Lets the Edit dialog stay mounted (and so run its own close-focus + * transition) when the row disappears mid-edit, instead of losing its data + * to an abrupt unmount the instant `boardRow` goes null. + */ +function useLastResolvedBoardRow( + taskId: string | null, + boardRow: TaskActionsMenuBoardRow | null, +): TaskActionsMenuBoardRow | null { + const ref = useRef<{ taskId: string | null; boardRow: TaskActionsMenuBoardRow | null }>({ + taskId, + boardRow, + }); + if (ref.current.taskId !== taskId || boardRow != null) { + ref.current = { taskId, boardRow }; + } + return ref.current.boardRow; +} + +/** + * Mounts every dialog a task actions menu entry can open. Hosted per surface + * so a dialog outlives the menu that opened it. + */ +export function TaskActionsMenuDialogs({ + taskId, + taskTitle, + workspaceId, + boardRow, + isArchiving, + isDeleting, + menu, + subjectExecutorType, +}: TaskActionsMenuDialogsProps) { + const repositories = useActiveWorkspaceRepositories(); + const editBoardRow = useLastResolvedBoardRow(taskId, boardRow); + if (!taskId) return null; + const linkDialogTask = buildLinkDialogTask(taskId, taskTitle, boardRow); + + return ( + <> + {editBoardRow && ( + + )} + menu.onConfirmArchive(values)} + /> + menu.onConfirmDelete(opts)} + focusReturnRef={menu.triggerRef} + /> + + + + {workspaceId && ( + + )} + {menu.externalLinkProvider && workspaceId && ( + { + if (!open) menu.setExternalLinkProvider(null); + }} + provider={menu.externalLinkProvider} + task={linkDialogTask} + workspaceId={workspaceId} + focusReturnRef={menu.triggerRef} + /> + )} + + ); +} diff --git a/apps/web/components/task/task-actions-menu-trigger.tsx b/apps/web/components/task/task-actions-menu-trigger.tsx new file mode 100644 index 0000000000..d60892f4b5 --- /dev/null +++ b/apps/web/components/task/task-actions-menu-trigger.tsx @@ -0,0 +1,65 @@ +"use client"; + +import type { RefObject } from "react"; +import { IconDots } from "@tabler/icons-react"; +import { useTranslation } from "react-i18next"; +import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from "@kandev/ui/dropdown-menu"; +import { + KanbanCardDropdownMenuItems, + type KanbanCardMenuEntry, +} from "@/components/kanban-card-menu-items"; + +type TaskActionsMenuTriggerProps = { + entries: KanbanCardMenuEntry[]; + testId: string; + triggerRef: RefObject; + open: boolean; + onOpenChange: (open: boolean) => void; +}; + +/** The "More options" dots trigger shared by the preview and detail surfaces. */ +export function TaskActionsMenuTrigger({ + entries, + testId, + triggerRef, + open, + onOpenChange, +}: TaskActionsMenuTriggerProps) { + const { t } = useTranslation(); + return ( + + + + + { + // Consume this Escape ourselves instead of letting Radix's default + // dismiss handle it: closing the menu via `onOpenChange` still + // triggers Radix's synchronous focus-restoration flush, but calling + // `preventDefault()` here (during document capture, before this + // same event reaches any bubble-phase `window` listener a host + // surface owns) lets that listener check `event.defaultPrevented` + // and know this keypress was already handled by the menu, rather + // than racing a state update that may not have re-rendered yet + // (AC-TASKS-TASK-ACTIONS-MENU-001.11). + event.preventDefault(); + onOpenChange(false); + }} + > + + + + ); +} diff --git a/apps/web/components/task/task-archive-confirm-dialog.tsx b/apps/web/components/task/task-archive-confirm-dialog.tsx index 82f9054b62..75972f097e 100644 --- a/apps/web/components/task/task-archive-confirm-dialog.tsx +++ b/apps/web/components/task/task-archive-confirm-dialog.tsx @@ -32,7 +32,12 @@ import { useTranslation } from "react-i18next"; type TaskArchiveConfirmDialogProps = { open: boolean; onOpenChange: (open: boolean) => void; + /** Element to return keyboard focus to on close, confirmed or cancelled + * (AC-TASKS-TASK-ACTIONS-MENU-001.12). Omitted callers keep Radix's + * default restore-to-previously-focused-element behavior. */ focusReturnRef?: RefObject; + /** Restore focus after confirmation when the task surface remains mounted. */ + restoreFocusOnConfirm?: boolean; taskTitle?: string; isBulkOperation?: boolean; count?: number; @@ -52,6 +57,10 @@ type TaskArchiveConfirmDialogProps = { type ArchiveOpenMode = "pending" | "confirm" | "bypass"; +function shouldRestoreFocus(confirmed: boolean, restoreFocusOnConfirm?: boolean): boolean { + return !confirmed || restoreFocusOnConfirm === true; +} + function useArchiveConfirmationMode( open: boolean, confirmTaskArchive: boolean, @@ -108,6 +117,7 @@ export function TaskArchiveConfirmDialog({ open, onOpenChange, focusReturnRef, + restoreFocusOnConfirm, taskTitle, isBulkOperation, count, @@ -137,7 +147,7 @@ export function TaskArchiveConfirmDialog({ const [cascade, setCascade] = useState(false); const confirmedRef = useRef(false); const restoreFocus = () => { - if (confirmedRef.current) return; + if (!shouldRestoreFocus(confirmedRef.current, restoreFocusOnConfirm)) return; const focusReturnTarget = focusReturnRef?.current; if (focusReturnTarget?.isConnected) focusReturnTarget.focus(); }; diff --git a/apps/web/components/task/task-archive-confirmation.test.tsx b/apps/web/components/task/task-archive-confirmation.test.tsx index ed5fc30aad..c240960d1c 100644 --- a/apps/web/components/task/task-archive-confirmation.test.tsx +++ b/apps/web/components/task/task-archive-confirmation.test.tsx @@ -309,3 +309,52 @@ describe("TaskArchiveConfirmation cleanup copy", () => { expect(screen.getByTestId(CLEANUP_NOTES_TEST_ID).tagName).toBe("SPAN"); }); }); + +describe("TaskArchiveConfirmation focus return", () => { + it("returns focus to the trigger when a confirmed archive fails", async () => { + pointerState.isFinePointer = false; + getSubtaskCountMock.mockResolvedValue({ count: 0 }); + const onConfirm = vi.fn(() => { + const failure = Promise.reject(new Error("archive failed")); + void failure.catch(() => undefined); + return failure; + }); + + function FocusHarness() { + const [open, setOpen] = useState(true); + const anchorRef = useRef(null); + return ( + <> + + + + ); + } + + render( + + + , + ); + + fireEvent.click(await screen.findByTestId(CONFIRM_TEST_ID)); + + await waitFor(() => expect(onConfirm).toHaveBeenCalledOnce()); + await waitFor(() => + expect(document.activeElement).toBe(screen.getByTestId("archive-focus-anchor")), + ); + }); +}); diff --git a/apps/web/components/task/task-archive-confirmation.tsx b/apps/web/components/task/task-archive-confirmation.tsx index c24fcc1b9b..b906544884 100644 --- a/apps/web/components/task/task-archive-confirmation.tsx +++ b/apps/web/components/task/task-archive-confirmation.tsx @@ -34,6 +34,7 @@ export type TaskArchiveConfirmationProps = { anchorRef: RefObject; focusReturnRef?: RefObject; focusBoundaryRef?: RefObject; + restoreFocusOnConfirm?: boolean; onConfirm: (values: ArchiveConfirmValues) => void | Promise; confirmTestId?: string; /** Render a simple confirmation inside an existing action surface. */ @@ -110,6 +111,7 @@ function ArchiveConfirmPopover({ anchorRef, focusReturnRef, focusBoundaryRef, + restoreFocusOnConfirm = false, taskTitle, executorType, isInFlight, @@ -122,6 +124,7 @@ function ArchiveConfirmPopover({ anchorRef: RefObject; focusReturnRef?: RefObject; focusBoundaryRef?: RefObject; + restoreFocusOnConfirm?: boolean; taskTitle?: string; executorType?: string | null; isInFlight: boolean; @@ -140,6 +143,7 @@ function ArchiveConfirmPopover({ anchorRef={anchorRef} focusReturnRef={focusReturnRef} focusBoundaryRef={focusBoundaryRef ?? anchorRef} + restoreFocusOnConfirm={restoreFocusOnConfirm} title={t("task:archiveTaskTitle")} description={ @@ -208,6 +212,7 @@ type ArchiveDialogProps = Pick< | "executorTypes" | "onConfirm" | "confirmTestId" + | "restoreFocusOnConfirm" > & { subtaskClassification: SubtaskCountResult; }; @@ -297,6 +302,7 @@ function ArchiveConfirmationContent({ anchorRef={anchorRef} focusReturnRef={focusReturnRef} focusBoundaryRef={focusBoundaryRef} + restoreFocusOnConfirm={dialogProps.restoreFocusOnConfirm} taskTitle={dialogProps.taskTitle} executorType={dialogProps.executorType} isInFlight={taskIsInFlight} @@ -323,6 +329,7 @@ export function TaskArchiveConfirmation({ anchorRef, focusReturnRef, focusBoundaryRef, + restoreFocusOnConfirm, onConfirm, confirmTestId = DEFAULT_CONFIRM_TEST_ID, inline = false, @@ -344,6 +351,7 @@ export function TaskArchiveConfirmation({ taskIsInFlight={taskIsInFlight} anchorRef={anchorRef} focusReturnRef={focusReturnRef} + restoreFocusOnConfirm={restoreFocusOnConfirm} focusBoundaryRef={focusBoundaryRef} inline={inline} onOpenChange={onOpenChange} diff --git a/apps/web/components/task/task-delete-confirm-dialog.tsx b/apps/web/components/task/task-delete-confirm-dialog.tsx index fb4cf6d236..012748b0ca 100644 --- a/apps/web/components/task/task-delete-confirm-dialog.tsx +++ b/apps/web/components/task/task-delete-confirm-dialog.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { useState, type RefObject } from "react"; import { IconLoader } from "@tabler/icons-react"; import { AlertDialog, @@ -50,6 +50,10 @@ type TaskDeleteConfirmDialogProps = { requireDiscardConsent?: boolean; onConfirm: (opts: { cascade: boolean; discardWorktreeChanges: boolean }) => void; confirmTestId?: string; + /** Element to return keyboard focus to on close, confirmed or cancelled + * (AC-TASKS-TASK-ACTIONS-MENU-001.12). Omitted callers keep Radix's + * default restore-to-previously-focused-element behavior. */ + focusReturnRef?: RefObject; }; type DiscardWorktreeChangesOptionProps = { @@ -254,6 +258,7 @@ export function TaskDeleteConfirmDialog({ requireDiscardConsent = false, onConfirm, confirmTestId, + focusReturnRef, }: TaskDeleteConfirmDialogProps) { const { t } = useTranslation(); const safeCount = count ?? 0; @@ -284,7 +289,17 @@ export function TaskDeleteConfirmDialog({ return ( - + { + const target = focusReturnRef?.current; + if (!target || !document.contains(target)) return; + event.preventDefault(); + target.focus(); + }} + > {title} diff --git a/apps/web/components/task/task-detach-confirm-dialog.test.tsx b/apps/web/components/task/task-detach-confirm-dialog.test.tsx index 0028c5e9a3..f2b289d220 100644 --- a/apps/web/components/task/task-detach-confirm-dialog.test.tsx +++ b/apps/web/components/task/task-detach-confirm-dialog.test.tsx @@ -21,6 +21,8 @@ function PopoverHarness({ onConfirm = vi.fn() }: { onConfirm?: () => void | Prom { expect(screen.queryByTestId("detach-task-inline-confirmation")).toBeNull(); }); + it("returns focus to the menu trigger after confirmation", async () => { + const onConfirm = vi.fn(); + render(); + + fireEvent.click(screen.getByTestId("detach-task-confirm")); + + await waitFor(() => expect(onConfirm).toHaveBeenCalledOnce()); + await waitFor(() => expect(document.activeElement).toBe(screen.getByTestId("detach-anchor"))); + }); + it("keeps task-switcher detachment on its existing modal branch", async () => { const longTaskTitle = `Child task ${"x".repeat(180)}`; render( diff --git a/apps/web/components/task/task-detach-confirm-dialog.tsx b/apps/web/components/task/task-detach-confirm-dialog.tsx index 9b5695e037..4c12eb17cf 100644 --- a/apps/web/components/task/task-detach-confirm-dialog.tsx +++ b/apps/web/components/task/task-detach-confirm-dialog.tsx @@ -74,6 +74,7 @@ export type TaskDetachConfirmPopoverProps = TaskDetachConfirmationCopyProps & { open: boolean; anchorRef: RefObject; focusReturnRef?: RefObject; + restoreFocusOnConfirm?: boolean; focusBoundaryRef?: RefObject; onOpenChange: (open: boolean) => void; onCancel?: () => void; @@ -84,6 +85,7 @@ export function TaskDetachConfirmPopover({ open, anchorRef, focusReturnRef, + restoreFocusOnConfirm = false, focusBoundaryRef, taskTitle, sharesParentWorkspace, @@ -97,6 +99,7 @@ export function TaskDetachConfirmPopover({ open={open} anchorRef={anchorRef} focusReturnRef={focusReturnRef} + restoreFocusOnConfirm={restoreFocusOnConfirm} focusBoundaryRef={focusBoundaryRef} title={t(DETACH_TASK_FROM_PARENT_KEY)} description={ @@ -157,6 +160,7 @@ export type TaskDetachConfirmationSurfaceProps = TaskDetachConfirmationCopyProps open: boolean; anchorRef: RefObject; focusReturnRef?: RefObject; + restoreFocusOnConfirm?: boolean; onOpenChange: (open: boolean) => void; onConfirm: () => void | Promise; }; @@ -165,6 +169,7 @@ export function TaskDetachConfirmationSurface({ open, anchorRef, focusReturnRef, + restoreFocusOnConfirm = false, taskTitle, sharesParentWorkspace, onOpenChange, @@ -177,6 +182,7 @@ export function TaskDetachConfirmationSurface({ open={open} anchorRef={anchorRef} focusReturnRef={focusReturnRef} + restoreFocusOnConfirm={restoreFocusOnConfirm} focusBoundaryRef={anchorRef} taskTitle={taskTitle} sharesParentWorkspace={sharesParentWorkspace} diff --git a/apps/web/components/task/task-external-link-dialog.tsx b/apps/web/components/task/task-external-link-dialog.tsx index 20c19d5844..1bd6ddf426 100644 --- a/apps/web/components/task/task-external-link-dialog.tsx +++ b/apps/web/components/task/task-external-link-dialog.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useState } from "react"; +import { useEffect, useState, type RefObject } from "react"; import { Button } from "@kandev/ui/button"; import { Dialog, @@ -23,6 +23,7 @@ import { JIRA_KEY_RE } from "@/components/jira/jira-ticket-common"; import { LINEAR_KEY_RE } from "@/components/linear/linear-issue-common"; import { extractSentryShortId } from "@/components/sentry/sentry-issue-common"; import { useSentryInstances } from "@/hooks/domains/sentry/use-sentry-availability"; +import { createFocusReturnHandler } from "@/lib/dialog-focus-return"; import { findTaskInSnapshots } from "@/lib/kanban/find-task"; import type { SentryIssue } from "@/lib/types/sentry"; import { buildLinkedIssueTitle } from "./task-external-link-utils"; @@ -41,6 +42,9 @@ type TaskExternalLinkDialogProps = { provider: ExternalLinkProvider; task: TaskExternalLinkTarget; workspaceId: string; + /** Element to return keyboard focus to on close (AC-TASKS-TASK-ACTIONS-MENU-001.12). + * Omitted callers keep Radix's default restore-to-previously-focused-element behavior. */ + focusReturnRef?: RefObject; }; /** Copy fields hold CATALOG KEYS, not English: `PROVIDERS` is a module-scope @@ -252,6 +256,7 @@ export function TaskExternalLinkDialog({ provider, task, workspaceId, + focusReturnRef, }: TaskExternalLinkDialogProps) { const { t } = useTranslation(); const config = PROVIDERS[provider]; @@ -260,7 +265,10 @@ export function TaskExternalLinkDialog({ return ( - + {t(config.titleKey)} {t(config.descriptionKey)} diff --git a/apps/web/components/task/task-github-issue-dialog.tsx b/apps/web/components/task/task-github-issue-dialog.tsx index 11d3f5580e..3cc8e7e4b5 100644 --- a/apps/web/components/task/task-github-issue-dialog.tsx +++ b/apps/web/components/task/task-github-issue-dialog.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useState, type RefObject } from "react"; import { Button } from "@kandev/ui/button"; import { Dialog, @@ -14,6 +14,7 @@ import { Input } from "@kandev/ui/input"; import { Label } from "@kandev/ui/label"; import { useToast } from "@/components/toast-provider"; import { linkTaskIssue, unlinkTaskIssue } from "@/lib/api/domains/github-api"; +import { createFocusReturnHandler } from "@/lib/dialog-focus-return"; import type { Repository } from "@/lib/types/http"; import { useTranslation } from "react-i18next"; @@ -40,6 +41,9 @@ type TaskGitHubIssueDialogProps = { onOpenChange: (open: boolean) => void; task: TaskIssue; repositories: Repository[]; + /** Element to return keyboard focus to on close (AC-TASKS-TASK-ACTIONS-MENU-001.12). + * Omitted callers keep Radix's default restore-to-previously-focused-element behavior. */ + focusReturnRef?: RefObject; }; type DialogFieldsProps = { @@ -163,6 +167,7 @@ export function TaskGitHubIssueDialog({ onOpenChange, task, repositories, + focusReturnRef, }: TaskGitHubIssueDialogProps) { const { t } = useTranslation(); const { toast } = useToast(); @@ -219,7 +224,10 @@ export function TaskGitHubIssueDialog({ return ( - + {currentLabel ? t("task:changeGithubIssue") : t("task:linkGithubIssue")} diff --git a/apps/web/components/task/task-github-pr-dialog.tsx b/apps/web/components/task/task-github-pr-dialog.tsx index 856e3d9418..2469ce3a1c 100644 --- a/apps/web/components/task/task-github-pr-dialog.tsx +++ b/apps/web/components/task/task-github-pr-dialog.tsx @@ -1,6 +1,6 @@ "use client"; -import { useMemo } from "react"; +import { useMemo, type RefObject } from "react"; import { Dialog, DialogContent, @@ -10,6 +10,7 @@ import { } from "@kandev/ui/dialog"; import { TaskChangeRequestLinkForm } from "@/components/integrations/task-change-request-link-form"; import { createTaskPR } from "@/lib/api/domains/github-api"; +import { createFocusReturnHandler } from "@/lib/dialog-focus-return"; import type { Repository } from "@/lib/types/http"; import { githubReposForTask, @@ -28,6 +29,9 @@ type TaskGitHubPRDialogProps = { onOpenChange: (open: boolean) => void; task: TaskPullRequestLinkTarget; repositories: Repository[]; + /** Element to return keyboard focus to on close (AC-TASKS-TASK-ACTIONS-MENU-001.12). + * Omitted callers keep Radix's default restore-to-previously-focused-element behavior. */ + focusReturnRef?: RefObject; }; export function TaskGitHubPRDialog({ @@ -36,6 +40,7 @@ export function TaskGitHubPRDialog({ onOpenChange, task, repositories, + focusReturnRef, }: TaskGitHubPRDialogProps) { const { t } = useTranslation(); const githubRepos = useMemo(() => githubReposForTask(task, repositories), [task, repositories]); @@ -59,7 +64,10 @@ export function TaskGitHubPRDialog({ return ( - + {t("task:linkGithubPullRequest")} diff --git a/apps/web/components/task/task-page-content-helpers-board-row.test.tsx b/apps/web/components/task/task-page-content-helpers-board-row.test.tsx new file mode 100644 index 0000000000..533c243018 --- /dev/null +++ b/apps/web/components/task/task-page-content-helpers-board-row.test.tsx @@ -0,0 +1,91 @@ +import { renderHook } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { StateProvider } from "@/components/state-provider"; +import type { Task } from "@/lib/types/http"; +import { useTaskActionsMenuBoardRow } from "./task-page-content-helpers"; + +const TASK_ID = "task-1"; + +function makeTask(overrides: Partial = {}): Task { + return { + id: TASK_ID, + title: "Fix the sidebar", + workspace_id: "ws-1", + workflow_id: "wf-1", + workflow_step_id: "step-1", + priority: "medium", + repositories: [], + created_at: "", + updated_at: "", + ...overrides, + } as Task; +} + +describe("useTaskActionsMenuBoardRow (AC-TASKS-TASK-ACTIONS-MENU-002.5)", () => { + it("resolves the board row from a workflow snapshot, not the page's own task record", () => { + function wrapper({ children }: { children: React.ReactNode }) { + return ( + + {children} + + ); + } + + const { result } = renderHook(() => useTaskActionsMenuBoardRow(makeTask()), { wrapper }); + + expect(result.current).toEqual( + expect.objectContaining({ + id: TASK_ID, + title: "Fix the sidebar (live)", + workflowStepId: "step-1", + parentTaskId: "task-parent", + priority: "medium", + }), + ); + }); + + it("returns null (the identifier-only tier) for a task with no board row anywhere on the board", () => { + function wrapper({ children }: { children: React.ReactNode }) { + return {children}; + } + + const { result } = renderHook(() => useTaskActionsMenuBoardRow(makeTask()), { wrapper }); + + expect(result.current).toBeNull(); + }); + + it("returns null when there is no subject task at all", () => { + function wrapper({ children }: { children: React.ReactNode }) { + return {children}; + } + + const { result } = renderHook(() => useTaskActionsMenuBoardRow(null), { wrapper }); + + expect(result.current).toBeNull(); + }); +}); diff --git a/apps/web/components/task/task-page-content-helpers.ts b/apps/web/components/task/task-page-content-helpers.ts index 48d0169d37..70c61c175e 100644 --- a/apps/web/components/task/task-page-content-helpers.ts +++ b/apps/web/components/task/task-page-content-helpers.ts @@ -1,3 +1,4 @@ +import { useMemo } from "react"; import { taskId as toTaskId, workflowId as toWorkflowId, @@ -8,6 +9,9 @@ import { import type { KanbanState } from "@/lib/state/slices"; import { issueFieldsFromMetadata } from "@/lib/metadata-utils"; import { repositorySlug } from "@/lib/repository-slug"; +import type { TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; +import { useAppStore } from "@/components/state-provider"; +import { findTaskInSnapshots } from "@/lib/kanban/find-task"; const EMPTY_REPOSITORIES: Repository[] = []; @@ -235,6 +239,7 @@ export function resolveTaskIds(task: Task | null) { workspaceId: taskValues.workspace_id ?? null, projectId: taskValues.project_id ?? null, workflowStepId: taskValues.workflow_step_id ?? null, + primaryExecutorType: taskValues.primary_executor_type ?? null, baseBranch: primaryRepository?.base_branch, pullRequestTarget: primaryRepository?.branch_policy_pull_request_target || undefined, isArchived: !!taskValues.archived_at, @@ -262,6 +267,41 @@ function buildPullRequestTargetsByRepository( return targets; } +/** + * The detail top bar's actions-menu subject: a live board-row lookup against + * the workflow snapshots with the flat task list as fallback (the same lookup + * `useSidebarTaskEdit` performs), NOT the detail page's own task record. The + * page's own record stays loaded on tasks the board has pruned (e.g. after a + * peer archives/deletes it, or for a task that was never on any board, such + * as an Office-managed task), so using it directly would make the + * "unresolved-row" identifier-only tier (AC-TASKS-TASK-ACTIONS-MENU-002.5) + * unreachable. Returns null when the subject genuinely has no board row. + */ +export function useTaskActionsMenuBoardRow(task: Task | null): TaskActionsMenuBoardRow | null { + const taskId = task?.id ?? null; + const taskPriority = task?.priority; + const kanbanTasks = useAppStore((state) => state.kanban.tasks); + const snapshots = useAppStore((state) => state.kanbanMulti.snapshots); + return useMemo(() => { + if (!taskId) return null; + const boardTask = findTaskInSnapshots(taskId, snapshots, kanbanTasks); + if (!boardTask) return null; + return { + id: boardTask.id, + title: boardTask.title, + description: boardTask.description, + workflowStepId: boardTask.workflowStepId, + state: boardTask.state, + priority: boardTask.priority ?? taskPriority, + repositoryId: boardTask.repositoryId, + repositories: boardTask.repositories, + parentTaskId: boardTask.parentTaskId, + primaryExecutorType: boardTask.primaryExecutorType, + workspaceMode: boardTask.workspaceMode, + }; + }, [kanbanTasks, snapshots, taskId, taskPriority]); +} + export function resolveTaskPullRequestProps( task: Pick | null, repositories: Repository[] = [], diff --git a/apps/web/components/task/task-page-inner.tsx b/apps/web/components/task/task-page-inner.tsx index 3448150b4c..761ded62e5 100644 --- a/apps/web/components/task/task-page-inner.tsx +++ b/apps/web/components/task/task-page-inner.tsx @@ -27,6 +27,7 @@ import { buildDebugEntries, buildArchivedValue, resolveTaskProps, + useTaskActionsMenuBoardRow, selectWorkspaceRepositories, } from "@/components/task/task-page-content-helpers"; import type { useSessionResumption } from "@/hooks/domains/session/use-session-resumption"; @@ -104,6 +105,7 @@ function resolveCurrentStepId( function buildTaskTopBarProps(params: { taskProps: ReturnType; + actionsMenuBoardRow: ReturnType; workflowSteps: ReturnType; showDebugOverlay: boolean; onToggleDebugOverlay: () => void; @@ -134,6 +136,13 @@ function buildTaskTopBarProps(params: { remoteExecutorType: params.remote.remoteExecutorType, officeTaskHref: params.officeTaskHref, onTaskUnarchived: params.onTaskUnarchived, + actionsMenuBoardRow: params.actionsMenuBoardRow, + // The subject's own last-known values, independent of `actionsMenuBoardRow`: + // the board excludes archived (and can lag/miss cross-workflow) tasks, so + // these stay available for the actions menu's plugin context and + // executor-aware confirmation copy even when the board row is unresolvable. + subjectWorkflowStepId: taskProps.workflowStepId, + subjectPrimaryExecutorType: taskProps.primaryExecutorType, }; } @@ -243,6 +252,7 @@ function useTaskPageDerivedProps({ selectWorkspaceRepositories(state.repositories.itemsByWorkspaceId, task?.workspace_id), ); const taskProps = resolveTaskProps(task, repository, workspaceRepositories); + const actionsMenuBoardRow = useTaskActionsMenuBoardRow(task); const remote = resolveRemoteExecutor(resumption.sessionStatus as RemoteExecutorStatus | null); const embeddedVscode = useEmbeddedVscodeSupport(effectiveSessionId, resumption.sessionStatus); const activeSessionMetadata = useAppStore((state) => @@ -261,6 +271,7 @@ function useTaskPageDerivedProps({ }); const topBarProps = buildTaskTopBarProps({ taskProps, + actionsMenuBoardRow, workflowSteps, showDebugOverlay, onToggleDebugOverlay, diff --git a/apps/web/components/task/task-top-bar-actions-menu.test.tsx b/apps/web/components/task/task-top-bar-actions-menu.test.tsx new file mode 100644 index 0000000000..3de313c7ea --- /dev/null +++ b/apps/web/components/task/task-top-bar-actions-menu.test.tsx @@ -0,0 +1,486 @@ +import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { StoreApi } from "zustand"; +import { StateProvider, useAppStoreApi } from "@/components/state-provider"; +import { ToastProvider } from "@/components/toast-provider"; +import { pluginRegistry } from "@/lib/plugins/registry"; +import type { AppState } from "@/lib/state/store"; +import { TaskTopBarActionsMenu } from "./task-top-bar-actions-menu"; +import type { TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; + +type WsHandler = (message: { payload: Record }) => void; +const wsHandlers = new Map>(); + +function emitWs(type: string, payload: Record) { + const handlers = wsHandlers.get(type); + if (!handlers) return; + for (const handler of handlers) handler({ payload }); +} + +const mockClient = { + subscribe: vi.fn(() => vi.fn()), + on: vi.fn((type: string, handler: WsHandler) => { + const handlers = wsHandlers.get(type) ?? new Set(); + handlers.add(handler); + wsHandlers.set(type, handlers); + return () => { + handlers.delete(handler); + }; + }), +}; +vi.mock("@/lib/ws/connection", () => ({ + getWebSocketClient: () => mockClient, +})); + +afterEach(() => { + cleanup(); + mockClient.subscribe.mockClear(); + mockClient.on.mockClear(); + wsHandlers.clear(); +}); + +const TASK_ID = "task-1"; +const OLD_WORKFLOW_ID = "wf-old"; +const NEW_WORKFLOW_ID = "wf-new"; +const TRIGGER_TEST_ID = "task-topbar-actions-menu"; +const MOVE_TO_TEST_ID = "task-context-move-to"; + +function initialStateWithTaskInActiveKanban(): Partial { + return { + kanban: { + workflowId: OLD_WORKFLOW_ID, + tasks: [ + { + id: TASK_ID, + workflowId: OLD_WORKFLOW_ID, + workflowStepId: "step-a", + title: "Task 1", + position: 0, + }, + ], + }, + } as never; +} + +function initialStateWithMoveTargets(): Partial { + return { + kanban: { + workflowId: OLD_WORKFLOW_ID, + tasks: [ + { + id: TASK_ID, + workflowId: OLD_WORKFLOW_ID, + workflowStepId: "step-a", + title: "Task 1", + position: 0, + }, + ], + }, + kanbanMulti: { + snapshots: { + [OLD_WORKFLOW_ID]: { + workflowId: OLD_WORKFLOW_ID, + workflowName: "Workflow", + steps: [ + { id: "step-a", title: "Step A", position: 0 }, + { id: "step-b", title: "Step B", position: 1 }, + ], + tasks: [ + { + id: TASK_ID, + workflowId: OLD_WORKFLOW_ID, + workflowStepId: "step-a", + title: "Task 1", + position: 0, + }, + ], + }, + }, + }, + workflows: { + items: [{ id: OLD_WORKFLOW_ID, name: "Workflow", workspaceId: "ws-1", hidden: false }], + }, + } as never; +} + +const RESOLVED_BOARD_ROW: TaskActionsMenuBoardRow = { + id: TASK_ID, + title: "Task 1", + workflowStepId: "step-a", +}; + +let capturedStore: StoreApi | null = null; + +function StoreCapture() { + capturedStore = useAppStoreApi(); + return null; +} + +function renderMenu(initialState: Partial) { + capturedStore = null; + render( + + + + + + , + ); + return () => capturedStore!; +} + +function renderMenuWithBoardRow( + initialState: Partial, + boardRow: TaskActionsMenuBoardRow | null, +) { + capturedStore = null; + const { rerender } = render( + + + + + + , + ); + return { + getStore: () => capturedStore!, + setBoardRow: (next: TaskActionsMenuBoardRow | null) => + rerender( + + + + + + , + ), + }; +} + +function openMenu(trigger: HTMLElement) { + act(() => { + trigger.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true })); + trigger.click(); + }); +} + +function expectMenuOpen(trigger: HTMLElement, open: boolean) { + expect(trigger.getAttribute("aria-expanded")).toBe(String(open)); +} + +describe("TaskTopBarActionsMenu — WS subscription lifetime", () => { + it("subscribes to the subject task's own WS updates for the lifetime of the trigger", () => { + const { unmount } = render( + + + + + , + ); + + expect(mockClient.subscribe).toHaveBeenCalledWith(TASK_ID); + const unsubscribe = mockClient.subscribe.mock.results[0]?.value as ReturnType; + + unmount(); + + expect(unsubscribe).toHaveBeenCalledTimes(1); + }); + + it("keeps the trigger mounted when a same-message workflow move lands the task in a workflow with no prior snapshot", () => { + const getStore = renderMenu(initialStateWithTaskInActiveKanban()); + expect(screen.getByTestId(TRIGGER_TEST_ID)).toBeTruthy(); + + // A single task.updated for a cross-workflow move removes the task from + // the old workflow's collections and adds it to the new one's snapshot + // in one state transition (mirroring applyTaskUpdatedCache), so the + // subject is never simultaneously absent from both. + act(() => { + getStore().setState((state) => ({ + kanban: { ...state.kanban, tasks: [] }, + kanbanMulti: { + ...state.kanbanMulti, + snapshots: { + ...state.kanbanMulti.snapshots, + [NEW_WORKFLOW_ID]: { + workflowId: NEW_WORKFLOW_ID, + workflowName: "New workflow", + steps: [], + tasks: [ + { + id: TASK_ID, + workflowId: NEW_WORKFLOW_ID, + workflowStepId: "step-b", + title: "Task 1", + position: 0, + }, + ], + }, + }, + }, + })); + }); + + expect(screen.getByTestId(TRIGGER_TEST_ID)).toBeTruthy(); + }); +}); + +describe("TaskTopBarActionsMenu — live demotion on board-row loss (AC-TASKS-TASK-ACTIONS-MENU-002.6/004.1c)", () => { + it("demotes Move to out of the menu in place, keeping the top-level menu open, when the board row is lost while the menu is open", () => { + const { setBoardRow } = renderMenuWithBoardRow( + initialStateWithMoveTargets(), + RESOLVED_BOARD_ROW, + ); + const trigger = screen.getByTestId(TRIGGER_TEST_ID); + + openMenu(trigger); + expectMenuOpen(trigger, true); + expect(screen.getByTestId(MOVE_TO_TEST_ID)).toBeTruthy(); + + act(() => { + setBoardRow(null); + }); + + expectMenuOpen(trigger, true); + expect(screen.queryByTestId(MOVE_TO_TEST_ID)).toBeNull(); + }); + + it("re-promotes Move to back into the menu in place when the board row resolves again while the menu stays open", () => { + const { setBoardRow } = renderMenuWithBoardRow(initialStateWithMoveTargets(), null); + const trigger = screen.getByTestId(TRIGGER_TEST_ID); + + openMenu(trigger); + expectMenuOpen(trigger, true); + expect(screen.queryByTestId(MOVE_TO_TEST_ID)).toBeNull(); + + act(() => { + setBoardRow(RESOLVED_BOARD_ROW); + }); + + expectMenuOpen(trigger, true); + expect(screen.getByTestId(MOVE_TO_TEST_ID)).toBeTruthy(); + }); +}); + +describe("TaskTopBarActionsMenu — closes on genuine removal (AC-TASKS-TASK-ACTIONS-MENU-004.5)", () => { + it("closes the trigger's open menu when task.deleted arrives for this task", () => { + renderMenu(initialStateWithTaskInActiveKanban()); + const trigger = screen.getByTestId(TRIGGER_TEST_ID); + + openMenu(trigger); + expectMenuOpen(trigger, true); + + act(() => { + emitWs("task.deleted", { task_id: TASK_ID }); + }); + + expectMenuOpen(trigger, false); + }); + + it("closes the trigger's open menu when task.updated arrives with archived_at set for this task", () => { + renderMenu(initialStateWithTaskInActiveKanban()); + const trigger = screen.getByTestId(TRIGGER_TEST_ID); + + openMenu(trigger); + expectMenuOpen(trigger, true); + + act(() => { + emitWs("task.updated", { task_id: TASK_ID, archived_at: "2026-09-03T00:00:00Z" }); + }); + + expectMenuOpen(trigger, false); + }); + + it("ignores task.deleted for a different task id", () => { + renderMenu(initialStateWithTaskInActiveKanban()); + const trigger = screen.getByTestId(TRIGGER_TEST_ID); + + openMenu(trigger); + expectMenuOpen(trigger, true); + + act(() => { + emitWs("task.deleted", { task_id: "some-other-task" }); + }); + + expectMenuOpen(trigger, true); + }); + + it("ignores task.updated for this task when archived_at is absent (an ordinary field update)", () => { + renderMenu(initialStateWithTaskInActiveKanban()); + const trigger = screen.getByTestId(TRIGGER_TEST_ID); + + openMenu(trigger); + expectMenuOpen(trigger, true); + + act(() => { + emitWs("task.updated", { task_id: TASK_ID, title: "Renamed" }); + }); + + expectMenuOpen(trigger, true); + }); +}); + +describe("TaskTopBarActionsMenu — board-row loss alone does not close the menu (AC-TASKS-TASK-ACTIONS-MENU-002.6/004.1c)", () => { + it("stays open and only demotes entries when the board row disappears from the store without a task.deleted/archived lifecycle event", () => { + const { getStore, setBoardRow } = renderMenuWithBoardRow( + initialStateWithMoveTargets(), + RESOLVED_BOARD_ROW, + ); + const trigger = screen.getByTestId(TRIGGER_TEST_ID); + + openMenu(trigger); + expectMenuOpen(trigger, true); + expect(screen.getByTestId(MOVE_TO_TEST_ID)).toBeTruthy(); + + // Mirrors what a real board-row loss looks like: the task's row vanishes + // from the store's collections (e.g. `useTaskActionsMenuBoardRow` can no + // longer resolve it) and the parent recomputes `boardRow` as null -- but + // no task.deleted/archived_at lifecycle event ever arrives, because the + // task itself is still alive. This is the exact state combination the + // old `existsInBoard`-based implementation could not tell apart from a + // genuine removal. + act(() => { + getStore().setState((state) => ({ + kanban: { ...state.kanban, tasks: [] }, + kanbanMulti: { + ...state.kanbanMulti, + snapshots: { + ...state.kanbanMulti.snapshots, + [OLD_WORKFLOW_ID]: { + ...state.kanbanMulti.snapshots[OLD_WORKFLOW_ID], + tasks: [], + }, + }, + }, + })); + setBoardRow(null); + }); + + expectMenuOpen(trigger, true); + expect(screen.queryByTestId(MOVE_TO_TEST_ID)).toBeNull(); + }); +}); + +describe("TaskTopBarActionsMenu — plugin context stays independent of the board row (AC-TASKS-TASK-ACTIONS-MENU-002.4b)", () => { + const PLUGIN_ID = "test-plugin-subject-workflow-step"; + + afterEach(() => pluginRegistry.unregisterPlugin(PLUGIN_ID)); + + it("passes the subject's own workflowStepId to a plugin primary action for an archived subject with no board row", () => { + const visible = vi.fn(() => true); + pluginRegistry.forPlugin(PLUGIN_ID).registerTaskMenuAction({ + id: "primary-action", + group: "primary", + label: "Plugin primary action", + visible, + run: vi.fn(), + }); + + render( + + + + + , + ); + + expect(visible).toHaveBeenCalledWith( + expect.objectContaining({ workflowStepId: "step-last-known" }), + ); + }); + + it("falls back to the board row's workflowStepId when no independent subject value is supplied", () => { + const visible = vi.fn(() => true); + pluginRegistry.forPlugin(PLUGIN_ID).registerTaskMenuAction({ + id: "primary-action", + group: "primary", + label: "Plugin primary action", + visible, + run: vi.fn(), + }); + + render( + + + + + , + ); + + expect(visible).toHaveBeenCalledWith(expect.objectContaining({ workflowStepId: "step-a" })); + }); +}); + +describe("TaskTopBarActionsMenu — Delete confirmation keeps executor-specific cleanup copy without a board row", () => { + it("shows the worktree-specific cleanup effect for an archived subject using the subject's own executor type", async () => { + render( + + + + + , + ); + + const trigger = screen.getByTestId(TRIGGER_TEST_ID); + openMenu(trigger); + (await screen.findByRole("menuitem", { name: "Delete" })).click(); + + const dialog = await screen.findByRole("alertdialog"); + expect(dialog.textContent).toContain("The task's git worktree and its branch will be deleted."); + }); +}); + +describe("TaskTopBarActionsMenu — Escape closes and returns focus (AC-TASKS-TASK-ACTIONS-MENU-001.9)", () => { + it("closes the menu and returns focus to the trigger on Escape", async () => { + renderMenu(initialStateWithTaskInActiveKanban()); + const trigger = screen.getByTestId(TRIGGER_TEST_ID); + + openMenu(trigger); + expectMenuOpen(trigger, true); + + const menu = await screen.findByRole("menu"); + act(() => { + menu.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); + }); + + expectMenuOpen(trigger, false); + await waitFor(() => expect(document.activeElement).toBe(trigger)); + }); +}); diff --git a/apps/web/components/task/task-top-bar-actions-menu.tsx b/apps/web/components/task/task-top-bar-actions-menu.tsx new file mode 100644 index 0000000000..a9c0ded7f5 --- /dev/null +++ b/apps/web/components/task/task-top-bar-actions-menu.tsx @@ -0,0 +1,173 @@ +"use client"; + +import { useCallback, useEffect, useState } from "react"; +import { getWebSocketClient } from "@/lib/ws/connection"; +import { TaskActionsMenuTrigger } from "@/components/task/task-actions-menu-trigger"; +import { TaskActionsMenuDialogs } from "@/components/task/task-actions-menu-dialogs"; +import { useTaskActionsMenu, type TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; +import { useArchiveAndSwitchTask, useDeleteAndSwitchTask } from "@/hooks/use-task-actions"; + +/** + * The detail surface has no board of its own to lean on, so a workflow move + * (Move to / Send to workflow) landing the subject in a workflow this client + * has never tracked would otherwise leave it absent from every board + * collection even though it still exists. Subscribing to the task directly + * guarantees its `task.updated` reaches this client regardless of workflow + * tracking, and the resulting cache update (removal from the old workflow, + * placeholder-snapshot insertion into the new one) lands in the same state + * transition, so `existsInBoard` below never observes a false gap for it. + */ +function useSubscribeToTask(taskId: string | null) { + useEffect(() => { + if (!taskId) return; + const client = getWebSocketClient(); + if (!client) return; + const unsubscribe = client.subscribe(taskId); + return () => { + unsubscribe(); + }; + }, [taskId]); +} + +/** + * Closes the menu only when the subject is genuinely gone -- deleted, or + * archived by another actor (AC-TASKS-TASK-ACTIONS-MENU-004.5). This + * deliberately listens for the task's own lifecycle events rather than + * inferring removal from board-collection presence: a task's board row can + * also disappear while the task is still alive (e.g. a move to a workflow + * this client has not tracked as a snapshot yet), which is the unresolved-row + * case AC-TASKS-TASK-ACTIONS-MENU-002.6/004.1c governs and must demote + * entries in place rather than close the menu. Board presence and "genuinely + * gone" happen to coincide for an ordinary delete/archive, but they are not + * the same predicate, so this hook never reads board state at all. Also + * resets any pending confirm/link dialog: a subject that is gone must not + * leave a stale, still-actionable confirmation open + * (AC-TASKS-TASK-ACTIONS-MENU-004.5a's no-retarget rule applies here too). + */ +function useCloseMenuOnTaskGone( + taskId: string | null, + setOpen: (open: boolean) => void, + closeDialogs: () => void, +) { + useSubscribeToTask(taskId); + useEffect(() => { + if (!taskId) return; + const client = getWebSocketClient(); + if (!client) return; + const close = () => { + setOpen(false); + closeDialogs(); + }; + const unsubscribeDeleted = client.on("task.deleted", (message) => { + if (message.payload.task_id === taskId) close(); + }); + const unsubscribeUpdated = client.on("task.updated", (message) => { + if (message.payload.task_id === taskId && message.payload.archived_at) close(); + }); + return () => { + unsubscribeDeleted(); + unsubscribeUpdated(); + }; + }, [taskId, setOpen, closeDialogs]); +} + +type TaskTopBarActionsMenuProps = { + /** The subject's own identifier/title, independent of `boardRow`: known as + * soon as the detail page has loaded the task, even when the board row is + * unresolvable, so the identifier-only tier (AC-TASKS-TASK-ACTIONS-MENU-002.5) + * stays reachable instead of the trigger disappearing entirely. */ + taskId: string | null; + taskTitle: string; + boardRow: TaskActionsMenuBoardRow | null; + workspaceId: string | null; + isArchived?: boolean; + /** The subject's own workflow step and executor type, independent of + * `boardRow`: the board excludes archived tasks and can lag/miss a + * cross-workflow row, but plugin context and the delete confirmation's + * executor-specific cleanup copy should not go stale just because the + * board row is momentarily or permanently unresolvable. */ + subjectWorkflowStepId?: string | null; + subjectPrimaryExecutorType?: string | null; +}; + +/** Tracks in-flight state for a one-shot switch-after-action call, since + * `useArchiveAndSwitchTask`/`useDeleteAndSwitchTask` carry no pending flag of + * their own (unlike `useTaskCRUD`, which the board and preview surfaces use). */ +function useTrackedSwitchAfterAction( + run: (taskId: string, opts?: { cascade?: boolean }) => Promise, +) { + const [pending, setPending] = useState(false); + const invoke = useCallback( + async (taskId: string, opts: { cascade: boolean }) => { + setPending(true); + try { + await run(taskId, opts); + } finally { + setPending(false); + } + }, + [run], + ); + return { pending, invoke }; +} + +/** + * Detail top bar's "More options" trigger. Unlike the board and preview + * surfaces, the detail surface has no board subscription to prune + * optimistically, so a confirmed Archive or Delete uses the same + * archive-and-switch / switch-to-next-task outcome as every other + * detail-surface entry point (AC-TASKS-TASK-ACTIONS-MENU-003.4/003.5). + */ +export function TaskTopBarActionsMenu({ + taskId, + taskTitle, + boardRow, + workspaceId, + isArchived, + subjectWorkflowStepId, + subjectPrimaryExecutorType, +}: TaskTopBarActionsMenuProps) { + const [open, setOpen] = useState(false); + const archiveAndSwitch = useArchiveAndSwitchTask(); + const deleteAndSwitch = useDeleteAndSwitchTask(); + const archiving = useTrackedSwitchAfterAction(archiveAndSwitch); + const deleting = useTrackedSwitchAfterAction(deleteAndSwitch); + + const menu = useTaskActionsMenu({ + taskId, + taskTitle, + workspaceId, + isArchived: Boolean(isArchived), + boardRow, + isArchiving: archiving.pending, + isDeleting: deleting.pending, + onArchive: (opts) => (taskId ? archiving.invoke(taskId, opts) : undefined), + onDelete: (opts) => (taskId ? deleting.invoke(taskId, opts) : undefined), + subjectWorkflowStepId, + }); + useCloseMenuOnTaskGone(taskId, setOpen, menu.closeDialogs); + + if (!taskId) return null; + + return ( + <> + + + + ); +} diff --git a/apps/web/components/task/task-top-bar.test.tsx b/apps/web/components/task/task-top-bar.test.tsx index 39e0b4294f..d2993ad501 100644 --- a/apps/web/components/task/task-top-bar.test.tsx +++ b/apps/web/components/task/task-top-bar.test.tsx @@ -1,8 +1,11 @@ -import { cleanup, render, screen, within } from "@testing-library/react"; +import { useEffect } from "react"; +import { act, cleanup, fireEvent, render, screen, within } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { StateProvider } from "@/components/state-provider"; +import { StateProvider, useAppStoreApi } from "@/components/state-provider"; import { ToastProvider } from "@/components/toast-provider"; +import { defaultState } from "@/lib/state/default-state"; import { TaskTopBar } from "./task-top-bar"; +import type { TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; afterEach(() => cleanup()); @@ -165,7 +168,7 @@ describe("TaskTopBar repository crumb", () => { it("names the task's repository so an open task says which project it belongs to", () => { renderTopBar( - , + , ); const crumb = repositoryCrumb("kdlbs/kandev"); @@ -177,7 +180,7 @@ describe("TaskTopBar repository crumb", () => { }); it("renders no repository crumb for a task with no repository", () => { - renderTopBar(); + renderTopBar(); // Any static crumb, not just a slug-shaped one: a repository with no // provider falls back to a bare name like "scratchpad", which a @@ -202,3 +205,155 @@ function renderTopBar(ui: React.ReactNode) { , ); } + +const TASK_TITLE = "Fix the sidebar"; +const TRIGGER_TEST_ID = "task-topbar-actions-menu"; + +const NORMAL_BOARD_ROW: TaskActionsMenuBoardRow = { + id: "task-1", + title: TASK_TITLE, + workflowStepId: "step-1", +}; + +function getTrigger() { + return screen.getByTestId(TRIGGER_TEST_ID); +} + +function openMenu() { + const trigger = getTrigger(); + fireEvent.pointerDown(trigger, { button: 0, pointerId: 1 }); + fireEvent.click(trigger); + return trigger; +} + +function expectMenuOpen(open: boolean) { + expect(getTrigger().getAttribute("aria-expanded")).toBe(String(open)); +} + +describe("TaskTopBar actions menu trigger", () => { + it("renders no trigger when the top bar has no subject task", () => { + renderTopBar(); + + expect(screen.queryByTestId(TRIGGER_TEST_ID)).toBeNull(); + }); + + it("renders the trigger last in the right control group, with the More options accessible name", () => { + renderTopBar( + , + ); + + const trigger = getTrigger(); + expect(trigger.getAttribute("aria-label")).toBe("More options"); + expect(trigger.getAttribute("aria-haspopup")).toBe("menu"); + expectMenuOpen(false); + + // Positioned after every other control already rendered in the group + // (AC-TASKS-TASK-ACTIONS-MENU-001.2): every earlier control precedes it + // in document order. + const controls = screen.getAllByRole("button"); + expect(controls.at(-1)).toBe(trigger); + }); + + it("opens a menu presenting Edit, Archive, and Delete for a normal (non-archived, resolvable) subject", () => { + renderTopBar( + , + ); + + openMenu(); + + expect(screen.getByRole("menuitem", { name: "Edit" })).toBeTruthy(); + expect(screen.getByRole("menuitem", { name: "Archive" })).toBeTruthy(); + expect(screen.getByRole("menuitem", { name: "Delete" })).toBeTruthy(); + }); + + it("renders the trigger and identifier-only tier (Archive/Delete only) when the board row can't be resolved (AC-TASKS-TASK-ACTIONS-MENU-002.5)", () => { + renderTopBar(); + + expect(screen.queryByTestId(TRIGGER_TEST_ID)).toBeTruthy(); + openMenu(); + + expect(screen.getByRole("menuitem", { name: "Archive" })).toBeTruthy(); + expect(screen.getByRole("menuitem", { name: "Delete" })).toBeTruthy(); + expect(screen.queryByRole("menuitem", { name: "Edit" })).toBeNull(); + expect(screen.queryByRole("menuitem", { name: "Move to" })).toBeNull(); + }); + + it("presents only Delete (no Archive, no Edit) for an archived subject", () => { + renderTopBar( + , + ); + + openMenu(); + + expect(screen.getByRole("menuitem", { name: "Delete" })).toBeTruthy(); + expect(screen.queryByRole("menuitem", { name: "Archive" })).toBeNull(); + expect(screen.queryByRole("menuitem", { name: "Edit" })).toBeNull(); + }); +}); + +function StoreProbe({ onReady }: { onReady: (store: ReturnType) => void }) { + const store = useAppStoreApi(); + useEffect(() => onReady(store), [store, onReady]); + return null; +} + +describe("TaskTopBar actions menu — subject removed from the board (AC-TASKS-TASK-ACTIONS-MENU-004.5)", () => { + // The subject leaving `kanban.tasks` alone is board-row loss, not genuine + // removal (AC-TASKS-TASK-ACTIONS-MENU-002.6/004.1c): it must demote the + // menu in place, never close it. `actionsMenuBoardRow` is a prop this + // harness holds static, so only a real "gone" signal (task.deleted, or + // task.updated with archived_at) may close the menu here, and this test has + // no WS client wired up to send one. + it("keeps an open menu open when the subject merely leaves the board's task collections", () => { + let store: ReturnType | null = null; + renderTopBar( + + { + store = api; + }} + /> + + , + ); + + openMenu(); + expectMenuOpen(true); + + act(() => { + store!.setState((state) => ({ kanban: { ...state.kanban, tasks: [] } })); + }); + + expectMenuOpen(true); + }); + + it("does not close the menu for a task that has simply not loaded into the board yet", () => { + renderTopBar( + , + ); + + openMenu(); + + expectMenuOpen(true); + }); +}); diff --git a/apps/web/components/task/task-top-bar.tsx b/apps/web/components/task/task-top-bar.tsx index 603bd6f569..9976c5316e 100644 --- a/apps/web/components/task/task-top-bar.tsx +++ b/apps/web/components/task/task-top-bar.tsx @@ -23,10 +23,12 @@ import { TaskUnarchiveButton } from "@/components/task/task-unarchive-button"; import { TaskAssigneeControl } from "@/components/task/task-assignee-control"; import { WorkflowStepper, type WorkflowStepperStep } from "@/components/task/workflow-stepper"; import { TaskTopBarPluginActions } from "@/components/task/task-top-bar-plugin-actions"; +import { TaskTopBarActionsMenu } from "@/components/task/task-top-bar-actions-menu"; import { TopbarMetrics } from "@/components/system-metrics/topbar-metrics"; import { RegisteredChangeRequestStatus } from "@/components/integrations/registered-change-request-status"; import { isDebugUI } from "@/lib/config"; import { useTranslation } from "react-i18next"; +import type { TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; type TaskTopBarProps = { taskId?: string | null; @@ -50,6 +52,11 @@ type TaskTopBarProps = { onTaskUnarchived?: (taskId: string) => void; onMoveStart?: () => void; onMoveError?: (error: unknown) => void; + actionsMenuBoardRow?: TaskActionsMenuBoardRow | null; + /** The subject task's own values, independent of `actionsMenuBoardRow` (see + * `TaskTopBarActionsMenuProps` for why the board row cannot be relied on). */ + subjectWorkflowStepId?: string | null; + subjectPrimaryExecutorType?: string | null; }; const TaskTopBar = memo(function TaskTopBar({ @@ -73,6 +80,9 @@ const TaskTopBar = memo(function TaskTopBar({ onTaskUnarchived, onMoveStart, onMoveError, + actionsMenuBoardRow, + subjectWorkflowStepId, + subjectPrimaryExecutorType, }: TaskTopBarProps) { const { t } = useTranslation(); // Projects only exist for office-owned tasks, so kanban-mode tasks render no @@ -128,6 +138,9 @@ const TaskTopBar = memo(function TaskTopBar({ issueNumber={issueNumber} officeTaskHref={officeTaskHref} onTaskUnarchived={onTaskUnarchived} + actionsMenuBoardRow={actionsMenuBoardRow} + subjectWorkflowStepId={subjectWorkflowStepId} + subjectPrimaryExecutorType={subjectPrimaryExecutorType} /> } /> @@ -366,6 +379,9 @@ function TopBarRight({ issueNumber, officeTaskHref, onTaskUnarchived, + actionsMenuBoardRow, + subjectWorkflowStepId, + subjectPrimaryExecutorType, }: { taskId?: string | null; activeSessionId?: string | null; @@ -379,6 +395,9 @@ function TopBarRight({ issueNumber?: number; officeTaskHref?: string | null; onTaskUnarchived?: (taskId: string) => void; + actionsMenuBoardRow?: TaskActionsMenuBoardRow | null; + subjectWorkflowStepId?: string | null; + subjectPrimaryExecutorType?: string | null; }) { const { t } = useTranslation(); return ( @@ -431,6 +450,15 @@ function TopBarRight({ isArchived={isArchived} embeddedVscodeSupported={embeddedVscodeSupported} /> +
); } diff --git a/apps/web/e2e/tests/kanban/task-actions-menu-detail.spec.ts b/apps/web/e2e/tests/kanban/task-actions-menu-detail.spec.ts new file mode 100644 index 0000000000..c870e5ab61 --- /dev/null +++ b/apps/web/e2e/tests/kanban/task-actions-menu-detail.spec.ts @@ -0,0 +1,56 @@ +import { test, expect } from "../../fixtures/test-base"; +import { KanbanPage } from "../../pages/kanban-page"; +import { SessionPage } from "../../pages/session-page"; + +// Covers REQ-TASKS-TASK-ACTIONS-MENU-001/002/003 on the desktop task detail +// top bar: the trigger this requirement adds, its parity entries, and the +// detail-scoped Archive outcome (archive-and-switch away from the archived +// task, matching every other task-scoped archive entry point). +test.describe("Task detail top bar actions menu", () => { + test("opens a menu with Edit, Archive, and Delete, and confirming Archive switches away from the task", async ({ + testPage, + apiClient, + seedData, + }) => { + const task = await apiClient.createTask(seedData.workspaceId, "Detail Menu Archive Task", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + const card = kanban.taskCardByTitle("Detail Menu Archive Task"); + await expect(card).toBeVisible({ timeout: 10_000 }); + await card.click(); + await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 }); + + const session = new SessionPage(testPage); + await session.waitForLoad(); + + const topBar = testPage.getByTestId("task-topbar"); + const trigger = topBar.getByTestId("task-topbar-actions-menu"); + await expect(trigger).toHaveAttribute("aria-label", "More options"); + await trigger.click(); + + await expect(testPage.getByRole("menuitem", { name: "Edit" })).toBeVisible(); + await expect(testPage.getByRole("menuitem", { name: "Delete" })).toBeVisible(); + const archiveItem = testPage.getByRole("menuitem", { name: "Archive" }); + await expect(archiveItem).toBeVisible(); + await archiveItem.click(); + + const confirmation = testPage.getByTestId("task-archive-confirm-popover"); + await expect(confirmation).toBeVisible(); + await expect(confirmation).toContainText("Detail Menu Archive Task"); + await confirmation.getByTestId("archive-task-confirm").click(); + + // Archive-and-switch (AC-TASKS-TASK-ACTIONS-MENU-003.4): the detail route + // no longer points at the archived task. + await expect + .poll(() => testPage.url(), { + timeout: 15_000, + message: "Waiting for switch-away navigation", + }) + .not.toMatch(new RegExp(`/t/${task.id}(?:$|[/?])`)); + }); +}); diff --git a/apps/web/e2e/tests/kanban/task-actions-menu-preview.spec.ts b/apps/web/e2e/tests/kanban/task-actions-menu-preview.spec.ts new file mode 100644 index 0000000000..e9e83f2364 --- /dev/null +++ b/apps/web/e2e/tests/kanban/task-actions-menu-preview.spec.ts @@ -0,0 +1,143 @@ +import { test, expect } from "../../fixtures/test-base"; +import { KanbanPage } from "../../pages/kanban-page"; + +// Covers REQ-TASKS-TASK-ACTIONS-MENU-001/002/003 on the desktop task preview +// panel: the trigger this requirement adds, its parity entries, and the +// preview-scoped Archive and Delete outcomes (remove from board, close the +// panel, no navigation to the task detail route). +test.describe("Task preview panel actions menu", () => { + test("opens a menu with Edit, Archive, and Delete, and confirming Archive closes the preview without navigating", async ({ + testPage, + apiClient, + seedData, + }) => { + await apiClient.saveUserSettings({ enable_preview_on_click: true }); + await apiClient.createTask(seedData.workspaceId, "Preview Menu Archive Task", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + const card = kanban.taskCardByTitle("Preview Menu Archive Task"); + await expect(card).toBeVisible({ timeout: 10_000 }); + await card.click(); + await expect(testPage).toHaveURL(/taskId=/, { timeout: 10_000 }); + + const previewPanel = testPage.getByTestId("task-preview-panel"); + await expect(previewPanel).toBeVisible(); + const startUrl = testPage.url(); + + const trigger = previewPanel.getByTestId("task-preview-actions-menu"); + await expect(trigger).toHaveAttribute("aria-label", "More options"); + await trigger.click(); + + const archiveItem = testPage.getByRole("menuitem", { name: "Archive" }); + await expect(archiveItem).toBeVisible(); + await archiveItem.click(); + + const confirmation = testPage.getByTestId("task-archive-confirm-popover"); + await expect(confirmation).toBeVisible(); + await expect(confirmation).toContainText("Preview Menu Archive Task"); + await confirmation.getByTestId("archive-task-confirm").click(); + + // AC-TASKS-TASK-ACTIONS-MENU-003.3: remove from the board, close the + // preview panel, no navigation to the task detail route. + await expect(kanban.taskCardByTitle("Preview Menu Archive Task")).not.toBeVisible({ + timeout: 10_000, + }); + await expect(previewPanel).not.toBeVisible(); + expect(testPage.url()).not.toBe(startUrl); + expect(testPage.url()).not.toMatch(/\/t\//); + }); + + test("opens a menu with Edit, Archive, and Delete, and confirming Delete closes the preview without navigating", async ({ + testPage, + apiClient, + seedData, + }) => { + await apiClient.saveUserSettings({ enable_preview_on_click: true }); + await apiClient.createTask(seedData.workspaceId, "Preview Menu Delete Task", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + const card = kanban.taskCardByTitle("Preview Menu Delete Task"); + await expect(card).toBeVisible({ timeout: 10_000 }); + await card.click(); + await expect(testPage).toHaveURL(/taskId=/, { timeout: 10_000 }); + + const previewPanel = testPage.getByTestId("task-preview-panel"); + await expect(previewPanel).toBeVisible(); + const startUrl = testPage.url(); + + const trigger = previewPanel.getByTestId("task-preview-actions-menu"); + await expect(trigger).toHaveAttribute("aria-label", "More options"); + await trigger.click(); + + await expect(testPage.getByRole("menuitem", { name: "Edit" })).toBeVisible(); + await expect(testPage.getByRole("menuitem", { name: "Archive" })).toBeVisible(); + const deleteItem = testPage.getByRole("menuitem", { name: "Delete" }); + await expect(deleteItem).toBeVisible(); + await deleteItem.click(); + + const dialog = testPage.getByRole("alertdialog"); + await expect(dialog).toBeVisible(); + await expect(dialog).toContainText("Preview Menu Delete Task"); + await dialog.getByTestId("delete-discard-worktree-checkbox").click(); + await dialog.getByRole("button", { name: "Delete" }).click(); + + await expect(kanban.taskCardByTitle("Preview Menu Delete Task")).not.toBeVisible({ + timeout: 10_000, + }); + await expect(previewPanel).not.toBeVisible(); + // No navigation to the task detail route (AC-TASKS-TASK-ACTIONS-MENU-003.3). + expect(testPage.url()).not.toBe(startUrl); + expect(testPage.url()).not.toMatch(/\/t\//); + }); + + test("Escape closes only the open menu, keeping the preview open, and returns focus to the trigger; a second Escape closes the preview", async ({ + testPage, + apiClient, + seedData, + }) => { + await apiClient.saveUserSettings({ enable_preview_on_click: true }); + await apiClient.createTask(seedData.workspaceId, "Preview Menu Escape Task", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + const card = kanban.taskCardByTitle("Preview Menu Escape Task"); + await expect(card).toBeVisible({ timeout: 10_000 }); + await card.click(); + await expect(testPage).toHaveURL(/taskId=/, { timeout: 10_000 }); + + const previewPanel = testPage.getByTestId("task-preview-panel"); + await expect(previewPanel).toBeVisible(); + + const trigger = previewPanel.getByTestId("task-preview-actions-menu"); + await trigger.click(); + await expect(testPage.getByRole("menuitem", { name: "Edit" })).toBeVisible(); + await expect(trigger).toHaveAttribute("aria-expanded", "true"); + + // First Escape (AC-TASKS-TASK-ACTIONS-MENU-001.9/001.11): closes only the + // menu, leaves the preview open on the same subject, and returns focus to + // the trigger that opened it. + await testPage.keyboard.press("Escape"); + await expect(testPage.getByRole("menuitem", { name: "Edit" })).not.toBeVisible(); + await expect(previewPanel).toBeVisible(); + await expect(trigger).toBeFocused(); + + // Second Escape, with no menu open, closes the preview panel as it does + // today. + await testPage.keyboard.press("Escape"); + await expect(previewPanel).not.toBeVisible(); + }); +}); diff --git a/apps/web/hooks/domains/kanban/use-kanban-actions.ts b/apps/web/hooks/domains/kanban/use-kanban-actions.ts index c07efa2bcf..6e63f09b24 100644 --- a/apps/web/hooks/domains/kanban/use-kanban-actions.ts +++ b/apps/web/hooks/domains/kanban/use-kanban-actions.ts @@ -72,7 +72,7 @@ function hydrateCreatedTask( } /** Handle editing an existing task - only update dialog-editable fields. */ -function hydrateEditedTask( +export function hydrateEditedTask( store: ReturnType, task: BackendTask, currentKanban: KanbanState, diff --git a/apps/web/hooks/use-task-actions-menu-move-targets.test.tsx b/apps/web/hooks/use-task-actions-menu-move-targets.test.tsx new file mode 100644 index 0000000000..cccadf299c --- /dev/null +++ b/apps/web/hooks/use-task-actions-menu-move-targets.test.tsx @@ -0,0 +1,121 @@ +import { describe, expect, it } from "vitest"; +import { renderHook } from "@testing-library/react"; +import { StateProvider } from "@/components/state-provider"; +import { useTaskActionsMenuMoveTargets } from "./use-task-actions-menu-move-targets"; + +describe("useTaskActionsMenuMoveTargets — hidden step fallback (AC-TASKS-TASK-ACTIONS-MENU-002.3b)", () => { + const WORKFLOW_ID = "wf-1"; + const TASK_ID = "task-1"; + const HIDDEN_STEP_ID = "step-hidden"; + + function wrapper({ children }: { children: React.ReactNode }) { + return ( + + {children} + + ); + } + + it("excludes hidden steps from the current workflow when the caller passes no explicit steps", () => { + const { result } = renderHook(() => useTaskActionsMenuMoveTargets(TASK_ID), { wrapper }); + + expect(result.current.currentWorkflowId).toBe(WORKFLOW_ID); + const stepIds = result.current.stepsByWorkflowId[WORKFLOW_ID].map((step) => step.id); + expect(stepIds).toEqual(["step-a", "step-b"]); + }); + + it("uses the caller's explicit steps as-is, bypassing the hidden-step fallback", () => { + const explicitSteps = [{ id: HIDDEN_STEP_ID, title: "Hidden step", color: "gray" }]; + const { result } = renderHook(() => useTaskActionsMenuMoveTargets(TASK_ID, explicitSteps), { + wrapper, + }); + + const stepIds = result.current.stepsByWorkflowId[WORKFLOW_ID].map((step) => step.id); + expect(stepIds).toEqual([HIDDEN_STEP_ID]); + }); +}); + +describe("useTaskActionsMenuMoveTargets — flat-list fallback for a WS-arrived task (AC-TASKS-TASK-ACTIONS-MENU-002.1/002.2)", () => { + const WORKFLOW_ID = "wf-1"; + const TASK_ID = "task-1"; + + function wrapper({ children }: { children: React.ReactNode }) { + return ( + + {children} + + ); + } + + it("resolves currentWorkflowId from the flat kanban.tasks list when no snapshot lists the task yet", () => { + const { result } = renderHook(() => useTaskActionsMenuMoveTargets(TASK_ID), { wrapper }); + + expect(result.current.currentWorkflowId).toBe(WORKFLOW_ID); + const stepIds = result.current.stepsByWorkflowId[WORKFLOW_ID].map((step) => step.id); + expect(stepIds).toEqual(["step-a", "step-b"]); + }); +}); diff --git a/apps/web/hooks/use-task-actions-menu-move-targets.ts b/apps/web/hooks/use-task-actions-menu-move-targets.ts new file mode 100644 index 0000000000..86b799382c --- /dev/null +++ b/apps/web/hooks/use-task-actions-menu-move-targets.ts @@ -0,0 +1,79 @@ +"use client"; + +import { useMemo } from "react"; +import { useAppStore } from "@/components/state-provider"; +import type { WorkflowStep } from "@/components/kanban-card"; +import type { KanbanCardMoveTargets } from "@/components/kanban-card-menu-items"; +import type { TaskMoveStep, TaskMoveWorkflow } from "@/components/task/task-move-context-menu"; +import { sortWorkflowStepsByPosition } from "@/lib/kanban/workflow-step-order"; + +/** + * Move-target resolution for callers with no board-column context (the + * preview panel, the detail top bar). Layers two fallbacks the card's hot + * render path (`useKanbanCardMoveTargets`) deliberately omits (system + * design: the card hook keeps a fixed, minimal subscription set so + * unrelated writes to these slices never re-render every visible card): + * - resolves `currentWorkflowId` from the flat `kanban.tasks` list when a + * WS-arrived task hasn't yet been merged into its workflow's snapshot + * (the same race `findTaskInSnapshots` falls back for). + * - filters the current workflow's steps by the user's hidden-step + * preference when the caller has no `steps` list of its own to pass + * (AC-TASKS-TASK-ACTIONS-MENU-002.3b: Move to must match the card). + */ +export function useTaskActionsMenuMoveTargets( + taskId: string, + steps?: WorkflowStep[], +): KanbanCardMoveTargets { + const workflows = useAppStore((state) => state.workflows.items); + const snapshots = useAppStore((state) => state.kanbanMulti.snapshots); + const kanbanTasks = useAppStore((state) => state.kanban.tasks); + const hiddenWorkflowStepIds = useAppStore((state) => state.userSettings.hiddenWorkflowStepIds); + + const currentWorkflowId = useMemo(() => { + for (const [workflowId, snapshot] of Object.entries(snapshots)) { + if (snapshot.tasks.some((task) => task.id === taskId)) return workflowId; + } + return kanbanTasks.find((task) => task.id === taskId)?.workflowId ?? null; + }, [snapshots, kanbanTasks, taskId]); + + const workflowItems = useMemo(() => { + const current = workflows.find((workflow) => workflow.id === currentWorkflowId); + return workflows + .filter((workflow) => workflow.workspaceId === current?.workspaceId && !workflow.hidden) + .map((workflow) => ({ id: workflow.id, name: workflow.name, hidden: workflow.hidden })); + }, [workflows, currentWorkflowId]); + + const fallbackCurrentWorkflowSteps = useMemo(() => { + if (!currentWorkflowId || steps) return undefined; + const snapshotSteps = snapshots[currentWorkflowId]?.steps; + if (!snapshotSteps) return undefined; + const hiddenIds = hiddenWorkflowStepIds[currentWorkflowId]; + const hiddenSet = hiddenIds && hiddenIds.length > 0 ? new Set(hiddenIds) : null; + const sorted = sortWorkflowStepsByPosition(snapshotSteps); + return hiddenSet ? sorted.filter((step) => !hiddenSet.has(step.id)) : sorted; + }, [currentWorkflowId, steps, snapshots, hiddenWorkflowStepIds]); + + const stepsByWorkflowId = useMemo>(() => { + const result: Record = {}; + for (const [workflowId, snapshot] of Object.entries(snapshots)) { + result[workflowId] = sortWorkflowStepsByPosition(snapshot.steps).map((step) => ({ + id: step.id, + title: step.title, + color: step.color, + events: step.events, + })); + } + const effectiveCurrentWorkflowSteps = steps ?? fallbackCurrentWorkflowSteps; + if (currentWorkflowId && effectiveCurrentWorkflowSteps) { + result[currentWorkflowId] = effectiveCurrentWorkflowSteps.map((step) => ({ + id: step.id, + title: step.title, + color: step.color, + events: step.events, + })); + } + return result; + }, [snapshots, currentWorkflowId, steps, fallbackCurrentWorkflowSteps]); + + return { currentWorkflowId, workflowItems, stepsByWorkflowId }; +} diff --git a/apps/web/hooks/use-task-actions-menu.ts b/apps/web/hooks/use-task-actions-menu.ts new file mode 100644 index 0000000000..2d1e8f4993 --- /dev/null +++ b/apps/web/hooks/use-task-actions-menu.ts @@ -0,0 +1,319 @@ +"use client"; + +import { useCallback, useEffect, useRef } from "react"; +import type { KanbanPresentation } from "@/components/kanban-card"; +import type { KanbanCardMoveTargets } from "@/components/kanban-card-menu-items"; +import type { WorkflowStep } from "@/components/kanban-card"; +import { useTaskActionsMenuMoveTargets } from "@/hooks/use-task-actions-menu-move-targets"; +import { useTaskWorkflowMove } from "@/hooks/use-task-workflow-move"; +import { + useTaskMenuDialogState, + useTaskMenuEditDialogState, +} from "@/hooks/use-task-menu-dialog-state"; +import { useDetachTask } from "@/hooks/use-detach-task"; +import { useUpdateTaskPriority } from "@/hooks/use-update-task-priority"; +import { useTaskPluginLinkActions } from "@/components/task/task-session-sidebar-link-actions"; +import { useKanbanExternalLinkAvailability } from "@/components/kanban-external-link-availability"; +import { usePluginRegistry } from "@/lib/plugins/registry"; +import type { PluginTaskMenuContext } from "@/lib/plugins/types"; +import type { TaskPriority, TaskState } from "@/lib/types/http"; +import { buildTaskActionsMenuEntries } from "@/lib/kanban/task-actions-menu-entries"; + +export type TaskActionsMenuBoardRow = { + id: string; + title: string; + description?: string; + workflowStepId?: string | null; + state?: TaskState; + priority?: TaskPriority; + repositoryId?: string; + repositories?: Array<{ + id: string; + repository_id: string; + position: number; + base_branch?: string; + checkout_branch?: string; + branch_policy_id?: string; + branch_policy_name?: string; + branch_policy_base_branch?: string; + branch_policy_branch_template?: string; + branch_policy_pull_request_target?: string; + }>; + parentTaskId?: string | null; + primaryExecutorType?: string | null; + workspaceMode?: "inherit_parent" | "new_workspace" | "shared_group"; +}; + +function resolveTaskActionsMenuTier( + isArchived: boolean, + boardRow: TaskActionsMenuBoardRow | null, +): "archived" | "normal" | "unresolved-row" { + if (isArchived) return "archived"; + return boardRow ? "normal" : "unresolved-row"; +} + +function buildLinkHandlers( + dialogs: ReturnType, + availability: ReturnType, +) { + return { + onLinkPullRequest: () => dialogs.setShowPRDialog(true), + onLinkIssue: () => dialogs.setShowIssueDialog(true), + onLinkMergeRequest: availability.gitlab ? () => dialogs.setShowMRDialog(true) : undefined, + onLinkJiraTicket: availability.jira ? () => dialogs.setExternalLinkProvider("jira") : undefined, + onLinkLinearIssue: availability.linear + ? () => dialogs.setExternalLinkProvider("linear") + : undefined, + onLinkSentryIssue: availability.sentry + ? () => dialogs.setExternalLinkProvider("sentry") + : undefined, + }; +} + +type ComputeEntriesArgs = { + taskId: string | null; + tier: ReturnType; + boardRow: TaskActionsMenuBoardRow | null; + moveTargets: KanbanCardMoveTargets; + disabled: boolean; + isDeleting?: boolean; + isArchiving?: boolean; + isDetaching: boolean; + editDialog: ReturnType; + dialogs: ReturnType; + requestArchiveConfirmation: () => void; + requestDetachConfirmation: () => void; + linkHandlers: ReturnType; + pluginLinkActions: ReturnType; + onMoveToStep: (stepId: string) => void; + onSendToWorkflow: (workflowId: string, stepId: string) => void; + pluginMenuContext: PluginTaskMenuContext; + onSelectPriority: (priority: TaskPriority) => void; +}; + +function useTaskPrioritySelector(taskId: string | null) { + const updateTaskPriority = useUpdateTaskPriority(); + return useCallback( + (priority: TaskPriority) => { + if (!taskId) return; + void updateTaskPriority(taskId, priority); + }, + [taskId, updateTaskPriority], + ); +} + +function computeTaskActionsMenuEntries(args: ComputeEntriesArgs) { + if (args.taskId == null) return []; + const { boardRow, moveTargets, editDialog, dialogs } = args; + return buildTaskActionsMenuEntries(args.tier, { + currentWorkflowId: moveTargets.currentWorkflowId, + currentStepId: boardRow?.workflowStepId ?? null, + workflows: moveTargets.workflowItems, + stepsByWorkflowId: moveTargets.stepsByWorkflowId, + disabled: args.disabled, + isDeleting: args.isDeleting, + isArchiving: args.isArchiving, + isDetaching: args.isDetaching, + parentTaskId: boardRow?.parentTaskId, + currentPriority: boardRow?.priority, + onSelectPriority: args.onSelectPriority, + onEdit: boardRow ? () => editDialog.setShowEditDialog(true) : undefined, + onArchive: args.requestArchiveConfirmation, + onDelete: () => dialogs.setShowDeleteConfirm(true), + onDetach: boardRow?.parentTaskId ? args.requestDetachConfirmation : undefined, + ...args.linkHandlers, + pluginLinkActions: args.pluginLinkActions, + onMoveToStep: args.onMoveToStep, + onSendToWorkflow: args.onSendToWorkflow, + pluginMenuContext: args.pluginMenuContext, + }); +} + +/** + * Keeps every confirm/link dialog scoped to the subject that opened it + * (AC-TASKS-TASK-ACTIONS-MENU-004.5a's no-retarget rule): resets them all on + * a subject-identity change, and guards the two 300ms-delayed confirmations + * so a subject swap during that window silently drops the request instead of + * opening for the new subject. + */ +function useSubjectScopedDialogs( + taskId: string | null, + dialogs: ReturnType, + editDialog: ReturnType, +) { + const taskIdRef = useRef(taskId); + taskIdRef.current = taskId; + + const closeDialogs = useCallback(() => { + dialogs.closeAll(); + editDialog.closeAll(); + }, [dialogs, editDialog]); + + // The preview panel re-renders this hook with a new `task` prop without + // unmounting, and the confirm/link popovers are non-modal, so nothing else + // would close a dialog opened for the previous subject. + useEffect(() => { + closeDialogs(); + // Only the subject identity should trigger this; `closeDialogs` is stable. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [taskId]); + + const requestDetachConfirmation = useCallback(() => { + // Let Radix finish the menu's pointer sequence before the non-modal + // popover opens; otherwise the initiating menu event is an outside click. + const requestedForTaskId = taskId; + window.setTimeout(() => { + if (taskIdRef.current !== requestedForTaskId) return; + dialogs.setShowDetachConfirm(true); + }, 300); + }, [dialogs, taskId]); + + const requestArchiveConfirmation = useCallback(() => { + const requestedForTaskId = taskId; + window.setTimeout(() => { + if (taskIdRef.current !== requestedForTaskId) return; + dialogs.setShowArchiveConfirm(true); + }, 300); + }, [dialogs, taskId]); + + return { closeDialogs, requestDetachConfirmation, requestArchiveConfirmation }; +} + +type UseTaskActionsMenuArgs = { + taskId: string | null; + taskTitle: string; + workspaceId: string | null; + presentation?: KanbanPresentation; + isArchived: boolean; + /** The subject's board row, when resolvable. Null demotes to the identifier-only tier. */ + boardRow: TaskActionsMenuBoardRow | null; + /** Current-workflow steps for the Move to submenu. */ + steps?: WorkflowStep[]; + isArchiving?: boolean; + isDeleting?: boolean; + onArchive: (opts: { cascade: boolean }) => void | Promise; + onDelete: (opts: { cascade: boolean }) => void | Promise; + /** The subject's own workflow step, independent of `boardRow` (AC-002.4b's + * plugin task-menu context must not go stale just because the board + * excludes the subject, e.g. an archived task). Falls back to + * `boardRow?.workflowStepId` for callers with no independent source. */ + subjectWorkflowStepId?: string | null; +}; + +export function useTaskActionsMenu({ + taskId, + taskTitle, + workspaceId, + presentation = "desktop", + isArchived, + boardRow, + steps, + isArchiving, + isDeleting, + onArchive, + onDelete, + subjectWorkflowStepId, +}: UseTaskActionsMenuArgs) { + const dialogs = useTaskMenuDialogState(); + const editDialog = useTaskMenuEditDialogState(); + const { detachTask, detachingTaskId } = useDetachTask(); + const onSelectPriority = useTaskPrioritySelector(taskId); + const isDetaching = taskId != null && detachingTaskId === taskId; + const externalLinkAvailability = useKanbanExternalLinkAvailability(workspaceId); + const pluginLinkActions = useTaskPluginLinkActions(taskId ?? "", boardRow?.repositories ?? []); + // Re-render on any registry mutation so a menu action a plugin registers or + // disables at runtime appears/disappears in an already-open menu (AC-002.8). + usePluginRegistry(); + const moveTargets = useTaskActionsMenuMoveTargets(taskId ?? "", steps); + const moveTasks = useTaskWorkflowMove(); + + const triggerRef = useRef(null); + + const disabled = Boolean(isArchiving || isDeleting || isDetaching); + + const { closeDialogs, requestDetachConfirmation, requestArchiveConfirmation } = + useSubjectScopedDialogs(taskId, dialogs, editDialog); + + const handleDetachConfirm = useCallback(async () => { + if (!taskId) return; + try { + await detachTask(taskId); + dialogs.setShowDetachConfirm(false); + } catch (error) { + console.error("Failed to detach task:", error); + } + }, [detachTask, dialogs, taskId]); + + const onMoveToStep = useCallback( + (stepId: string) => { + if (!taskId || !moveTargets.currentWorkflowId) return; + void moveTasks([taskId], moveTargets.currentWorkflowId, stepId, "step").catch(() => { + // useTaskWorkflowMove already shows the failure toast. + }); + }, + [moveTasks, moveTargets.currentWorkflowId, taskId], + ); + + const onSendToWorkflow = useCallback( + (workflowId: string, stepId: string) => { + if (!taskId) return; + void moveTasks([taskId], workflowId, stepId, "workflow").catch(() => { + // useTaskWorkflowMove already shows the failure toast. + }); + }, + [moveTasks, taskId], + ); + + const pluginMenuContext: PluginTaskMenuContext = { + workspaceId: workspaceId ?? "", + taskId: taskId ?? "", + taskTitle, + workflowStepId: subjectWorkflowStepId ?? boardRow?.workflowStepId ?? null, + presentation, + }; + + const tier = resolveTaskActionsMenuTier(Boolean(isArchived), boardRow); + const linkHandlers = buildLinkHandlers(dialogs, externalLinkAvailability); + + const entries = computeTaskActionsMenuEntries({ + taskId, + tier, + boardRow, + moveTargets, + disabled, + isDeleting, + isArchiving, + isDetaching, + editDialog, + dialogs, + requestArchiveConfirmation, + requestDetachConfirmation, + linkHandlers, + pluginLinkActions, + onMoveToStep, + onSendToWorkflow, + pluginMenuContext, + onSelectPriority, + }); + + // Strip each state slice's own `closeAll` before spreading: `closeDialogs` + // above is the single composed reset callers should use, and leaving both + // `dialogs.closeAll`/`editDialog.closeAll` in the spread would collide under + // one `closeAll` key that only resets the edit dialog (last spread wins). + const { closeAll: _dialogsCloseAll, ...dialogsRest } = dialogs; + const { closeAll: _editDialogCloseAll, ...editDialogRest } = editDialog; + + return { + entries, + ...dialogsRest, + ...editDialogRest, + isDetaching, + handleDetachConfirm, + triggerRef, + currentWorkflowId: moveTargets.currentWorkflowId, + stepsByWorkflowId: moveTargets.stepsByWorkflowId, + onConfirmArchive: onArchive, + onConfirmDelete: onDelete, + closeDialogs, + }; +} diff --git a/apps/web/hooks/use-task-actions.test.ts b/apps/web/hooks/use-task-actions.test.ts index f6c2b705d4..6d1dbedaff 100644 --- a/apps/web/hooks/use-task-actions.test.ts +++ b/apps/web/hooks/use-task-actions.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { renderHook, act } from "@testing-library/react"; +const NETWORK_ERROR_MESSAGE = "network error"; + const archiveTaskMock = vi.fn(); const deleteTaskMock = vi.fn(); const removeTaskFromBoardMock = vi.fn(); @@ -40,7 +42,11 @@ vi.mock("@/components/toast-provider", () => ({ useToast: () => ({ toast: toastMock }), })); -import { useArchiveAndSwitchTask, useTaskActions } from "./use-task-actions"; +import { + useArchiveAndSwitchTask, + useDeleteAndSwitchTask, + useTaskActions, +} from "./use-task-actions"; function deferred() { let resolve!: (value: T | PromiseLike) => void; @@ -90,7 +96,7 @@ describe("useArchiveAndSwitchTask", () => { }); it("restores active task when archive API rejects after switching", async () => { - const error = new Error("network error"); + const error = new Error(NETWORK_ERROR_MESSAGE); archiveTaskMock.mockRejectedValueOnce(error); removeTaskFromBoardMock.mockImplementationOnce(async () => { storeState.tasks.activeTaskId = "task-B"; @@ -98,7 +104,7 @@ describe("useArchiveAndSwitchTask", () => { }); const { result } = renderHook(() => useArchiveAndSwitchTask()); - await expect(result.current("task-A")).rejects.toThrow("network error"); + await expect(result.current("task-A")).rejects.toThrow(NETWORK_ERROR_MESSAGE); expect(removeTaskFromBoardMock).toHaveBeenCalledTimes(1); expect(removeTaskFromBoardMock).toHaveBeenCalledWith("task-A", { @@ -151,6 +157,49 @@ describe("useArchiveAndSwitchTask", () => { }); }); +describe("useDeleteAndSwitchTask", () => { + it("removes and switches away from the active task before delete API resolves", async () => { + const deletion = deferred(); + deleteTaskMock.mockReturnValueOnce(deletion.promise); + const { result } = renderHook(() => useDeleteAndSwitchTask()); + + let deleteAndSwitchPromise!: Promise; + act(() => { + deleteAndSwitchPromise = result.current("task-A"); + }); + + expect(removeTaskFromBoardMock).toHaveBeenCalledWith("task-A", { + wasActiveTaskId: "task-A", + wasActiveSessionId: "sess-A", + switchOnly: true, + }); + + deletion.resolve(); + await deleteAndSwitchPromise; + expect(deleteTaskMock).toHaveBeenCalledWith("task-A", undefined); + expect(removeTaskFromBoardMock).toHaveBeenLastCalledWith("task-A", { + wasActiveTaskId: "task-A", + wasActiveSessionId: "sess-A", + }); + }); + + it("restores active task when delete API rejects after switching", async () => { + const error = new Error(NETWORK_ERROR_MESSAGE); + deleteTaskMock.mockRejectedValueOnce(error); + removeTaskFromBoardMock.mockImplementationOnce(async () => { + storeState.tasks.activeTaskId = "task-B"; + return { switchedTaskId: "task-B" }; + }); + const { result } = renderHook(() => useDeleteAndSwitchTask()); + + await expect(result.current("task-A")).rejects.toThrow(NETWORK_ERROR_MESSAGE); + + expect(setActiveSessionMock).toHaveBeenCalledWith("task-A", "sess-A"); + expect(replaceTaskUrlMock).toHaveBeenCalledWith("task-A"); + expect(deleteTaskMock).toHaveBeenCalledWith("task-A", undefined); + }); +}); + describe("useTaskActions", () => { it("shows localized retry guidance for a dirty-worktree delete conflict", async () => { const { ApiError } = await import("@/lib/api/client"); diff --git a/apps/web/hooks/use-task-actions.ts b/apps/web/hooks/use-task-actions.ts index d4c29b62a5..2c77631bc0 100644 --- a/apps/web/hooks/use-task-actions.ts +++ b/apps/web/hooks/use-task-actions.ts @@ -53,22 +53,26 @@ export function useTaskActions() { } /** - * Archives a task and switches to the next available task. - * Shared between the PR merged banner and the sidebar archive action. + * Runs a one-shot task action (archive or delete) and switches to the next + * available task, restoring the previous active task if the action rejects + * after an optimistic switch. Shared shape behind `useArchiveAndSwitchTask` + * and `useDeleteAndSwitchTask`. */ -export function useArchiveAndSwitchTask(opts?: { useLayoutSwitch?: boolean }) { +function useSwitchAfterTaskAction( + runAction: (taskId: string, opts?: { cascade?: boolean }) => Promise, + opts?: { useLayoutSwitch?: boolean }, +) { const store = useAppStoreApi(); - const { archiveTaskById } = useTaskActions(); const { removeTaskFromBoard } = useTaskRemoval({ store, useLayoutSwitch: opts?.useLayoutSwitch, }); return useCallback( - async (taskId: string, opts?: TaskActionOptions) => { + async (taskId: string, actionOpts?: { cascade?: boolean }) => { const { activeTaskId: wasActiveTaskId, activeSessionId: wasActiveSessionId } = store.getState().tasks; - const removalOptions = opts?.cascade ? { excludeTaskTree: true } : {}; + const removalOptions = actionOpts?.cascade ? { excludeTaskTree: true } : {}; const initialSwitch = await removeTaskFromBoard(taskId, { wasActiveTaskId, @@ -78,7 +82,7 @@ export function useArchiveAndSwitchTask(opts?: { useLayoutSwitch?: boolean }) { }); try { - await archiveTaskById(taskId, opts); + await runAction(taskId, actionOpts); await removeTaskFromBoard(taskId, { wasActiveTaskId, wasActiveSessionId, @@ -103,6 +107,24 @@ export function useArchiveAndSwitchTask(opts?: { useLayoutSwitch?: boolean }) { throw error; } }, - [archiveTaskById, removeTaskFromBoard, store], + [runAction, removeTaskFromBoard, store], ); } + +/** + * Archives a task and switches to the next available task. + * Shared between the PR merged banner and the sidebar archive action. + */ +export function useArchiveAndSwitchTask(opts?: { useLayoutSwitch?: boolean }) { + const { archiveTaskById } = useTaskActions(); + return useSwitchAfterTaskAction(archiveTaskById, opts); +} + +/** + * Deletes a task and switches to the next available task, mirroring + * `useArchiveAndSwitchTask`'s outcome for the task detail surface. + */ +export function useDeleteAndSwitchTask(opts?: { useLayoutSwitch?: boolean }) { + const { deleteTaskById } = useTaskActions(); + return useSwitchAfterTaskAction(deleteTaskById, opts); +} diff --git a/apps/web/hooks/use-task-menu-dialog-state.ts b/apps/web/hooks/use-task-menu-dialog-state.ts new file mode 100644 index 0000000000..d67c334e33 --- /dev/null +++ b/apps/web/hooks/use-task-menu-dialog-state.ts @@ -0,0 +1,53 @@ +"use client"; + +import { useCallback, useState } from "react"; +import type { ExternalLinkProvider } from "@/components/task/task-external-link-dialog"; + +/** Every confirm/link-dialog open flag a task actions menu and its dialogs share. + * `closeAll` resets every flag; callers use it whenever the subject a dialog + * was opened for is no longer the current one (AC-TASKS-TASK-ACTIONS-MENU-004.5a's + * no-retarget rule applies to these dialogs, not just the trigger itself). */ +export function useTaskMenuDialogState() { + const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); + const [showArchiveConfirm, setShowArchiveConfirm] = useState(false); + const [showDetachConfirm, setShowDetachConfirm] = useState(false); + const [showPRDialog, setShowPRDialog] = useState(false); + const [showIssueDialog, setShowIssueDialog] = useState(false); + const [showMRDialog, setShowMRDialog] = useState(false); + const [externalLinkProvider, setExternalLinkProvider] = useState( + null, + ); + const closeAll = useCallback(() => { + setShowDeleteConfirm(false); + setShowArchiveConfirm(false); + setShowDetachConfirm(false); + setShowPRDialog(false); + setShowIssueDialog(false); + setShowMRDialog(false); + setExternalLinkProvider(null); + }, []); + return { + showDeleteConfirm, + setShowDeleteConfirm, + showArchiveConfirm, + setShowArchiveConfirm, + showDetachConfirm, + setShowDetachConfirm, + showPRDialog, + setShowPRDialog, + showIssueDialog, + setShowIssueDialog, + showMRDialog, + setShowMRDialog, + externalLinkProvider, + setExternalLinkProvider, + closeAll, + }; +} + +/** Edit-dialog open flag for a single-subject task actions menu (preview/detail). */ +export function useTaskMenuEditDialogState() { + const [showEditDialog, setShowEditDialog] = useState(false); + const closeAll = useCallback(() => setShowEditDialog(false), []); + return { showEditDialog, setShowEditDialog, closeAll }; +} diff --git a/apps/web/lib/dialog-focus-return.ts b/apps/web/lib/dialog-focus-return.ts new file mode 100644 index 0000000000..075867c81f --- /dev/null +++ b/apps/web/lib/dialog-focus-return.ts @@ -0,0 +1,18 @@ +import type { RefObject } from "react"; + +/** + * Builds a Radix `onCloseAutoFocus` handler that returns keyboard focus to + * `focusReturnRef` on close. Radix's own default restores focus to whatever + * element had it when the dialog opened, which has nothing valid to return to + * when the dialog was opened from a menu item that has already unmounted + * (e.g. the task actions menu closes on entry activation before its dialog + * opens). + */ +export function createFocusReturnHandler(focusReturnRef?: RefObject) { + return (event: Event) => { + const target = focusReturnRef?.current; + if (!target || !document.contains(target)) return; + event.preventDefault(); + target.focus(); + }; +} diff --git a/apps/web/lib/kanban/task-actions-menu-entries.test.ts b/apps/web/lib/kanban/task-actions-menu-entries.test.ts new file mode 100644 index 0000000000..d5bfe688f0 --- /dev/null +++ b/apps/web/lib/kanban/task-actions-menu-entries.test.ts @@ -0,0 +1,178 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { pluginRegistry } from "@/lib/plugins/registry"; +import { buildTaskActionsMenuEntries } from "./task-actions-menu-entries"; +import type { KanbanCardMenuEntry } from "@/components/kanban-card-menu-items"; + +const PLUGIN_ID = "test-task-actions-menu-plugin"; +const DELETE_SEPARATOR = "delete-separator"; + +function itemKeys(entries: KanbanCardMenuEntry[]): string[] { + return entries.map((entry) => entry.key); +} + +const baseArgs = { + currentWorkflowId: "workflow-1", + currentStepId: "step-1", + workflows: [ + { id: "workflow-1", name: "Workflow 1" }, + { id: "workflow-2", name: "Workflow 2" }, + ], + stepsByWorkflowId: { + "workflow-1": [ + { id: "step-1", title: "Step 1" }, + { id: "step-2", title: "Step 2" }, + ], + "workflow-2": [{ id: "step-3", title: "Step 3" }], + }, + onEdit: vi.fn(), + onArchive: vi.fn(), + onDelete: vi.fn(), + onDetach: vi.fn(), + parentTaskId: "parent-1", + currentPriority: "high", + onSelectPriority: vi.fn(), + onMoveToStep: vi.fn(), + onSendToWorkflow: vi.fn(), + // A real card always wires at least one link handler; omitting it here + // would silently drop "link" from the entries array and let the order + // assertion below pass without ever exercising its actual position. + onLinkPullRequest: vi.fn(), +}; + +describe("buildTaskActionsMenuEntries — normal tier", () => { + afterEach(() => pluginRegistry.unregisterPlugin(PLUGIN_ID)); + + it("always uses the flat Edit item, even when a plugin edit-group action is registered", () => { + pluginRegistry.forPlugin(PLUGIN_ID).registerTaskMenuAction({ + id: "edit-action", + group: "edit", + label: "Plugin edit action", + run: vi.fn(), + }); + + const entries = buildTaskActionsMenuEntries("normal", baseArgs); + const edit = entries.find((entry) => entry.key === "edit"); + + expect(edit?.kind).toBe("item"); + }); + + it("presents the full card order for a resolved, unarchived subject", () => { + const entries = buildTaskActionsMenuEntries("normal", baseArgs); + expect(itemKeys(entries)).toEqual([ + "edit", + "priority", + "move-to", + "send-to-workflow", + "link", + "archive", + "detach", + DELETE_SEPARATOR, + "delete", + ]); + }); +}); + +describe("buildTaskActionsMenuEntries — in-flight disabling (AC-TASKS-TASK-ACTIONS-MENU-004.1)", () => { + function itemByKey(entries: KanbanCardMenuEntry[], key: string) { + return entries.find((entry) => entry.key === key); + } + + it("disables Archive, Detach, and Delete while an archive request from this menu is in flight", () => { + const entries = buildTaskActionsMenuEntries("normal", { ...baseArgs, isArchiving: true }); + + expect(itemByKey(entries, "archive")).toMatchObject({ disabled: true }); + expect(itemByKey(entries, "detach")).toMatchObject({ disabled: true }); + expect(itemByKey(entries, "delete")).toMatchObject({ disabled: true }); + }); + + it("disables Archive, Detach, and Delete while a delete request from this menu is in flight", () => { + const entries = buildTaskActionsMenuEntries("normal", { ...baseArgs, isDeleting: true }); + + expect(itemByKey(entries, "archive")).toMatchObject({ disabled: true }); + expect(itemByKey(entries, "detach")).toMatchObject({ disabled: true }); + expect(itemByKey(entries, "delete")).toMatchObject({ disabled: true }); + }); + + it("disables Archive, Detach, and Delete while a detach request from this menu is in flight", () => { + const entries = buildTaskActionsMenuEntries("normal", { ...baseArgs, isDetaching: true }); + + expect(itemByKey(entries, "archive")).toMatchObject({ disabled: true }); + expect(itemByKey(entries, "detach")).toMatchObject({ disabled: true }); + expect(itemByKey(entries, "delete")).toMatchObject({ disabled: true }); + }); + + it("leaves every entry enabled when nothing from this menu is in flight", () => { + const entries = buildTaskActionsMenuEntries("normal", baseArgs); + + expect(itemByKey(entries, "archive")).toMatchObject({ disabled: false }); + expect(itemByKey(entries, "detach")).toMatchObject({ disabled: false }); + expect(itemByKey(entries, "delete")).toMatchObject({ disabled: false }); + }); +}); + +describe("buildTaskActionsMenuEntries — archived tier", () => { + afterEach(() => pluginRegistry.unregisterPlugin(PLUGIN_ID)); + + it("presents Delete alone with no leading separator when no plugin action is admitted", () => { + const entries = buildTaskActionsMenuEntries("archived", baseArgs); + expect(itemKeys(entries)).toEqual(["delete"]); + }); + + it("presents admitted plugin primary actions, a separator, then Delete", () => { + pluginRegistry.forPlugin(PLUGIN_ID).registerTaskMenuAction({ + id: "primary-action", + group: "primary", + label: "Plugin primary action", + run: vi.fn(), + }); + + const entries = buildTaskActionsMenuEntries("archived", baseArgs); + expect(itemKeys(entries)).toEqual([ + `plugin-primary-${PLUGIN_ID}-primary-action`, + DELETE_SEPARATOR, + "delete", + ]); + }); + + it("omits Edit, Move to, Send to workflow, Link, Detach from parent, and Archive", () => { + const entries = buildTaskActionsMenuEntries("archived", baseArgs); + const keys = itemKeys(entries); + for (const omitted of ["edit", "move-to", "send-to-workflow", "link", "archive", "detach"]) { + expect(keys).not.toContain(omitted); + } + }); +}); + +describe("buildTaskActionsMenuEntries — unresolved board row tier", () => { + afterEach(() => pluginRegistry.unregisterPlugin(PLUGIN_ID)); + + it("presents only Archive and Delete when no plugin action is admitted", () => { + const entries = buildTaskActionsMenuEntries("unresolved-row", baseArgs); + expect(itemKeys(entries)).toEqual(["archive", DELETE_SEPARATOR, "delete"]); + }); + + it("orders admitted plugin primary actions before Archive", () => { + pluginRegistry.forPlugin(PLUGIN_ID).registerTaskMenuAction({ + id: "primary-action", + group: "primary", + label: "Plugin primary action", + run: vi.fn(), + }); + + const entries = buildTaskActionsMenuEntries("unresolved-row", baseArgs); + expect(itemKeys(entries)).toEqual([ + `plugin-primary-${PLUGIN_ID}-primary-action`, + "archive", + DELETE_SEPARATOR, + "delete", + ]); + }); + + it("omits Edit, Move to, Send to workflow, Link, and Detach from parent even with a parentTaskId", () => { + const entries = buildTaskActionsMenuEntries("unresolved-row", baseArgs); + const keys = itemKeys(entries); + for (const omitted of ["edit", "move-to", "send-to-workflow", "link", "detach"]) { + expect(keys).not.toContain(omitted); + } + }); +}); diff --git a/apps/web/lib/kanban/task-actions-menu-entries.ts b/apps/web/lib/kanban/task-actions-menu-entries.ts new file mode 100644 index 0000000000..8565f3b113 --- /dev/null +++ b/apps/web/lib/kanban/task-actions-menu-entries.ts @@ -0,0 +1,66 @@ +import { + buildArchiveEntry, + buildDeleteEntry, + buildKanbanCardMenuEntries, + resolvePluginMenuContext, + type KanbanCardMenuEntry, +} from "@/components/kanban-card-menu-items"; +import { buildPrimaryPluginEntries } from "@/components/plugins/task-menu-actions"; +import type { BuildKanbanCardMenuEntriesArgs } from "@/components/kanban-card-menu-items"; + +/** + * `normal`: the subject is not archived and its board row is resolved, so + * the full card entry set applies. `archived`: the board excludes archived + * tasks, so this is a detail-surface-only branch the card has none to + * inherit from. `unresolved-row`: the subject's board row cannot be found, + * so only identifier-only entries are available. + */ +export type TaskActionsMenuTier = "normal" | "archived" | "unresolved-row"; + +export function buildTaskActionsMenuEntries( + tier: TaskActionsMenuTier, + args: BuildKanbanCardMenuEntriesArgs, +): KanbanCardMenuEntry[] { + if (tier === "archived") return buildArchivedTaskActionsMenuEntries(args); + if (tier === "unresolved-row") return buildUnresolvedRowTaskActionsMenuEntries(args); + return buildKanbanCardMenuEntries({ ...args, forceFlatEdit: true }); +} + +function buildArchivedTaskActionsMenuEntries({ + disabled, + isDeleting, + onDelete, + pluginMenuContext, +}: BuildKanbanCardMenuEntriesArgs): KanbanCardMenuEntry[] { + const isProcessing = Boolean(disabled || isDeleting); + const pluginEntries = buildPrimaryPluginEntries({ + disabled: isProcessing, + context: resolvePluginMenuContext(pluginMenuContext), + }); + const entries: KanbanCardMenuEntry[] = [...pluginEntries]; + if (pluginEntries.length > 0) { + entries.push({ kind: "separator", key: "delete-separator" }); + } + entries.push(buildDeleteEntry({ isDeleting, isProcessing, onDelete })); + return entries; +} + +function buildUnresolvedRowTaskActionsMenuEntries({ + disabled, + isDeleting, + isArchiving, + onArchive, + onDelete, + pluginMenuContext, +}: BuildKanbanCardMenuEntriesArgs): KanbanCardMenuEntry[] { + const isProcessing = Boolean(disabled || isDeleting || isArchiving); + return [ + ...buildPrimaryPluginEntries({ + disabled: isProcessing, + context: resolvePluginMenuContext(pluginMenuContext), + }), + buildArchiveEntry({ isArchiving, isProcessing, onArchive }), + { kind: "separator", key: "delete-separator" }, + buildDeleteEntry({ isDeleting, isProcessing, onDelete }), + ]; +} diff --git a/docs/specs/tasks/README.md b/docs/specs/tasks/README.md index 84f6dd1b19..ab089caa36 100644 --- a/docs/specs/tasks/README.md +++ b/docs/specs/tasks/README.md @@ -85,6 +85,9 @@ signals, and task-scoped scheduling contracts. - [Runtime Task-State Publication Order](requirements/runtime-state-publication-order.md) - [Session Delete Preserves Task Workspaces](requirements/session-delete-resource-cleanup.md) - [Sidebar Task Editing](requirements/sidebar-task-edit.md) +- [Task Actions Menu on Preview and Detail Surfaces](requirements/task-actions-menu.md) +- [Task Actions Menu In-Flight and Concurrency](requirements/task-actions-menu-concurrency.md) +- [Task Actions Menu Action Outcomes](requirements/task-actions-menu-outcomes.md) - [Subtasks as Workflow Checklist](requirements/subtask-checklist.md) - [Subtask Completion Trigger](requirements/subtask-completion-trigger.md) - [Subtask detachment](requirements/subtask-detachment.md) @@ -150,6 +153,7 @@ signals, and task-scoped scheduling contracts. - [Saved Prompt Delivery](system-design/saved-prompt-delivery.md) - [Passthrough Initial Prompt Turn Boundary](system-design/passthrough-initial-prompt-turn-boundary.md) - [Prompt attachments](system-design/prompt-attachments.md) +- [Task Actions Menu on Preview and Detail Surfaces](system-design/task-actions-menu.md) - [Task Archive Confirmation](system-design/archive-confirmation.md) - [Task plan content size limit](system-design/plan-content-size-limit.md) - [Task plan write consistency](system-design/plan-write-consistency.md) diff --git a/docs/specs/tasks/requirements/task-actions-menu-concurrency.md b/docs/specs/tasks/requirements/task-actions-menu-concurrency.md new file mode 100644 index 0000000000..46cb99eee1 --- /dev/null +++ b/docs/specs/tasks/requirements/task-actions-menu-concurrency.md @@ -0,0 +1,151 @@ +--- +status: draft +system: tasks +created: 2026-09-03 +owners: + - Kandev +--- + +# Task Actions Menu In-Flight and Concurrency Requirements + +## Overview + +The actions menu added to the task preview and task detail surfaces by +[Task Actions Menu on Preview and Detail Surfaces](task-actions-menu.md) creates +two new entry points to task actions that already exist. This file owns what +those entry points do while a request is in flight, when two surfaces act on one +task, and when a request fails. Terminology, entry content, and post-action +navigation are defined in the parent requirement and are not restated here. + +## Requirements + +### REQ-TASKS-TASK-ACTIONS-MENU-004: In-flight, concurrent, and failing actions + +**Intent:** The new entry points cannot produce a duplicate destructive request, +a stuck menu, or a silent failure. + +#### Acceptance criteria + +- **AC-TASKS-TASK-ACTIONS-MENU-004.1:** While an archive, delete, or detach + request started from a surface's actions menu is in flight, the system shall + present that surface's actions-menu entries as disabled and shall not start a + second request from that menu. Because + AC-TASKS-TASK-ACTIONS-MENU-004.2 closes the menu on a terminal activation, + and archive, delete and detach are terminal, this state is observed by + reopening the menu while the request is still in flight. +- **AC-TASKS-TASK-ACTIONS-MENU-004.1a:** Move to, Send to workflow, and Link are + deliberately outside AC-TASKS-TASK-ACTIONS-MENU-004.1, matching the card, + whose menu carries no pending state for them either. Selecting a Link entry + opens a dialog and starts no task-state request, so the dialog owns its own + submit state. For Move to and Send to workflow the system shall permit a + second target to be selected before the first request settles, shall issue + each as an independent request, and shall let the last one the server applies + determine the task's step, on + AC-TASKS-TASK-ACTIONS-MENU-004.3a's terms. The system shall not introduce a + menu-level in-flight lock for these entries. +- **AC-TASKS-TASK-ACTIONS-MENU-004.1b:** The in-flight disabled state of + AC-TASKS-TASK-ACTIONS-MENU-004.1 is scoped to the surface that started the + request. A surface's menu shall disable its entries only for a request that + surface itself started, and shall not disable them because a Kanban card or + the other surface has a request in flight for the same task. The system shall + not introduce shared, task-keyed pending state to close that gap. A card + mid-archive and a preview panel showing the same task may therefore present + different enabled states for the duration of that request. This is deliberate, + it is the named exception in AC-TASKS-TASK-ACTIONS-MENU-002.1, and it matches + the card, whose own pending flags are board-scoped or hook-local and are + already not shared with any other surface. +- **AC-TASKS-TASK-ACTIONS-MENU-004.1c:** When the subject task's board row stops + being resolvable while a Move to or Send to workflow submenu is open, the + system shall demote those entries per AC-TASKS-TASK-ACTIONS-MENU-002.6, which + removes the submenu's parent entry and so closes the open submenu while + leaving the top-level menu open. A move request already in flight shall be + left to complete or fail on its own terms; the system shall not cancel it, and + shall keep the surface open on the subject task per + AC-TASKS-TASK-ACTIONS-MENU-003.6. Losing the board row is not losing the task: + AC-TASKS-TASK-ACTIONS-MENU-004.5 governs the latter. +- **AC-TASKS-TASK-ACTIONS-MENU-004.2:** When the user activates a *terminal* + entry, meaning one that opens a dialog, dispatches a request, or invokes a + plugin action, the system + shall close the menu, so a single activation produces at most one confirmation + and at most one request. Opening a submenu is not a terminal activation: when + the user opens the Move to or Send to workflow submenu required by + AC-TASKS-TASK-ACTIONS-MENU-002.2, the system shall leave both that submenu and + the top-level menu open until a target inside it is chosen, and choosing that + target is itself a terminal activation and closes both. +- **AC-TASKS-TASK-ACTIONS-MENU-004.3:** When the same task is acted on + concurrently from two surfaces by a *one-shot* action, meaning Archive or + Delete, the system shall send each confirmed request independently; the first + request to be applied determines task state, and the later request shall + surface the existing failure feedback for its action rather than a new error + surface. Two actions are deliberately outside this criterion because neither + is one-shot: Move to and Send to workflow are repeatable and are governed by + AC-TASKS-TASK-ACTIONS-MENU-004.3a, and Detach from parent is idempotent and is + governed by AC-TASKS-TASK-ACTIONS-MENU-004.3b. +- **AC-TASKS-TASK-ACTIONS-MENU-004.3a:** When the same task is moved + concurrently, whether from two surfaces or twice from one menu, the system + shall send each move independently and shall not treat the later move as a + conflict. A move is repeatable and carries no precondition on the step it + moves from, so the last move the server applies determines the task's step, + and the system shall raise no failure feedback for an earlier move merely + being superseded. Failure feedback for a move is raised only when the server + rejects that move on its own terms + (AC-TASKS-TASK-ACTIONS-MENU-004.4). +- **AC-TASKS-TASK-ACTIONS-MENU-004.3b:** Detach from parent is idempotent rather + than one-shot, and the system shall not treat a concurrent or repeated detach + of one task as a conflict. A task that is already parentless shall remain + parentless and shall produce no failure feedback. Failure feedback for a + detach is raised only when the detach operation fails, per + AC-TASKS-TASK-ACTIONS-MENU-004.4. +- **AC-TASKS-TASK-ACTIONS-MENU-004.4:** When an archive, delete, move, detach, + or link request started from either surface fails, the system shall surface + the existing failure feedback for that action, shall leave the task in its + last confirmed state, and shall keep the surface open on that task. +- **AC-TASKS-TASK-ACTIONS-MENU-004.4a:** AC-TASKS-TASK-ACTIONS-MENU-004.5 takes + precedence over AC-TASKS-TASK-ACTIONS-MENU-004.4 whenever both apply. When a + request fails because another actor archived or deleted the subject task, + the surface shall apply its existing missing-task behavior instead of staying + open on a task that is gone. The system shall still surface the existing + failure feedback for the request. For every other failure, the surface shall + stay open on the task and show that feedback. +- **AC-TASKS-TASK-ACTIONS-MENU-004.5:** When a task disappears from beneath a + surface for a reason other than the user's own action, such as another client + archiving it, the system shall apply that surface's existing missing-task + behavior and shall not leave an actions menu open over a subject that no + longer exists. +- **AC-TASKS-TASK-ACTIONS-MENU-004.5a:** An actions menu shall never act on a + task other than the one it was opened on. There are three ways a menu can stop + matching its subject, and this criterion owns the third. The board row + becoming unresolvable is governed by AC-TASKS-TASK-ACTIONS-MENU-002.6 and + AC-TASKS-TASK-ACTIONS-MENU-004.1c, which demote entries but keep the menu + open. The subject being removed is governed by + AC-TASKS-TASK-ACTIONS-MENU-004.5, which closes it. The subject being *replaced + by a different task* is governed here: when the preview surface's subject task + changes identity while its actions menu is open, which is reachable because + the preview panel re-renders with a different task rather than unmounting, the + system shall close that menu and shall not re-target it at the new subject. + Reopening the trigger then opens a menu on the new subject, per + AC-TASKS-TASK-ACTIONS-MENU-002.6. For this criterion the subject's identity is + the subject task's *identifier*, compared across renders, and not the identity + of the object the surface holds that task in. The system shall not treat a new + task object bearing the same identifier as a change of subject: the preview + rebuilds that object whenever the board's task collections change, so a + field-only update to the subject, or an unrelated task's update anywhere on + the board, shall leave an open menu open, which is what + AC-TASKS-TASK-ACTIONS-MENU-002.6 requires of an in-place entry update. Only a + change of the identifier itself qualifies. A subject replaced by *no* subject + is not this case: the preview losing its subject entirely is governed by + AC-TASKS-TASK-ACTIONS-MENU-004.5 when the task went away and by + AC-TASKS-TASK-ACTIONS-MENU-001.6 for the trigger itself. The detail surface needs no equivalent rule + because it remounts on navigation, which destroys any open menu; the criterion + is stated for both surfaces so that a later change to that behavior does not + silently create the gap. +- **AC-TASKS-TASK-ACTIONS-MENU-004.6:** Repeating an action that already + succeeded, for example archiving a task another client has already archived + (whether by confirming it, or by issuing it directly where + AC-TASKS-TASK-ACTIONS-MENU-003.1a shows no confirmation), shall not corrupt + client state: the system shall reconcile to the server's task state and + surface the failure feedback for the rejected request. Where the server + accepts the repeat instead of rejecting it, as a repeated move does under + AC-TASKS-TASK-ACTIONS-MENU-004.3a and a repeated detach does under + AC-TASKS-TASK-ACTIONS-MENU-004.3b, there is no rejection and the system shall + surface no failure feedback. diff --git a/docs/specs/tasks/requirements/task-actions-menu-outcomes.md b/docs/specs/tasks/requirements/task-actions-menu-outcomes.md new file mode 100644 index 0000000000..946135b20d --- /dev/null +++ b/docs/specs/tasks/requirements/task-actions-menu-outcomes.md @@ -0,0 +1,95 @@ +--- +status: draft +system: tasks +created: 2026-09-03 +owners: + - Kandev +--- + +# Task Actions Menu Action Outcomes Requirements + +## Overview + +The actions menu added to the task preview and task detail surfaces by +[Task Actions Menu on Preview and Detail Surfaces](task-actions-menu.md) reaches +task actions that already exist. This file owns what each confirmed action does +to task state and to navigation, so that an action taken from one of these two +surfaces has the same effect as the same action taken from a Kanban card, and +leaves the user somewhere coherent rather than on a surface whose subject no +longer exists. Terminology and entry content are defined in the parent +requirement and are not restated here; in-flight, concurrent, and failing +actions are owned by +[Task Actions Menu In-Flight and Concurrency](task-actions-menu-concurrency.md). + +## Requirements + +### REQ-TASKS-TASK-ACTIONS-MENU-003: Action outcomes and post-action navigation + +**Intent:** An action taken from one of these surfaces has the same effect on +task state as the same action taken from a card, and leaves the user somewhere +coherent rather than on a surface whose subject no longer exists. + +#### Acceptance criteria + +- **AC-TASKS-TASK-ACTIONS-MENU-003.1:** When the user selects Archive or Delete + from either surface and a confirmation is shown for that action, the system + shall open the same confirmation the card actions menu opens, carrying the + subject task's title and the same cascade choice, and shall make no state + change until the user confirms. Whether a confirmation is shown at all is not + decided by this requirement; see AC-TASKS-TASK-ACTIONS-MENU-003.1a. +- **AC-TASKS-TASK-ACTIONS-MENU-003.1a:** Whether Archive shows a confirmation is + owned by [Archive confirmation](archive-confirmation.md), not by this + requirement. When the archive-confirmation preference is disabled, + `AC-TASKS-ARCHIVE-CONFIRMATION-001.2` archives the requested task alone, + without a dialog and without cascading, from any task surface. These two + surfaces are task surfaces, so the system shall take that same path here and + shall not show a confirmation the card would not show: this is parity under + AC-TASKS-TASK-ACTIONS-MENU-002.1, not an exception to it. On that path the + archive request is issued immediately with no cascade, and no confirm gesture + exists, so every criterion worded around confirming an archive + (AC-TASKS-TASK-ACTIONS-MENU-003.3, AC-TASKS-TASK-ACTIONS-MENU-003.4, + AC-TASKS-TASK-ACTIONS-MENU-004.3 and AC-TASKS-TASK-ACTIONS-MENU-004.6) shall + be read as taking effect when the request is issued. Where the preference has + no stored value the system shall confirm, which is the existing default and + not a new one. Delete is unaffected: + `AC-TASKS-ARCHIVE-CONFIRMATION-001.7` leaves delete confirmations unchanged, + so Delete always confirms. +- **AC-TASKS-TASK-ACTIONS-MENU-003.2:** When the user cancels or dismisses a + confirmation opened from either surface, the system shall leave the task + unchanged, shall leave the surface open on the same subject task, and shall + make no request. +- **AC-TASKS-TASK-ACTIONS-MENU-003.3:** When the user confirms Archive or + Delete from the preview surface and the request succeeds, the system shall + remove the task from the board and close the preview panel, and shall not + navigate to the task detail route. +- **AC-TASKS-TASK-ACTIONS-MENU-003.4:** When the user confirms Archive from the + detail surface and the request succeeds, the system shall use the same + archive-and-switch outcome the existing task-scoped archive entry points use: + the next eligible task opens, or the task overview opens when no eligible + task remains. +- **AC-TASKS-TASK-ACTIONS-MENU-003.5:** When the user confirms Delete from the + detail surface and the request succeeds, the system shall remove the task and + apply the same switch-to-next-task outcome as + AC-TASKS-TASK-ACTIONS-MENU-003.4. +- **AC-TASKS-TASK-ACTIONS-MENU-003.6:** When the user selects a Move to or Send + to workflow target from either surface, the system shall move the subject + task alone to that step, shall keep the surface open on that task, and shall + not navigate. +- **AC-TASKS-TASK-ACTIONS-MENU-003.7:** When a move made from the detail + surface succeeds, the system shall show the new step as current in the top + bar's workflow stepper. +- **AC-TASKS-TASK-ACTIONS-MENU-003.8:** When the user selects Edit from either + surface, the system shall open the existing task edit dialog initialized from + the subject task, and shall not navigate or change the active or previewed + task; saving shall use the existing task update contract and cancelling shall + leave the task unchanged. +- **AC-TASKS-TASK-ACTIONS-MENU-003.9:** When the user selects a Link entry from + either surface, the system shall open the same link dialog the card actions + menu opens for that provider, scoped to the subject task. +- **AC-TASKS-TASK-ACTIONS-MENU-003.10:** When the user confirms Detach from + parent from either surface, the system shall detach the subject task from its + parent and shall leave the surface open on that task. +- **AC-TASKS-TASK-ACTIONS-MENU-003.11:** The system shall introduce no API + endpoint, request payload, persisted field, permission, or feature flag for + any action reached from these surfaces; every action shall use the contract + it already uses from the card actions menu. diff --git a/docs/specs/tasks/requirements/task-actions-menu.md b/docs/specs/tasks/requirements/task-actions-menu.md new file mode 100644 index 0000000000..92f7201279 --- /dev/null +++ b/docs/specs/tasks/requirements/task-actions-menu.md @@ -0,0 +1,302 @@ +--- +status: draft +system: tasks +created: 2026-09-03 +owners: + - Kandev +--- + +# Task Actions Menu on Preview and Detail Surfaces Requirements + +## Overview + +Every task action Kandev offers (edit, move to step, send to workflow, link an +external item, run a plugin task action, detach from parent, archive, delete) is +reachable from a Kanban card, through its visible "more options" button or its +context menu. Two other surfaces show a single task and offer none of them: the +**desktop task preview panel**, whose header renders only Maximize and Close, +and the **desktop task detail top bar**, which renders metrics, plugin actions, +PR/MR/issue status, unarchive, and tools, but no task actions. + +From either surface, archiving the task means returning to the board to find its +card, right-clicking a task switcher sidebar row that only appears on hover, or +knowing the command palette shortcut. This requirement makes the same actions +reachable from both surfaces through the affordance the card already uses. + +The task system owns this contract, not the UI system. Per +[structure and ownership](../../guide/structure-and-ownership.md), user +visibility does not make a capability UI-owned: the contract is *which task +actions are reachable, from which task surface, and what each does to task state +and navigation*, which has no meaning without task state. The UI system keeps +the menu, dropdown, and popover primitives this composes. + +## Terminology + +- **Actions menu:** The menu of task actions this requirement places on a + surface, opened from a visible trigger. +- **Card actions menu:** The existing menu opened from a Kanban card's visible + "more options" button. It is this capability's reference for content and + ordering. +- **Preview surface:** The desktop task preview panel rendered beside the + Kanban board. It has no mobile equivalent. +- **Detail surface:** The desktop task detail top bar rendered above an open + task. It has a separate mobile top bar, which is out of scope. +- **Subject task:** The single task an actions menu acts on. On the preview + surface it is the previewed task; on the detail surface it is the open task. +- **Control group:** A cluster of controls rendered together at one end of a + surface's header or top bar. On the preview surface it is the single group + holding Maximize and Close; on the detail surface it is the top bar's + right-hand group, whose last member today is the tools cluster. +- **Board row:** The task's entry in the board's task collections. Entries such + as Edit, Detach, Move to, Send to workflow, and Link need fields that only + the board row carries. + +## Prior art + +**Wiki leg (receipt).** Searched the `@henry`-pinned vault +`OBSIDIAN_VAULT_PATH=/Users/henry/Documents/henry/wiki`, collection +`QMD_WIKI_COLLECTION="wiki"`. No `qmd` MCP server and no `obsidian-wiki` CLI, so +both the semantic and GraphRAG passes were skipped; the grep fallback then hit +`Operation not permitted` on every content read while `ls -la` on the same paths +succeeded. **This leg returned nothing useful.** + +**Cross-product leg (receipt).** Intended query: `saas-kb` `search_fsm_docs`, +`category: "ai_sdlc"`. Server and tool not connected to this session. +**This leg returned nothing useful.** + +**In-product prior art (read directly; the basis for the decisions below).** + +- The card actions menu is already built by a pure, surface-independent builder + emitting an entry list that a dropdown or a context-menu renderer draws. + Bringing it to a new surface is wiring, not a new menu. +- [Sidebar Task Editing](sidebar-task-edit.md) is the same shape of change, + owned by this system, and set the precedent that a second surface reuses the + first's dialog and update contract rather than inventing its own. +- The task switcher sidebar already exposes task actions from a visible dots + button, but only on hover, and only per sidebar row. Its archive flow is the + same shared confirmation the PR banners and command palette use. +- The command palette already offers Archive from the detail surface, but it is + keyboard-driven and not a visible affordance, so it does not satisfy this. + +**What this capability does differently:** it standardises on the *card's* +entry set and ordering rather than the sidebar's divergent one, because two +menus that already disagree should not become four. + +## Requirements + +### REQ-TASKS-TASK-ACTIONS-MENU-001: Actions menu trigger on the preview and detail surfaces + +**Intent:** A user looking at a single task can reach that task's actions from +the surface they are already on, without returning to the board, hovering to +reveal a control, or knowing a keyboard shortcut. + +#### Acceptance criteria + +- **AC-TASKS-TASK-ACTIONS-MENU-001.1:** When the preview surface has a subject + task, the system shall render an actions-menu trigger in the preview header's + control group, positioned before the Maximize control, with the accessible + name `More options`. +- **AC-TASKS-TASK-ACTIONS-MENU-001.2:** When the detail surface has a subject + task, the system shall render an actions-menu trigger in the top bar's right + control group, positioned after every other control in that group, with the + accessible name `More options`. +- **AC-TASKS-TASK-ACTIONS-MENU-001.3:** The system shall render each trigger at + full opacity whenever its surface is rendered, without requiring pointer + hover or keyboard focus to reveal it. +- **AC-TASKS-TASK-ACTIONS-MENU-001.4:** The system shall expose each trigger + with `aria-haspopup="menu"` and an `aria-expanded` value that is `false` while + its menu is closed and `true` while its menu is open. +- **AC-TASKS-TASK-ACTIONS-MENU-001.5:** The system shall give the preview + trigger the stable test id `task-preview-actions-menu` and the detail trigger + the stable test id `task-topbar-actions-menu`, so that a test can address one + surface while the board's card triggers carry the same accessible name. +- **AC-TASKS-TASK-ACTIONS-MENU-001.6:** When the preview surface has no subject + task, the system shall render no actions-menu trigger, and shall leave the + Close control unchanged. +- **AC-TASKS-TASK-ACTIONS-MENU-001.7:** When the detail surface has no subject + task identifier, the system shall render no actions-menu trigger. +- **AC-TASKS-TASK-ACTIONS-MENU-001.8:** When the user activates a trigger, the + system shall open that surface's actions menu right-aligned to the trigger, + and shall not navigate, shall not change the active or previewed task, shall + not close the preview panel, and shall not activate any other control on the + surface. +- **AC-TASKS-TASK-ACTIONS-MENU-001.9:** When an actions menu is open and the + user presses Escape, the system shall close the menu and return keyboard + focus to the trigger that opened it. +- **AC-TASKS-TASK-ACTIONS-MENU-001.10:** The system shall introduce no new + localization key for either trigger, reusing the existing `More options` + string, which is already present in every shipped catalog. +- **AC-TASKS-TASK-ACTIONS-MENU-001.11:** When an actions menu is open on the + preview surface and the user presses Escape, the system shall close only the + menu and shall leave the preview panel open on the same subject task; a + second Escape, with no menu open, shall then close the preview panel as it + does today. +- **AC-TASKS-TASK-ACTIONS-MENU-001.12:** When a dialog opened from an actions + menu closes, whether confirmed, cancelled, or dismissed, and its surface is + still mounted on the same subject task, the system shall return keyboard + focus to the trigger that opened the menu. +- **AC-TASKS-TASK-ACTIONS-MENU-001.13:** The system shall place each trigger in + the document tab order at its rendered position within its control group, so + a keyboard user reaches it by tabbing through that group and opens its menu + with Enter or Space. + +### REQ-TASKS-TASK-ACTIONS-MENU-002: Menu content parity with the card actions menu + +**Intent:** A user learns one task menu. The same task offers the same actions, +in the same order, with the same labels, wherever the menu is opened. + +#### Acceptance criteria + +- **AC-TASKS-TASK-ACTIONS-MENU-002.1:** When an actions menu is open on a + subject task that is not archived, the system shall present the same entries, + in the same order, with the same labels, the same submenu nesting, and the + same enabled or disabled state as the card actions menu presents for that + same task at that same moment, except where + AC-TASKS-TASK-ACTIONS-MENU-002.4 through + AC-TASKS-TASK-ACTIONS-MENU-002.9 state otherwise, except for plugin + `edit`-group actions (AC-TASKS-TASK-ACTIONS-MENU-002.2a), and except for the + in-flight disabled state, which is per surface under + AC-TASKS-TASK-ACTIONS-MENU-004.1b and so may differ from the card's for the + duration of a request. +- **AC-TASKS-TASK-ACTIONS-MENU-002.2:** The order named in + AC-TASKS-TASK-ACTIONS-MENU-002.1 is: Edit, Priority, Move to, Send to + workflow, plugin primary actions, Link, Archive, Detach from parent, a + separator, Delete. The system shall omit Priority when the card actions menu + omits it. It shall omit any of Move to, Send to workflow, Link, and Detach + from parent whose availability condition in the card actions menu is unmet, + and shall not reorder the entries that remain. +- **AC-TASKS-TASK-ACTIONS-MENU-002.2a:** The Edit entry on these two surfaces + shall always be the flat Edit item, never the card's submenu form: the system + shall not present plugin task-menu actions registered with group `edit` on + either surface. Group `edit` is a card-only plugin contract and this + requirement does not widen it; group `primary` is unaffected and appears per + AC-TASKS-TASK-ACTIONS-MENU-002.2. +- **AC-TASKS-TASK-ACTIONS-MENU-002.3:** Within the Move to and Send to workflow + submenus the system shall order steps by ascending `position`, breaking a tie + by ascending step `id`. The new surfaces shall use the same order as the card. +- **AC-TASKS-TASK-ACTIONS-MENU-002.3a:** Within the Send to workflow submenu the + system shall order workflows in the order the workflow collection holds them, + which is the order the card actions menu uses, and shall omit workflows marked + hidden. +- **AC-TASKS-TASK-ACTIONS-MENU-002.3b:** The system shall exclude from a Move to + target list every step the user has hidden for the subject task's current + workflow, matching the card, whose current-workflow targets are already + hidden-filtered. Within Send to workflow the system shall not filter another + workflow's steps by hidden state, also matching the card. This asymmetry is + inherited deliberately; see `## Out of scope`. +- **AC-TASKS-TASK-ACTIONS-MENU-002.3c:** The system shall never present the + display-only orphan sentinel step (`__kandev_orphan__`, labelled "Needs + Reassignment") as a move target on either surface. +- **AC-TASKS-TASK-ACTIONS-MENU-002.4:** When the subject task is archived, the + system shall omit Edit, Move to, Send to workflow, Link, Detach from parent, + and Archive from the actions menu, and shall present exactly, in this order: + the admitted plugin primary actions, then a separator, then Delete. The card + has no archived branch to inherit from, so the order is stated in full here; + it is the card's own order with the omitted entries removed. +- **AC-TASKS-TASK-ACTIONS-MENU-002.4c:** When no plugin primary action is + admitted for an archived subject, the system shall present Delete alone, with + no leading separator, so the menu never opens on a separator. +- **AC-TASKS-TASK-ACTIONS-MENU-002.4a:** AC-TASKS-TASK-ACTIONS-MENU-002.4 takes + precedence over AC-TASKS-TASK-ACTIONS-MENU-002.5 whenever both apply. An + archived subject is normally board-row-unresolvable too, since the board + excludes archived tasks; in that overlap the archived rule governs, so the + system shall present Delete and not Archive. +- **AC-TASKS-TASK-ACTIONS-MENU-002.4b:** "The admitted plugin primary actions" + means the ordinary `visible(context)` result for the `primary` group, in + plugin registration order, evaluated against the unchanged plugin task-menu + context. The system shall not add an archived field to that + context, and shall not filter plugin actions by archived state: the context + carries no archived signal, and a plugin that should hide itself for an + archived task owns that. +- **AC-TASKS-TASK-ACTIONS-MENU-002.5:** When the subject task's board row is + not resolvable, the system shall omit Edit, Move to, Send to workflow, Link, + and Detach from parent, and shall present Archive and Delete, which need only + the task identifier. This criterion yields to + AC-TASKS-TASK-ACTIONS-MENU-002.4 when the subject is also archived. +- **AC-TASKS-TASK-ACTIONS-MENU-002.6:** The entry set shall be live, not + snapshotted when the menu opens. When the subject task's board row becomes + resolvable, or stops being resolvable, the system shall update that surface's + entries in place, whether its menu is currently open or closed, without + requiring a page reload and without closing an open menu. When the subject + task itself is removed rather than its row merely becoming unresolvable, + AC-TASKS-TASK-ACTIONS-MENU-004.5 governs and the menu closes; when the subject + is replaced by a different task, AC-TASKS-TASK-ACTIONS-MENU-004.5a governs. +- **AC-TASKS-TASK-ACTIONS-MENU-002.7:** When the board holds a multi-task + selection that includes the subject task, the system shall still act on the + subject task alone, shall present the single-task labels rather than any + count-bearing label, and shall leave the selection unchanged. Entry + membership shall not depend on the selection either: the system shall still + present Detach from parent for a subject that has a parent, even while a board + multi-selection includes it. This removes the selection as a reason to omit + Detach; it does not override AC-TASKS-TASK-ACTIONS-MENU-002.4 or + AC-TASKS-TASK-ACTIONS-MENU-002.5, which still omit Detach for an archived + subject and for an unresolved board row. The card suppresses Detach under a + multi-selection because its own menu can act on that selection; these surfaces + never can, so that cause is absent here. +- **AC-TASKS-TASK-ACTIONS-MENU-002.8:** When a plugin registers, is enabled, or + is disabled while a surface is mounted, the system shall reflect that change + in that surface's plugin entries immediately, including in a menu that is + already open, and without closing it. Plugin entries are not a separate tier: + they are live on the same terms as every other entry under + AC-TASKS-TASK-ACTIONS-MENU-002.6, which is what the card already does, since + it subscribes to the plugin registry and rebuilds its entries each render. +- **AC-TASKS-TASK-ACTIONS-MENU-002.9:** The system shall present no entry on + these two surfaces that the card actions menu does not present for a single, + unselected task, and shall present no bulk-selection variant of the menu. +- **AC-TASKS-TASK-ACTIONS-MENU-002.10:** The system shall leave the card actions + menu, the card context menu, and the task switcher sidebar menu unchanged in + entry membership, labels, top-level order, and behavior, with one named + exception. Adopting `sortWorkflowStepsByPosition` inside the shared + move-target derivation also applies its ascending-`id` tiebreak to the steps + the card lists under Send to workflow, which are today sorted by `position` + alone. That changes rendered order only where two steps of a non-current + workflow share a `position` value, a case whose order is arbitrary today. No + other card-facing change is permitted; the card's Move to submenu is + unaffected, already sorting through that helper. + +### Action outcomes and post-action navigation + +`REQ-TASKS-TASK-ACTIONS-MENU-003` covers what each confirmed action does to task +state and navigation. It lives in +[Task Actions Menu Action Outcomes](task-actions-menu-outcomes.md). + +### In-flight, concurrent, and failing actions + +`REQ-TASKS-TASK-ACTIONS-MENU-004` covers in-flight, concurrent, and failing +actions. It lives in +[Task Actions Menu In-Flight and Concurrency](task-actions-menu-concurrency.md). + +## Out of scope + +- **The mobile task detail top bar.** Different chrome budget, and mobile + already routes task actions through the task switcher sheet's per-row menus. + A follow-up wanting parity there should start from that sheet's menu. +- **The mobile preview surface.** There is none: on mobile the board navigates + straight to the task detail route and never renders the preview panel, so + there is no surface to add a trigger to. +- **Changing the card actions menu, the card context menu, or the task + switcher sidebar menu.** Their entry sets, ordering, and labels are frozen by + this requirement, not revised by it. +- **Reconciling the card menu's entry set with the task switcher sidebar's + divergent one.** The sidebar offers Pin, Rename, Create subtask, Duplicate, + Color, and Nest, and omits several the card has. These surfaces standardise on + the card's set; the divergence stays. +- **Bulk or multi-task actions from these surfaces.** Both surfaces address a + single task; the board's multi-select toolbar remains the bulk path. +- **Unarchive.** The detail top bar already renders a dedicated Unarchive + control for an archived task, and this requirement does not duplicate it into + the menu. +- **A right-click context menu on either surface.** This requirement adds the + visible trigger only; the card keeps its context menu. +- **Correcting the card's hidden-step asymmetry in move targets.** The card + filters user-hidden steps out of Move to but not out of Send to workflow. + `AC-TASKS-TASK-ACTIONS-MENU-002.3b` reproduces that asymmetry rather than + fixing it, because fixing it would change card behavior that + `AC-TASKS-TASK-ACTIONS-MENU-002.10` freezes. A follow-up should change both + surfaces together and update the card's move-target tests. +- **Presenting plugin `edit`-group actions on these surfaces.** That group is + documented as card-only, and `AC-TASKS-TASK-ACTIONS-MENU-002.2a` keeps it that + way. Widening it changes a published plugin contract and every registered + plugin's expectations, so it is a plugin-system decision, not this one's. +- **Any new backend endpoint, persistence model, permission, feature flag, or + localization key.** diff --git a/docs/specs/tasks/system-design/task-actions-menu.md b/docs/specs/tasks/system-design/task-actions-menu.md new file mode 100644 index 0000000000..1df970b363 --- /dev/null +++ b/docs/specs/tasks/system-design/task-actions-menu.md @@ -0,0 +1,362 @@ +--- +status: draft +system: tasks +requirements: + - REQ-TASKS-TASK-ACTIONS-MENU-001 + - REQ-TASKS-TASK-ACTIONS-MENU-002 + - REQ-TASKS-TASK-ACTIONS-MENU-003 + - REQ-TASKS-TASK-ACTIONS-MENU-004 +--- + +# Task Actions Menu on Preview and Detail Surfaces System Design + +## Purpose and boundaries + +This design places the existing task actions menu on the desktop preview panel +and the desktop task detail top bar. It owns how a surface obtains the subject +task, how the shared entry list is built for that surface, and what each +confirmed action does to task state and navigation. + +Contracts this design uses but does not own: + +- The task update, archive, delete, move, detach, and link request contracts, + owned by their existing task-system requirements. +- The archive confirmation contract, owned by + [Archive confirmation](../requirements/archive-confirmation.md). +- The dropdown, context-menu, popover, and dialog primitives, owned by the UI + system. +- The plugin task-menu action contract, owned by the plugin system. + +## Requirement mapping + +| Requirement | Design section | +| --- | --- | +| `REQ-TASKS-TASK-ACTIONS-MENU-001` | [Components and responsibilities](#components-and-responsibilities) | +| `REQ-TASKS-TASK-ACTIONS-MENU-002` | [Data and contracts](#data-and-contracts) | +| `REQ-TASKS-TASK-ACTIONS-MENU-003` ([own file](../requirements/task-actions-menu-outcomes.md)) | [Control flow](#control-flow) | +| `REQ-TASKS-TASK-ACTIONS-MENU-004` ([own file](../requirements/task-actions-menu-concurrency.md)) | [Failure and recovery](#failure-and-recovery) | + +## Components and responsibilities + +**Shared menu state (new).** The Kanban card today builds its entry list and +its dialog state inside a card-scoped hook that also carries board concerns: +board-supplied edit/delete/archive/move callbacks, the multi-selection, and the +card's own step list. Parity in +`AC-TASKS-TASK-ACTIONS-MENU-002.1` cannot be guaranteed by a second, +independent implementation, so the surface-independent part of that hook is +extracted into a shared hook that takes a subject task and a presentation, and +returns the entry list plus the dialog state. The card hook keeps the +board-only concerns (multi-selection move targets, board callbacks) and +composes the shared one. The extracted hook carries no pending state: each +caller supplies its own in-flight flags, which is what keeps the card's existing +board-scoped flags working untouched and what makes the surface-local rule below +fall out of the structure rather than needing enforcement. + +This extraction sits on the board's hot render path, one instance per visible +card. It must not change the card's rendered entries, their identity keys, or +the number of store subscriptions per card. That is the constraint the +implementation is held to, and the existing card menu tests are the evidence. + +**Preview trigger (new).** Renders inside the preview header's control group, +before Maximize. The preview already receives the full task object, so its +subject task needs no store lookup. + +**Detail trigger (new).** Renders inside the top bar's right control group, +after the tools cluster. The top bar receives only identifiers, so it resolves +its board row from the store (see below) and degrades per +`AC-TASKS-TASK-ACTIONS-MENU-002.5` when the row is absent. + +**Dialog host (new, one per surface).** Mounts the confirmation and link +dialogs the entries open. Each surface hosts its own so a dialog outlives the +menu that opened it and is not nested inside a portal that unmounts on select. + +## Data and contracts + +**Subject task resolution.** + +- Preview surface: the task object the panel already receives. When it is + `null` the trigger is not rendered. +- Detail surface: the task identifier from the route, plus a board-row lookup + against the workflow snapshots with the flat task list as fallback (the same + lookup the sidebar and the shared archive-confirmation hook already perform + from inside the detail view, so the snapshots are populated there). + +**Entry availability tiers.** Two tiers, which is what makes +`AC-TASKS-TASK-ACTIONS-MENU-002.5` implementable: + +- *Identifier-only*: Archive, Delete. Available whenever a task identifier is + known. +- *Board-row*: Edit, Priority, Move to, Send to workflow, Link, Detach from + parent. Each needs fields carried by the board row (priority, workflow + membership, step, repositories, or parent task id), and is omitted while + that row is unresolved. + +The tiers describe availability, not precedence. An archived subject is filtered +by the archived rule first, which is why Archive -- an identifier-only entry -- +is still absent for it. + +The tiers are evaluated live. The entry list is derived from store state on each +render rather than captured when the menu opens, so a board row that resolves +while the menu is open promotes the board-row entries in place +(`AC-TASKS-TASK-ACTIONS-MENU-002.6`). Nothing needs to close and reopen the menu +to converge. Two things do close it: the subject task disappearing +(`AC-TASKS-TASK-ACTIONS-MENU-004.5`) and the subject being replaced by a +different task (`AC-TASKS-TASK-ACTIONS-MENU-004.5a`). Losing the board row is +neither, and demotes entries in place. + +**There is no memoized-at-open tier.** Plugin entries are live on exactly the +same terms as every other entry (`AC-TASKS-TASK-ACTIONS-MENU-002.8`), including +inside a menu that is currently open. This matches the card rather than relaxing +against it: `usePluginRegistry()` is a `useSyncExternalStore` subscription that +re-renders the card on any registry mutation, and the card's entry list is +rebuilt unmemoized on each render, so an action a plugin registers after the +card mounted already appears and one whose plugin was just disabled already +stops appearing. A single liveness rule for every tier is also the cheaper +contract to hold: a plugin-only exception would need its own snapshot point, +its own invalidation, and its own tests, to buy nothing a user can see. + +**Ordering.** The entry order is fixed by the shared builder, so parity is +structural rather than asserted per surface. + +For a normal resolved row, the top-level order is Edit, Priority, Move to, Send +to workflow, plugin primary actions, Link, Archive, Detach from parent, a +separator, and Delete. The Priority submenu uses the row's current priority and +the same update hook as the card. If the card builder omits an entry because its +callback is unavailable, the new surface omits that entry too. + +Step ordering is ascending `position`, tie-broken by ascending step `id`. That +rule is already implemented, once, by `sortWorkflowStepsByPosition` +(`lib/kanban/auto-hide-empty-columns.ts`), and the card's Move to submenu +already renders through it: the board derives its move targets with that helper +and passes them down, and the shared move-target derivation then uses that +supplied list verbatim for the current workflow. So the tiebreak is not new +behavior being introduced here, and the card's Move to submenu does not change. + +The one place the shared derivation is weaker is *other* workflows: it sorts +their steps with `position` alone. Adopting `sortWorkflowStepsByPosition` there +gives the new surfaces the same order the card's Move to already has, and +changes the card's Send to workflow submenu only where two steps of a +non-current workflow share a `position` -- an order that is arbitrary today. +`AC-TASKS-TASK-ACTIONS-MENU-002.10` names that as its one permitted exception. + +Two exclusions travel with the target list. User-hidden steps are filtered out +for the current workflow and not for other workflows, which is what the card +does today and what `AC-TASKS-TASK-ACTIONS-MENU-002.3b` reproduces rather than +corrects. The orphan sentinel `__kandev_orphan__` ("Needs Reassignment") is a +display-only node synthesized per view and never reaches a snapshot, so it is +absent from these surfaces' targets by construction; the criterion states it so +that a future view-specific target list cannot leak it in. + +Workflow ordering is the workflow collection's own order, minus hidden +workflows, unchanged. + +**Plugin action groups.** Only the `primary` group crosses to these surfaces. +Group `edit` actions, which the card nests under its Edit entry, are documented +as card-only (`docs/plans/plugins/PLUGIN-API.md`), and this design does not +widen that contract from the task system's side, so the Edit entry here is +always the flat item and never the card's submenu form +(`AC-TASKS-TASK-ACTIONS-MENU-002.2a`). Concretely, `buildEditMenuEntry` returns +a flat item when no `edit`-group action is visible and a submenu when one is; +these surfaces take the flat form unconditionally rather than calling that +builder with the plugin context. + +**Archived subject.** The archived case reaches only the detail surface: the +board excludes archived tasks, so the preview panel cannot hold one. That same +exclusion is why an archived subject is normally board-row-unresolvable too, so +the archived rule and the unresolved-row rule overlap by default rather than by +accident. `AC-TASKS-TASK-ACTIONS-MENU-002.4a` resolves the overlap in one +direction: archived wins, and Archive is not offered on a task that is already +archived. + +The archived entry set is therefore the admitted plugin primary actions, a +separator, and Delete -- stated in full because the card menu has no archived +branch for it to inherit from (`buildKanbanCardMenuEntries` takes no archived +input). "Admitted" is the ordinary `visible(context)` result in registration +order; the plugin task-menu context carries no archived signal and this design +does not add one, so archived-awareness stays the plugin's own concern. + +The detail surface already receives `isArchived`, so no new input is needed to +evaluate this branch. + +**Accessible names and test ids.** Both triggers use the existing +`More options` string. Because the preview renders beside a board whose cards +carry the same accessible name, each trigger also carries a surface-specific +test id (`task-preview-actions-menu`, `task-topbar-actions-menu`) so a test can +address one surface unambiguously. + +**Localization.** No new key. Every entry label and both trigger names already +exist in `en`, `pt-pt`, `zh-cn`, `zh-tw`, `zh-hk`, and `pseudo`. + +## Control flow + +1. The user activates a surface's trigger. The surface opens its menu. No + navigation, no active-task change, no preview close: the trigger stops + pointer and click propagation so the surrounding header controls and the + board's card click dispatch never see the event. +2. Selecting an entry closes the menu and either opens a dialog hosted by that + surface or dispatches a move. +3. Archive and Delete open the shared confirmation; nothing is requested until + the user confirms. +3a. Except when the archive-confirmation preference is off. That preference is + read *inside* the shared confirmation component, not by its callers: when it + is disabled the component renders no dialog, immediately invokes its confirm + callback with no cascade, and closes. A surface that mounts the same shared + confirmation therefore inherits the preference-honouring path for free, which + is why `AC-TASKS-TASK-ACTIONS-MENU-003.1a` is parity with the card rather + than an exception to it: the card mounts that same component. Neither new + surface reads the preference itself, and this design adds no second bypass. +4. On confirmed Archive or Delete: + - Preview surface: the removal prunes the task from the board collections. + The preview's existing missing-subject guard then closes the panel. No + new navigation code path is introduced. + - Detail surface: the removal routes through the existing + archive-and-switch and remove-from-board flows, which pick the next + eligible task or fall back to the task overview. +4a. Escape while a preview menu is open must close only the menu + (`AC-TASKS-TASK-ACTIONS-MENU-001.11`). The preview panel's own Escape + handler is currently a bare `window` keydown listener that does not consult + `defaultPrevented`, so without a change it would close the panel on the same + keypress that closes the menu. The panel's handler is the one that changes: + it must ignore an Escape the open menu has already handled. The menu + primitive's own Escape behavior is not modified. +4b. The preview panel re-renders with a different subject task rather than + unmounting, so an open menu can outlive the task it was opened on, and its + terminal entries include Delete. Both triggers therefore use the default + modal dropdown behavior the card already uses, so a single outside click + dismisses the menu rather than also landing on another card; and the preview + additionally closes its menu when the subject's identity changes + (`AC-TASKS-TASK-ACTIONS-MENU-004.5a`). Modality alone is not relied on: it is + a presentation default a later `modal={false}` could remove, whereas the + identity check is behavioral. That check compares the subject's task *id* + across renders, never the identity of the task object: `useSelectedTask` + memoizes on the board's task collections and returns a freshly constructed + object, so its result gets a new reference whenever any task on the board + changes while the selected id stays put. Keying the close on the object would + therefore close the menu on unrelated board churn and contradict + `AC-TASKS-TASK-ACTIONS-MENU-002.6`, which requires an open menu to survive an + in-place entry update. The detail route remounts on navigation and needs + neither. +5. On a move: the subject task alone is moved. Both surfaces stay on the task. + The detail top bar's stepper re-reads the current step from state. +5a. If the board row is lost while a Move to or Send to workflow submenu is + open, the live demotion of `AC-TASKS-TASK-ACTIONS-MENU-002.6` removes the + submenu's parent entry, so the submenu closes with it and the top-level menu + stays open on the identifier-only entries. A move already in flight is not + cancelled: it was dispatched against the server, which remains the arbiter, + and it lands or fails on its own terms + (`AC-TASKS-TASK-ACTIONS-MENU-004.1c`). This is a rare flash rather than a + state machine -- the row is lost by a store update, and the alternative, + pinning an open submenu to a target list the store no longer backs, would + mean offering move targets that may no longer exist. +6. Edit opens the existing edit dialog seeded from the subject task; save uses + the existing task update contract. + +## Failure and recovery + +**In-flight state is surface-local, and that is a decision, not an oversight.** +Each surface owns the pending flag for the requests it starts. There is no +task-keyed pending registry, and this design does not add one +(`AC-TASKS-TASK-ACTIONS-MENU-004.1b`). + +The alternative -- lifting archive/delete/detach pending state into shared, +task-keyed store state so every surface showing the task disables together -- +was rejected. It buys strict parity during a window that is hard to even +observe (`AC-TASKS-TASK-ACTIONS-MENU-004.2` closes the menu on a terminal +activation, and archive/delete/detach are terminal, so the +disabled state is only reachable by reopening mid-flight), and it pays for that +with a new piece of client state architecture on the board's hot render path. +It would also be a divergence dressed as parity: the card has no such registry +either. `isDeleting` and `isArchiving` arrive as props from board-level state +(`deletingTaskId === task.id` in `virtualized-column-task-list.tsx` and +`swimlane-graph2-content.tsx`), and `isDetaching` comes from `useDetachTask`, +whose `detachingTaskId` is a `useState` inside the hook and therefore local to +each caller. Two cards for different tasks already do not share it, and nothing +outside the board can read it. + +The consequence is explicit and testable: a card mid-archive and a preview panel +open on that same task will show different enabled states until the request +settles. `AC-TASKS-TASK-ACTIONS-MENU-002.1` names this as its one in-flight +exception rather than leaving the parity rule to over-claim. + +- In-flight archive, delete, and detach requests mark that surface's entries + disabled, and the menu is already closed by the selection, so a second + request cannot be started from the same menu. The disabled state is therefore + observable by reopening the menu mid-flight, not by watching the menu that + dispatched the request. +- Move to, Send to workflow, and Link are outside that rule by decision, not by + omission (`AC-TASKS-TASK-ACTIONS-MENU-004.1a`). Link starts no task-state + request on select -- it opens a dialog, which owns its own submit state. Move + carries no menu-level pending state on the card either, so adding one here + would be a new divergence rather than parity. Two moves issued in quick + succession are two independent requests and the server arbitrates, which is + the same rule the detail surface's own workflow stepper already applies with + its last-request-wins guard. +- Two surfaces acting on one task issue independent requests. The server is the + arbiter: the losing request fails and raises the existing per-action failure + feedback. No new error surface, no client-side lock across surfaces. +- Detach is the exception, because it is idempotent rather than one-shot + (`AC-TASKS-TASK-ACTIONS-MENU-004.3b`). Two existing layers already guarantee + that, and this design leans on both rather than adding a third: the client + detach request layer keys in-flight requests by task id and hands a second + caller the existing promise instead of issuing another request, and the detach + endpoint treats a detach of an already-parentless task as success with no row + update. A second detach of one task therefore produces no second request and + no failure feedback. This is a REQUEST-level coalesce and does not contradict + `AC-TASKS-TASK-ACTIONS-MENU-004.1b`, which governs the per-surface pending + flag behind the disabled state, not the request layer. +- When the losing request fails *because* the subject was archived or deleted by + another actor, the subject is gone, so the surface's missing-task behavior + wins over the keep-the-surface-open rule + (`AC-TASKS-TASK-ACTIONS-MENU-004.4a`). The failure feedback is raised at + application level, so it still reaches the user after the preview has closed + or the detail route has switched away. +- **That cause is detected from store state, not from the failed response, and + that is the authoritative mechanism rather than an incidental race.** The + surface never inspects the rejected request to decide whether the subject + survived; it reacts to the subject leaving the store, which is the same signal + `AC-TASKS-TASK-ACTIONS-MENU-004.5` already runs on. The response could not + carry that decision anyway without a backend change this capability does not + make: a delete of a removed task resolves through the handler's not-found + branch, but the already-archived sentinel matches none of the handler's typed + branches and is answered as an undifferentiated server error, so the archive + path has no distinguishable losing-race status to read. Ordering between the + rejection and the store update is therefore not load-bearing. If the rejection + lands first, the surface stays open, shows the failure feedback, and closes + when the store update arrives; if the store update lands first, the surface is + already gone and the application-level feedback still reaches the user. If the + store update never arrives, the surface simply stays open under + `AC-TASKS-TASK-ACTIONS-MENU-004.4`, which is the safe default. +- A failed request leaves the task at its last confirmed state and leaves the + surface open on it. The archive-and-switch flow already restores the previous + active task and route when its archive request rejects after an optimistic + switch. +- A task removed by another client is handled by each surface's existing + missing-subject behavior. The menu is closed by then, because the removal is + observed through state rather than through a menu interaction. + +## Persistence + +None. This capability adds no storage, no migration, and no restart behavior. +All state is client-side and derived from the existing task store. + +## Security + +None added. Every action reuses its existing request, so backend authorization +is unchanged and no surface bypasses it. + +## Observability + +None added. Failures surface through the existing per-action feedback, and +failed archive and delete requests keep their existing console diagnostics. + +## Testing + +- Unit: shared builder parity for a not-archived task, an archived task, and an + unresolved board row; the step ordering tiebreak. +- Component: each trigger renders, opens, closes on Escape with focus returned, + and does not activate its neighbouring controls. +- End-to-end: the two surfaces are user-visible, so each gets a scenario that + opens the menu by its surface test id and confirms Archive, asserting the + post-action outcome named in `REQ-TASKS-TASK-ACTIONS-MENU-003`. The existing + Kanban card-menu scenarios must continue to pass unchanged, as the evidence + for `AC-TASKS-TASK-ACTIONS-MENU-002.10`.