From b651b4be6d03e2ec8266c1cda6c7d16ece41315c Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 25 Jun 2026 17:43:45 -0700 Subject: [PATCH] Fix tab switching after missed drag cleanup (#6392) Co-authored-by: Orca --- .../tab-group/useTabDragSplit.test.ts | 62 +++++++++++++++++++ .../components/tab-group/useTabDragSplit.ts | 52 ++++++++++++++-- tests/e2e/tabs.spec.ts | 57 +++++++++++++++++ 3 files changed, 167 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/components/tab-group/useTabDragSplit.test.ts b/src/renderer/src/components/tab-group/useTabDragSplit.test.ts index 453096115..9c526ce25 100644 --- a/src/renderer/src/components/tab-group/useTabDragSplit.test.ts +++ b/src/renderer/src/components/tab-group/useTabDragSplit.test.ts @@ -245,6 +245,68 @@ describe('canDropTabIntoPaneBody', () => { }) describe('useTabDragSplit', () => { + it.each(['pointerup', 'pointercancel', 'blur'])( + 'clears a stuck active drag when %s arrives without a dnd end event', + async (eventName) => { + const activeData = makeDragData('group-1') + const drag = renderDragHook() + + act(() => { + drag.onDragStart( + makeDragEvent(activeData, { x: 120, y: 20 }) as unknown as Parameters< + typeof drag.onDragStart + >[0] + ) + // Why: dispatch in the same turn as drag start so the fallback must be + // installed synchronously, before React can run passive effects. + window.dispatchEvent(new MouseEvent(eventName, { bubbles: true })) + }) + expect(drag.isTabDragActiveRef.current).toBe(true) + + await act(async () => { + await new Promise((resolve) => window.setTimeout(resolve, 0)) + }) + + expect(drag.isTabDragActiveRef.current).toBe(false) + } + ) + + it('does not cancel a legitimate drag end when the fallback timer is pending', async () => { + addPanelGeometry( + 'group-2', + rect({ left: 500, top: 0, width: 400, height: 600 }), + rect({ left: 500, top: 32, width: 400, height: 568 }) + ) + const activeData = makeDragData('group-1') + const dropUnifiedTab = vi.fn(() => true) + useAppStore.setState({ dropUnifiedTab } as Partial>) + + const drag = renderDragHook() + + act(() => { + drag.onDragStart( + makeDragEvent(activeData, { x: 120, y: 20 }) as unknown as Parameters< + typeof drag.onDragStart + >[0] + ) + window.dispatchEvent(new MouseEvent('pointerup', { bubbles: true })) + drag.onDragEnd( + makeDragEvent(activeData, { x: 880, y: 300 }) as unknown as Parameters< + typeof drag.onDragEnd + >[0] + ) + }) + await act(async () => { + await new Promise((resolve) => window.setTimeout(resolve, 0)) + }) + + expect(drag.isTabDragActiveRef.current).toBe(false) + expect(dropUnifiedTab).toHaveBeenCalledWith('tab-1', { + groupId: 'group-2', + splitDirection: 'right' + }) + }) + it('commits a geometry-only pane split when drag end has no over target', () => { addPanelGeometry( 'group-2', diff --git a/src/renderer/src/components/tab-group/useTabDragSplit.ts b/src/renderer/src/components/tab-group/useTabDragSplit.ts index 8515942da..6d945d071 100644 --- a/src/renderer/src/components/tab-group/useTabDragSplit.ts +++ b/src/renderer/src/components/tab-group/useTabDragSplit.ts @@ -186,6 +186,7 @@ export function useTabDragSplit({ const lastHoveredTabPreviewRef = useRef<{ groupId: string; tabId: string } | null>(null) const tabDragActiveRef = useRef(false) const dragGeometryRef = useRef(null) + const releaseMissedEndFallbackRef = useRef<(() => void) | null>(null) const tabInsertion = useHoveredTabInsertion(isTabDragData, getDragPointer) // Why: hidden worktrees stay mounted so their PTYs survive worktree @@ -204,6 +205,44 @@ export function useTabDragSplit({ releaseWebviewDragPassthroughRef.current = null }, []) + const releaseMissedEndFallback = useCallback(() => { + releaseMissedEndFallbackRef.current?.() + releaseMissedEndFallbackRef.current = null + }, []) + + const clearDragStateRef = useRef<() => void>(() => {}) + + const installMissedEndFallback = useCallback(() => { + releaseMissedEndFallback() + + let cleanupTimer: number | null = null + const clearIfDndMissedEnd = (): void => { + if (cleanupTimer !== null) { + window.clearTimeout(cleanupTimer) + } + cleanupTimer = window.setTimeout(() => { + cleanupTimer = null + if (tabDragActiveRef.current) { + // Why: Electron/dnd-kit can occasionally miss drag end/cancel; a + // stuck drag ref makes all later tab clicks look like drag releases. + clearDragStateRef.current() + } + }, 0) + } + + window.addEventListener('pointerup', clearIfDndMissedEnd) + window.addEventListener('pointercancel', clearIfDndMissedEnd) + window.addEventListener('blur', clearIfDndMissedEnd) + releaseMissedEndFallbackRef.current = () => { + if (cleanupTimer !== null) { + window.clearTimeout(cleanupTimer) + } + window.removeEventListener('pointerup', clearIfDndMissedEnd) + window.removeEventListener('pointercancel', clearIfDndMissedEnd) + window.removeEventListener('blur', clearIfDndMissedEnd) + } + }, [releaseMissedEndFallback]) + const acquireWebviewDragPassthrough = useCallback(() => { // Why: dnd-kit tab drags are pointer-driven, so the native drag listeners // in webview-registry never fire. Put webviews in passthrough explicitly. @@ -217,15 +256,18 @@ export function useTabDragSplit({ return } // Why: this root owns the dnd-kit gesture that temporarily puts browser - // webviews in pointer passthrough, so root teardown must release it. + // webviews in pointer passthrough and installs global fallback listeners, + // so root teardown must release both. releaseWebviewDragPassthrough() + releaseMissedEndFallback() }, - [releaseWebviewDragPassthrough] + [releaseMissedEndFallback, releaseWebviewDragPassthrough] ) const clearDragState = useCallback(() => { tabDragActiveRef.current = false releaseWebviewDragPassthrough() + releaseMissedEndFallback() setActiveDrag(null) setHoveredDropTarget(null) tabInsertion.clear() @@ -233,7 +275,8 @@ export function useTabDragSplit({ lastPreviewRef.current = null lastHoveredTabPreviewRef.current = null dragGeometryRef.current = null - }, [releaseWebviewDragPassthrough, tabInsertion]) + }, [releaseMissedEndFallback, releaseWebviewDragPassthrough, tabInsertion]) + clearDragStateRef.current = clearDragState const restorePreDragActivation = useCallback(() => { const snapshot = preDragActivationSnapshotRef.current @@ -363,11 +406,12 @@ export function useTabDragSplit({ setActiveDrag(dragData) tabDragActiveRef.current = true + installMissedEndFallback() dragGeometryRef.current = captureTabGroupPanelGeometrySnapshot(worktreeId) preDragActivationSnapshotRef.current = captureTabDragActivationSnapshot(worktreeId) acquireWebviewDragPassthrough() }, - [acquireWebviewDragPassthrough, clearDragState, worktreeId] + [acquireWebviewDragPassthrough, clearDragState, installMissedEndFallback, worktreeId] ) const onDragMove = useCallback( diff --git a/tests/e2e/tabs.spec.ts b/tests/e2e/tabs.spec.ts index 5b1e2f0d9..cb00bd0d4 100644 --- a/tests/e2e/tabs.spec.ts +++ b/tests/e2e/tabs.spec.ts @@ -279,6 +279,63 @@ test.describe('Tabs', () => { .toEqual([domOrderBefore[1], domOrderBefore[0], ...domOrderBefore.slice(2)]) }) + test('clicking tabs still switches after a tab drag gesture releases', async ({ orcaPage }) => { + const worktreeId = (await getActiveWorktreeId(orcaPage))! + + await orcaPage.evaluate((targetWorktreeId) => { + const store = window.__store + if (!store) { + return + } + const state = store.getState() + const existing = (state.tabsByWorktree[targetWorktreeId] ?? []).length + for (let i = existing; i < 2; i++) { + state.createTab(targetWorktreeId) + } + }, worktreeId) + await expect + .poll(() => countRenderedTabs(orcaPage), { timeout: 5_000 }) + .toBeGreaterThanOrEqual(2) + + const domOrder = await orcaPage.$$eval(SORTABLE_TAB, (nodes) => + nodes.map((n) => (n as HTMLElement).dataset.tabId ?? '') + ) + const [firstTabId, secondTabId] = domOrder + expect(firstTabId).toBeTruthy() + expect(secondTabId).toBeTruthy() + + await orcaPage.evaluate((tabId) => { + window.__store?.getState().setActiveTab(tabId) + }, firstTabId) + await expect.poll(() => getDomActiveTabId(orcaPage), { timeout: 3_000 }).toBe(firstTabId) + + const firstTabBox = await tabLocator(orcaPage, firstTabId).boundingBox() + expect(firstTabBox).not.toBeNull() + const startX = firstTabBox!.x + firstTabBox!.width / 2 + const startY = firstTabBox!.y + firstTabBox!.height / 2 + await orcaPage.mouse.move(startX, startY) + await orcaPage.mouse.down() + // Why: exceed dnd-kit's 12px tab-drag threshold so this exercises the + // drag/click handshake, not just an ordinary tab press. + await orcaPage.mouse.move(startX + 24, startY, { steps: 4 }) + await orcaPage.mouse.up() + + // Reset selection through setup state, then prove the user-visible click + // path still activates another tab after the drag release. + await orcaPage.evaluate((tabId) => { + window.__store?.getState().setActiveTab(tabId) + }, firstTabId) + await expect.poll(() => getDomActiveTabId(orcaPage), { timeout: 3_000 }).toBe(firstTabId) + + await tabLocator(orcaPage, secondTabId).click({ force: true }) + await expect + .poll(() => getDomActiveTabId(orcaPage), { + timeout: 5_000, + message: 'Tab click did not activate after a completed tab drag gesture' + }) + .toBe(secondTabId) + }) + /** * Regression: after a drag-reorder, Cmd/Ctrl+Shift+[ must walk tabs in * the new visible order. The pre-fix bug read a stale legacy order