diff --git a/src/renderer/src/components/tab-bar/tab-strip-overflow-navigation.ts b/src/renderer/src/components/tab-bar/tab-strip-overflow-navigation.ts index 72d66783239..77af550b748 100644 --- a/src/renderer/src/components/tab-bar/tab-strip-overflow-navigation.ts +++ b/src/renderer/src/components/tab-bar/tab-strip-overflow-navigation.ts @@ -5,6 +5,7 @@ import { sameTabStripScrollMetrics, type TabStripScrollMetrics } from './tab-strip-scroll-metrics' +import { isTabStripPointerGestureActive } from './tab-strip-pointer-gesture' const TAB_STRIP_SCROLL_FRACTION = 0.75 const TAB_STRIP_MIN_SCROLL_STEP_PX = 120 @@ -114,6 +115,9 @@ export function useTabStripOverflowNavigation({ if (!stickToEndRef.current) { return } + if (isTabStripPointerGestureActive()) { + return + } el.scrollLeft = Math.max(0, el.scrollWidth - el.clientWidth) } @@ -137,7 +141,8 @@ export function useTabStripOverflowNavigation({ updateTabStripOverflowState() return } - if (stickToEndRef.current) { + const pointerGestureActive = isTabStripPointerGestureActive() + if (stickToEndRef.current && !pointerGestureActive) { const scrollToEnd = (): void => { const el = tabStripRef.current if (!el) { @@ -149,7 +154,7 @@ export function useTabStripOverflowNavigation({ scrollToEnd() requestAnimationFrame(scrollToEnd) } - if (tabCount > prev.len) { + if (tabCount > prev.len && !pointerGestureActive) { const scrollToEnd = (): void => { const el = tabStripRef.current if (!el) { @@ -178,6 +183,12 @@ export function useTabStripOverflowNavigation({ if (!activeTab) { return } + if (isTabStripPointerGestureActive()) { + // Why: active-tab preview changes during a tab press must not move the + // strip under a stationary pointer before the release decides click/drag. + requestAnimationFrame(updateTabStripOverflowState) + return + } activeTab.scrollIntoView({ block: 'nearest', inline: 'nearest' }) requestAnimationFrame(updateTabStripOverflowState) }, [activeVisibleTabId, updateTabStripOverflowState]) diff --git a/src/renderer/src/components/tab-bar/tab-strip-pointer-activation.test.tsx b/src/renderer/src/components/tab-bar/tab-strip-pointer-activation.test.tsx index 3bd467e49b0..d1d644f7660 100644 --- a/src/renderer/src/components/tab-bar/tab-strip-pointer-activation.test.tsx +++ b/src/renderer/src/components/tab-bar/tab-strip-pointer-activation.test.tsx @@ -41,22 +41,22 @@ describe('useTabStripPointerActivation', () => { const { result } = renderHook(() => useTabStripPointerActivation({ onActivate })) act(() => result.current.onPointerDown(pointerDownEvent(10, 10))) - firePointer('pointermove', 10 + TAB_DRAG_ACTIVATION_DISTANCE_PX + 5, 10) - firePointer('pointerup', 300, 40) + firePointer('pointerup', 10 + TAB_DRAG_ACTIVATION_DISTANCE_PX + 5, 10) expect(onActivate).not.toHaveBeenCalled() }) - it('stays a drag even if the pointer returns near the start before release', () => { + it('activates a stationary click after a single stale over-threshold move', () => { const onActivate = vi.fn() const { result } = renderHook(() => useTabStripPointerActivation({ onActivate })) act(() => result.current.onPointerDown(pointerDownEvent(10, 10))) - // Cross the threshold, then come back over the origin and release. + // Why: packaged Chromium can deliver one stale/coalesced move immediately + // after pointerdown; the release position is the click/drag authority. firePointer('pointermove', 200, 200) firePointer('pointerup', 11, 11) - expect(onActivate).not.toHaveBeenCalled() + expect(onActivate).toHaveBeenCalledTimes(1) }) it('does not activate when the press is cancelled', () => { @@ -108,4 +108,15 @@ describe('useTabStripPointerActivation', () => { firePointer('pointerup', 400, 10) expect(onActivate).toHaveBeenCalledTimes(1) }) + + it('flushes a pending press on window focus', () => { + const onActivate = vi.fn() + const { result } = renderHook(() => useTabStripPointerActivation({ onActivate })) + + act(() => result.current.onPointerDown(pointerDownEvent(10, 10))) + act(() => window.dispatchEvent(new Event('focus'))) + firePointer('pointerup', 10, 10) + + 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 index e1a32e1e0d8..e581e73ba9c 100644 --- a/src/renderer/src/components/tab-bar/tab-strip-pointer-activation.ts +++ b/src/renderer/src/components/tab-bar/tab-strip-pointer-activation.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef } from 'react' import { TAB_DRAG_ACTIVATION_DISTANCE_PX } from '../tab-group/useTabDragSplit' +import { beginTabStripPointerGesture } from './tab-strip-pointer-gesture' /** * Defer tab activation to pointer-up and suppress it when the press turns into a @@ -11,10 +12,10 @@ import { TAB_DRAG_ACTIVATION_DISTANCE_PX } from '../tab-group/useTabDragSplit' * We gate on measured pointer DISPLACEMENT, not the drag-active context ref the * old hook used — that ref clears asynchronously relative to the drop's * pointerup, which is what made #6395's click-after-reorder misfire. Displacement - * mirrors dnd-kit's own activation threshold exactly: once the pointer travels - * past it the gesture is a drag (activation suppressed); a release within it is a - * click (activate). Because each press measures its own gesture, a click after a - * reorder always activates. + * mirrors dnd-kit's own activation threshold, but the authority is the release + * position. A release within it is a click (activate); a release outside it is a + * drag (activation suppressed). Because each press measures its own gesture, a + * click after a reorder always activates. */ export function useTabStripPointerActivation({ onActivate, @@ -49,28 +50,23 @@ export function useTabStripPointerActivation({ cleanupRef.current?.() const startX = event.clientX const startY = event.clientY - let draggedPastThreshold = false + const releaseTabStripPointerGesture = beginTabStripPointerGesture() const cleanup = (): void => { - window.removeEventListener('pointermove', onPointerMove) window.removeEventListener('pointerup', onPointerUp) window.removeEventListener('pointercancel', onPointerCancel) + window.removeEventListener('blur', onPointerCancel) + window.removeEventListener('focus', onPointerCancel) + releaseTabStripPointerGesture() cleanupRef.current = null } - const onPointerMove = (moveEvent: PointerEvent): void => { - if ( - Math.hypot(moveEvent.clientX - startX, moveEvent.clientY - startY) >= + const onPointerUp = (upEvent: PointerEvent): void => { + const wasDrag = + Math.hypot(upEvent.clientX - startX, upEvent.clientY - startY) >= TAB_DRAG_ACTIVATION_DISTANCE_PX - ) { - draggedPastThreshold = true - } - } - const onPointerUp = (): void => { - const wasDrag = draggedPastThreshold cleanup() - // Why: only a release that never crossed the drag threshold is a click. - // Activating after a real drag would yank the just-dropped tab's pane - // back to the source selection. + // Why: packaged Chromium can deliver a stale first pointermove after + // focus; the final release position is the click/drag authority. if (!wasDrag) { onActivateRef.current() } @@ -79,9 +75,10 @@ export function useTabStripPointerActivation({ cleanup() } - window.addEventListener('pointermove', onPointerMove) window.addEventListener('pointerup', onPointerUp) window.addEventListener('pointercancel', onPointerCancel) + window.addEventListener('blur', onPointerCancel) + window.addEventListener('focus', onPointerCancel) cleanupRef.current = cleanup }, [disabled] diff --git a/src/renderer/src/components/tab-bar/tab-strip-pointer-gesture.ts b/src/renderer/src/components/tab-bar/tab-strip-pointer-gesture.ts new file mode 100644 index 00000000000..d00475d71c8 --- /dev/null +++ b/src/renderer/src/components/tab-bar/tab-strip-pointer-gesture.ts @@ -0,0 +1,18 @@ +let activeTabStripPointerGestureCount = 0 + +export function beginTabStripPointerGesture(): () => void { + activeTabStripPointerGestureCount += 1 + let released = false + + return () => { + if (released) { + return + } + released = true + activeTabStripPointerGestureCount = Math.max(0, activeTabStripPointerGestureCount - 1) + } +} + +export function isTabStripPointerGestureActive(): boolean { + return activeTabStripPointerGestureCount > 0 +} diff --git a/src/renderer/src/components/tab-group/tab-drag-pointer-sensor.ts b/src/renderer/src/components/tab-group/tab-drag-pointer-sensor.ts new file mode 100644 index 00000000000..ab96dc5e394 --- /dev/null +++ b/src/renderer/src/components/tab-group/tab-drag-pointer-sensor.ts @@ -0,0 +1,312 @@ +import type { PointerEvent as ReactPointerEvent } from 'react' +import type { + Activators, + DistanceMeasurement, + PointerActivationConstraint, + PointerSensorOptions, + SensorInstance, + SensorProps +} from '@dnd-kit/core' + +type PointerCoordinates = { x: number; y: number } + +const DEFAULT_COORDINATES: PointerCoordinates = { x: 0, y: 0 } +const TAB_DRAG_EARLY_MOVE_CONFIRMATION_MS = 50 +const TAB_DRAG_CONFIRMED_DISTANCE_SAMPLE_COUNT = 2 + +type ListenerEntry = { + eventName: string + handler: EventListener + options?: AddEventListenerOptions | boolean + target: EventTarget +} + +class ListenerBag { + private readonly listeners: ListenerEntry[] = [] + + add( + target: EventTarget | null, + eventName: string, + handler: (event: T) => void, + options?: AddEventListenerOptions | boolean + ): void { + if (!target) { + return + } + const listener = handler as EventListener + target.addEventListener(eventName, listener, options) + this.listeners.push({ eventName, handler: listener, options, target }) + } + + removeAll = (): void => { + for (const { eventName, handler, options, target } of this.listeners) { + target.removeEventListener(eventName, handler, options) + } + this.listeners.length = 0 + } +} + +function isDistanceConstraint( + constraint: PointerActivationConstraint +): constraint is Extract { + return 'distance' in constraint +} + +function isDelayConstraint( + constraint: PointerActivationConstraint +): constraint is Extract { + return 'delay' in constraint +} + +function getOwnerDocument(target: EventTarget | null): Document { + if (target instanceof Document) { + return target + } + if (target instanceof Node) { + return target.ownerDocument ?? document + } + return document +} + +function getPointerCoordinates(event: Event): PointerCoordinates | null { + if ('clientX' in event && 'clientY' in event) { + const pointerEvent = event as PointerEvent + return { x: pointerEvent.clientX, y: pointerEvent.clientY } + } + return null +} + +function subtractCoordinates( + start: PointerCoordinates, + current: PointerCoordinates +): PointerCoordinates { + return { + x: start.x - current.x, + y: start.y - current.y + } +} + +function hasExceededDistance(delta: PointerCoordinates, measurement: DistanceMeasurement): boolean { + const dx = Math.abs(delta.x) + const dy = Math.abs(delta.y) + + if (typeof measurement === 'number') { + return Math.hypot(dx, dy) > measurement + } + if ('x' in measurement && 'y' in measurement) { + return dx > measurement.x && dy > measurement.y + } + if ('x' in measurement) { + return dx > measurement.x + } + if ('y' in measurement) { + return dy > measurement.y + } + return false +} + +export function shouldActivateTabDragFromDistanceSample({ + elapsedMs, + overThresholdSampleCount +}: { + elapsedMs: number + overThresholdSampleCount: number +}): boolean { + // Why: one immediate over-threshold sample can be stale/coalesced after + // window focus; a second sample or a short grace period confirms real motion. + return ( + elapsedMs >= TAB_DRAG_EARLY_MOVE_CONFIRMATION_MS || + overThresholdSampleCount >= TAB_DRAG_CONFIRMED_DISTANCE_SAMPLE_COUNT + ) +} + +export class TabDragPointerSensor implements SensorInstance { + static activators: Activators = [ + { + eventName: 'onPointerDown', + handler: ( + { nativeEvent: event }: ReactPointerEvent, + { onActivation }: PointerSensorOptions + ): boolean => { + if (!event.isPrimary || event.button !== 0) { + return false + } + onActivation?.({ event }) + return true + } + } + ] + + autoScrollEnabled = true + + private activated = false + private readonly document: Document + private readonly initialCoordinates: PointerCoordinates + private readonly pointerDownTime = performance.now() + private readonly props: SensorProps + private readonly documentListeners = new ListenerBag() + private readonly pointerListeners = new ListenerBag() + private readonly windowListeners = new ListenerBag() + private overThresholdSampleCount = 0 + private timeoutId: number | null = null + + constructor(props: SensorProps) { + this.props = props + this.document = getOwnerDocument(props.event.target) + this.initialCoordinates = getPointerCoordinates(props.event) ?? DEFAULT_COORDINATES + this.handleStart = this.handleStart.bind(this) + this.handleMove = this.handleMove.bind(this) + this.handleEnd = this.handleEnd.bind(this) + this.handleCancel = this.handleCancel.bind(this) + this.handleKeydown = this.handleKeydown.bind(this) + this.removeTextSelection = this.removeTextSelection.bind(this) + this.attach() + } + + private attach(): void { + const win = this.document.defaultView + const { activationConstraint, bypassActivationConstraint } = this.props.options + + this.pointerListeners.add(this.document, 'pointermove', this.handleMove, { passive: false }) + this.pointerListeners.add(this.document, 'pointerup', this.handleEnd) + this.pointerListeners.add(this.document, 'pointercancel', this.handleCancel) + this.windowListeners.add(win, 'resize', this.handleCancel) + this.windowListeners.add(win, 'dragstart', preventDefault) + this.windowListeners.add(win, 'visibilitychange', this.handleCancel) + this.windowListeners.add(win, 'contextmenu', preventDefault) + this.windowListeners.add(win, 'focus', this.handleCancel) + this.documentListeners.add(this.document, 'keydown', this.handleKeydown) + + if (!activationConstraint) { + this.handleStart() + return + } + if ( + bypassActivationConstraint?.({ + activeNode: this.props.activeNode, + event: this.props.event, + options: this.props.options + }) + ) { + this.handleStart() + return + } + if (isDelayConstraint(activationConstraint)) { + this.timeoutId = window.setTimeout(this.handleStart, activationConstraint.delay) + this.handlePending(activationConstraint) + return + } + this.handlePending(activationConstraint) + } + + private detach(): void { + this.pointerListeners.removeAll() + this.windowListeners.removeAll() + window.setTimeout(this.documentListeners.removeAll, 50) + if (this.timeoutId !== null) { + window.clearTimeout(this.timeoutId) + this.timeoutId = null + } + } + + private handlePending( + constraint: PointerActivationConstraint, + offset?: PointerCoordinates | undefined + ): void { + this.props.onPending(this.props.active, constraint, this.initialCoordinates, offset) + } + + private handleStart(): void { + if (this.activated) { + return + } + this.activated = true + this.documentListeners.add(this.document, 'click', stopPropagation, { capture: true }) + this.removeTextSelection() + this.documentListeners.add(this.document, 'selectionchange', this.removeTextSelection) + this.props.onStart(this.initialCoordinates) + } + + private handleMove(event: PointerEvent): void { + const coordinates = getPointerCoordinates(event) + const { activationConstraint } = this.props.options + if (!coordinates) { + return + } + const delta = subtractCoordinates(this.initialCoordinates, coordinates) + + if (!this.activated && activationConstraint) { + if (isDistanceConstraint(activationConstraint)) { + if ( + activationConstraint.tolerance != null && + hasExceededDistance(delta, activationConstraint.tolerance) + ) { + this.handleCancel() + return + } + if (hasExceededDistance(delta, activationConstraint.distance)) { + this.overThresholdSampleCount += 1 + if ( + shouldActivateTabDragFromDistanceSample({ + elapsedMs: performance.now() - this.pointerDownTime, + overThresholdSampleCount: this.overThresholdSampleCount + }) + ) { + this.handleStart() + return + } + } else { + this.overThresholdSampleCount = 0 + } + } + if ( + isDelayConstraint(activationConstraint) && + hasExceededDistance(delta, activationConstraint.tolerance) + ) { + this.handleCancel() + return + } + this.handlePending(activationConstraint, delta) + return + } + + if (event.cancelable) { + event.preventDefault() + } + this.props.onMove(coordinates) + } + + private handleEnd(): void { + this.detach() + if (!this.activated) { + this.props.onAbort(this.props.active) + } + this.props.onEnd() + } + + private handleCancel(): void { + this.detach() + if (!this.activated) { + this.props.onAbort(this.props.active) + } + this.props.onCancel() + } + + private handleKeydown(event: KeyboardEvent): void { + if (event.code === 'Escape') { + this.handleCancel() + } + } + + private removeTextSelection(): void { + this.document.getSelection()?.removeAllRanges() + } +} + +function preventDefault(event: Event): void { + event.preventDefault() +} + +function stopPropagation(event: Event): void { + event.stopPropagation() +} diff --git a/src/renderer/src/components/tab-group/useTabDragSplit.test.ts b/src/renderer/src/components/tab-group/useTabDragSplit.test.ts index 9c526ce25e3..fa825031762 100644 --- a/src/renderer/src/components/tab-group/useTabDragSplit.test.ts +++ b/src/renderer/src/components/tab-group/useTabDragSplit.test.ts @@ -7,6 +7,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { Tab, TabGroup, TabGroupLayoutNode } from '../../../../shared/types' import { useAppStore } from '../../store' import type { TabDragItemData } from './useTabDragSplit' +import { shouldActivateTabDragFromDistanceSample } from './tab-drag-pointer-sensor' import { canDropTabForPaneColumnSplit, canDropTabIntoPaneBody, @@ -192,6 +193,27 @@ describe('tab drag activation distance', () => { it('uses an impossible activation distance when tab dragging is disabled', () => { expect(getTabDragActivationDistance(false)).toBe(Number.MAX_SAFE_INTEGER) }) + + it('requires confirmation for an immediate over-threshold distance sample', () => { + expect( + shouldActivateTabDragFromDistanceSample({ + elapsedMs: 10, + overThresholdSampleCount: 1 + }) + ).toBe(false) + expect( + shouldActivateTabDragFromDistanceSample({ + elapsedMs: 10, + overThresholdSampleCount: 2 + }) + ).toBe(true) + expect( + shouldActivateTabDragFromDistanceSample({ + elapsedMs: 60, + overThresholdSampleCount: 1 + }) + ).toBe(true) + }) }) describe('canDropTabIntoPaneBody', () => { @@ -245,7 +267,7 @@ describe('canDropTabIntoPaneBody', () => { }) describe('useTabDragSplit', () => { - it.each(['pointerup', 'pointercancel', 'blur'])( + it.each(['pointerup', 'pointercancel', 'blur', 'focus'])( 'clears a stuck active drag when %s arrives without a dnd end event', async (eventName) => { const activeData = makeDragData('group-1') diff --git a/src/renderer/src/components/tab-group/useTabDragSplit.ts b/src/renderer/src/components/tab-group/useTabDragSplit.ts index 6d945d07145..090e41b047f 100644 --- a/src/renderer/src/components/tab-group/useTabDragSplit.ts +++ b/src/renderer/src/components/tab-group/useTabDragSplit.ts @@ -5,7 +5,6 @@ import { useCallback, useRef, useState, type RefObject } from 'react' import { closestCenter, pointerWithin, - PointerSensor, type CollisionDetection, type DragEndEvent, type DragMoveEvent, @@ -34,6 +33,7 @@ import { } from './tab-drag-preview-activation' import { resolveDragPreviewTabId, resolveSourceGroupRestoreOnDrop } from './tab-drag-preview-target' import { getDragPointer } from './tab-drag-pointer' +import { TabDragPointerSensor } from './tab-drag-pointer-sensor' import { captureTabGroupPanelGeometrySnapshot, resolveActivePaneColumnSplitTarget, @@ -195,7 +195,7 @@ export function useTabDragSplit({ // useSensors(ptr) / useSensors(), because dnd-kit internally spreads // the sensors array into a useEffect dependency list — changing its // length between renders violates React's rules of hooks. - const pointerSensor = useSensor(PointerSensor, { + const pointerSensor = useSensor(TabDragPointerSensor, { activationConstraint: { distance: getTabDragActivationDistance(enabled) } }) const sensors = useSensors(pointerSensor) @@ -233,6 +233,7 @@ export function useTabDragSplit({ window.addEventListener('pointerup', clearIfDndMissedEnd) window.addEventListener('pointercancel', clearIfDndMissedEnd) window.addEventListener('blur', clearIfDndMissedEnd) + window.addEventListener('focus', clearIfDndMissedEnd) releaseMissedEndFallbackRef.current = () => { if (cleanupTimer !== null) { window.clearTimeout(cleanupTimer) @@ -240,6 +241,7 @@ export function useTabDragSplit({ window.removeEventListener('pointerup', clearIfDndMissedEnd) window.removeEventListener('pointercancel', clearIfDndMissedEnd) window.removeEventListener('blur', clearIfDndMissedEnd) + window.removeEventListener('focus', clearIfDndMissedEnd) } }, [releaseMissedEndFallback])