From a7cbf4f61ddd5adf78970e8c6584b6e8f5c78880 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 17 Jul 2026 13:00:47 +0200 Subject: [PATCH] fix(app): make sidebar reordering respond immediately Mouse input inherited the touch hold delay. Split activation by input so mouse dragging begins after deliberate movement while touch retains long-press arbitration. --- packages/app/e2e/sidebar-reorder.spec.ts | 71 ++++++ .../app/src/components/drag-reorder/index.ts | 5 +- .../drag-reorder/pointer-activation.test.ts | 21 +- .../drag-reorder/pointer-activation.ts | 29 ++- .../app/src/components/draggable-list.web.tsx | 25 +- .../src/components/sidebar-workspace-list.tsx | 222 +----------------- .../use-long-press-drag-interaction.ts | 5 +- 7 files changed, 124 insertions(+), 254 deletions(-) create mode 100644 packages/app/e2e/sidebar-reorder.spec.ts diff --git a/packages/app/e2e/sidebar-reorder.spec.ts b/packages/app/e2e/sidebar-reorder.spec.ts new file mode 100644 index 000000000..0979694f0 --- /dev/null +++ b/packages/app/e2e/sidebar-reorder.spec.ts @@ -0,0 +1,71 @@ +import type { Locator } from "@playwright/test"; +import { expect, test } from "./fixtures"; +import { gotoAppShell } from "./helpers/app"; +import { getServerId } from "./helpers/server-id"; +import { seedWorkspace } from "./helpers/seed-client"; +import { waitForSidebarHydration } from "./helpers/workspace-ui"; + +async function rowTestIds(rows: Locator) { + return rows.evaluateAll((elements) => + elements.map((element) => element.getAttribute("data-testid")), + ); +} + +async function visibleBoundingBox(row: Locator) { + const box = await row.boundingBox(); + if (!box) throw new Error("Expected a visible draggable row"); + return box; +} + +async function quickDragFirstRowAfterSecond(rows: Locator) { + await expect(rows).toHaveCount(2); + const before = await rowTestIds(rows); + const sourceBox = await visibleBoundingBox(rows.nth(0)); + const targetBox = await visibleBoundingBox(rows.nth(1)); + + const page = rows.page(); + const source = { x: sourceBox.x + sourceBox.width / 2, y: sourceBox.y + sourceBox.height / 2 }; + const target = { x: targetBox.x + targetBox.width / 2, y: targetBox.y + targetBox.height / 2 }; + + await page.mouse.move(source.x, source.y); + await page.mouse.down(); + await page.mouse.move(source.x, source.y + 7); + await page.mouse.move(target.x, target.y, { steps: 4 }); + await page.mouse.up(); + + await expect.poll(() => rowTestIds(rows)).toEqual([before[1], before[0]]); +} + +test("projects and workspaces reorder with an immediate mouse drag", async ({ page }) => { + const firstProject = await seedWorkspace({ repoPrefix: "sidebar-reorder-first-" }); + const secondProject = await seedWorkspace({ repoPrefix: "sidebar-reorder-second-" }); + + try { + const secondWorkspace = await firstProject.client.createWorkspace({ + source: { + kind: "directory", + path: firstProject.repoPath, + projectId: firstProject.projectId, + }, + title: "Second workspace", + }); + if (!secondWorkspace.workspace) { + throw new Error(secondWorkspace.error ?? "Failed to seed a second workspace"); + } + + await gotoAppShell(page); + await waitForSidebarHydration(page); + + await quickDragFirstRowAfterSecond(page.locator('[data-testid^="sidebar-project-row-"]')); + const firstWorkspaceTestId = `sidebar-workspace-row-${getServerId()}:${firstProject.workspaceId}`; + const secondWorkspaceTestId = `sidebar-workspace-row-${getServerId()}:${secondWorkspace.workspace.id}`; + await quickDragFirstRowAfterSecond( + page.locator( + `[data-testid="${firstWorkspaceTestId}"], [data-testid="${secondWorkspaceTestId}"]`, + ), + ); + } finally { + await firstProject.cleanup(); + await secondProject.cleanup(); + } +}); diff --git a/packages/app/src/components/drag-reorder/index.ts b/packages/app/src/components/drag-reorder/index.ts index 4016bd1e6..ba31937e4 100644 --- a/packages/app/src/components/drag-reorder/index.ts +++ b/packages/app/src/components/drag-reorder/index.ts @@ -1,8 +1,9 @@ export { reorderItemsOnDragEnd } from "./reorder-items"; export type { DragEndInput } from "./reorder-items"; export { - getPointerActivationConstraint, - type PointerActivationConfig, + getDragActivationConstraints, + type DragActivationConfig, + type DragActivationConstraints, type PointerActivationConstraint, } from "./pointer-activation"; export { diff --git a/packages/app/src/components/drag-reorder/pointer-activation.test.ts b/packages/app/src/components/drag-reorder/pointer-activation.test.ts index d1c6fa041..921dd79a2 100644 --- a/packages/app/src/components/drag-reorder/pointer-activation.test.ts +++ b/packages/app/src/components/drag-reorder/pointer-activation.test.ts @@ -1,14 +1,21 @@ import { describe, expect, it } from "vitest"; -import { getPointerActivationConstraint } from "./pointer-activation"; +import { getDragActivationConstraints } from "./pointer-activation"; -const config = { defaultDistance: 6, holdDelayMs: 250, holdTolerance: 8 }; +const config = { movementDistance: 6, touchHoldDelayMs: 180, touchHoldTolerance: 8 }; -describe("getPointerActivationConstraint", () => { - it("uses distance activation for default draggable rows", () => { - expect(getPointerActivationConstraint(false, config)).toEqual({ distance: 6 }); +describe("getDragActivationConstraints", () => { + it("starts mouse drags after deliberate pointer movement", () => { + expect(getDragActivationConstraints(true, config).mouse).toEqual({ distance: 6 }); }); - it("requires a held pointer before activating handle-based drags", () => { - expect(getPointerActivationConstraint(true, config)).toEqual({ delay: 250, tolerance: 8 }); + it("requires a short hold before starting touch drags", () => { + expect(getDragActivationConstraints(true, config).touch).toEqual({ + delay: 180, + tolerance: 8, + }); + }); + + it("starts ordinary touch rows after deliberate movement", () => { + expect(getDragActivationConstraints(false, config).touch).toEqual({ distance: 6 }); }); }); diff --git a/packages/app/src/components/drag-reorder/pointer-activation.ts b/packages/app/src/components/drag-reorder/pointer-activation.ts index cb68e2bda..9b929c8f7 100644 --- a/packages/app/src/components/drag-reorder/pointer-activation.ts +++ b/packages/app/src/components/drag-reorder/pointer-activation.ts @@ -2,18 +2,25 @@ export type PointerActivationConstraint = | { distance: number } | { delay: number; tolerance: number }; -export interface PointerActivationConfig { - defaultDistance: number; - holdDelayMs: number; - holdTolerance: number; +export interface DragActivationConfig { + movementDistance: number; + touchHoldDelayMs: number; + touchHoldTolerance: number; } -export function getPointerActivationConstraint( +export interface DragActivationConstraints { + mouse: PointerActivationConstraint; + touch: PointerActivationConstraint; +} + +export function getDragActivationConstraints( useDragHandle: boolean, - config: PointerActivationConfig, -): PointerActivationConstraint { - if (useDragHandle) { - return { delay: config.holdDelayMs, tolerance: config.holdTolerance }; - } - return { distance: config.defaultDistance }; + config: DragActivationConfig, +): DragActivationConstraints { + const movement = { distance: config.movementDistance }; + const touch = useDragHandle + ? { delay: config.touchHoldDelayMs, tolerance: config.touchHoldTolerance } + : movement; + + return { mouse: movement, touch }; } diff --git a/packages/app/src/components/draggable-list.web.tsx b/packages/app/src/components/draggable-list.web.tsx index 3f16fc072..7c1e683e5 100644 --- a/packages/app/src/components/draggable-list.web.tsx +++ b/packages/app/src/components/draggable-list.web.tsx @@ -4,7 +4,8 @@ import { DndContext, closestCenter, KeyboardSensor, - PointerSensor, + MouseSensor, + TouchSensor, type Modifier, useSensor, useSensors, @@ -17,7 +18,7 @@ import { } from "@dnd-kit/sortable"; import { CSS } from "@dnd-kit/utilities"; import type { DraggableListProps, DraggableRenderItemInfo } from "./draggable-list.types"; -import { getPointerActivationConstraint, useDragReorderState } from "./drag-reorder"; +import { getDragActivationConstraints, useDragReorderState } from "./drag-reorder"; export type { DraggableListProps, DraggableRenderItemInfo }; @@ -27,10 +28,10 @@ const restrictToVerticalAxis: Modifier = ({ transform }) => ({ }); const DND_MODIFIERS = [restrictToVerticalAxis]; -const POINTER_ACTIVATION_CONFIG = { - defaultDistance: 6, - holdDelayMs: 250, - holdTolerance: 8, +const DRAG_ACTIVATION_CONFIG = { + movementDistance: 6, + touchHoldDelayMs: 180, + touchHoldTolerance: 8, }; interface SortableItemProps { @@ -145,14 +146,14 @@ export function DraggableList({ onDragEnd, onDragBegin, }); - const pointerActivationConstraint = getPointerActivationConstraint( - useDragHandle, - POINTER_ACTIVATION_CONFIG, - ); + const activationConstraints = getDragActivationConstraints(useDragHandle, DRAG_ACTIVATION_CONFIG); const sensors = useSensors( - useSensor(PointerSensor, { - activationConstraint: pointerActivationConstraint, + useSensor(MouseSensor, { + activationConstraint: activationConstraints.mouse, + }), + useSensor(TouchSensor, { + activationConstraint: activationConstraints.touch, }), useSensor(KeyboardSensor, { coordinateGetter: sortableKeyboardCoordinates, diff --git a/packages/app/src/components/sidebar-workspace-list.tsx b/packages/app/src/components/sidebar-workspace-list.tsx index bad34715b..ffef8dfd0 100644 --- a/packages/app/src/components/sidebar-workspace-list.tsx +++ b/packages/app/src/components/sidebar-workspace-list.tsx @@ -2,15 +2,12 @@ import { View, Text, Pressable, - Platform, ActivityIndicator, - StatusBar, ScrollView, type GestureResponderEvent, type PressableStateCallbackType, type ViewStyle, } from "react-native"; -import * as Haptics from "expo-haptics"; import { useMutation } from "@tanstack/react-query"; import { ProjectIconView } from "@/components/project-icon-view"; import { AdaptiveRenameModal } from "@/components/rename-modal"; @@ -87,7 +84,6 @@ import { useToast } from "@/contexts/toast-context"; import { getForgePresentation, normalizeForge } from "@/git/forge"; import { toWorktreeArchiveRisk } from "@/git/worktree-archive-warning"; import { hasVisibleOrderChanged, mergeWithRemainder } from "@/utils/sidebar-reorder"; -import { decideLongPressMove } from "@/utils/sidebar-gesture-arbitration"; import { confirmDialog } from "@/utils/confirm-dialog"; import { projectIconPlaceholderLabelFromDisplayName } from "@/utils/project-display-name"; import { shouldRenderSyncedStatusLoader } from "@/utils/status-loader"; @@ -96,6 +92,7 @@ import type { SidebarStateBucket } from "@/utils/sidebar-agent-state"; import { SidebarStatusWorkspaceList } from "@/components/sidebar/sidebar-status-list"; import type { StatusGroup } from "@/hooks/sidebar-status-view-model"; import { SidebarWorkspaceMenu } from "@/components/sidebar/sidebar-workspace-menu"; +import { useLongPressDragInteraction } from "@/components/sidebar/use-long-press-drag-interaction"; import { PinnedSectionHeader } from "@/components/sidebar/pinned-section-header"; import { SidebarWorkspaceRowFrame, @@ -948,223 +945,6 @@ function NewWorkspaceGhostRow({ ); } -function useLongPressDragInteraction(input: { - drag: () => void; - menuController: ReturnType | null; -}) { - const didLongPressRef = useRef(false); - const dragArmedRef = useRef(false); - const dragActivatedRef = useRef(false); - const didStartDragRef = useRef(false); - const scrollIntentRef = useRef(false); - const menuOpenedRef = useRef(false); - const touchStartRef = useRef<{ x: number; y: number } | null>(null); - const touchCurrentRef = useRef<{ x: number; y: number } | null>(null); - const dragArmTimerRef = useRef | null>(null); - const contextMenuTimerRef = useRef | null>(null); - - const clearTimers = useCallback(() => { - if (dragArmTimerRef.current) { - clearTimeout(dragArmTimerRef.current); - dragArmTimerRef.current = null; - } - if (contextMenuTimerRef.current) { - clearTimeout(contextMenuTimerRef.current); - contextMenuTimerRef.current = null; - } - }, []); - - const openContextMenuAtStartPoint = useCallback(() => { - if (!input.menuController || !touchStartRef.current) { - return; - } - const statusBarHeight = Platform.OS === "android" ? (StatusBar.currentHeight ?? 0) : 0; - input.menuController.setAnchorRect({ - x: touchStartRef.current.x, - y: touchStartRef.current.y + statusBarHeight, - width: 0, - height: 0, - }); - input.menuController.setOpen(true); - menuOpenedRef.current = true; - didLongPressRef.current = true; - }, [input.menuController]); - - const handleLongPress = useCallback(() => { - // Manual timers own long-press behavior on mobile. - }, []); - - useEffect(() => { - return () => { - clearTimers(); - }; - }, [clearTimers]); - - const armTimers = useCallback(() => { - clearTimers(); - - const DRAG_ARM_DELAY_MS = 180; - const DRAG_ARM_STATIONARY_SLOP_PX = 4; - const CONTEXT_MENU_DELAY_MS = 450; - const CONTEXT_MENU_STATIONARY_SLOP_PX = 6; - - dragArmTimerRef.current = setTimeout(() => { - if (scrollIntentRef.current || didStartDragRef.current || menuOpenedRef.current) { - return; - } - const start = touchStartRef.current; - const current = touchCurrentRef.current ?? start; - if (!start || !current) { - return; - } - const dx = current.x - start.x; - const dy = current.y - start.y; - const distance = Math.sqrt(dx * dx + dy * dy); - if (distance > DRAG_ARM_STATIONARY_SLOP_PX) { - return; - } - dragArmedRef.current = true; - dragActivatedRef.current = true; - didLongPressRef.current = true; - void Haptics.selectionAsync().catch(() => {}); - input.drag(); - }, DRAG_ARM_DELAY_MS); - - if (!input.menuController || platformIsWeb) { - return; - } - - contextMenuTimerRef.current = setTimeout(() => { - if (scrollIntentRef.current || didStartDragRef.current || menuOpenedRef.current) { - return; - } - const start = touchStartRef.current; - const current = touchCurrentRef.current ?? start; - if (!start || !current) { - return; - } - const dx = current.x - start.x; - const dy = current.y - start.y; - const distance = Math.sqrt(dx * dx + dy * dy); - if (distance > CONTEXT_MENU_STATIONARY_SLOP_PX) { - return; - } - void Haptics.selectionAsync().catch(() => {}); - openContextMenuAtStartPoint(); - }, CONTEXT_MENU_DELAY_MS); - }, [clearTimers, input, openContextMenuAtStartPoint]); - - const handleDragIntent = useCallback( - (_details: { dx: number; dy: number; distance: number }) => { - if (!dragActivatedRef.current) { - return; - } - didStartDragRef.current = true; - didLongPressRef.current = true; - clearTimers(); - void Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Medium).catch(() => {}); - }, - [clearTimers], - ); - - const handleScrollIntent = useCallback( - (_details: { dx: number; dy: number; distance: number }) => { - scrollIntentRef.current = true; - didLongPressRef.current = true; - clearTimers(); - }, - [clearTimers], - ); - - const handleSwipeIntent = useCallback( - (_details: { dx: number; dy: number; distance: number }) => { - didLongPressRef.current = true; - clearTimers(); - }, - [clearTimers], - ); - - const handlePressIn = useCallback( - (event: GestureResponderEvent) => { - didLongPressRef.current = false; - dragArmedRef.current = false; - dragActivatedRef.current = false; - didStartDragRef.current = false; - scrollIntentRef.current = false; - menuOpenedRef.current = false; - touchStartRef.current = { - x: event.nativeEvent.pageX, - y: event.nativeEvent.pageY, - }; - touchCurrentRef.current = { - x: event.nativeEvent.pageX, - y: event.nativeEvent.pageY, - }; - armTimers(); - }, - [armTimers], - ); - - const handleTouchMove = useCallback( - (event: GestureResponderEvent) => { - const start = touchStartRef.current; - if (!start || didStartDragRef.current || menuOpenedRef.current) { - return; - } - - const touch = event?.nativeEvent?.touches?.[0] ?? event?.nativeEvent; - const x = touch?.pageX; - const y = touch?.pageY; - if (typeof x !== "number" || typeof y !== "number") { - return; - } - - const current = { x, y }; - touchCurrentRef.current = current; - const dx = current.x - start.x; - const dy = current.y - start.y; - const distance = Math.sqrt(dx * dx + dy * dy); - const decision = decideLongPressMove({ - dragArmed: dragArmedRef.current, - didStartDrag: didStartDragRef.current, - startPoint: start, - currentPoint: current, - }); - - if (decision === "vertical_scroll") { - handleScrollIntent({ dx, dy, distance }); - return; - } - - if (decision === "horizontal_swipe" || decision === "cancel_long_press") { - handleSwipeIntent({ dx, dy, distance }); - return; - } - - if (decision === "start_drag") { - handleDragIntent({ dx, dy, distance }); - } - }, - [handleDragIntent, handleScrollIntent, handleSwipeIntent], - ); - - const handlePressOut = useCallback(() => { - clearTimers(); - dragArmedRef.current = false; - dragActivatedRef.current = false; - touchStartRef.current = null; - touchCurrentRef.current = null; - }, [clearTimers]); - - return { - didLongPressRef, - handleLongPress, - handlePressIn, - handleTouchMove, - handlePressOut, - }; -} - function ProjectHeaderRow({ project, displayName, diff --git a/packages/app/src/components/sidebar/use-long-press-drag-interaction.ts b/packages/app/src/components/sidebar/use-long-press-drag-interaction.ts index 80f15d926..747f2b6bc 100644 --- a/packages/app/src/components/sidebar/use-long-press-drag-interaction.ts +++ b/packages/app/src/components/sidebar/use-long-press-drag-interaction.ts @@ -59,6 +59,9 @@ export function useLongPressDragInteraction(input: { const armTimers = useCallback(() => { clearTimers(); + if (platformIsWeb) { + return; + } const DRAG_ARM_DELAY_MS = 180; const DRAG_ARM_STATIONARY_SLOP_PX = 4; @@ -87,7 +90,7 @@ export function useLongPressDragInteraction(input: { input.drag(); }, DRAG_ARM_DELAY_MS); - if (!input.menuController || platformIsWeb) { + if (!input.menuController) { return; }