diff --git a/src/renderer/src/components/tab-bar/BrowserTab.test.tsx b/src/renderer/src/components/tab-bar/BrowserTab.test.tsx index 1c8b183821c..a53e80e0117 100644 --- a/src/renderer/src/components/tab-bar/BrowserTab.test.tsx +++ b/src/renderer/src/components/tab-bar/BrowserTab.test.tsx @@ -36,13 +36,6 @@ vi.mock('@dnd-kit/sortable', () => ({ }) })) -vi.mock('./tab-strip-pointer-activation', () => ({ - useTabStripPointerActivation: () => ({ - isPressed: false, - onPointerDown: vi.fn() - }) -})) - vi.mock('lucide-react', () => ({ Columns2: function Columns2(props: Record) { return { type: 'Columns2', props } diff --git a/src/renderer/src/components/tab-bar/BrowserTab.tsx b/src/renderer/src/components/tab-bar/BrowserTab.tsx index 04eb73649f5..83ad2c5f45f 100644 --- a/src/renderer/src/components/tab-bar/BrowserTab.tsx +++ b/src/renderer/src/components/tab-bar/BrowserTab.tsx @@ -20,13 +20,11 @@ import { getDropIndicatorClasses, getTabRootStateClasses, getTabStripBorderClasses, - showsTabSelectionChrome, type DropIndicator } from './drop-indicator' import { preventMiddleButtonDefault } from './middle-button-default-guard' import { translate } from '@/i18n/i18n' import { TAB_CONTAINER_WIDTH_CLASSES, TAB_LABEL_WIDTH_CLASSES } from './tab-width-rules' -import { useTabStripPointerActivation } from './tab-strip-pointer-activation' import { TabWorkspaceLayoutMenuSection } from './TabWorkspaceLayoutMenuSection' function formatBrowserTabUrlLabel(url: string): string { @@ -171,11 +169,6 @@ export default function BrowserTab({ return () => window.removeEventListener('blur', dismiss) }, [menuOpen]) - const { isPressed, onPointerDown: onTabPointerDown } = useTabStripPointerActivation({ - onActivate - }) - const showsSelectionChrome = showsTabSelectionChrome(isActive, isPressed) - const tabRoot = (
{ - onTabPointerDown( - e, - listeners?.onPointerDown as ((event: React.PointerEvent) => void) | undefined - ) + if (e.button !== 0) { + return + } + onActivate() + listeners?.onPointerDown?.(e) }} onMouseDown={(e) => { if (e.button === 1) { @@ -207,7 +201,7 @@ export default function BrowserTab({ } }} > - {showsSelectionChrome && } + {isActive && } {/* Why: the browser tab icon is the only non-terminal, non-editor surface in the tab strip. Coloring the Globe blue (matching the in-app browser's identity and the default tab insertion bar) @@ -224,7 +218,7 @@ export default function BrowserTab({ {!isPinned && ( - - - ) -} - -let root: Root | null = null -let container: HTMLDivElement | null = null - -function renderProbe(onActivate = vi.fn()): { - onActivate: ReturnType - tabButton: HTMLButtonElement - dragButton: HTMLButtonElement -} { - container = document.createElement('div') - document.body.appendChild(container) - root = createRoot(container) - act(() => { - root?.render() - }) - const buttons = container.querySelectorAll('button') - return { - onActivate, - tabButton: buttons[0] as HTMLButtonElement, - dragButton: buttons[1] as HTMLButtonElement - } -} - -function dispatchPointer(target: EventTarget, type: string, button = 0): void { - target.dispatchEvent(new MouseEvent(type, { bubbles: true, button })) -} - -afterEach(() => { - act(() => root?.unmount()) - container?.remove() - root = null - container = null -}) - -describe('useTabStripPointerActivation', () => { - it('defers activation until pointerup', () => { - const { onActivate, tabButton } = renderProbe() - - act(() => dispatchPointer(tabButton, 'pointerdown')) - expect(tabButton.dataset.pressed).toBe('true') - expect(onActivate).not.toHaveBeenCalled() - - act(() => dispatchPointer(window, 'pointerup')) - expect(tabButton.dataset.pressed).toBe('false') - expect(onActivate).toHaveBeenCalledTimes(1) - }) - - it('activates when the release event reports no changed button', () => { - const { onActivate, tabButton } = renderProbe() - - act(() => dispatchPointer(tabButton, 'pointerdown')) - act(() => dispatchPointer(window, 'pointerup', -1)) - - expect(tabButton.dataset.pressed).toBe('false') - expect(onActivate).toHaveBeenCalledTimes(1) - }) - - it('cancels pending activation on pointercancel', () => { - const { onActivate, tabButton } = renderProbe() - - act(() => dispatchPointer(tabButton, 'pointerdown')) - act(() => dispatchPointer(window, 'pointercancel')) - - expect(tabButton.dataset.pressed).toBe('false') - expect(onActivate).not.toHaveBeenCalled() - }) - - it('clears pending activation when a drag starts', () => { - const { onActivate, tabButton, dragButton } = renderProbe() - - act(() => dispatchPointer(tabButton, 'pointerdown')) - act(() => dragButton.click()) - act(() => dispatchPointer(window, 'pointerup')) - - expect(tabButton.dataset.pressed).toBe('false') - expect(onActivate).not.toHaveBeenCalled() - }) -}) diff --git a/src/renderer/src/components/tab-bar/tab-strip-pointer-activation.ts b/src/renderer/src/components/tab-bar/tab-strip-pointer-activation.ts deleted file mode 100644 index a61748038c4..00000000000 --- a/src/renderer/src/components/tab-bar/tab-strip-pointer-activation.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { useCallback, useLayoutEffect, useRef, useState } from 'react' -import { useTabDragActive, useTabDragActiveRef } from '../tab-group/tab-drag-context' - -export function useTabStripPointerActivation({ - onActivate, - disabled = false -}: { - onActivate: () => void - disabled?: boolean -}): { - isPressed: boolean - onPointerDown: ( - event: React.PointerEvent, - dragListener?: (event: React.PointerEvent) => void - ) => void -} { - const isTabDragActive = useTabDragActive() - const isTabDragActiveRef = useTabDragActiveRef() - const [isPressed, setIsPressed] = useState(false) - const pendingActivationRef = useRef(false) - const onActivateRef = useRef(onActivate) - onActivateRef.current = onActivate - - useLayoutEffect(() => { - if (!isTabDragActive) { - return - } - pendingActivationRef.current = false - setIsPressed(false) - }, [isTabDragActive]) - - useLayoutEffect(() => { - if (!isPressed) { - return - } - const finishPointerPress = (): void => { - // Why: pointerup often reports button -1/no changed button; the left-button - // gate is on pointerdown, so release must always clear the pending click. - const shouldActivate = pendingActivationRef.current && !isTabDragActiveRef.current - pendingActivationRef.current = false - setIsPressed(false) - if (shouldActivate) { - onActivateRef.current() - } - } - const cancelPointerPress = (): void => { - pendingActivationRef.current = false - setIsPressed(false) - } - window.addEventListener('pointerup', finishPointerPress) - window.addEventListener('pointercancel', cancelPointerPress) - return () => { - window.removeEventListener('pointerup', finishPointerPress) - window.removeEventListener('pointercancel', cancelPointerPress) - } - }, [isPressed, isTabDragActiveRef]) - - const onPointerDown = useCallback( - (event: React.PointerEvent, dragListener?: (event: React.PointerEvent) => void) => { - if (disabled || event.button !== 0) { - return - } - pendingActivationRef.current = true - setIsPressed(true) - dragListener?.(event) - }, - [disabled] - ) - - return { isPressed, onPointerDown } -} diff --git a/src/renderer/src/components/tab-group/tab-insertion.test.ts b/src/renderer/src/components/tab-group/tab-insertion.test.ts index 5acc6569b46..1008956d9c3 100644 --- a/src/renderer/src/components/tab-group/tab-insertion.test.ts +++ b/src/renderer/src/components/tab-group/tab-insertion.test.ts @@ -172,7 +172,7 @@ describe('resolveTabInsertion', () => { // --------------------------------------------------------------------------- describe('resolveTabIndicatorEdges', () => { - it('marks both tabs around a left-edge insertion slot', () => { + it('marks one edge for a left-edge insertion slot', () => { const hovered: HoveredTabInsertion = { groupId: 'group-1', visibleTabId: 'tab-2', @@ -180,12 +180,11 @@ describe('resolveTabIndicatorEdges', () => { } expect(resolveTabIndicatorEdges(['tab-1', 'tab-2', 'tab-3'], hovered)).toEqual([ - { visibleTabId: 'tab-1', side: 'right' }, { visibleTabId: 'tab-2', side: 'left' } ]) }) - it('marks both tabs around a right-edge insertion slot', () => { + it('marks one edge for a right-edge insertion slot', () => { const hovered: HoveredTabInsertion = { groupId: 'group-1', visibleTabId: 'tab-2', @@ -193,7 +192,6 @@ describe('resolveTabIndicatorEdges', () => { } expect(resolveTabIndicatorEdges(['tab-1', 'tab-2', 'tab-3'], hovered)).toEqual([ - { visibleTabId: 'tab-2', side: 'right' }, { visibleTabId: 'tab-3', side: 'left' } ]) }) diff --git a/src/renderer/src/components/tab-group/tab-insertion.ts b/src/renderer/src/components/tab-group/tab-insertion.ts index acd40776fda..708e4616b19 100644 --- a/src/renderer/src/components/tab-group/tab-insertion.ts +++ b/src/renderer/src/components/tab-group/tab-insertion.ts @@ -62,18 +62,10 @@ export function resolveTabIndicatorEdges( } const insertionIndex = hoveredIndex + (hoveredTabInsertion.side === 'right' ? 1 : 0) - const edges: TabIndicatorEdge[] = [] - - // Why: VS Code draws the insertion cue by marking both tabs adjacent to the - // slot so the two 1px edges read as one continuous bar between them. - if (insertionIndex > 0) { - edges.push({ visibleTabId: orderedVisibleTabIds[insertionIndex - 1]!, side: 'right' }) - } if (insertionIndex < orderedVisibleTabIds.length) { - edges.push({ visibleTabId: orderedVisibleTabIds[insertionIndex]!, side: 'left' }) + return [{ visibleTabId: orderedVisibleTabIds[insertionIndex]!, side: 'left' }] } - - return edges + return [{ visibleTabId: orderedVisibleTabIds[insertionIndex - 1]!, side: 'right' }] } function equal(a: HoveredTabInsertion | null, b: HoveredTabInsertion | null): boolean { diff --git a/tests/e2e/tabs.spec.ts b/tests/e2e/tabs.spec.ts index cb00bd0d482..369d11ac81c 100644 --- a/tests/e2e/tabs.spec.ts +++ b/tests/e2e/tabs.spec.ts @@ -279,7 +279,9 @@ test.describe('Tabs', () => { .toEqual([domOrderBefore[1], domOrderBefore[0], ...domOrderBefore.slice(2)]) }) - test('clicking tabs still switches after a tab drag gesture releases', async ({ orcaPage }) => { + test('clicking tabs still switches after dragging a terminal tab to reorder', async ({ + orcaPage + }) => { const worktreeId = (await getActiveWorktreeId(orcaPage))! await orcaPage.evaluate((targetWorktreeId) => { @@ -297,41 +299,49 @@ test.describe('Tabs', () => { .poll(() => countRenderedTabs(orcaPage), { timeout: 5_000 }) .toBeGreaterThanOrEqual(2) - const domOrder = await orcaPage.$$eval(SORTABLE_TAB, (nodes) => + const domOrderBefore = await orcaPage.$$eval(SORTABLE_TAB, (nodes) => nodes.map((n) => (n as HTMLElement).dataset.tabId ?? '') ) - const [firstTabId, secondTabId] = domOrder + const [firstTabId, secondTabId] = domOrderBefore expect(firstTabId).toBeTruthy() expect(secondTabId).toBeTruthy() - await orcaPage.evaluate((tabId) => { - window.__store?.getState().setActiveTab(tabId) - }, firstTabId) + await tabLocator(orcaPage, firstTabId).click({ force: true }) await expect.poll(() => getDomActiveTabId(orcaPage), { timeout: 3_000 }).toBe(firstTabId) const firstTabBox = await tabLocator(orcaPage, firstTabId).boundingBox() + const secondTabBox = await tabLocator(orcaPage, secondTabId).boundingBox() expect(firstTabBox).not.toBeNull() + expect(secondTabBox).not.toBeNull() const startX = firstTabBox!.x + firstTabBox!.width / 2 const startY = firstTabBox!.y + firstTabBox!.height / 2 + const endX = secondTabBox!.x + secondTabBox!.width * 0.75 + const endY = secondTabBox!.y + secondTabBox!.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 }) + // Why: this mirrors the release repro: drag a terminal tab across another + // tab far enough for dnd-kit to commit a reorder, then release on the tab + // strip before clicking tabs again. + await orcaPage.mouse.move(endX, endY, { steps: 8 }) 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 expect + .poll( + async () => + orcaPage.$$eval(SORTABLE_TAB, (nodes) => + nodes.map((n) => (n as HTMLElement).dataset.tabId ?? '') + ), + { timeout: 5_000, message: 'Terminal tab drag did not reorder the tab strip' } + ) + .toEqual([secondTabId, firstTabId, ...domOrderBefore.slice(2)]) + await tabLocator(orcaPage, firstTabId).click({ force: true }) + 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' + message: 'Tab click did not activate after a terminal tab reorder drag' }) .toBe(secondTabId) })