diff --git a/src/composables/canvas/useSelectionToolboxPosition.ts b/src/composables/canvas/useSelectionToolboxPosition.ts index 426604f824..8296ee955a 100644 --- a/src/composables/canvas/useSelectionToolboxPosition.ts +++ b/src/composables/canvas/useSelectionToolboxPosition.ts @@ -66,6 +66,14 @@ export function useSelectionToolboxPosition( lgCanvas.canvas ) + // Unified dragging state - combines both LiteGraph and Vue node dragging + const isDragging = computed((): boolean => { + const litegraphDragging = canvasStore.canvas?.state?.draggingItems ?? false + const vueNodeDragging = + shouldRenderVueNodes.value && layoutStore.isDraggingVueNodes.value + return litegraphDragging || vueNodeDragging + }) + /** * Update position based on selection */ @@ -77,6 +85,12 @@ export function useSelectionToolboxPosition( return } + // Don't show toolbox while dragging + if (isDragging.value) { + visible.value = false + return + } + visible.value = true // Get bounds for all selected items @@ -241,14 +255,6 @@ export function useSelectionToolboxPosition( }) } - // Unified dragging state - combines both LiteGraph and Vue node dragging - const isDragging = computed((): boolean => { - const litegraphDragging = canvasStore.canvas?.state?.draggingItems ?? false - const vueNodeDragging = - shouldRenderVueNodes.value && layoutStore.isDraggingVueNodes.value - return litegraphDragging || vueNodeDragging - }) - watch(isDragging, handleDragStateChange) onUnmounted(() => { diff --git a/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts b/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts index 05373c5223..b5a425378a 100644 --- a/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts +++ b/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts @@ -15,24 +15,7 @@ import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle' import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions' import { useNodeZIndex } from '@/renderer/extensions/vueNodes/composables/useNodeZIndex' -import { isLGraphNode } from '@/utils/litegraphUtil' - -/** - * Check if multiple nodes are selected - * Optimized to return early when 2+ nodes found - */ -function hasMultipleNodesSelected(selectedItems: unknown[]): boolean { - let count = 0 - for (let i = 0; i < selectedItems.length; i++) { - if (isLGraphNode(selectedItems[i])) { - count++ - if (count >= 2) { - return true - } - } - } - return false -} +import { isMultiSelectKey } from '@/renderer/extensions/vueNodes/utils/selectionUtils' function useNodeEventHandlersIndividual() { const canvasStore = useCanvasStore() @@ -52,24 +35,19 @@ function useNodeEventHandlersIndividual() { const node = nodeManager.value.getNode(nodeData.id) if (!node) return - const isMultiSelect = event.ctrlKey || event.metaKey || event.shiftKey + const multiSelect = isMultiSelectKey(event) + const selectedItemsCount = canvasStore.selectedItems.length + const preserveExistingSelection = + !multiSelect && node.selected && selectedItemsCount > 1 - if (isMultiSelect) { - // Ctrl/Cmd+click -> toggle selection - if (node.selected) { - canvasStore.canvas.deselect(node) - } else { - canvasStore.canvas.select(node) - } - } else { - const selectedMultipleNodes = hasMultipleNodesSelected( - canvasStore.selectedItems - ) - if (!selectedMultipleNodes) { - // Single-select the node - canvasStore.canvas.deselectAll() + if (multiSelect) { + if (!node.selected) { canvasStore.canvas.select(node) } + } else if (!preserveExistingSelection) { + // Regular click -> single select + canvasStore.canvas.deselectAll() + canvasStore.canvas.select(node) } // Bring node to front when clicked (similar to LiteGraph behavior) @@ -219,6 +197,32 @@ function useNodeEventHandlersIndividual() { canvasStore.updateSelectedItems() } + /** + * Ensure node is selected for shift-drag operations + * Handles special logic for promoting a node to selection when shift-dragging + * @param event - The pointer event (for multi-select key detection) + * @param nodeData - The node data for the node being dragged + * @param wasSelectedAtPointerDown - Whether the node was selected when pointer-down occurred + */ + const ensureNodeSelectedForShiftDrag = ( + event: PointerEvent, + nodeData: VueNodeData, + wasSelectedAtPointerDown: boolean + ) => { + if (wasSelectedAtPointerDown) return + + const multiSelectKeyPressed = isMultiSelectKey(event) + if (!multiSelectKeyPressed) return + + if (!canvasStore.canvas || !nodeManager.value) return + const node = nodeManager.value.getNode(nodeData.id) + if (!node || node.selected) return + + const selectionCount = canvasStore.selectedItems.length + const addToSelection = selectionCount > 0 + selectNodes([nodeData.id], addToSelection) + } + /** * Deselect specific nodes */ @@ -229,14 +233,58 @@ function useNodeEventHandlersIndividual() { nodeIds.forEach((nodeId) => { const node = nodeManager.value?.getNode(nodeId) - if (node) { - node.selected = false + if (node && canvasStore.canvas) { + canvasStore.canvas.deselect(node) } }) canvasStore.updateSelectedItems() } + const deselectNode = (nodeId: string) => { + const node = nodeManager.value?.getNode(nodeId) + if (node) { + canvasStore.canvas?.deselect(node) + canvasStore.updateSelectedItems() + } + } + + const toggleNodeSelectionAfterPointerUp = ( + nodeId: string, + { + wasSelectedAtPointerDown, + multiSelect + }: { + wasSelectedAtPointerDown: boolean + multiSelect: boolean + } + ) => { + if (!shouldHandleNodePointerEvents.value) return + + if (!canvasStore.canvas || !nodeManager.value) return + + const node = nodeManager.value.getNode(nodeId) + if (!node) return + + if (!multiSelect) { + const multipleSelected = canvasStore.selectedItems.length > 1 + if (multipleSelected && wasSelectedAtPointerDown) { + canvasStore.canvas.deselectAll() + canvasStore.canvas.select(node) + canvasStore.updateSelectedItems() + } + return + } + + if (wasSelectedAtPointerDown) { + canvasStore.canvas.deselect(node) + canvasStore.updateSelectedItems() + } + + // No action needed when the node was not previously selected since the pointer-down + // handler already added it to the selection. + } + return { // Core event handlers handleNodeSelect, @@ -248,7 +296,10 @@ function useNodeEventHandlersIndividual() { // Batch operations selectNodes, - deselectNodes + deselectNodes, + deselectNode, + ensureNodeSelectedForShiftDrag, + toggleNodeSelectionAfterPointerUp } } diff --git a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts index b4bd1d8394..0874573dcc 100644 --- a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts +++ b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts @@ -6,6 +6,11 @@ import type { VueNodeData } from '@/composables/graph/useGraphNodeManager' import { useNodePointerInteractions } from '@/renderer/extensions/vueNodes/composables/useNodePointerInteractions' const forwardEventToCanvasMock = vi.fn() +const deselectNodeMock = vi.fn() +const selectNodesMock = vi.fn() +const toggleNodeSelectionAfterPointerUpMock = vi.fn() +const ensureNodeSelectedForShiftDragMock = vi.fn() +const selectedItemsState: { items: Array<{ id?: string }> } = { items: [] } // Mock the dependencies vi.mock('@/renderer/core/canvas/useCanvasInteractions', () => ({ @@ -29,6 +34,37 @@ vi.mock('@/renderer/core/layout/store/layoutStore', () => ({ } })) +vi.mock('@/renderer/core/canvas/canvasStore', () => ({ + useCanvasStore: () => ({ + get selectedItems() { + return selectedItemsState.items + } + }) +})) + +vi.mock( + '@/renderer/extensions/vueNodes/composables/useNodeEventHandlers', + () => ({ + useNodeEventHandlers: () => ({ + deselectNode: deselectNodeMock, + selectNodes: selectNodesMock, + toggleNodeSelectionAfterPointerUp: toggleNodeSelectionAfterPointerUpMock, + ensureNodeSelectedForShiftDrag: ensureNodeSelectedForShiftDragMock + }) + }) +) + +vi.mock('@/composables/graph/useVueNodeLifecycle', () => ({ + useVueNodeLifecycle: () => ({ + nodeManager: ref({ + getNode: vi.fn((id: string) => ({ + id, + selected: false // Default to not selected + })) + }) + }) +})) + const createMockVueNodeData = ( overrides: Partial = {} ): VueNodeData => ({ @@ -72,6 +108,7 @@ const createMouseEvent = ( describe('useNodePointerInteractions', () => { beforeEach(async () => { vi.clearAllMocks() + selectedItemsState.items = [] setActivePinia(createPinia()) // Reset layout store state between tests const { layoutStore } = await import( @@ -141,9 +178,16 @@ describe('useNodePointerInteractions', () => { ) // Test pointer cancel - selection happens on pointer down - pointerHandlers.onPointerdown(createPointerEvent('pointerdown')) + pointerHandlers.onPointerdown( + createPointerEvent('pointerdown', { clientX: 100, clientY: 100 }) + ) expect(mockOnNodeSelect).toHaveBeenCalledTimes(1) + // Simulate drag by moving pointer beyond threshold + pointerHandlers.onPointermove( + createPointerEvent('pointermove', { clientX: 110, clientY: 110 }) + ) + pointerHandlers.onPointercancel(createPointerEvent('pointercancel')) // Selection should have been called on pointer down only @@ -152,7 +196,13 @@ describe('useNodePointerInteractions', () => { mockOnNodeSelect.mockClear() // Test context menu during drag prevents default - pointerHandlers.onPointerdown(createPointerEvent('pointerdown')) + pointerHandlers.onPointerdown( + createPointerEvent('pointerdown', { clientX: 100, clientY: 100 }) + ) + // Simulate drag by moving pointer beyond threshold + pointerHandlers.onPointermove( + createPointerEvent('pointermove', { clientX: 110, clientY: 110 }) + ) const contextMenuEvent = createMouseEvent('contextmenu') const preventDefaultSpy = vi.spyOn(contextMenuEvent, 'preventDefault') @@ -193,8 +243,16 @@ describe('useNodePointerInteractions', () => { mockOnNodeSelect ) - // Start drag - pointerHandlers.onPointerdown(createPointerEvent('pointerdown')) + // Pointer down alone shouldn't set dragging state + pointerHandlers.onPointerdown( + createPointerEvent('pointerdown', { clientX: 100, clientY: 100 }) + ) + expect(layoutStore.isDraggingVueNodes.value).toBe(false) + + // Move pointer beyond threshold to start drag + pointerHandlers.onPointermove( + createPointerEvent('pointermove', { clientX: 110, clientY: 110 }) + ) await nextTick() expect(layoutStore.isDraggingVueNodes.value).toBe(true) @@ -273,14 +331,17 @@ describe('useNodePointerInteractions', () => { expect(mockOnNodeSelect).toHaveBeenCalledWith(downEvent, mockNodeData) expect(mockOnNodeSelect).toHaveBeenCalledTimes(1) - // Dragging state should be active - expect(layoutStore.isDraggingVueNodes.value).toBe(true) + // Dragging state should NOT be active yet + expect(layoutStore.isDraggingVueNodes.value).toBe(false) - // Move the pointer (start dragging) + // Move the pointer beyond threshold (start dragging) pointerHandlers.onPointermove( createPointerEvent('pointermove', { clientX: 150, clientY: 150 }) ) + // Now dragging state should be active + expect(layoutStore.isDraggingVueNodes.value).toBe(true) + // Selection should still only have been called once (on pointer down) expect(mockOnNodeSelect).toHaveBeenCalledTimes(1) @@ -292,4 +353,146 @@ describe('useNodePointerInteractions', () => { // Selection should still only have been called once expect(mockOnNodeSelect).toHaveBeenCalledTimes(1) }) + + it('on ctrl+click: calls toggleNodeSelectionAfterPointerUp on pointer up (not pointer down)', async () => { + const mockNodeData = createMockVueNodeData() + const mockOnNodeSelect = vi.fn() + + const { pointerHandlers } = useNodePointerInteractions( + ref(mockNodeData), + mockOnNodeSelect + ) + + // Pointer down with ctrl + const downEvent = createPointerEvent('pointerdown', { + ctrlKey: true, + clientX: 100, + clientY: 100 + }) + pointerHandlers.onPointerdown(downEvent) + + // On pointer down: toggle handler should NOT be called yet + expect(toggleNodeSelectionAfterPointerUpMock).not.toHaveBeenCalled() + + // Pointer up with ctrl (no drag - same position) + const upEvent = createPointerEvent('pointerup', { + ctrlKey: true, + clientX: 100, + clientY: 100 + }) + pointerHandlers.onPointerup(upEvent) + + // On pointer up: toggle handler IS called with correct params + expect(toggleNodeSelectionAfterPointerUpMock).toHaveBeenCalledWith( + mockNodeData.id, + { + wasSelectedAtPointerDown: false, + multiSelect: true + } + ) + }) + + it('on ctrl+drag: does NOT call toggleNodeSelectionAfterPointerUp', async () => { + const mockNodeData = createMockVueNodeData() + const mockOnNodeSelect = vi.fn() + + const { pointerHandlers } = useNodePointerInteractions( + ref(mockNodeData), + mockOnNodeSelect + ) + + // Pointer down with ctrl + const downEvent = createPointerEvent('pointerdown', { + ctrlKey: true, + clientX: 100, + clientY: 100 + }) + pointerHandlers.onPointerdown(downEvent) + + // Move beyond drag threshold + pointerHandlers.onPointermove( + createPointerEvent('pointermove', { + ctrlKey: true, + clientX: 110, + clientY: 110 + }) + ) + + // Pointer up after drag + const upEvent = createPointerEvent('pointerup', { + ctrlKey: true, + clientX: 110, + clientY: 110 + }) + pointerHandlers.onPointerup(upEvent) + + // When dragging: toggle handler should NOT be called + expect(toggleNodeSelectionAfterPointerUpMock).not.toHaveBeenCalled() + }) + + it('selects node when shift drag starts without multi selection', async () => { + selectedItemsState.items = [] + const mockNodeData = createMockVueNodeData() + const mockOnNodeSelect = vi.fn() + + const { pointerHandlers } = useNodePointerInteractions( + ref(mockNodeData), + mockOnNodeSelect + ) + + const pointerDownEvent = createPointerEvent('pointerdown', { + clientX: 0, + clientY: 0, + shiftKey: true + }) + + pointerHandlers.onPointerdown(pointerDownEvent) + + const pointerMoveEvent = createPointerEvent('pointermove', { + clientX: 10, + clientY: 10, + shiftKey: true + }) + + pointerHandlers.onPointermove(pointerMoveEvent) + + expect(ensureNodeSelectedForShiftDragMock).toHaveBeenCalledWith( + pointerMoveEvent, + mockNodeData, + false + ) + }) + + it('still ensures selection when shift drag starts with existing multi select', async () => { + selectedItemsState.items = [{ id: 'a' }, { id: 'b' }] + const mockNodeData = createMockVueNodeData() + const mockOnNodeSelect = vi.fn() + + const { pointerHandlers } = useNodePointerInteractions( + ref(mockNodeData), + mockOnNodeSelect + ) + + const pointerDownEvent = createPointerEvent('pointerdown', { + clientX: 0, + clientY: 0, + shiftKey: true + }) + + pointerHandlers.onPointerdown(pointerDownEvent) + + const pointerMoveEvent = createPointerEvent('pointermove', { + clientX: 10, + clientY: 10, + shiftKey: true + }) + + pointerHandlers.onPointermove(pointerMoveEvent) + + expect(ensureNodeSelectedForShiftDragMock).toHaveBeenCalledWith( + pointerMoveEvent, + mockNodeData, + false + ) + }) }) diff --git a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts index e9ca19e97b..3a9c452ea6 100644 --- a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts +++ b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts @@ -3,9 +3,12 @@ import type { MaybeRefOrGetter } from 'vue' import { isMiddlePointerInput } from '@/base/pointerUtils' import type { VueNodeData } from '@/composables/graph/useGraphNodeManager' +import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle' import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions' import { layoutStore } from '@/renderer/core/layout/store/layoutStore' import { useNodeLayout } from '@/renderer/extensions/vueNodes/layout/useNodeLayout' +import { useNodeEventHandlers } from '@/renderer/extensions/vueNodes/composables/useNodeEventHandlers' +import { isMultiSelectKey } from '@/renderer/extensions/vueNodes/utils/selectionUtils' export function useNodePointerInteractions( nodeDataMaybe: MaybeRefOrGetter, @@ -28,6 +31,9 @@ export function useNodePointerInteractions( // Use canvas interactions for proper wheel event handling and pointer event capture control const { forwardEventToCanvas, shouldHandleNodePointerEvents } = useCanvasInteractions() + const { toggleNodeSelectionAfterPointerUp, ensureNodeSelectedForShiftDrag } = + useNodeEventHandlers() + const { nodeManager } = useVueNodeLifecycle() const forwardMiddlePointerIfNeeded = (event: PointerEvent) => { if (!isMiddlePointerInput(event)) return false @@ -37,6 +43,8 @@ export function useNodePointerInteractions( // Drag state for styling const isDragging = ref(false) + const isPointerDown = ref(false) + const wasSelectedAtPointerDown = ref(false) // Track if node was selected when pointer down occurred const dragStyle = computed(() => { if (nodeData.value?.flags?.pinned) { return { cursor: 'default' } @@ -44,6 +52,7 @@ export function useNodePointerInteractions( return { cursor: isDragging.value ? 'grabbing' : 'grab' } }) const startPosition = ref({ x: 0, y: 0 }) + const DRAG_THRESHOLD = 3 // pixels const handlePointerDown = (event: PointerEvent) => { if (!nodeData.value) { @@ -66,8 +75,10 @@ export function useNodePointerInteractions( return } - // Record position for drag threshold calculation - startPosition.value = { x: event.clientX, y: event.clientY } + // Track if node was selected before this pointer down + // IMPORTANT: Read from actual LGraphNode, not nodeData, to get correct state + const lgNode = nodeManager.value?.getNode(nodeData.value.id) + wasSelectedAtPointerDown.value = lgNode?.selected ?? false onNodeSelect(event, nodeData.value) @@ -75,18 +86,35 @@ export function useNodePointerInteractions( return } - // Start drag using layout system - isDragging.value = true - - // Set Vue node dragging state for selection toolbox - layoutStore.isDraggingVueNodes.value = true + // Record position for drag threshold calculation + startPosition.value = { x: event.clientX, y: event.clientY } + isPointerDown.value = true + // Don't start drag yet - wait for pointer move to exceed threshold startDrag(event) } const handlePointerMove = (event: PointerEvent) => { if (forwardMiddlePointerIfNeeded(event)) return + // Check if we should start dragging (pointer moved beyond threshold) + if (isPointerDown.value && !isDragging.value) { + const dx = event.clientX - startPosition.value.x + const dy = event.clientY - startPosition.value.y + const distance = Math.sqrt(dx * dx + dy * dy) + + if (distance > DRAG_THRESHOLD && nodeData.value) { + // Start drag + isDragging.value = true + layoutStore.isDraggingVueNodes.value = true + ensureNodeSelectedForShiftDrag( + event, + nodeData.value, + wasSelectedAtPointerDown.value + ) + } + } + if (isDragging.value) { void handleDrag(event) } @@ -98,6 +126,8 @@ export function useNodePointerInteractions( */ const cleanupDragState = () => { isDragging.value = false + isPointerDown.value = false + wasSelectedAtPointerDown.value = false layoutStore.isDraggingVueNodes.value = false } @@ -128,12 +158,28 @@ export function useNodePointerInteractions( const handlePointerUp = (event: PointerEvent) => { if (forwardMiddlePointerIfNeeded(event)) return - if (isDragging.value) { + const wasDragging = isDragging.value + const multiSelect = isMultiSelectKey(event) + const canHandlePointer = shouldHandleNodePointerEvents.value + + if (wasDragging) { handleDragTermination(event, 'drag end') + } else { + // Clean up pointer state even if not dragging + isPointerDown.value = false + const wasSelected = wasSelectedAtPointerDown.value + wasSelectedAtPointerDown.value = false + + if (nodeData.value && canHandlePointer) { + toggleNodeSelectionAfterPointerUp(nodeData.value.id, { + wasSelectedAtPointerDown: wasSelected, + multiSelect + }) + } } // Don't handle pointer events when canvas is in panning mode - forward to canvas instead - if (!shouldHandleNodePointerEvents.value) { + if (!canHandlePointer) { forwardEventToCanvas(event) return } diff --git a/src/renderer/extensions/vueNodes/utils/selectionUtils.ts b/src/renderer/extensions/vueNodes/utils/selectionUtils.ts new file mode 100644 index 0000000000..21ae852a63 --- /dev/null +++ b/src/renderer/extensions/vueNodes/utils/selectionUtils.ts @@ -0,0 +1,10 @@ +/** + * Checks if a pointer/mouse event has multi-select modifier keys pressed. + * Multi-select keys are: Ctrl (Windows/Linux), Cmd (Mac), or Shift + * + * @param event - The pointer or mouse event to check + * @returns true if any multi-select modifier key is pressed + */ +export function isMultiSelectKey(event: PointerEvent | MouseEvent): boolean { + return event.ctrlKey || event.metaKey || event.shiftKey +} diff --git a/tests-ui/tests/renderer/extensions/vueNodes/composables/useNodeEventHandlers.test.ts b/tests-ui/tests/renderer/extensions/vueNodes/composables/useNodeEventHandlers.test.ts index f19104f0fa..c5d5032262 100644 --- a/tests-ui/tests/renderer/extensions/vueNodes/composables/useNodeEventHandlers.test.ts +++ b/tests-ui/tests/renderer/extensions/vueNodes/composables/useNodeEventHandlers.test.ts @@ -1,10 +1,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { computed, shallowRef } from 'vue' -import { - type GraphNodeManager, - type VueNodeData, - useGraphNodeManager +import { useGraphNodeManager } from '@/composables/graph/useGraphNodeManager' +import type { + GraphNodeManager, + VueNodeData } from '@/composables/graph/useGraphNodeManager' import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle' import type { @@ -16,6 +16,8 @@ import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations' import { useNodeEventHandlers } from '@/renderer/extensions/vueNodes/composables/useNodeEventHandlers' +const canvasSelectedItems = vi.hoisted(() => [] as Array<{ id?: string }>) + vi.mock('@/renderer/core/canvas/canvasStore', () => { const canvas: Partial = { select: vi.fn(), @@ -23,12 +25,13 @@ vi.mock('@/renderer/core/canvas/canvasStore', () => { deselectAll: vi.fn() } const updateSelectedItems = vi.fn() + const canvasStoreInstance = { + canvas: canvas as LGraphCanvas, + updateSelectedItems, + selectedItems: canvasSelectedItems + } return { - useCanvasStore: vi.fn(() => ({ - canvas: canvas as LGraphCanvas, - updateSelectedItems, - selectedItems: [] - })) + useCanvasStore: vi.fn(() => canvasStoreInstance) } }) @@ -89,6 +92,8 @@ describe('useNodeEventHandlers', () => { beforeEach(async () => { vi.restoreAllMocks() + vi.clearAllMocks() + canvasSelectedItems.length = 0 }) describe('handleNodeSelect', () => { @@ -109,11 +114,10 @@ describe('useNodeEventHandlers', () => { expect(updateSelectedItems).toHaveBeenCalledOnce() }) - it('should toggle selection on ctrl+click', () => { + it('on pointer down with ctrl+click: selects node immediately', () => { const { handleNodeSelect } = useNodeEventHandlers() const { canvas } = useCanvasStore() - // Test selecting unselected node with ctrl mockNode!.selected = false const ctrlClickEvent = new PointerEvent('pointerdown', { @@ -124,16 +128,23 @@ describe('useNodeEventHandlers', () => { handleNodeSelect(ctrlClickEvent, testNodeData) + // On pointer down with multi-select: bring to front + expect(mockLayoutMutations.bringNodeToFront).toHaveBeenCalledWith( + 'node-1' + ) + + // Selection happens immediately so dragging includes this node expect(canvas?.deselectAll).not.toHaveBeenCalled() expect(canvas?.select).toHaveBeenCalledWith(mockNode) + expect(canvas?.deselect).not.toHaveBeenCalled() }) - it('should deselect on ctrl+click of selected node', () => { + it('on pointer down with ctrl+click of selected node: brings node to front only', () => { const { handleNodeSelect } = useNodeEventHandlers() const { canvas } = useCanvasStore() - // Test deselecting selected node with ctrl mockNode!.selected = true + mockNode!.flags.pinned = false const ctrlClickEvent = new PointerEvent('pointerdown', { bubbles: true, @@ -143,15 +154,22 @@ describe('useNodeEventHandlers', () => { handleNodeSelect(ctrlClickEvent, testNodeData) - expect(canvas?.deselect).toHaveBeenCalledWith(mockNode) + // On pointer down: bring to front + expect(mockLayoutMutations.bringNodeToFront).toHaveBeenCalledWith( + 'node-1' + ) + + // But don't deselect yet (deferred to pointer up) + expect(canvas?.deselect).not.toHaveBeenCalled() expect(canvas?.select).not.toHaveBeenCalled() }) - it('should handle meta key (Cmd) on Mac', () => { + it('on pointer down with meta key (Cmd): selects node immediately', () => { const { handleNodeSelect } = useNodeEventHandlers() const { canvas } = useCanvasStore() mockNode!.selected = false + mockNode!.flags.pinned = false const metaClickEvent = new PointerEvent('pointerdown', { bubbles: true, @@ -161,8 +179,59 @@ describe('useNodeEventHandlers', () => { handleNodeSelect(metaClickEvent, testNodeData) + // On pointer down with meta key: bring to front + expect(mockLayoutMutations.bringNodeToFront).toHaveBeenCalledWith( + 'node-1' + ) + + // Selection happens immediately expect(canvas?.select).toHaveBeenCalledWith(mockNode) expect(canvas?.deselectAll).not.toHaveBeenCalled() + expect(canvas?.deselect).not.toHaveBeenCalled() + }) + + it('on pointer down with shift key: selects node immediately', () => { + const { handleNodeSelect } = useNodeEventHandlers() + const { canvas } = useCanvasStore() + + mockNode!.selected = false + mockNode!.flags.pinned = false + + const shiftClickEvent = new PointerEvent('pointerdown', { + bubbles: true, + shiftKey: true + }) + + handleNodeSelect(shiftClickEvent, testNodeData) + + // On pointer down with shift: bring to front + expect(mockLayoutMutations.bringNodeToFront).toHaveBeenCalledWith( + 'node-1' + ) + + // Selection happens immediately for shift-click as well + expect(canvas?.select).toHaveBeenCalledWith(mockNode) + expect(canvas?.deselectAll).not.toHaveBeenCalled() + expect(canvas?.deselect).not.toHaveBeenCalled() + }) + + it('keeps existing multi-selection when dragging selected node without modifiers', () => { + const { handleNodeSelect } = useNodeEventHandlers() + const { canvas } = useCanvasStore() + + mockNode!.selected = true + canvasSelectedItems.push({ id: 'node-1' }, { id: 'node-2' }) + + const event = new PointerEvent('pointerdown', { + bubbles: true, + ctrlKey: false, + metaKey: false + }) + + handleNodeSelect(event, testNodeData) + + expect(canvas?.deselectAll).not.toHaveBeenCalled() + expect(canvas?.select).not.toHaveBeenCalled() }) it('should bring node to front when not pinned', () => { @@ -189,4 +258,144 @@ describe('useNodeEventHandlers', () => { expect(mockLayoutMutations.bringNodeToFront).not.toHaveBeenCalled() }) }) + + describe('toggleNodeSelectionAfterPointerUp', () => { + it('on pointer up with multi-select: deselects node that was selected at pointer down', () => { + const { toggleNodeSelectionAfterPointerUp } = useNodeEventHandlers() + const { canvas, updateSelectedItems } = useCanvasStore() + + mockNode!.selected = true + + toggleNodeSelectionAfterPointerUp('node-1', { + wasSelectedAtPointerDown: true, + multiSelect: true + }) + + expect(canvas?.deselect).toHaveBeenCalledWith(mockNode) + expect(updateSelectedItems).toHaveBeenCalledOnce() + }) + + it('on pointer up with multi-select and node not previously selected: no-op', () => { + const { toggleNodeSelectionAfterPointerUp } = useNodeEventHandlers() + const { canvas, updateSelectedItems } = useCanvasStore() + + mockNode!.selected = true + + toggleNodeSelectionAfterPointerUp('node-1', { + wasSelectedAtPointerDown: false, + multiSelect: true + }) + + expect(canvas?.select).not.toHaveBeenCalled() + expect(updateSelectedItems).not.toHaveBeenCalled() + }) + + it('on pointer up without multi-select: collapses multi-selection to clicked node', () => { + const { toggleNodeSelectionAfterPointerUp } = useNodeEventHandlers() + const { canvas, updateSelectedItems } = useCanvasStore() + + mockNode!.selected = true + canvasSelectedItems.push({ id: 'node-1' }, { id: 'node-2' }) + + toggleNodeSelectionAfterPointerUp('node-1', { + wasSelectedAtPointerDown: true, + multiSelect: false + }) + + expect(canvas?.deselectAll).toHaveBeenCalledOnce() + expect(canvas?.select).toHaveBeenCalledWith(mockNode) + expect(updateSelectedItems).toHaveBeenCalledOnce() + }) + + it('on pointer up without multi-select: keeps single selection intact', () => { + const { toggleNodeSelectionAfterPointerUp } = useNodeEventHandlers() + const { canvas, updateSelectedItems } = useCanvasStore() + + mockNode!.selected = true + canvasSelectedItems.push({ id: 'node-1' }) + + toggleNodeSelectionAfterPointerUp('node-1', { + wasSelectedAtPointerDown: true, + multiSelect: false + }) + + expect(canvas?.deselectAll).not.toHaveBeenCalled() + expect(canvas?.select).not.toHaveBeenCalled() + expect(updateSelectedItems).not.toHaveBeenCalled() + }) + }) + + describe('ensureNodeSelectedForShiftDrag', () => { + it('does nothing when multi-select key is not pressed', () => { + const { ensureNodeSelectedForShiftDrag } = useNodeEventHandlers() + const { canvas } = useCanvasStore() + + const event = new PointerEvent('pointermove', { shiftKey: false }) + + ensureNodeSelectedForShiftDrag(event, testNodeData, false) + + expect(canvas?.select).not.toHaveBeenCalled() + expect(canvas?.deselectAll).not.toHaveBeenCalled() + }) + + it('selects node and clears existing selection when shift-dragging with no other selections', () => { + const { ensureNodeSelectedForShiftDrag } = useNodeEventHandlers() + const { canvas } = useCanvasStore() + + mockNode!.selected = false + + const event = new PointerEvent('pointermove', { shiftKey: true }) + + ensureNodeSelectedForShiftDrag(event, testNodeData, false) + + expect(canvas?.deselectAll).toHaveBeenCalledOnce() + expect(canvas?.select).toHaveBeenCalledWith(mockNode) + }) + + it('adds node to existing multi-selection without clearing other nodes', () => { + const { ensureNodeSelectedForShiftDrag } = useNodeEventHandlers() + const { canvas, selectedItems } = useCanvasStore() + + // Create mock Positionable objects for existing selection + const mockExisting1 = { + id: 'existing-1', + pos: [0, 0] as [number, number], + move: vi.fn(), + snapToGrid: vi.fn(), + boundingRect: vi.fn(() => [0, 0, 100, 100] as const) + } as unknown as LGraphNode + const mockExisting2 = { + id: 'existing-2', + pos: [0, 0] as [number, number], + move: vi.fn(), + snapToGrid: vi.fn(), + boundingRect: vi.fn(() => [0, 0, 100, 100] as const) + } as unknown as LGraphNode + selectedItems.push(mockExisting1, mockExisting2) + mockNode!.selected = false + if (canvas?.select) vi.mocked(canvas.select).mockClear() + if (canvas?.deselectAll) vi.mocked(canvas.deselectAll).mockClear() + + const event = new PointerEvent('pointermove', { shiftKey: true }) + + ensureNodeSelectedForShiftDrag(event, testNodeData, false) + + expect(canvas?.deselectAll).not.toHaveBeenCalled() + expect(canvas?.select).toHaveBeenCalledWith(mockNode) + }) + + it('does nothing if node is already selected (selection happened on pointer down)', () => { + const { ensureNodeSelectedForShiftDrag } = useNodeEventHandlers() + const { canvas } = useCanvasStore() + + mockNode!.selected = true + + const event = new PointerEvent('pointermove', { shiftKey: true }) + + ensureNodeSelectedForShiftDrag(event, testNodeData, false) + + expect(canvas?.select).not.toHaveBeenCalled() + expect(canvas?.deselectAll).not.toHaveBeenCalled() + }) + }) })