Fix tab switching after missed drag cleanup (#6392)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo Hong 2026-06-25 17:43:45 -07:00 committed by GitHub
parent e257d79294
commit b651b4be6d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 167 additions and 4 deletions

View File

@ -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<ReturnType<typeof useAppStore.getState>>)
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',

View File

@ -186,6 +186,7 @@ export function useTabDragSplit({
const lastHoveredTabPreviewRef = useRef<{ groupId: string; tabId: string } | null>(null)
const tabDragActiveRef = useRef(false)
const dragGeometryRef = useRef<TabGroupPanelGeometrySnapshot | null>(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(

View File

@ -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