fix(web): stop task context menu from starting a row drag (#2662)

This commit is contained in:
Clément Fiere
2026-08-14 16:56:11 +02:00
committed by GitHub
parent cb0c10aa9f
commit 4d54c20041
8 changed files with 893 additions and 7 deletions
@@ -0,0 +1,291 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { StateProvider } from "@/components/state-provider";
import { ToastProvider } from "@/components/toast-provider";
import { TaskItemWithContextMenu } from "./task-switcher-context-menu";
import type { TaskSwitcherItem } from "./task-switcher-types";
afterEach(() => cleanup());
function task(overrides: Partial<TaskSwitcherItem> = {}): TaskSwitcherItem {
return { id: "task-1", title: "Task 1", state: "IN_PROGRESS", ...overrides };
}
/**
* Stands in for the dnd-kit drag handle that wraps the row. In the real tree
* the handle's sensor listeners (`onMouseDown`, `onTouchStart`, `onPointerDown`)
* are fiber ancestors of the context-menu portal, so a pointer-start event on a
* menu item fiber-bubbles into them and starts a row drag — unless the menu
* stops it. `onClick` stands in for the row's `onSelectTask` click handler.
*/
function renderWithDragHandle(overrides: Partial<TaskSwitcherItem> = {}) {
const onMouseDown = vi.fn();
const onPointerDown = vi.fn();
const onTouchStart = vi.fn();
const onClick = vi.fn();
const onArchiveTask = vi.fn();
render(
<StateProvider>
<ToastProvider>
<div
data-testid="drag-handle"
onMouseDown={onMouseDown}
onPointerDown={onPointerDown}
onTouchStart={onTouchStart}
onClick={onClick}
>
<TaskItemWithContextMenu task={task(overrides)} onArchiveTask={onArchiveTask}>
<div data-testid="task-row">Task 1</div>
</TaskItemWithContextMenu>
</div>
</ToastProvider>
</StateProvider>,
);
return { onMouseDown, onPointerDown, onTouchStart, onClick, onArchiveTask };
}
async function openContextMenu() {
fireEvent.contextMenu(screen.getByTestId("task-row"));
await screen.findByRole("menuitem", { name: /color/i });
}
// happy-dom's TouchEvent drops the touches/changedTouches init, so the touch
// events in the cancellation tests stub it faithfully.
function stubTouchEvent() {
vi.stubGlobal(
"TouchEvent",
class StubTouchEvent extends Event {
touches: unknown[];
changedTouches: unknown[];
constructor(
type: string,
init?: EventInit & { touches?: unknown[]; changedTouches?: unknown[] },
) {
// Forward bubbles/cancelable so the stub faithfully models the
// production `new TouchEvent("touchcancel", { bubbles: true,
// cancelable: true })` dispatch.
super(type, init);
this.touches = init?.touches ?? [];
this.changedTouches = init?.changedTouches ?? [];
}
},
);
}
describe("TaskItemWithContextMenu — pointer containment", () => {
// Regression: the menu renders in a portal whose fiber ancestors include the
// drag handle. Without a guard, mousedown/pointerdown on any menu item
// bubbles through the React fiber tree to the handle's dnd-kit sensor
// listeners and starts a row drag (MouseSensor ignores only right-click).
it("mousedown and pointerdown on the Color submenu trigger do not reach the drag handle", async () => {
const { onMouseDown, onPointerDown } = renderWithDragHandle();
await openContextMenu();
fireEvent.mouseDown(screen.getByRole("menuitem", { name: /color/i }));
fireEvent.pointerDown(screen.getByRole("menuitem", { name: /color/i }));
expect(onMouseDown).not.toHaveBeenCalled();
expect(onPointerDown).not.toHaveBeenCalled();
});
it("touchstart on the Color submenu trigger and swatch does not reach the drag handle", async () => {
const { onTouchStart } = renderWithDragHandle();
await openContextMenu();
fireEvent.touchStart(screen.getByRole("menuitem", { name: /color/i }));
expect(onTouchStart).not.toHaveBeenCalled();
fireEvent.pointerMove(screen.getByRole("menuitem", { name: /color/i }), {
pointerType: "mouse",
});
const redSwatch = await screen.findByRole("menuitem", { name: /red/i });
fireEvent.touchStart(redSwatch);
expect(onTouchStart).not.toHaveBeenCalled();
});
it("mousedown and pointerdown on a color swatch do not reach the drag handle", async () => {
const { onMouseDown, onPointerDown } = renderWithDragHandle();
await openContextMenu();
fireEvent.pointerMove(screen.getByRole("menuitem", { name: /color/i }), {
pointerType: "mouse",
});
const redSwatch = await screen.findByRole("menuitem", { name: /red/i });
fireEvent.mouseDown(redSwatch);
fireEvent.pointerDown(redSwatch);
expect(onMouseDown).not.toHaveBeenCalled();
expect(onPointerDown).not.toHaveBeenCalled();
});
it("clicking a menu item runs its action without activating the row", async () => {
const { onClick, onArchiveTask } = renderWithDragHandle();
await openContextMenu();
fireEvent.click(screen.getByRole("menuitem", { name: /archive/i }));
expect(onArchiveTask).toHaveBeenCalledTimes(1);
expect(onArchiveTask).toHaveBeenCalledWith("task-1");
expect(onClick).not.toHaveBeenCalled();
});
});
describe("TaskItemWithContextMenu — touch drag cancellation", () => {
it("opening the menu cancels an in-flight touch drag at the touchstart target", async () => {
// A touch long-press arms the row's TouchSensor (250ms) before the menu
// opens (~700ms); dnd-kit cancels the drag when a touchcancel reaches the
// element the touch began on, so opening the menu must emit one there.
stubTouchEvent();
const touchCancelListener = vi.fn();
try {
renderWithDragHandle();
const row = screen.getByTestId("task-row");
row.addEventListener("touchcancel", touchCancelListener);
fireEvent.touchStart(row, { touches: [{ identifier: 1 }] });
fireEvent.contextMenu(row);
await screen.findByRole("menuitem", { name: /color/i });
expect(touchCancelListener).toHaveBeenCalledTimes(1);
expect(touchCancelListener.mock.calls[0]?.[0].type).toBe("touchcancel");
expect(touchCancelListener.mock.calls[0]?.[0].target).toBe(row);
row.removeEventListener("touchcancel", touchCancelListener);
} finally {
vi.unstubAllGlobals();
}
});
it("a second concurrent touch does not overwrite the captured drag target", async () => {
// dnd-kit's TouchSensor ignores a second finger (touches.length > 1), so
// the first touch's target must stay the cancel target. The listener sits
// on the row and `secondFinger` nests inside it, so a buggy overwrite
// would still bubble here — assert the event target itself.
stubTouchEvent();
try {
renderWithDragHandle();
const row = screen.getByTestId("task-row");
const secondFinger = document.createElement("div");
row.appendChild(secondFinger);
const touchCancelListener = vi.fn();
row.addEventListener("touchcancel", touchCancelListener);
fireEvent.touchStart(row, { touches: [{ identifier: 1 }] });
fireEvent.touchStart(secondFinger, {
touches: [{ identifier: 1 }, { identifier: 2 }],
});
fireEvent.contextMenu(row);
await screen.findByRole("menuitem", { name: /color/i });
// The cancel went to the first touch's target (the row), not the second.
expect(touchCancelListener).toHaveBeenCalledTimes(1);
expect(touchCancelListener.mock.calls[0]?.[0].target).toBe(row);
row.removeEventListener("touchcancel", touchCancelListener);
} finally {
vi.unstubAllGlobals();
}
});
it("a completed touch leaves no stale cancel target for a later menu open", async () => {
// Stub TouchEvent so the touchstart genuinely stores a target (happy-dom
// drops the touches init); without it this assertion would pass vacuously.
stubTouchEvent();
try {
renderWithDragHandle();
const row = screen.getByTestId("task-row");
const touchCancelListener = vi.fn();
row.addEventListener("touchcancel", touchCancelListener);
fireEvent.touchStart(row, { touches: [{ identifier: 1 }] });
fireEvent.touchEnd(row, { changedTouches: [{ identifier: 1 }] });
fireEvent.contextMenu(row);
await screen.findByRole("menuitem", { name: /color/i });
// No gesture is active when the menu opens, so nothing is dispatched.
expect(touchCancelListener).not.toHaveBeenCalled();
row.removeEventListener("touchcancel", touchCancelListener);
} finally {
vi.unstubAllGlobals();
}
});
it("a non-primary finger lifting does not clear the tracked drag target", async () => {
// Finger 1 holds the row (long-press in progress); finger 2 lifts first.
// Only finger 1's touchend may clear the target, so the menu-open cancel
// still reaches the first touch's element.
stubTouchEvent();
try {
renderWithDragHandle();
const row = screen.getByTestId("task-row");
const touchCancelListener = vi.fn();
row.addEventListener("touchcancel", touchCancelListener);
fireEvent.touchStart(row, { touches: [{ identifier: 1 }] });
fireEvent.touchEnd(row, { changedTouches: [{ identifier: 2 }] });
fireEvent.contextMenu(row);
await screen.findByRole("menuitem", { name: /color/i });
expect(touchCancelListener).toHaveBeenCalledTimes(1);
expect(touchCancelListener.mock.calls[0]?.[0].type).toBe("touchcancel");
expect(touchCancelListener.mock.calls[0]?.[0].target).toBe(row);
row.removeEventListener("touchcancel", touchCancelListener);
} finally {
vi.unstubAllGlobals();
}
});
});
describe("TaskItemWithContextMenu — touch drag cancellation identifier lifecycle", () => {
it("a non-primary touchcancel leaves the tracked drag target live", async () => {
// Same rule on the touchcancel path: finger 2's cancel must not clear
// finger 1's tracked target.
stubTouchEvent();
try {
renderWithDragHandle();
const row = screen.getByTestId("task-row");
const touchCancelListener = vi.fn();
row.addEventListener("touchcancel", touchCancelListener);
fireEvent.touchStart(row, { touches: [{ identifier: 1 }] });
fireEvent.touchCancel(row, { changedTouches: [{ identifier: 2 }] });
// The fired touchcancel above is caught by the row listener; count only
// what the menu-open path dispatches.
touchCancelListener.mockClear();
fireEvent.contextMenu(row);
await screen.findByRole("menuitem", { name: /color/i });
expect(touchCancelListener).toHaveBeenCalledTimes(1);
expect(touchCancelListener.mock.calls[0]?.[0].target).toBe(row);
row.removeEventListener("touchcancel", touchCancelListener);
} finally {
vi.unstubAllGlobals();
}
});
it("the tracked touch's touchcancel clears the cancel target", async () => {
// When finger 1 itself is cancelled, no synthetic cancel may fire later.
stubTouchEvent();
try {
renderWithDragHandle();
const row = screen.getByTestId("task-row");
const touchCancelListener = vi.fn();
row.addEventListener("touchcancel", touchCancelListener);
fireEvent.touchStart(row, { touches: [{ identifier: 1 }] });
fireEvent.touchCancel(row, { changedTouches: [{ identifier: 1 }] });
touchCancelListener.mockClear();
fireEvent.contextMenu(row);
await screen.findByRole("menuitem", { name: /color/i });
expect(touchCancelListener).not.toHaveBeenCalled();
row.removeEventListener("touchcancel", touchCancelListener);
} finally {
vi.unstubAllGlobals();
}
});
});
@@ -1,6 +1,6 @@
"use client";
import { cloneElement, isValidElement, useState } from "react";
import { cloneElement, isValidElement, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import {
IconCopy,
@@ -67,6 +67,92 @@ type ContextMenuProps = TaskLinkHandlers & {
isMixedWorkflowSelection?: boolean;
};
/**
* dnd-kit's TouchSensor arms on touchstart and activates after the 250ms
* delay — before a long-press (≈700ms) can open this context menu. A
* stationary long-press therefore starts a row drag that is still live when
* the menu opens. While a drag is active the TouchSensor listens for
* `touchcancel` on the element the touch started on, so dispatching one at
* that element aborts the drag (onDragCancel) instead of dropping it: the
* row stays put and the menu remains usable. Inert when no touch has started
* on this row (desktop right-click, or a sensor that already detached after a
* quick tap), because then nothing listens for the event.
*/
type CancelTouchDrag = (touchStartTarget: EventTarget | null) => void;
const cancelTouchDrag: CancelTouchDrag = (touchStartTarget) => {
if (touchStartTarget instanceof Element && typeof TouchEvent === "function") {
touchStartTarget.dispatchEvent(
new TouchEvent("touchcancel", { bubbles: true, cancelable: true }),
);
}
};
/**
* Coordinates the context menu with the row's touch-drag sensor: remembers
* the element the touch began on and cancels the in-flight drag when the menu
* opens. Returns the menu `onOpenChange` handler and the trigger-wrapper
* capture props.
*/
function useMenuTouchDragCancel(onOpenChange: (open: boolean) => void) {
const touchStartRef = useRef<{ target: EventTarget; identifier: number } | null>(null);
const menuOpenRef = useRef(false);
const handleOpenChange = (open: boolean) => {
onOpenChange(open);
menuOpenRef.current = open;
if (open) {
// A touch long-press has already armed the row's TouchSensor (250ms)
// when the menu opens (~700ms); cancel that drag at the touchstart
// target so the menu gesture never moves the row.
const target = touchStartRef.current?.target ?? null;
touchStartRef.current = null;
cancelTouchDrag(target);
} else {
touchStartRef.current = null;
}
};
return {
handleOpenChange,
triggerProps: {
// The TouchSensor attaches its touchcancel listener to the element the
// touch began on while a drag is active. Track only the first touch of
// a single-touch gesture (dnd-kit's TouchSensor rejects multi-touch)
// and only while the menu is closed, and drop the target when that
// touch ends or the menu closes — not when another finger lifts — so a
// later open never dispatches a synthetic touchcancel for a gesture
// that is no longer active (pull-to-refresh and touch-scroll listen for
// bubbled touchcancel).
onTouchStartCapture: (event: React.TouchEvent) => {
if (menuOpenRef.current || event.touches.length !== 1) return;
if (!touchStartRef.current) {
touchStartRef.current = {
target: event.target,
identifier: event.touches[0].identifier,
};
}
},
onTouchEndCapture: (event: React.TouchEvent) => {
const tracked = touchStartRef.current;
if (
tracked &&
Array.from(event.changedTouches).some((t) => t.identifier === tracked.identifier)
) {
touchStartRef.current = null;
}
},
onTouchCancelCapture: (event: React.TouchEvent) => {
const tracked = touchStartRef.current;
if (
tracked &&
Array.from(event.changedTouches).some((t) => t.identifier === tracked.identifier)
) {
touchStartRef.current = null;
}
},
},
};
}
export function TaskItemWithContextMenu({
task,
workflows,
@@ -105,13 +191,27 @@ export function TaskItemWithContextMenu({
setContextOpen(false);
setMenuKey((k) => k + 1);
};
const { handleOpenChange, triggerProps } = useMenuTouchDragCancel(setContextOpen);
return (
<ContextMenu key={menuKey} onOpenChange={setContextOpen}>
<ContextMenu key={menuKey} onOpenChange={handleOpenChange}>
<ContextMenuTrigger asChild>
<div>{cloneWithMenuOpen(children, contextOpen)}</div>
<div {...triggerProps}>{cloneWithMenuOpen(children, contextOpen)}</div>
</ContextMenuTrigger>
<ContextMenuContent className="w-48">
<ContextMenuContent
className="w-48"
// The menu renders in a portal whose fiber ancestors include the
// dnd-kit drag handle that wraps the row. React synthetic events
// bubble through the fiber tree, not the DOM, so without these guards
// a mousedown/pointerdown/touchstart on any menu item (e.g. the Color
// submenu trigger or a swatch) reaches the handle's sensor listeners
// and starts a row drag, and a click activates the row. Bubble-phase
// guards run after the item's own handlers, so menu actions still work.
onMouseDown={(event) => event.stopPropagation()}
onPointerDown={(event) => event.stopPropagation()}
onTouchStart={(event) => event.stopPropagation()}
onClick={(event) => event.stopPropagation()}
>
<TaskContextMenuItems
task={task}
workflows={workflows}
@@ -1,7 +1,8 @@
import { type CDPSession, type Page } from "@playwright/test";
import { test, expect } from "../../fixtures/test-base";
import { dwell } from "../../helpers/causal-waits";
import { SessionPage } from "../../pages/session-page";
import { settledBoundingBox } from "../../helpers/settled-box";
import { dwell } from "../../helpers/causal-waits";
/**
* Touch-drags a task row's handle onto a nest drop zone via CDP touch events.
@@ -12,8 +13,8 @@ import { dwell } from "../../helpers/causal-waits";
* coordinates are read.
*/
async function touchDragRowToNestZone(
page: import("@playwright/test").Page,
cdp: import("@playwright/test").CDPSession,
page: Page,
cdp: CDPSession,
handleLocator: ReturnType<Page["locator"]>,
zoneLocator: ReturnType<Page["locator"]>,
) {
@@ -107,4 +108,122 @@ test.describe("Mobile subtask re-parenting by drag and drop", () => {
})
.toEqual({ parentId: newParent.id, workspaceMode: "shared_group" });
});
test("long-press opens the context menu without dragging the row", async ({
testPage,
apiClient,
seedData,
}) => {
const placement = {
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
};
const parent = await apiClient.createTask(
seedData.workspaceId,
"Menu long-press parent",
placement,
);
const child = await apiClient.createTask(
seedData.workspaceId,
"Menu long-press child",
placement,
);
await testPage.goto(`/t/${parent.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
await testPage.getByTestId("mobile-session-menu").click();
const taskSheet = testPage.getByRole("dialog", { name: "Tasks" });
const taskList = taskSheet.getByTestId("mobile-task-switcher-list");
// The rendered order is the DOM order of the blocks inside the list.
// Bounding-box comparisons are unreliable here: content-visibility rows
// settle lazily, and a hidden desktop-sidebar copy of the tree duplicates
// the test ids outside the sheet list.
const orderOf = async () =>
(
await taskList
.locator('[data-testid="sortable-task-block"]')
.evaluateAll((els) => els.map((el) => el.getAttribute("data-task-id")))
).filter((id) => id === parent.id || id === child.id);
// Wait until both rows are in the DOM: an empty before/after pair would
// make the order assertion vacuous.
await expect
.poll(async () => await orderOf(), { timeout: 10_000 })
.toEqual(expect.arrayContaining([parent.id, child.id]));
const beforeOrder = await orderOf();
expect(beforeOrder).toHaveLength(2);
// Long-press the row: the TouchSensor arms at 250ms and the context menu
// opens at ~700ms; opening the menu must cancel that drag.
const cdp = await testPage.context().newCDPSession(testPage);
const handle = taskList
.locator(`[data-testid="sortable-task-block"][data-task-id="${child.id}"]`)
.getByTestId("sortable-task-handle");
await handle.scrollIntoViewIfNeeded();
// Sheet scroll and Radix open animation settle on their own timers before
// the handle box is measured.
await dwell(testPage, 100, "library-timer", "sheet scroll settles before measuring the handle");
const handleBox = await handle.boundingBox();
if (!handleBox) throw new Error("drag source handle has no bounding box");
const startX = handleBox.x + handleBox.width / 2;
const startY = handleBox.y + handleBox.height / 2;
await cdp.send("Input.dispatchTouchEvent", {
type: "touchStart",
touchPoints: [{ x: startX, y: startY }],
});
// Hold past the dnd-kit TouchSensor arming (250ms) and the long-press
// contextmenu (~700ms) so the menu opens while the drag is live; the menu
// must then cancel it.
await dwell(
testPage,
1200,
"library-timer",
"long-press hold outlasts the 250ms sensor and ~700ms menu",
);
// Scope to the menu this gesture opened: another mounted menu must not
// satisfy the assertion.
const openMenu = testPage.getByRole("menu");
await expect(openMenu.getByRole("menuitem", { name: /color/i })).toBeVisible();
// The long-press drag was cancelled when the menu opened: no nest zones
// render and the source row is not dimmed.
await expect(taskList.getByTestId("nest-drop-zone")).toHaveCount(0);
await expect(handle).not.toHaveCSS("opacity", "0.5");
// Keep the finger down and drag inside the open menu: the row must not
// move or reorder. Aim at an inert point in the menu content — Radix
// items click on pointerup when they did not receive the pointerdown, so
// ending on an item could run its action. Scan the content (p-1 padding)
// for a point that is not over a menu item.
const menuContent = testPage.locator('[data-slot="context-menu-content"]');
await expect(menuContent).toBeVisible();
const inertPoint = await testPage.evaluate(() => {
const menu = document.querySelector('[data-slot="context-menu-content"]');
if (!menu) return null;
const r = menu.getBoundingClientRect();
for (let dy = 1; dy <= 8; dy += 1) {
for (let dx = 1; dx <= 8; dx += 1) {
const el = document.elementFromPoint(r.x + dx, r.y + dy);
if (el && !el.closest('[role="menuitem"]')) {
return { x: r.x + dx, y: r.y + dy };
}
}
}
return null;
});
if (!inertPoint) throw new Error("no inert point inside the context menu content");
await cdp.send("Input.dispatchTouchEvent", {
type: "touchMove",
touchPoints: [{ x: inertPoint.x, y: inertPoint.y }],
});
// Let dnd-kit's sensor process the move before the touch ends.
await dwell(testPage, 100, "library-timer", "sensor move processing settles before touch end");
await cdp.send("Input.dispatchTouchEvent", { type: "touchEnd", touchPoints: [] });
// The gesture ended inside the menu: no item action ran, the menu is
// still open, and neither task was reordered or removed.
await expect(menuContent).toBeVisible();
await expect.poll(async () => await orderOf()).toEqual(beforeOrder);
await expect.poll(async () => (await apiClient.getTask(child.id)).parent_id ?? "").toBe("");
});
});
@@ -258,4 +258,62 @@ test.describe("Subtask re-parenting by drag and drop", () => {
})
.toBe(true);
});
test("starting a drag inside the row context menu does not drag the row", async ({
testPage,
apiClient,
seedData,
}) => {
const placement = {
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
};
const parent = await apiClient.createTask(seedData.workspaceId, "Menu drag parent", placement);
const child = await apiClient.createTask(seedData.workspaceId, "Menu drag child", placement);
await testPage.goto(`/t/${parent.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
const taskBlock = (taskId: string) =>
session.sidebar.locator(`[data-testid="sortable-task-block"][data-task-id="${taskId}"]`);
const before = {
parentY: (await taskBlock(parent.id).boundingBox())?.y ?? 0,
childY: (await taskBlock(child.id).boundingBox())?.y ?? 0,
};
// Right-click opens the context menu; the MouseSensor ignores right
// buttons, so opening the menu alone must not start a drag.
await taskBlock(child.id).getByTestId("sortable-task-handle").click({ button: "right" });
const colorItem = testPage.getByRole("menuitem", { name: /color/i });
await expect(colorItem).toBeVisible();
// Press and move inside the menu. The menu's pointer events must not
// reach the drag handle's sensor: a live drag would dim the source row
// and render nest drop zones on candidate rows.
const colorBox = await colorItem.boundingBox();
if (!colorBox) throw new Error("context menu color item has no bounding box");
await testPage.mouse.move(colorBox.x + colorBox.width / 2, colorBox.y + colorBox.height / 2);
await testPage.mouse.down();
await testPage.mouse.move(
colorBox.x + colorBox.width / 2 + 24,
colorBox.y + colorBox.height / 2,
{
steps: 3,
},
);
await expect(session.sidebar.getByTestId("nest-drop-zone")).toHaveCount(0);
await expect(taskBlock(child.id)).not.toHaveCSS("opacity", "0.5");
await testPage.mouse.up();
// The row never moved: both rows keep their vertical order.
await expect
.poll(async () => {
return {
parentY: (await taskBlock(parent.id).boundingBox())?.y ?? 0,
childY: (await taskBlock(child.id).boundingBox())?.y ?? 0,
};
})
.toEqual(before);
});
});
@@ -0,0 +1,111 @@
---
spec: ../../specs/tasks/subtask-reparenting-drag-drop.md
created: 2026-08-13
status: done
---
# Fix Plan: Task drag must not start from the context menu
## Root cause
The sidebar task tree (desktop sidebar and the mobile task switcher sheet)
wires dnd-kit's drag listeners onto a handle `div` that wraps the task row.
`TaskRow` renders `TaskItemWithContextMenu`, whose Radix `ContextMenuContent`
portal is a **fiber descendant** of that handle. React synthetic events do not
follow the DOM tree for portals; they bubble through the **React fiber tree**.
So a `mousedown`, `touchstart`, or `pointerdown` on any menu item (e.g. the
`Color` submenu trigger or one of its swatches) reaches the handle's sensor
listeners:
- `MouseSensor.activators` ignores only right-click (`button === 2`), so a
left-press on a menu item activates the sensor; after the 8px
`DRAG_ACTIVATION_DISTANCE` the row drags (dnd-kit source:
`MouseSensor.activators` in `@dnd-kit/core`).
- `TouchSensor` activates on `touchstart` after a 250ms hold — **before** a
~700ms long-press can open the context menu. A stationary long-press on the
row therefore starts a drag that is still live when the menu opens on top of
it; the drag only ends when the finger lifts, so continuing to drag inside
the open menu moves/reorders the row.
- A plain `click` on a menu item additionally fiber-bubbles to the row's
`onClick` → `onSelectTask`, activating the task as a side effect.
The kanban card context menu is **not** affected: there the
`ContextMenuContent` is a fiber *sibling* of the card shell that holds the
listeners, so no leak exists (verified structurally in
`apps/web/components/kanban-card-context-menu.tsx`). The kanban *dropdown*
menu was already fixed the same way (`DropdownEntry` guards
`onClick`/`onPointerDown`; see `kanban-card-menu-items.tsx` and its test).
## Fix
Contain pointer-start and click events at the menu boundary in
`TaskItemWithContextMenu` (`apps/web/components/task/task-switcher-context-menu.tsx`):
- **Bubble-phase containment** on the `ContextMenuContent` for `onMouseDown`,
`onPointerDown`, `onTouchStart`, and `onClick` stops events on any menu item
or submenu (fiber descendants) from reaching the drag handle or the row.
Item handlers run first (they are deeper in the fiber tree), so menu actions
still work; Radix outside-click dismissal and keyboard activation are
unaffected.
- **Touch long-press cancellation**: the touchstart target is captured on the
trigger wrapper (capture phase). When the menu opens, a `touchcancel`
`TouchEvent` is dispatched at that element — dnd-kit's `TouchSensor` listens
for `touchcancel` on the touchstart target while a drag is active, so the
in-flight long-press drag is cancelled (`onDragCancel`, no drop) instead of
reordering the row. Inert on desktop (no touch sensor) and for quick taps
(sensor already detached).
## Tests
### Unit regression (`apps/web/components/task/task-switcher-context-menu.test.tsx`)
Render `TaskItemWithContextMenu` inside a wrapper `div` with spy
`onMouseDown`/`onPointerDown`/`onTouchStart`/`onClick` handlers that stand in
for the drag handle and row click. Open the real menu and assert:
1. `mousedown`/`pointerdown` on the `Color` submenu trigger do not reach the
wrapper (fails before the fix: the spies are called).
2. `touchstart` on the `Color` submenu trigger and on a color swatch inside
the open submenu do not reach the wrapper.
3. `click` on a menu item (e.g. Archive) still invokes its action
(`onArchiveTask`) and does **not** reach the wrapper's `onClick`.
4. Opening the menu dispatches a `touchcancel` at the touchstart target (the
signal that cancels an in-flight touch drag).
i18n is initialized globally (`vitest.setup.ts`), so the real English labels
render; wrap in `StateProvider`/`ToastProvider` per `task-switcher.test.tsx`.
happy-dom does not construct `TouchEvent`, so tests 2/4 stub it.
### E2E (desktop, extend `apps/web/e2e/tests/task/subtask-reparent-drag-drop.spec.ts`)
Seed a parent with a subtask, right-click the subtask row, assert the context
menu opens (Color item visible), then press the mouse down on the Color item,
move ≥8px, assert **no** nest drop zone appears and the row order is unchanged,
then release.
### E2E (mobile, extend `apps/web/e2e/tests/task/mobile-subtask-reparent-drag-drop.spec.ts`)
Seed two root tasks, long-press a row via CDP touch (hold 1200ms), assert the
context menu opens while **no** nest drop zone renders and the row is not
dimmed, then drag inside the open menu and assert the row order and parent
relationship are unchanged (native list scroll may shift both rows together).
## Implementation Wave
1. [Guard context-menu pointer events](task-01-guard-context-menu-pointer-events.md) — done, sequential.
2. [Cancel touch drag when the context menu opens](task-02-cancel-touch-drag-on-menu-open.md) — done, sequential.
## Risks
- A capture-phase guard would break item selection (it would stop the event
before it reaches the items); the guard must be bubble-phase on the content.
- Guarding only the top-level items would miss submenu content (Color); the
content-level guard covers all fiber descendants.
- The kanban card context menu must stay untouched (no leak there); changing
it would be speculative.
- The touch long-press cancel dispatches a synthetic `touchcancel` at the
touchstart target. It is inert when no touch sensor is armed (desktop
right-click, quick taps) but any other listener of `touchcancel` on that
element would also see it; dnd-kit's sensor is the only known one. The
250–700ms window where the drag is live before the menu opens still shows a
brief row dim/nest-zone flash on touch long-press; the drag is cancelled the
moment the menu opens.
@@ -0,0 +1,98 @@
---
id: "task-context-menu-drag-guard-01"
title: "Guard context-menu pointer events"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/tasks/subtask-reparenting-drag-drop.md"
---
# Task 01: Guard context-menu pointer events
## Acceptance
1. Pressing and moving inside the task row context menu (including the Color
submenu trigger and its swatches) never starts a row drag: the menu's
pointer-start events (`mousedown`, `pointerdown`, `touchstart`) are stopped
at the menu content boundary before reaching the dnd-kit handle listeners.
2. Clicking a menu item still runs the item action and never activates or
selects the row as a side effect.
3. Unit regression tests prove 1 and 2 (they must fail before the fix), the
desktop E2E drag-from-menu regression passes, and the existing desktop and
mobile subtask drag-and-drop E2E specs stay green.
## Verification
First-time worktree install (node_modules is absent):
```sh
cd apps && pnpm install --frozen-lockfile
```
Targeted unit test (RED then GREEN):
```sh
cd apps && pnpm --filter @kandev/web test -- --run components/task/task-switcher-context-menu.test.tsx
```
E2E (rebuilds web + backend; run from `apps/web`):
```sh
cd apps/web && pnpm e2e:run tests/task/subtask-reparent-drag-drop.spec.ts
cd apps/web && pnpm e2e:run tests/task/mobile-subtask-reparent-drag-drop.spec.ts -- --project=mobile-chrome
```
Full gate:
```sh
make fmt && make typecheck && make test && make lint
```
## Files likely touched
- `apps/web/components/task/task-switcher-context-menu.tsx` — add
bubble-phase `stopPropagation` guards (`onMouseDown`, `onPointerDown`,
`onTouchStart`, `onClick`) to the `ContextMenuContent` in
`TaskItemWithContextMenu`.
- `apps/web/components/task/task-switcher-context-menu.test.tsx` — new unit
regression suite (wrapper spies stand in for the drag handle / row click).
- `apps/web/e2e/tests/task/subtask-reparent-drag-drop.spec.ts` — new desktop
regression test: drag started inside the open context menu must not move
the row.
Do **not** touch `apps/web/components/kanban-card-menu-items.tsx` or
`kanban-card-context-menu.tsx`: the kanban context menu is a fiber sibling of
the drag-listeners node and cannot leak (verified in the plan).
## Dependencies
None.
## Parallelism
Sequential; the guard and its regression tests change together.
## Inputs
- [Subtask re-parenting by drag and drop spec](../../specs/tasks/subtask-reparenting-drag-drop.md) (amended with the menu-containment scenarios)
- [Fix plan](plan.md)
- dnd-kit sensor behavior (`MouseSensor.activators` ignores only right-click;
`PointerSensor` requires primary button) from `@dnd-kit/core` source.
- Existing precedent: `DropdownEntry` in `kanban-card-menu-items.tsx` guards
portal menu `onClick`/`onPointerDown`; its test asserts the same containment.
## Completion record
- Unit regression suite (`task-switcher-context-menu.test.tsx`): RED confirmed
(3/3 failed on the containment assertions before the fix) then GREEN after
the guard; task-switcher suites 21/21.
- Desktop E2E `subtask-reparent-drag-drop.spec.ts`: 6/6 passed, including the
new "starting a drag inside the row context menu does not drag the row".
- Mobile E2E `mobile-subtask-reparent-drag-drop.spec.ts` (mobile-chrome): 1/1
passed — touch drag still works with the guard in place.
- `make fmt`, `make typecheck`, `make lint` passed. `make test`'s remaining
failures are environmental and unrelated (no Docker daemon for
`http-git-server` tests, missing `unzip` for `scripts/pr-state --job-log`,
untagged worktree HEAD and service detection for backend launcher/updates
tests, sandbox CPU timeouts in `git-base`/storage tests).
@@ -0,0 +1,104 @@
---
id: "task-context-menu-drag-guard-02"
title: "Cancel touch drag when the context menu opens"
status: done
wave: 1
depends_on: ["task-context-menu-drag-guard-01"]
plan: "plan.md"
spec: "../../specs/tasks/subtask-reparenting-drag-drop.md"
---
# Task 02: Cancel touch drag when the context menu opens
## Acceptance
1. On touch, a long-press that opens the row context menu never results in row
movement or reordering: the touch-drag the hold started is cancelled when
the menu opens (no nest drop zones remain, the row is not dimmed while the
menu is open, and dragging inside the open menu does not move the row).
2. Desktop is unaffected: right-click menus never dispatch a touchcancel that
matters, and the existing desktop drag-from-menu and drag-to-nest E2E
behavior is unchanged.
3. Regression coverage: a unit test asserts the menu-open touchcancel signal
at the touchstart target; a mobile E2E proves the long-press gesture opens
the menu without dragging the row; the existing mobile touch-drag
re-parenting E2E stays green.
## Verification
Targeted unit test:
```sh
cd apps && pnpm --filter @kandev/web test -- --run components/task/task-switcher-context-menu.test.tsx
```
E2E (rebuilds web + backend; run from `apps/web`):
```sh
cd apps/web && pnpm e2e:run tests/task/mobile-subtask-reparent-drag-drop.spec.ts -- --project=mobile-chrome
cd apps/web && pnpm e2e:run tests/task/subtask-reparent-drag-drop.spec.ts
```
Full gate:
```sh
make fmt && make typecheck && make test && make lint
```
## Files likely touched
- `apps/web/components/task/task-switcher-context-menu.tsx` — capture the
touchstart target on the `ContextMenuTrigger` wrapper; on menu open dispatch
a `touchcancel` `TouchEvent` at that element (dnd-kit `TouchSensor` cancels
the active drag on `touchcancel`).
- `apps/web/components/task/task-switcher-context-menu.test.tsx` — touchstart
containment assertions + menu-open `touchcancel` dispatch assertion
(stub `TouchEvent` for happy-dom).
- `apps/web/e2e/tests/task/mobile-subtask-reparent-drag-drop.spec.ts` — new
"long-press opens the context menu without dragging the row" test.
- `docs/specs/tasks/subtask-reparenting-drag-drop.md` — touch long-press
cancellation contract (What bullet + scenario).
## Dependencies
Task 01 (the content-boundary guards) — this task adds the touch long-press
cancellation on top of it.
## Parallelism
Sequential; task 02 depends on task 01 and changes the same component.
## Inputs
- [Subtask re-parenting by drag and drop spec](../../specs/tasks/subtask-reparenting-drag-drop.md) (amended)
- [Fix plan](plan.md)
- dnd-kit `TouchSensor` source: activation after the 250ms delay; `touchcancel`
listener attached to the touchstart target element while a drag is active;
`handleCancel` → `onCancel` → `onDragCancel` (no drop).
- Adversarial review round 1 finding (major): the mobile long-press race,
reproduced live on the dev instance (drag live at 300ms, menu at 900ms).
## Completion record
- Live dev-instance verification (CDP touch long-press, 390px viewport):
drag live at ~300ms (row dim + nest zone), menu opens at ~900ms, drag
cancelled at menu open (zones/dim clear while the menu stays open); moving
the finger inside the open menu no longer moves or reorders the row.
Initial document-level touchcancel dispatch did not cancel (the TouchSensor
listens on the touchstart target element, not the document); corrected to
dispatch at the captured touchstart target.
- Unit suite (`task-switcher-context-menu.test.tsx`): 10/10, including the
touchstart containment, menu-open touchcancel (with dispatch-target
assertions), multi-touch retention, stale-target clearing, and
non-primary-finger touchend/touchcancel vs tracked-touch touchcancel
identifier cases.
- Mobile E2E (`mobile-subtask-reparent-drag-drop.spec.ts`, mobile-chrome):
2/2 — new "long-press opens the context menu without dragging the row" plus
the existing touch-drag re-parenting test.
- Desktop E2E (`subtask-reparent-drag-drop.spec.ts`): 6/6 — desktop
unaffected. The mobile test's initial bounding-box order assertion proved
unreliable (duplicate hidden tree + content-visibility rows) and was
replaced with DOM-order assertions; diagnostics confirmed the gesture
caused no API, preference, or DOM-order change.
- `make fmt`, `make typecheck`, `make lint` passed. `make test`'s remaining
failures are the environmental ones recorded in task 01.
@@ -21,6 +21,8 @@ Users can detach a subtask or nest a task under another via context-menu actions
- No confirmation dialog is shown on drop (unlike the detach menu action): the gesture is explicit, the operation is non-destructive (workspace membership is retained) and reversible via the menu or another drag.
- Successful re-parenting is reflected across sidebar, board, and task-detail views without a reload, via the existing optimistic snapshot update and the `task.updated` WebSocket event.
- The mobile task switcher sheet offers the same touch drag-and-drop re-parenting.
- A drag starts only from a row's body. Interacting with a row's context menu (including the Color submenu and every item inside it) never starts a drag and never activates the row: pointer-start and click events inside the menu are contained within the menu and do not reach the row's drag sensor listeners or its click handler.
- On touch, the long-press that opens the context menu cancels any touch-drag the hold started: the menu opens without the row moving, no nest drop zones remain while the menu is open, and continuing to drag inside the open menu does not move or reorder the row. (The drag sensor arms after a 250ms hold, before the ≈700ms long-press opens the menu, so the menu cancels the in-flight drag instead of dropping it.)
## Data model
@@ -58,6 +60,9 @@ No new endpoint. Reuses and extends existing contracts:
- **GIVEN** a drop that lands outside every nest zone, **WHEN** the drop completes, **THEN** the task keeps its original parent and no request is sent (a plain no-op); a request-error toast appears only when a valid-zone drop's request is rejected by the backend.
- **GIVEN** a re-parented task, **WHEN** its `task.updated` event arrives over WebSocket, **THEN** cached parent relationships in sidebar, board, and task-detail views are updated.
- **GIVEN** the mobile task switcher sheet, **WHEN** a user touch-drags a subtask onto a root's nest zone, **THEN** the same re-parenting occurs.
- **GIVEN** a task row whose context menu is open, **WHEN** the user presses and moves inside the menu (for example on the Color submenu trigger or one of its swatches), **THEN** no drag starts: no nest drop zones appear, the row neither dims nor moves, and the menu item still works.
- **GIVEN** a task row whose context menu is open, **WHEN** the user clicks a menu item, **THEN** the row is not activated or selected as a side effect of the click.
- **GIVEN** a touch long-press on a task row, **WHEN** the context menu opens, **THEN** the touch-drag started by the hold is cancelled: no nest drop zones remain and the row is not dimmed while the menu is open, and dragging inside the open menu does not move or reorder the row.
## Out of scope