Stabilize workspace board selection and drag (#2369)
This commit is contained in:
parent
38ccb999d4
commit
75f4044156
|
|
@ -1,5 +1,5 @@
|
|||
/* eslint-disable max-lines -- Why: the board drawer owns shared board state, drag/drop, and settings callbacks that need one coordinated surface. */
|
||||
import React, { useCallback, useMemo, useRef, useState } from 'react'
|
||||
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||
import { useAppStore } from '@/store'
|
||||
import { useAllWorktrees, useRepoMap } from '@/store/selectors'
|
||||
import { Sheet, SheetContent } from '@/components/ui/sheet'
|
||||
|
|
@ -88,6 +88,7 @@ export default function WorkspaceKanbanDrawer({
|
|||
selectionAnchorId,
|
||||
updateSelectionForGesture,
|
||||
updateSelectionForArea,
|
||||
clearSelection,
|
||||
selectForContextMenu
|
||||
} = useWorkspaceKanbanSelection(open, boardWorktrees)
|
||||
const { handleAreaSelectionPointerDown } = useWorkspaceKanbanAreaSelection({
|
||||
|
|
@ -318,6 +319,29 @@ export default function WorkspaceKanbanDrawer({
|
|||
useWorkspaceKanbanShiftWheelScroll(boardRef, laneScrollerRef, open, isPointerDragActiveRef)
|
||||
useWorkspaceKanbanOutsideDismiss({ open, boardRef, preserveOpenForMenu, onOpenChange })
|
||||
|
||||
useEffect(() => {
|
||||
if (!open || selectedWorktreeIds.size === 0) {
|
||||
return
|
||||
}
|
||||
|
||||
const clearSelectionOutsideBoard = (event: PointerEvent): void => {
|
||||
const content = boardRef.current?.closest<HTMLElement>('[data-slot="sheet-content"]')
|
||||
const target = event.target
|
||||
if (target instanceof Node && content?.contains(target)) {
|
||||
return
|
||||
}
|
||||
if (isWorkspaceBoardKeepOpenTarget(target)) {
|
||||
return
|
||||
}
|
||||
clearSelection()
|
||||
}
|
||||
|
||||
// Why: clicks in the sidebar are outside the companion board but do not
|
||||
// close it; they still need to behave like "click off" for board selection.
|
||||
document.addEventListener('pointerdown', clearSelectionOutsideBoard, true)
|
||||
return () => document.removeEventListener('pointerdown', clearSelectionOutsideBoard, true)
|
||||
}, [clearSelection, open, selectedWorktreeIds.size])
|
||||
|
||||
const opacityPercent = Math.round(workspaceBoardOpacity * 100)
|
||||
const drawerLeft = sidebarOpen ? sidebarWidth : 0
|
||||
const drawerLeftCss = sidebarOpen
|
||||
|
|
|
|||
|
|
@ -45,6 +45,18 @@ type UseWorkspaceKanbanAreaSelectionParams = {
|
|||
|
||||
const AREA_SELECTION_DRAG_THRESHOLD = 4
|
||||
|
||||
export function shouldCommitWorkspaceKanbanAreaSelection({
|
||||
additive,
|
||||
started
|
||||
}: {
|
||||
additive: boolean
|
||||
started: boolean
|
||||
}): boolean {
|
||||
// Why: a plain click on empty board space is the user's "click off" gesture;
|
||||
// modifier-clicking empty space should not accidentally drop a selected batch.
|
||||
return started || !additive
|
||||
}
|
||||
|
||||
export function useWorkspaceKanbanAreaSelection({
|
||||
open,
|
||||
boardRef,
|
||||
|
|
@ -154,7 +166,7 @@ export function useWorkspaceKanbanAreaSelection({
|
|||
state.frameId = null
|
||||
}
|
||||
flushAreaSelectionDrag()
|
||||
if (state.started) {
|
||||
if (shouldCommitWorkspaceKanbanAreaSelection(state)) {
|
||||
updateSelectionForAreaRef.current(
|
||||
state.finalAreaIds,
|
||||
state.additive,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,32 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { shouldStartWorkspaceKanbanCardPointerDrag } from './use-workspace-kanban-card-pointer-drag'
|
||||
|
||||
function pointerEvent(overrides: Partial<PointerEvent> = {}): PointerEvent {
|
||||
return {
|
||||
button: 0,
|
||||
ctrlKey: false,
|
||||
metaKey: false,
|
||||
pointerType: 'mouse',
|
||||
shiftKey: false,
|
||||
...overrides
|
||||
} as PointerEvent
|
||||
}
|
||||
|
||||
describe('workspace kanban card pointer drag start', () => {
|
||||
it('starts for plain primary mouse drags', () => {
|
||||
expect(shouldStartWorkspaceKanbanCardPointerDrag(pointerEvent())).toBe(true)
|
||||
})
|
||||
|
||||
it('does not steal modifier gestures from selection', () => {
|
||||
expect(shouldStartWorkspaceKanbanCardPointerDrag(pointerEvent({ metaKey: true }))).toBe(false)
|
||||
expect(shouldStartWorkspaceKanbanCardPointerDrag(pointerEvent({ ctrlKey: true }))).toBe(false)
|
||||
expect(shouldStartWorkspaceKanbanCardPointerDrag(pointerEvent({ shiftKey: true }))).toBe(false)
|
||||
})
|
||||
|
||||
it('ignores touch and non-primary buttons', () => {
|
||||
expect(shouldStartWorkspaceKanbanCardPointerDrag(pointerEvent({ pointerType: 'touch' }))).toBe(
|
||||
false
|
||||
)
|
||||
expect(shouldStartWorkspaceKanbanCardPointerDrag(pointerEvent({ button: 1 }))).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
@ -39,6 +39,17 @@ type UseWorkspaceKanbanCardPointerDragParams = {
|
|||
onPinDragTargetChange: (isOver: boolean) => void
|
||||
}
|
||||
|
||||
export function shouldStartWorkspaceKanbanCardPointerDrag(
|
||||
event: Pick<PointerEvent, 'button' | 'pointerType' | 'shiftKey' | 'metaKey' | 'ctrlKey'>
|
||||
): boolean {
|
||||
if (event.button !== 0 || event.pointerType === 'touch') {
|
||||
return false
|
||||
}
|
||||
// Why: modifier gestures are reserved for selection/context-menu intent.
|
||||
// Letting tiny pointer drift start a drag makes Cmd/Ctrl/Shift selection flaky.
|
||||
return !event.shiftKey && !event.metaKey && !event.ctrlKey
|
||||
}
|
||||
|
||||
function shouldIgnorePointerDown(target: EventTarget | null, card: HTMLElement): boolean {
|
||||
if (!(target instanceof Element)) {
|
||||
return false
|
||||
|
|
@ -185,7 +196,9 @@ export function useWorkspaceKanbanCardPointerDrag({
|
|||
}
|
||||
state.currentX = event.clientX
|
||||
state.currentY = event.clientY
|
||||
event.preventDefault()
|
||||
if (state.started) {
|
||||
event.preventDefault()
|
||||
}
|
||||
stopPointerDrag(true)
|
||||
}
|
||||
|
||||
|
|
@ -217,7 +230,7 @@ export function useWorkspaceKanbanCardPointerDrag({
|
|||
|
||||
const onCardPointerDownCapture = useCallback(
|
||||
(event: React.PointerEvent<HTMLElement>) => {
|
||||
if (!open || event.button !== 0 || event.pointerType === 'touch') {
|
||||
if (!open || !shouldStartWorkspaceKanbanCardPointerDrag(event.nativeEvent)) {
|
||||
return
|
||||
}
|
||||
const target = event.target
|
||||
|
|
|
|||
|
|
@ -91,12 +91,18 @@ export function useWorkspaceKanbanSelection(open: boolean, boardWorktrees: reado
|
|||
[boardWorktreeIds, selectedWorktreeIds, selectionAnchorId]
|
||||
)
|
||||
|
||||
const clearSelection = useCallback(() => {
|
||||
setSelectedWorktreeIds((previous) => (previous.size === 0 ? previous : new Set()))
|
||||
setSelectionAnchorId((previous) => (previous === null ? previous : null))
|
||||
}, [])
|
||||
|
||||
return {
|
||||
selectedWorktreeIds,
|
||||
selectedWorktrees,
|
||||
selectionAnchorId,
|
||||
updateSelectionForGesture,
|
||||
updateSelectionForArea,
|
||||
clearSelection,
|
||||
selectForContextMenu
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -37,7 +37,6 @@ export function shouldIgnoreAreaSelectionStart(target: EventTarget | null): bool
|
|||
target.closest(
|
||||
[
|
||||
'[data-workspace-board-card-id]',
|
||||
'[data-workspace-pin-drop-target]',
|
||||
'a',
|
||||
'button',
|
||||
'input',
|
||||
|
|
|
|||
|
|
@ -0,0 +1,31 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { shouldCommitWorkspaceKanbanAreaSelection } from './use-workspace-kanban-area-selection'
|
||||
|
||||
describe('workspace kanban area selection finish', () => {
|
||||
it('commits an empty non-additive surface click so selection clears', () => {
|
||||
expect(
|
||||
shouldCommitWorkspaceKanbanAreaSelection({
|
||||
additive: false,
|
||||
started: false
|
||||
})
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('ignores empty additive surface clicks so modifier-click off does not clear', () => {
|
||||
expect(
|
||||
shouldCommitWorkspaceKanbanAreaSelection({
|
||||
additive: true,
|
||||
started: false
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('commits marquee drags even when additive', () => {
|
||||
expect(
|
||||
shouldCommitWorkspaceKanbanAreaSelection({
|
||||
additive: true,
|
||||
started: true
|
||||
})
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { resolveWorkspaceStatusDropTargetFromRects } from './workspace-kanban-card-pointer-drag-dom'
|
||||
|
||||
const rects = [
|
||||
{ status: 'todo', left: 0, top: 0, right: 200, bottom: 600 },
|
||||
{ status: 'doing', left: 212, top: 0, right: 412, bottom: 600 },
|
||||
{ status: 'done', left: 424, top: 0, right: 624, bottom: 600 }
|
||||
]
|
||||
|
||||
describe('workspace kanban pointer drag drop target', () => {
|
||||
it('uses the containing lane when the pointer is inside one', () => {
|
||||
expect(resolveWorkspaceStatusDropTargetFromRects(rects, 240, 100)).toBe('doing')
|
||||
})
|
||||
|
||||
it('falls back to the nearest lane when the pointer is in a lane gap', () => {
|
||||
expect(resolveWorkspaceStatusDropTargetFromRects(rects, 206, 100)).toBe('todo')
|
||||
expect(resolveWorkspaceStatusDropTargetFromRects(rects, 418, 100)).toBe('doing')
|
||||
})
|
||||
|
||||
it('does not resolve a lane outside the lane row', () => {
|
||||
expect(resolveWorkspaceStatusDropTargetFromRects(rects, 206, 620)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
@ -4,6 +4,7 @@ export const CARD_SELECTOR = '[data-workspace-board-card-id]'
|
|||
export const STATUS_DROP_TARGET = '[data-workspace-status-drop-target]'
|
||||
export const PIN_DROP_TARGET = '[data-workspace-pin-drop-target]'
|
||||
|
||||
const STATUS_DROP_GAP_TOLERANCE_PX = 24
|
||||
const POINTER_CARD_DRAGGING_ATTR = 'data-workspace-board-card-pointer-dragging'
|
||||
const POINTER_DRAG_CARD_ATTR = 'data-workspace-board-card-drag-card'
|
||||
const POINTER_DRAG_COUNT_ATTR = 'data-workspace-board-card-drag-count'
|
||||
|
|
@ -23,6 +24,60 @@ type DragPreviewState = {
|
|||
previewOffsetY: number
|
||||
}
|
||||
|
||||
export type WorkspaceKanbanStatusDropRect = {
|
||||
status: WorkspaceStatus
|
||||
left: number
|
||||
top: number
|
||||
right: number
|
||||
bottom: number
|
||||
}
|
||||
|
||||
export function resolveWorkspaceStatusDropTargetFromRects(
|
||||
rects: readonly WorkspaceKanbanStatusDropRect[],
|
||||
x: number,
|
||||
y: number,
|
||||
gapTolerance = STATUS_DROP_GAP_TOLERANCE_PX
|
||||
): WorkspaceStatus | null {
|
||||
let nearest: { status: WorkspaceStatus; distance: number } | null = null
|
||||
|
||||
for (const rect of rects) {
|
||||
if (y < rect.top || y > rect.bottom) {
|
||||
continue
|
||||
}
|
||||
if (x >= rect.left && x <= rect.right) {
|
||||
return rect.status
|
||||
}
|
||||
const distance = x < rect.left ? rect.left - x : x - rect.right
|
||||
if (distance > gapTolerance) {
|
||||
continue
|
||||
}
|
||||
if (!nearest || distance < nearest.distance) {
|
||||
nearest = { status: rect.status, distance }
|
||||
}
|
||||
}
|
||||
|
||||
return nearest?.status ?? null
|
||||
}
|
||||
|
||||
function getStatusDropTargetRects(board: HTMLElement): WorkspaceKanbanStatusDropRect[] {
|
||||
return Array.from(board.querySelectorAll<HTMLElement>(STATUS_DROP_TARGET)).flatMap((element) => {
|
||||
const status = element.dataset.workspaceStatus
|
||||
if (!status) {
|
||||
return []
|
||||
}
|
||||
const rect = element.getBoundingClientRect()
|
||||
return [
|
||||
{
|
||||
status,
|
||||
left: rect.left,
|
||||
top: rect.top,
|
||||
right: rect.right,
|
||||
bottom: rect.bottom
|
||||
}
|
||||
]
|
||||
})
|
||||
}
|
||||
|
||||
export function getDropTarget(
|
||||
board: HTMLElement,
|
||||
x: number,
|
||||
|
|
@ -39,11 +94,16 @@ export function getDropTarget(
|
|||
}
|
||||
|
||||
const statusTarget = target.closest<HTMLElement>(STATUS_DROP_TARGET)
|
||||
const directStatus =
|
||||
statusTarget && board.contains(statusTarget)
|
||||
? (statusTarget.dataset.workspaceStatus ?? null)
|
||||
: null
|
||||
return {
|
||||
// Why: dropping in the visual gap between lanes should still land in the
|
||||
// nearest lane. Without this fallback, otherwise-valid drags appeared flaky.
|
||||
status:
|
||||
statusTarget && board.contains(statusTarget)
|
||||
? (statusTarget.dataset.workspaceStatus ?? null)
|
||||
: null,
|
||||
directStatus ??
|
||||
resolveWorkspaceStatusDropTargetFromRects(getStatusDropTargetRects(board), x, y),
|
||||
isPinDrop: false
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -118,4 +118,17 @@ describe('worktree multi selection', () => {
|
|||
expect([...result.selectedIds]).toEqual(['wt-2'])
|
||||
expect(result.anchorId).toBe('wt-2')
|
||||
})
|
||||
|
||||
it('clears the existing batch for an empty non-additive area', () => {
|
||||
const result = updateWorktreeAreaSelection({
|
||||
visibleIds,
|
||||
previousSelectedIds: new Set(['wt-2', 'wt-3']),
|
||||
previousAnchorId: 'wt-2',
|
||||
areaIds: [],
|
||||
additive: false
|
||||
})
|
||||
|
||||
expect([...result.selectedIds]).toEqual([])
|
||||
expect(result.anchorId).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue