diff --git a/src/client/TabBar.tsx b/src/client/TabBar.tsx index 4e38487..8cd92bf 100644 --- a/src/client/TabBar.tsx +++ b/src/client/TabBar.tsx @@ -74,6 +74,38 @@ export function TabBar(props: { const [dragOver, setDragOver] = useState(false) const listRef = useRef(null) + // Middle-click close: the press target is recorded on middle mousedown + // (preventDefaulted to disarm Chrome's middle-click autoscroll — its + // indicator is inert here because the strip hides its scrollbar and only + // the wheel handler scrolls) and the close settles on the first middle + // mouseup OVER that same tab. Release-position semantics match VS Code + // (microsoft/vscode#101028) and what users expect from Chrome tabs + // (crbug/40679924): pressing on a tab and releasing elsewhere cancels the + // close. The browser dispatches auxclick to the nearest common ancestor of + // the press/release targets when they differ, so any drift, autoscroll + // scroll, or tab-list reflow between press and release would otherwise + // swallow the close; settling on the recorded press target at mouseup + // keeps release semantics without depending on auxclick delivery. + const onCloseRef = useRef(onClose) + const middlePressed = useRef<{ id: string; node: HTMLElement } | null>(null) + useEffect(() => { + onCloseRef.current = onClose + }) + useEffect(() => { + const onMouseUp = (event: MouseEvent): void => { + if (event.button !== 1) return + const pressed = middlePressed.current + middlePressed.current = null + // Close only when the release lands on the pressed tab; a drag-away + // release cancels the press (one-shot per press). + if (pressed !== null && pressed.node.isConnected && pressed.node.contains(event.target as Node)) { + onCloseRef.current(pressed.id) + } + } + window.addEventListener('mouseup', onMouseUp) + return () => { window.removeEventListener('mouseup', onMouseUp) } + }, []) + // Wheel over the strip scrolls the tab row horizontally (a plain mouse // wheel emits deltaY, which overflow-x alone never consumes). Bound as a // native NON-passive listener: React registers onWheel passively at the @@ -151,11 +183,15 @@ export function TabBar(props: { if (payload !== null) onDropTab(payload, tab.id) }} onClick={() => { onActivate(tab.id) }} - onAuxClick={(event) => { - // Middle-click closes the tab (and suppresses autoscroll). + onMouseDown={(event) => { + // Middle-click close: record the press target and disarm + // Chrome's middle-click autoscroll (its indicator is inert + // here — the strip scrolls via the wheel handler only). The + // close itself settles on the first middle mouseup over this + // same tab (window-level), keeping release semantics. if (event.button === 1) { event.preventDefault() - onClose(tab.id) + middlePressed.current = { id: tab.id, node: event.currentTarget } } }} > diff --git a/tests/tab-bar-middle-click.spec.tsx b/tests/tab-bar-middle-click.spec.tsx new file mode 100644 index 0000000..e26d675 --- /dev/null +++ b/tests/tab-bar-middle-click.spec.tsx @@ -0,0 +1,191 @@ +/** + * Tab-strip middle-click close tests. The press target is recorded on middle + * mousedown (preventDefaulted to disarm Chrome's middle-click autoscroll) + * and the close settles on the first middle mouseup OVER that same tab — + * release-position semantics matching VS Code (microsoft/vscode#101028) and + * user expectations for Chrome tabs (crbug/40679924): pressing on a tab and + * releasing elsewhere cancels. The browser dispatches auxclick to the common + * ancestor of the press/release targets when they differ (or suppresses it + * entirely), so settling on the recorded press target keeps release + * semantics without depending on auxclick delivery. Left-button paths + * (activate / drag) and wheel scrolling are untouched. + */ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createElement } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { act } from 'react-dom/test-utils' + +// The act() environment flag (React 18.2 reads it before flushing effects). +;(globalThis as Record).IS_REACT_ACT_ENVIRONMENT = true + +import { TabBar } from '../src/client/TabBar.tsx' +import type { SidebarTab } from '../src/client/state.ts' + +function mountBar(): { + tab0: HTMLElement + tab1: HTMLElement + onClose: ReturnType + onActivate: ReturnType + unmount: () => void +} { + const container = document.createElement('div') + document.body.append(container) + const onClose = vi.fn() + const onActivate = vi.fn() + const tabs: SidebarTab[] = [ + { id: 't1', type: 'explorer', title: 'Explorer' }, + { id: 't2', type: 'git', title: 'Git' }, + ] + const root: Root = createRoot(container) + act(() => { + root.render(createElement(TabBar, { + paneId: 'pane:1', + tabs, + active: 't1', + onActivate, + onClose, + onNewTab: () => {}, + newTabOptions: [], + onDropTab: () => {}, + })) + }) + const tabEls = [...container.querySelectorAll('[class*="tabList"] > [class*="tab"]')] as HTMLElement[] + expect(tabEls.length).toBe(2) + // noUncheckedIndexedAccess: the length assertion above is the guard, but + // the compiler still sees `HTMLElement | undefined` through indexing. + const tab0 = tabEls[0]! + const tab1 = tabEls[1]! + return { + tab0, + tab1, + onClose, + onActivate, + unmount: () => { + act(() => { root.unmount() }) + container.remove() + }, + } +} + +/** Dispatch a native mouse event; returns the event (to read defaultPrevented). */ +function mouse(target: EventTarget, type: string, button: number): MouseEvent { + const event = new MouseEvent(type, { bubbles: true, cancelable: true, button }) + target.dispatchEvent(event) + return event +} + +afterEach(() => { + document.body.innerHTML = '' + vi.restoreAllMocks() +}) + +describe('TabBar middle-click close', () => { + it('closes when the middle mouseup lands on the same tab', () => { + const { tab0, onClose, unmount } = mountBar() + try { + mouse(tab0, 'mousedown', 1) + mouse(tab0, 'mouseup', 1) + expect(onClose).toHaveBeenCalledWith('t1') + } finally { + unmount() + } + }) + + it('preventDefaults the middle mousedown (disarms browser autoscroll)', () => { + const { tab0, unmount } = mountBar() + try { + const event = mouse(tab0, 'mousedown', 1) + expect(event.defaultPrevented).toBe(true) + } finally { + unmount() + } + }) + + it('a release elsewhere (drag-away) cancels the close — one-shot', () => { + const { tab0, onClose, unmount } = mountBar() + try { + // Press on tab 0, release on unrelated element: no close. + mouse(tab0, 'mousedown', 1) + mouse(document.body, 'mouseup', 1) + expect(onClose).not.toHaveBeenCalled() + // A later middle mouseup over the tab (no new press) must NOT close. + mouse(tab0, 'mouseup', 1) + expect(onClose).not.toHaveBeenCalled() + } finally { + unmount() + } + }) + + it('a release over a DIFFERENT tab does not close', () => { + const { tab0, tab1, onClose, unmount } = mountBar() + try { + mouse(tab0, 'mousedown', 1) + mouse(tab1, 'mouseup', 1) + expect(onClose).not.toHaveBeenCalled() + } finally { + unmount() + } + }) + + it('middle mousedown on the close button closes (release over the tab)', () => { + const { tab0, onClose, unmount } = mountBar() + try { + const button = tab0.querySelector('button') as HTMLButtonElement + expect(button).not.toBeNull() + mouse(button, 'mousedown', 1) + mouse(button, 'mouseup', 1) + expect(onClose).toHaveBeenCalledWith('t1') + } finally { + unmount() + } + }) + + it('left mousedown does not close; left click activates', () => { + const { tab0, onClose, onActivate, unmount } = mountBar() + try { + mouse(tab0, 'mousedown', 0) + mouse(tab0, 'mouseup', 0) + expect(onClose).not.toHaveBeenCalled() + mouse(tab0, 'click', 0) + expect(onActivate).toHaveBeenCalledWith('t1') + } finally { + unmount() + } + }) + + it('a middle auxclick alone (no mousedown) does not close', () => { + // Pins the contract: the close settles on the mouseup of the recorded + // press, not on auxclick delivery — the browser redirects auxclick to + // the common ancestor (or suppresses it) when press/release targets + // differ, which was the failure mode. + const { tab0, onClose, unmount } = mountBar() + try { + mouse(tab0, 'auxclick', 1) + expect(onClose).not.toHaveBeenCalled() + } finally { + unmount() + } + }) + + it('a left mouseup after a middle press does not consume the press', () => { + const { tab0, onClose, unmount } = mountBar() + try { + mouse(tab0, 'mousedown', 1) + mouse(window, 'mouseup', 0) + expect(onClose).not.toHaveBeenCalled() + mouse(tab0, 'mouseup', 1) + expect(onClose).toHaveBeenCalledWith('t1') + } finally { + unmount() + } + }) + + it('stops closing after unmount', () => { + const { tab0, onClose, unmount } = mountBar() + mouse(tab0, 'mousedown', 1) + unmount() + mouse(tab0, 'mouseup', 1) + expect(onClose).not.toHaveBeenCalled() + }) +})