From 5c343ab41645c12f4267edcb004cffd7a69df229 Mon Sep 17 00:00:00 2001 From: zipg Date: Sun, 9 Aug 2026 13:57:02 +0800 Subject: [PATCH] fix(tab): avoid accidental drag on slight tab movement --- .../src/components/layout/AppTabBar.vue | 4 +- .../composables/__tests__/useTabDrag.spec.ts | 93 +++++++++++++++++++ apps/desktop/src/composables/useTabDrag.ts | 19 ++-- apps/desktop/src/stores/queryStore.ts | 15 ++- packages/app-tests/queryStore.test.ts | 19 +++- 5 files changed, 132 insertions(+), 18 deletions(-) create mode 100644 apps/desktop/src/composables/__tests__/useTabDrag.spec.ts diff --git a/apps/desktop/src/components/layout/AppTabBar.vue b/apps/desktop/src/components/layout/AppTabBar.vue index 3656a61b39..eff8eecbab 100644 --- a/apps/desktop/src/components/layout/AppTabBar.vue +++ b/apps/desktop/src/components/layout/AppTabBar.vue @@ -47,7 +47,7 @@ const queryStore = useQueryStore(); const settingsStore = useSettingsStore(); const { toast } = useToast(); const tabDrag = useTabDrag((draggedId, targetId, position) => { - queryStore.reorderTab(draggedId, targetId, position); + return queryStore.reorderTab(draggedId, targetId, position); }); const editingTabId = ref(null); const editingTitle = ref(""); @@ -538,7 +538,7 @@ function tabMenuIcon(tab: QueryTab) { } function handleTabClick(tab: QueryTab) { - if (tabDrag.state.wasDragged) return; + if (tabDrag.state.suppressClick) return; activateTab(tab.id); } diff --git a/apps/desktop/src/composables/__tests__/useTabDrag.spec.ts b/apps/desktop/src/composables/__tests__/useTabDrag.spec.ts new file mode 100644 index 0000000000..ad3e32a3ac --- /dev/null +++ b/apps/desktop/src/composables/__tests__/useTabDrag.spec.ts @@ -0,0 +1,93 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { TAB_DRAG_HORIZONTAL_THRESHOLD, useTabDrag } from "@/composables/useTabDrag"; + +function createDragHarness(onDrop = vi.fn(() => true)) { + const drag = useTabDrag(onDrop); + const source = document.createElement("div"); + source.innerHTML = 'Source'; + source.addEventListener("mousedown", (event) => drag.startDrag(event, "source")); + document.body.appendChild(source); + return { drag, source, onDrop }; +} + +function beginDrag(source: HTMLElement, x = 100, y = 20) { + source.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, button: 0, clientX: x, clientY: y })); +} + +function movePointer(x: number, y = 20) { + document.dispatchEvent(new MouseEvent("mousemove", { bubbles: true, clientX: x, clientY: y })); +} + +function releasePointer() { + document.dispatchEvent(new MouseEvent("mouseup", { bubbles: true })); +} + +function enterDropTarget(drag: ReturnType, tabId = "target", x = 10) { + const target = document.createElement("div"); + target.getBoundingClientRect = () => ({ x: 0, y: 0, left: 0, top: 0, right: 100, bottom: 28, width: 100, height: 28, toJSON: () => ({}) }); + target.addEventListener("mousemove", (event) => drag.updateTarget(event, tabId)); + document.body.appendChild(target); + target.dispatchEvent(new MouseEvent("mousemove", { bubbles: true, clientX: x, clientY: 20 })); +} + +afterEach(() => { + releasePointer(); + document.body.innerHTML = ""; + document.body.style.cursor = ""; + document.body.style.userSelect = ""; +}); + +describe("useTabDrag", () => { + it("ignores sub-threshold horizontal movement and vertical pointer jitter", () => { + const { drag, source, onDrop } = createDragHarness(); + beginDrag(source); + + movePointer(100 + TAB_DRAG_HORIZONTAL_THRESHOLD - 1, 120); + + expect(drag.state.active).toBe(false); + expect(drag.state.suppressClick).toBe(false); + expect(onDrop).not.toHaveBeenCalled(); + }); + + it("does not suppress the click when a drag gesture has no valid drop target", () => { + const { drag, source, onDrop } = createDragHarness(); + beginDrag(source); + movePointer(100 + TAB_DRAG_HORIZONTAL_THRESHOLD); + expect(drag.state.active).toBe(true); + + releasePointer(); + + expect(onDrop).not.toHaveBeenCalled(); + expect(drag.state.suppressClick).toBe(false); + }); + + it("does not suppress the click when the drop callback reports no reorder", () => { + const onDrop = vi.fn(() => false); + const { drag, source } = createDragHarness(onDrop); + beginDrag(source); + movePointer(100 + TAB_DRAG_HORIZONTAL_THRESHOLD); + enterDropTarget(drag); + + releasePointer(); + + expect(onDrop).toHaveBeenCalledWith("source", "target", "before"); + expect(drag.state.suppressClick).toBe(false); + }); + + it("suppresses the click only after a completed reorder", () => { + const { drag, source, onDrop } = createDragHarness(); + beginDrag(source); + movePointer(100 + TAB_DRAG_HORIZONTAL_THRESHOLD); + enterDropTarget(drag, "target", 90); + + releasePointer(); + + expect(onDrop).toHaveBeenCalledWith("source", "target", "after"); + expect(drag.state.suppressClick).toBe(true); + + beginDrag(source); + expect(drag.state.suppressClick).toBe(false); + }); +}); diff --git a/apps/desktop/src/composables/useTabDrag.ts b/apps/desktop/src/composables/useTabDrag.ts index f798555809..d28826045e 100644 --- a/apps/desktop/src/composables/useTabDrag.ts +++ b/apps/desktop/src/composables/useTabDrag.ts @@ -7,19 +7,19 @@ interface TabDragState { draggedId: string | null; targetId: string | null; dropPosition: TabDropPosition | null; - wasDragged: boolean; + suppressClick: boolean; startX: number; startY: number; } -const DRAG_THRESHOLD = 5; +export const TAB_DRAG_HORIZONTAL_THRESHOLD = 12; const state = reactive({ active: false, draggedId: null, targetId: null, dropPosition: null, - wasDragged: false, + suppressClick: false, startX: 0, startY: 0, }); @@ -30,7 +30,7 @@ let pending: { y: number; sourceEl: HTMLElement | null; } | null = null; -let onDropCallback: ((draggedId: string, targetId: string, position: TabDropPosition) => void) | null = null; +let onDropCallback: ((draggedId: string, targetId: string, position: TabDropPosition) => boolean) | null = null; let ghostEl: HTMLElement | null = null; function createGhost(sourceEl: HTMLElement, x: number, y: number) { @@ -79,10 +79,8 @@ function onMouseMove(event: MouseEvent) { if (pending && !state.active) { const dx = event.clientX - pending.x; - const dy = event.clientY - pending.y; - if (Math.abs(dx) < DRAG_THRESHOLD && Math.abs(dy) < DRAG_THRESHOLD) return; + if (Math.abs(dx) < TAB_DRAG_HORIZONTAL_THRESHOLD) return; state.active = true; - state.wasDragged = true; state.draggedId = pending.id; state.startX = pending.x; state.startY = pending.y; @@ -100,8 +98,9 @@ function onMouseMove(event: MouseEvent) { } function onMouseUp() { + state.suppressClick = false; if (state.active && state.draggedId && state.targetId && state.dropPosition && onDropCallback) { - onDropCallback(state.draggedId, state.targetId, state.dropPosition); + state.suppressClick = onDropCallback(state.draggedId, state.targetId, state.dropPosition); } reset(); } @@ -128,7 +127,7 @@ function ensureListeners() { listenersAttached = true; } -export function useTabDrag(onDrop: (draggedId: string, targetId: string, position: TabDropPosition) => void) { +export function useTabDrag(onDrop: (draggedId: string, targetId: string, position: TabDropPosition) => boolean) { ensureListeners(); onDropCallback = onDrop; @@ -136,7 +135,7 @@ export function useTabDrag(onDrop: (draggedId: string, targetId: string, positio if (event.button !== 0) return; const target = event.target as HTMLElement; if (target.closest("button, input, [data-tab-title-input]")) return; - state.wasDragged = false; + state.suppressClick = false; const el = (event.currentTarget as HTMLElement) || null; pending = { id: tabId, x: event.clientX, y: event.clientY, sourceEl: el }; } diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index e7c6187b0a..1cc26addb9 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -2702,11 +2702,16 @@ export const useQueryStore = defineStore("query", () => { function reorderTab(id: string, targetId: string, position: "before" | "after") { const fromIdx = tabs.value.findIndex((t) => t.id === id); const toIdx = tabs.value.findIndex((t) => t.id === targetId); - if (fromIdx < 0 || toIdx < 0 || fromIdx === toIdx) return; - const [tab] = tabs.value.splice(fromIdx, 1); - const newToIdx = tabs.value.findIndex((t) => t.id === targetId); - tabs.value.splice(newToIdx + (position === "after" ? 1 : 0), 0, tab); - tabs.value = orderPinnedFirst(tabs.value, (item) => !!item.pinned); + if (fromIdx < 0 || toIdx < 0 || fromIdx === toIdx) return false; + + const reordered = [...tabs.value]; + const [tab] = reordered.splice(fromIdx, 1); + const newToIdx = reordered.findIndex((t) => t.id === targetId); + reordered.splice(newToIdx + (position === "after" ? 1 : 0), 0, tab); + const nextTabs = orderPinnedFirst(reordered, (item) => !!item.pinned); + if (nextTabs.every((item, index) => item.id === tabs.value[index]?.id)) return false; + tabs.value = nextTabs; + return true; } function persistSavedSqlExecutionTarget(tab: QueryTab, options: UpdateExecutionTargetOptions) { diff --git a/packages/app-tests/queryStore.test.ts b/packages/app-tests/queryStore.test.ts index bfb7ed41c0..2b7ea5a5d1 100644 --- a/packages/app-tests/queryStore.test.ts +++ b/packages/app-tests/queryStore.test.ts @@ -7302,9 +7302,26 @@ test("reorderTab with after position places tab correctly", () => { const tabC = store.createTab("conn-1", "db", "C", "query"); // Drag A after C - store.reorderTab(tabA, tabC, "after"); + assert.equal(store.reorderTab(tabA, tabC, "after"), true); assert.deepEqual( store.tabs.map((t) => t.id), [tabB, tabC, tabA], ); }); + +test("reorderTab reports adjacent no-op drops without replacing tab order", () => { + setActivePinia(createPinia()); + const store = useQueryStore(); + + const tabA = store.createTab("conn-1", "db", "A", "query"); + const tabB = store.createTab("conn-1", "db", "B", "query"); + const tabC = store.createTab("conn-1", "db", "C", "query"); + const originalTabs = [...store.tabs]; + + assert.equal(store.reorderTab(tabA, tabB, "before"), false); + assert.deepEqual(store.tabs, originalTabs); + assert.equal(store.reorderTab(tabB, tabA, "after"), false); + assert.deepEqual(store.tabs, originalTabs); + assert.equal(store.reorderTab(tabC, "missing", "before"), false); + assert.deepEqual(store.tabs, originalTabs); +});