From 555226a9bfde7ed2de89bcf9af4f75f50930eaee Mon Sep 17 00:00:00 2001 From: ntbang0901 <121003585+ntbang0901@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:31:10 +0700 Subject: [PATCH] fix(ui): start pointer drags only while the primary button is held (#1071) On macOS, with an editable element focused and an input method active, WebKit can deliver a tap as pointerup then pointerdown, so drag trackers saw every later pointermove as a held button: one tap plus a pointer move started a phantom drag (file drag label, tab detach, tool window move) and the next tap dropped it. A drag now starts only from a move with the primary button down, a move without it cancels the press, and late button-less moves after a drag started are ignored so the drop is kept. Resizers, dialog drag/resize, pan and the review gutter end instead; the Project view also listens for pointercancel. Refs #760 Co-authored-by: ntbang0901 <121003585+ntbang0901@users.noreply.github.com> --- e2e/projectTree.spec.ts | 41 +++++++++++++++ src/features/database/Grid.tsx | 4 +- .../project-tree/ProjectView.test.tsx | 29 +++++++++++ src/features/project-tree/ProjectView.tsx | 7 +++ .../services/ServicesTree.reorder.test.tsx | 11 +++- src/features/vcs/reviewGutter.ts | 1 + src/features/viewers/panZoom.ts | 1 + src/ui/Dialog.test.tsx | 6 +-- src/ui/Dialog.tsx | 2 + src/ui/Tabs.test.tsx | 11 +++- src/ui/Tree.reorder.test.tsx | 12 ++++- src/ui/pointerDrag.test.ts | 52 ++++++++++++++++++- src/ui/pointerDrag.ts | 5 ++ src/workbench/WindowGroupTabs.tsx | 1 + src/workbench/toolwindows/ToolWindows.tsx | 6 +++ 15 files changed, 180 insertions(+), 9 deletions(-) diff --git a/e2e/projectTree.spec.ts b/e2e/projectTree.spec.ts index 55f89f00..ccb12ed3 100644 --- a/e2e/projectTree.spec.ts +++ b/e2e/projectTree.spec.ts @@ -1,3 +1,4 @@ +import type { Page } from "@playwright/test"; import { expect, test } from "./fixtures"; test("keyboard navigation: arrows move, Right/Left expand and collapse, Enter opens", async ({ workbench: page }) => { @@ -48,3 +49,43 @@ test("Git ignore submenu offers shared and local destinations for files and fold await page.keyboard.press("Escape"); } }); + +// macOS WebKit can deliver a tap's pointerup before its pointerdown, leaving a press with no button held (#760). +const settle = (page: Page) => + page.evaluate(() => new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(done)))); + +async function fileRow(page: Page) { + // The start-up splash swallows raw mouse presses until it has faded out. + await expect(page.locator("#splash")).toHaveCount(0); + const file = page.getByRole("tree", { name: "Project" }).getByRole("treeitem", { name: "settings.gradle.kts" }); + await file.scrollIntoViewIfNeeded(); + const content = file.locator("[data-project-path]"); + const box = (await content.boundingBox())!; + return { + content, + x: box.x + 10, + y: box.y + box.height / 2, + dragLabel: page.locator('body > div[aria-hidden="true"]', { hasText: "settings.gradle.kts" }), + }; +} + +test("a press without a held button never starts a file drag", async ({ workbench: page }) => { + const { content, x, y, dragLabel } = await fileRow(page); + await content.evaluate((el) => + el.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, button: 0, buttons: 0, pointerType: "mouse" })), + ); + await page.mouse.move(x, y); + await page.mouse.move(x + 60, y + 30, { steps: 5 }); + await settle(page); + await expect(dragLabel).toHaveCount(0); +}); + +test("a real mouse drag still shows the drag label until release", async ({ workbench: page }) => { + const { x, y, dragLabel } = await fileRow(page); + await page.mouse.move(x, y); + await page.mouse.down(); + await page.mouse.move(x + 60, y + 30, { steps: 5 }); + await expect(dragLabel).toHaveCount(1); + await page.mouse.up(); + await expect(dragLabel).toHaveCount(0); +}); diff --git a/src/features/database/Grid.tsx b/src/features/database/Grid.tsx index f0d93202..aefd0f1d 100644 --- a/src/features/database/Grid.tsx +++ b/src/features/database/Grid.tsx @@ -282,8 +282,10 @@ export function Grid(props: GridProps) { e.stopPropagation(); const startX = e.clientX; const start = widths[col] ?? 100; - const move = (ev: globalThis.MouseEvent) => + const move = (ev: globalThis.MouseEvent) => { + if (!(ev.buttons & 1)) return up(); setWidths((w) => w.map((x, i) => (i === col ? Math.max(40, start + ev.clientX - startX) : x))); + }; const up = () => { window.removeEventListener("mousemove", move); window.removeEventListener("mouseup", up); diff --git a/src/features/project-tree/ProjectView.test.tsx b/src/features/project-tree/ProjectView.test.tsx index f916157d..8a2e4b90 100644 --- a/src/features/project-tree/ProjectView.test.tsx +++ b/src/features/project-tree/ProjectView.test.tsx @@ -242,6 +242,35 @@ describe("ProjectView", () => { } }); + it("starts a file drag only while the primary button is held", async () => { + const { row, rows } = setup(); + await waitFor(() => expect(rows()).toContain("README.md")); + document.elementFromPoint = () => null; // jsdom lacks it; the drop-target lookup needs it + // The drag label repeats the file name outside the tree row. + const ghost = () => (screen.queryAllByText("README.md").length > 1 ? true : null); + const move = (buttons: number, x: number) => + act(() => { + window.dispatchEvent(new MouseEvent("pointermove", { bubbles: true, buttons, clientX: x, clientY: 0 })); + }); + const release = () => act(() => void window.dispatchEvent(new MouseEvent("pointerup", { bubbles: true }))); + // jsdom has no PointerEvent, so React would see `button` undefined on a plain pointerDown. + const press = () => + fireEvent( + row("README.md").querySelector("[data-project-path]")!, + new MouseEvent("pointerdown", { bubbles: true, button: 0 }), + ); + // A macOS tap WebKit reorders: the press arrives late and no button is held. + press(); + move(0, 40); + expect(ghost()).toBeNull(); + release(); + press(); + move(1, 40); + expect(ghost()).not.toBeNull(); + release(); + expect(ghost()).toBeNull(); + }); + it("cancels a new file when its name field loses focus", async () => { const { rows } = setup(); await waitFor(() => expect(rows()).toContain("README.md")); diff --git a/src/features/project-tree/ProjectView.tsx b/src/features/project-tree/ProjectView.tsx index 26548a64..16002ef1 100644 --- a/src/features/project-tree/ProjectView.tsx +++ b/src/features/project-tree/ProjectView.tsx @@ -581,6 +581,11 @@ export function ProjectView() { // Files (not folders) can also be dropped on an editor to open them there. let files: DragSession | null = null; const move = (ev: PointerEvent) => { + // A press with no button held is a reordered macOS tap, not a drag (see pointerDrag.ts). + if (!(ev.buttons & 1)) { + if (!active) cleanup(); + return; + } if (!active) { if (Math.hypot(ev.clientX - startX, ev.clientY - startY) < DRAG_THRESHOLD) return; active = true; @@ -626,6 +631,7 @@ export function ProjectView() { const cleanup = () => { window.removeEventListener("pointermove", move); window.removeEventListener("pointerup", up); + window.removeEventListener("pointercancel", cleanup); window.removeEventListener("keydown", key, true); files?.cancel(); setDropTarget(null); @@ -633,6 +639,7 @@ export function ProjectView() { }; window.addEventListener("pointermove", move); window.addEventListener("pointerup", up); + window.addEventListener("pointercancel", cleanup); window.addEventListener("keydown", key, true); }, []); diff --git a/src/features/services/ServicesTree.reorder.test.tsx b/src/features/services/ServicesTree.reorder.test.tsx index 8ec8f36f..01140081 100644 --- a/src/features/services/ServicesTree.reorder.test.tsx +++ b/src/features/services/ServicesTree.reorder.test.tsx @@ -113,7 +113,16 @@ describe("service reordering integration", () => { const target = within(tree()).getByText("Gateway").closest('[role="treeitem"]')!; Object.defineProperty(document, "elementFromPoint", { configurable: true, value: () => target }); const pointer = (el: Element | Window, type: string, y: number) => - fireEvent(el, new MouseEvent(type, { bubbles: true, button: 0, clientX: 70, clientY: y })); + fireEvent( + el, + new MouseEvent(type, { + bubbles: true, + button: 0, + buttons: type === "pointerup" ? 0 : 1, + clientX: 70, + clientY: y, + }), + ); pointer(source, "pointerdown", 60); pointer(window, "pointermove", 26); expect(document.querySelector('[aria-hidden="true"][role="treeitem"]')).not.toBeNull(); diff --git a/src/features/vcs/reviewGutter.ts b/src/features/vcs/reviewGutter.ts index 8195bb00..7f8c38a8 100644 --- a/src/features/vcs/reviewGutter.ts +++ b/src/features/vcs/reviewGutter.ts @@ -252,6 +252,7 @@ export function reviewGutter(handlers: ReviewGutterHandlers): Extension { const startDrag = (view: EditorView, drag: Drag) => { view.dispatch({ effects: setDrag.of(drag) }); const move = (e: MouseEvent) => { + if (!(e.buttons & 1)) return up(); const line = lineAtY(view, e.clientY); const cur = view.state.field(field).drag; if (line === null || !cur) return; diff --git a/src/features/viewers/panZoom.ts b/src/features/viewers/panZoom.ts index c6ab748a..3b198845 100644 --- a/src/features/viewers/panZoom.ts +++ b/src/features/viewers/panZoom.ts @@ -47,6 +47,7 @@ export function attachPanZoom(viewport: HTMLElement): void { }); viewport.addEventListener("pointermove", (e) => { if (!drag || drag.id !== e.pointerId) return; + if (!(e.buttons & 1)) return end(e); apply({ ...view, x: view.x + e.clientX - drag.x, y: view.y + e.clientY - drag.y }); drag = { ...drag, x: e.clientX, y: e.clientY }; }); diff --git a/src/ui/Dialog.test.tsx b/src/ui/Dialog.test.tsx index 051bf266..d8695f1f 100644 --- a/src/ui/Dialog.test.tsx +++ b/src/ui/Dialog.test.tsx @@ -120,7 +120,7 @@ describe("Dialog chrome", () => { down(dialog.querySelector("div:last-child")!, 10); expect(dialog.style.translate).toBe(""); down(handle, 10); - fireEvent.pointerMove(window, { clientX: 200, clientY: 40 }); + fireEvent.pointerMove(window, { buttons: 1, clientX: 200, clientY: 40 }); fireEvent.pointerUp(window); expect(dialog.style.translate).toBe("190px 30px"); }); @@ -134,12 +134,12 @@ describe("Dialog chrome", () => { const dialog = screen.getByRole("dialog"); const bar = document.querySelector("[data-dialog-titlebar]") as HTMLElement; fireEvent.pointerDown(bar, { button: 0, clientX: 10, clientY: 10 }); - fireEvent.pointerMove(window, { clientX: 60, clientY: 40 }); + fireEvent.pointerMove(window, { buttons: 1, clientX: 60, clientY: 40 }); fireEvent.pointerUp(window); expect(dialog.style.translate).toBe("80px 30px"); // The close button never starts a drag. fireEvent.pointerDown(within(bar).getByRole("button", { name: "Close" }), { button: 0, clientX: 10, clientY: 10 }); - fireEvent.pointerMove(window, { clientX: 200, clientY: 40 }); + fireEvent.pointerMove(window, { buttons: 1, clientX: 200, clientY: 40 }); fireEvent.pointerUp(window); expect(dialog.style.translate).toBe("80px 30px"); }); diff --git a/src/ui/Dialog.tsx b/src/ui/Dialog.tsx index a7f148d9..6482f7e9 100644 --- a/src/ui/Dialog.tsx +++ b/src/ui/Dialog.tsx @@ -299,6 +299,7 @@ function DialogInner({ const rect = el.getBoundingClientRect(); const base = { left: rect.left - offset.x, top: rect.top - offset.y }; const move = (ev: globalThis.PointerEvent) => { + if (!(ev.buttons & 1)) return up(); // Keep the title bar reachable. const x = Math.min( Math.max(ev.clientX - start.x, -base.left - rect.width + 80), @@ -331,6 +332,7 @@ function DialogInner({ const minH = chrome + 120; const origin = offset; const move = (ev: globalThis.PointerEvent) => { + if (!(ev.buttons & 1)) return up(); const w = Math.min(Math.max(rect.width + ev.clientX - start.x, minW), window.innerWidth - 24); const h = Math.min(Math.max(rect.height + ev.clientY - start.y, minH), window.innerHeight - 24); setSize({ w, h }); diff --git a/src/ui/Tabs.test.tsx b/src/ui/Tabs.test.tsx index b43336a1..bb8ca7b5 100644 --- a/src/ui/Tabs.test.tsx +++ b/src/ui/Tabs.test.tsx @@ -226,7 +226,16 @@ describe("Tabs drag & drop", () => { .sort((a, b) => a.getBoundingClientRect().left - b.getBoundingClientRect().left) .map((el) => el.textContent); const pointer = (el: Element | Window, type: string, x: number) => - fireEvent(el, new MouseEvent(type, { bubbles: true, button: 0, clientX: x, clientY: 12 })); + fireEvent( + el, + new MouseEvent(type, { + bubbles: true, + button: 0, + buttons: type === "pointerup" ? 0 : 1, + clientX: x, + clientY: 12, + }), + ); it("moves the other tabs out of the way while a tab is dragged, like IDEA (#1052)", () => { render(); diff --git a/src/ui/Tree.reorder.test.tsx b/src/ui/Tree.reorder.test.tsx index ef640bc8..b8193b07 100644 --- a/src/ui/Tree.reorder.test.tsx +++ b/src/ui/Tree.reorder.test.tsx @@ -67,7 +67,17 @@ const labels = () => .getAllByRole("treeitem") .map((el) => el.textContent); const pointer = (el: Element | Window, type: string, y: number, altKey = false) => - fireEvent(el, new MouseEvent(type, { bubbles: true, button: 0, clientX: 30, clientY: y, altKey })); + fireEvent( + el, + new MouseEvent(type, { + bubbles: true, + button: 0, + buttons: type === "pointerup" ? 0 : 1, + clientX: 30, + clientY: y, + altKey, + }), + ); function drag(source: HTMLElement, target: HTMLElement, after = false, alt = false) { pointer(source, "pointerdown", source.getBoundingClientRect().top + 12, alt); hit = target; diff --git a/src/ui/pointerDrag.test.ts b/src/ui/pointerDrag.test.ts index 8e1e1ed3..843bb39a 100644 --- a/src/ui/pointerDrag.test.ts +++ b/src/ui/pointerDrag.test.ts @@ -1,5 +1,53 @@ -import { describe, expect, it } from "vitest"; -import { dropZoneAt, isOutsideWindow } from "./pointerDrag"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { dropZoneAt, isOutsideWindow, startPointerDrag } from "./pointerDrag"; + +const fire = (type: string, buttons: number, x = 0, y = 0) => + window.dispatchEvent(new MouseEvent(type, { bubbles: true, buttons, clientX: x, clientY: y })); + +describe("startPointerDrag with a reordered macOS tap", () => { + beforeEach(() => { + document.elementFromPoint = () => null; // jsdom lacks it; drop hit-testing needs it + }); + afterEach(() => (document.body.innerHTML = "")); + const start = () => { + const source = document.body.appendChild(document.createElement("div")); + const onStart = vi.fn(); + const onEnd = vi.fn(); + startPointerDrag( + { button: 0, clientX: 0, clientY: 0, altKey: false }, + source, + { type: "t", payload: "" }, + { onStart, onEnd }, + ); + return { onStart, onEnd }; + }; + + it("never starts a drag from moves with no button held", () => { + const { onStart, onEnd } = start(); + fire("pointermove", 0, 40, 0); + fire("pointerup", 0, 40, 0); + expect(onStart).not.toHaveBeenCalled(); + expect(onEnd).toHaveBeenCalledOnce(); + expect(onEnd).toHaveBeenCalledWith(false); + expect(document.querySelector("[aria-hidden]")).toBeNull(); + }); + + it("drags and drops while the button is held", () => { + const { onStart, onEnd } = start(); + fire("pointermove", 1, 40, 0); + fire("pointerup", 0, 40, 0); + expect(onStart).toHaveBeenCalledOnce(); + expect(onEnd).toHaveBeenCalledWith(true, expect.objectContaining({ x: 40 })); + }); + + it("keeps the drop when late moves arrive after the button was released", () => { + const { onEnd } = start(); + fire("pointermove", 1, 40, 0); + fire("pointermove", 0, 60, 0); + fire("pointerup", 0, 60, 0); + expect(onEnd).toHaveBeenCalledWith(true, expect.objectContaining({ x: 60 })); + }); +}); const viewport = { left: 0, top: 0, right: 1280, bottom: 800 }; diff --git a/src/ui/pointerDrag.ts b/src/ui/pointerDrag.ts index 6b5206af..4e2a3fb1 100644 --- a/src/ui/pointerDrag.ts +++ b/src/ui/pointerDrag.ts @@ -146,6 +146,9 @@ export function startPointerDrag( const session = dragSession(data); const onMove = (ev: PointerEvent) => { if (!ghost) { + // macOS WebKit can deliver a tap's pointerup before its pointerdown, so a + // press with no button held would otherwise start a phantom drag. + if (!(ev.buttons & 1)) return onCancel(); if (Math.hypot(ev.clientX - start.x, ev.clientY - start.y) < DRAG_THRESHOLD) return; const r = source.getBoundingClientRect(); ghost = source.cloneNode(true) as HTMLElement; @@ -165,6 +168,8 @@ export function startPointerDrag( document.documentElement.classList.add(styles.dragging!); opts.onStart?.(); } + // Late moves after release may still precede the pointerup; ignore, don't cancel the drop. + if (!(ev.buttons & 1)) return; ghost.style.transform = `translate(${ev.clientX - start.x}px, ${ev.clientY - start.y}px)`; ghost.classList.toggle(styles.noTarget!, !session.move(ev.clientX, ev.clientY)); }; diff --git a/src/workbench/WindowGroupTabs.tsx b/src/workbench/WindowGroupTabs.tsx index 49e873e3..588091b1 100644 --- a/src/workbench/WindowGroupTabs.tsx +++ b/src/workbench/WindowGroupTabs.tsx @@ -30,6 +30,7 @@ export function WindowGroupTabs() { drag.current = { label: tab.label, x: e.clientX, moved: false }; }} onPointerMove={(e) => { + if (drag.current && !(e.buttons & 1)) drag.current = null; // reordered macOS tap, not a drag if (drag.current && Math.abs(e.clientX - drag.current.x) > 5) { drag.current.moved = true; e.currentTarget.setPointerCapture(e.pointerId); diff --git a/src/workbench/toolwindows/ToolWindows.tsx b/src/workbench/toolwindows/ToolWindows.tsx index b6c7e4ec..07e7afd8 100644 --- a/src/workbench/toolwindows/ToolWindows.tsx +++ b/src/workbench/toolwindows/ToolWindows.tsx @@ -219,6 +219,7 @@ function StripeHandle({ document.body.style.cursor = "col-resize"; let next = { names, width, px: start }; const move = (ev: globalThis.PointerEvent) => { + if (!(ev.buttons & 1)) return end(); const r = resolve(start + sign * (ev.clientX - origin)); if (r.names !== next.names) onPreview(r.names); next = r; @@ -314,6 +315,8 @@ function startStripeDrag(e: PointerEvent, id: string) { }; const onMove = (ev: globalThis.PointerEvent) => { if (!ghost) { + // Reordered macOS tap: no button held, so not a drag (see pointerDrag.ts). + if (!(ev.buttons & 1)) return finish(false); if (Math.hypot(ev.clientX - start.x, ev.clientY - start.y) < DRAG_THRESHOLD) return; const r = button.getBoundingClientRect(); ghost = button.cloneNode(true) as HTMLElement; @@ -328,6 +331,7 @@ function startStripeDrag(e: PointerEvent, id: string) { document.body.appendChild(ghost); useStripeDrag.setState({ id, drop: null }); } + if (!(ev.buttons & 1)) return; place(ev.clientX, ev.clientY); const drop = stripeDropAt(ev.clientX, ev.clientY); const cur = useStripeDrag.getState().drop; @@ -812,6 +816,7 @@ function SplitHandle({ anchor, hidden }: { anchor: ToolWindowAnchor; hidden: boo document.body.style.cursor = stacked ? "row-resize" : "col-resize"; let ratio: number | null = null; const move = (ev: globalThis.PointerEvent) => { + if (!(ev.buttons & 1)) return end(); ratio = ratioOf(m.clamp(m.first + (stacked ? ev.clientY : ev.clientX) - origin), m.total); apply(m.areas, ratio); }; @@ -963,6 +968,7 @@ function GapHandle({ document.body.style.cursor = vertical ? "col-resize" : "row-resize"; let next = m.size; const move = (ev: globalThis.PointerEvent) => { + if (!(ev.buttons & 1)) return end(); const raw = m.size + sign * ((vertical ? ev.clientX : ev.clientY) - origin); // Dragged past half the minimum: collapse, as IDEA does. next = raw < MIN[anchor] / 2 ? 0 : clamp(raw, m.max);