Tab strip: judge click-vs-drag at pointerup and flush gesture state on window focus (#7316)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo Hong
2026-07-03 22:22:35 -07:00
committed by GitHub
co-authored by Orca
parent 22d5989ed9
commit 52e2eb01b1
7 changed files with 402 additions and 29 deletions
@@ -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])
@@ -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()
})
})
@@ -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]
@@ -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
}
@@ -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<T extends Event>(
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<PointerActivationConstraint, { distance: DistanceMeasurement }> {
return 'distance' in constraint
}
function isDelayConstraint(
constraint: PointerActivationConstraint
): constraint is Extract<PointerActivationConstraint, { delay: number }> {
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<PointerSensorOptions> = [
{
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<PointerSensorOptions>
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<PointerSensorOptions>) {
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()
}
@@ -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')
@@ -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])