Select Vue Nodes After Drag (#5863)

This pull request refactors the node selection logic in the Vue nodes
event handler composable to simplify the function signature and improve
single vs. multi-selection behavior. The main change is the removal of
the `wasDragging` parameter from the `handleNodeSelect` function, with
selection logic now determined by the current selection state. Related
test code is updated to match the new function signature.

**Node selection logic improvements:**

* Refactored the `handleNodeSelect` function in
`useNodeEventHandlersIndividual` to remove the `wasDragging` parameter,
making the function signature simpler and relying on selection state to
handle single vs. multi-selection.
* Updated the selection logic to check if multiple nodes are already
selected using `isLGraphNode`, and only perform single selection if not.

**Code and test updates:**

* Updated all calls to `handleNodeSelect` in the composable to remove
the `wasDragging` argument, ensuring consistent usage throughout the
codebase.
[[1]](diffhunk://#diff-8d3820a1ca9c569bce00671fdd6290af81315ae11b8f3d6f29a5a9d30379d084L125-R123)
[[2]](diffhunk://#diff-8d3820a1ca9c569bce00671fdd6290af81315ae11b8f3d6f29a5a9d30379d084L146-R144)
[[3]](diffhunk://#diff-8d3820a1ca9c569bce00671fdd6290af81315ae11b8f3d6f29a5a9d30379d084L173-R171)
* Updated all related test cases to use the new `handleNodeSelect`
signature without the third parameter.
[[1]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aL105-R105)
[[2]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aL125-R125)
[[3]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aL144-R144)
[[4]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aL162-R162)
[[5]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aL174-R174)
[[6]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aL187-R187)

**Utility import:**

* Added an import for `isLGraphNode` from `@/utils/litegraphUtil` to
support the updated selection logic.## Summary

<!-- One sentence describing what changed and why. -->


## Screenshots (if applicable)



https://github.com/user-attachments/assets/71e856d3-afc2-497d-826e-5b485066e7fe

---------

Co-authored-by: github-actions <github-actions@github.com>

browser_tests/tests/vueNodes/interactions/node/select.spec.ts

@@ -49,4 +49,36 @@ test.describe('Vue Node Selection', () => {
       expect(await comfyPage.vueNodes.getSelectedNodeCount()).toBe(0)
     })
   }
+
+  test('should select pinned node without dragging', async ({ comfyPage }) => {
+    const PIN_HOTKEY = 'p'
+    const PIN_INDICATOR = '[data-testid="node-pin-indicator"]'
+
+    // Select a node by clicking its title
+    const checkpointNodeHeader = comfyPage.page.getByText('Load Checkpoint')
+    await checkpointNodeHeader.click()
+
+    // Pin it using the hotkey (as a user would)
+    await comfyPage.page.keyboard.press(PIN_HOTKEY)
+
+    const checkpointNode = comfyPage.vueNodes.getNodeByTitle('Load Checkpoint')
+    const pinIndicator = checkpointNode.locator(PIN_INDICATOR)
+    await expect(pinIndicator).toBeVisible()
+
+    expect(await comfyPage.vueNodes.getSelectedNodeCount()).toBe(1)
+
+    const initialPos = await checkpointNodeHeader.boundingBox()
+    if (!initialPos) throw new Error('Failed to get header position')
+
+    await comfyPage.dragAndDrop(
+      { x: initialPos.x + 10, y: initialPos.y + 10 },
+      { x: initialPos.x + 100, y: initialPos.y + 100 }
+    )
+
+    const finalPos = await checkpointNodeHeader.boundingBox()
+    if (!finalPos) throw new Error('Failed to get header position after drag')
+    expect(finalPos).toEqual(initialPos)
+
+    expect(await comfyPage.vueNodes.getSelectedNodeCount()).toBe(1)
+  })
 })

src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts

@@ -15,6 +15,24 @@ 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
+}
 
 function useNodeEventHandlersIndividual() {
   const canvasStore = useCanvasStore()
@@ -26,11 +44,7 @@ function useNodeEventHandlersIndividual() {
    * Handle node selection events
    * Supports single selection and multi-select with Ctrl/Cmd
    */
-  const handleNodeSelect = (
-    event: PointerEvent,
-    nodeData: VueNodeData,
-    wasDragging: boolean
-  ) => {
+  const handleNodeSelect = (event: PointerEvent, nodeData: VueNodeData) => {
     if (!shouldHandleNodePointerEvents.value) return
 
     if (!canvasStore.canvas || !nodeManager.value) return
@@ -48,12 +62,14 @@ function useNodeEventHandlersIndividual() {
         canvasStore.canvas.select(node)
       }
     } else {
-      // If it wasn't a drag: single-select the node
-      if (!wasDragging) {
+      const selectedMultipleNodes = hasMultipleNodesSelected(
+        canvasStore.selectedItems
+      )
+      if (!selectedMultipleNodes) {
+        // Single-select the node
         canvasStore.canvas.deselectAll()
         canvasStore.canvas.select(node)
       }
-      // Regular click -> single select
     }
 
     // Bring node to front when clicked (similar to LiteGraph behavior)
@@ -122,7 +138,7 @@ function useNodeEventHandlersIndividual() {
     // TODO: add custom double-click behavior here
     // For now, ensure node is selected
     if (!node.selected) {
-      handleNodeSelect(event, nodeData, false)
+      handleNodeSelect(event, nodeData)
     }
   }
 
@@ -143,7 +159,7 @@ function useNodeEventHandlersIndividual() {
 
     // Select the node if not already selected
     if (!node.selected) {
-      handleNodeSelect(event, nodeData, false)
+      handleNodeSelect(event, nodeData)
     }
 
     // Let LiteGraph handle the context menu
@@ -170,7 +186,7 @@ function useNodeEventHandlersIndividual() {
         metaKey: event.metaKey,
         bubbles: true
       })
-      handleNodeSelect(syntheticEvent, nodeData, false)
+      handleNodeSelect(syntheticEvent, nodeData)
     }
 
     // Set drag data for potential drop operations

src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts

@@ -69,125 +69,85 @@ const createMouseEvent = (
 }
 
 describe('useNodePointerInteractions', () => {
-  beforeEach(() => {
+  beforeEach(async () => {
     vi.clearAllMocks()
-    forwardEventToCanvasMock.mockClear()
+    // Reset layout store state between tests
+    const { layoutStore } = await import(
+      '@/renderer/core/layout/store/layoutStore'
+    )
+    layoutStore.isDraggingVueNodes.value = false
   })
 
   it('should only start drag on left-click', async () => {
     const mockNodeData = createMockVueNodeData()
-    const mockOnPointerUp = vi.fn()
+    const mockOnNodeSelect = vi.fn()
 
     const { pointerHandlers } = useNodePointerInteractions(
       ref(mockNodeData),
-      mockOnPointerUp
+      mockOnNodeSelect
     )
 
-    // Right-click should not start drag
+    // Right-click should not trigger selection
     const rightClickEvent = createPointerEvent('pointerdown', { button: 2 })
     pointerHandlers.onPointerdown(rightClickEvent)
 
-    expect(mockOnPointerUp).not.toHaveBeenCalled()
+    expect(mockOnNodeSelect).not.toHaveBeenCalled()
 
-    // Left-click should start drag and emit callback
+    // Left-click should trigger selection on pointer down
     const leftClickEvent = createPointerEvent('pointerdown', { button: 0 })
     pointerHandlers.onPointerdown(leftClickEvent)
 
-    const pointerUpEvent = createPointerEvent('pointerup')
-    pointerHandlers.onPointerup(pointerUpEvent)
-
-    expect(mockOnPointerUp).toHaveBeenCalledWith(
-      pointerUpEvent,
-      mockNodeData,
-      false // wasDragging = false (same position)
-    )
+    expect(mockOnNodeSelect).toHaveBeenCalledWith(leftClickEvent, mockNodeData)
   })
 
-  it('forwards middle mouse interactions to the canvas', () => {
+  it('should call onNodeSelect on pointer down', async () => {
     const mockNodeData = createMockVueNodeData()
-    const mockOnPointerUp = vi.fn()
+    const mockOnNodeSelect = vi.fn()
 
     const { pointerHandlers } = useNodePointerInteractions(
       ref(mockNodeData),
-      mockOnPointerUp
+      mockOnNodeSelect
     )
 
-    const middlePointerDown = createPointerEvent('pointerdown', { button: 1 })
-    pointerHandlers.onPointerdown(middlePointerDown)
-    expect(forwardEventToCanvasMock).toHaveBeenCalledWith(middlePointerDown)
-
-    forwardEventToCanvasMock.mockClear()
-
-    const middlePointerMove = createPointerEvent('pointermove', { buttons: 4 })
-    pointerHandlers.onPointermove(middlePointerMove)
-    expect(forwardEventToCanvasMock).toHaveBeenCalledWith(middlePointerMove)
-
-    forwardEventToCanvasMock.mockClear()
-
-    const middlePointerUp = createPointerEvent('pointerup', { button: 1 })
-    pointerHandlers.onPointerup(middlePointerUp)
-    expect(forwardEventToCanvasMock).toHaveBeenCalledWith(middlePointerUp)
-
-    expect(mockOnPointerUp).not.toHaveBeenCalled()
-  })
-
-  it('should distinguish drag from click based on distance threshold', async () => {
-    const mockNodeData = createMockVueNodeData()
-    const mockOnPointerUp = vi.fn()
-
-    const { pointerHandlers } = useNodePointerInteractions(
-      ref(mockNodeData),
-      mockOnPointerUp
-    )
-
-    // Test drag (distance > 4px)
-    pointerHandlers.onPointerdown(
-      createPointerEvent('pointerdown', { clientX: 100, clientY: 100 })
-    )
-
-    const dragUpEvent = createPointerEvent('pointerup', {
-      clientX: 200,
-      clientY: 200
+    // Selection should happen on pointer down
+    const downEvent = createPointerEvent('pointerdown', {
+      clientX: 100,
+      clientY: 100
     })
-    pointerHandlers.onPointerup(dragUpEvent)
+    pointerHandlers.onPointerdown(downEvent)
 
-    expect(mockOnPointerUp).toHaveBeenCalledWith(
-      dragUpEvent,
-      mockNodeData,
-      true
+    expect(mockOnNodeSelect).toHaveBeenCalledWith(downEvent, mockNodeData)
+
+    mockOnNodeSelect.mockClear()
+
+    // Even if we drag, selection already happened on pointer down
+    pointerHandlers.onPointerup(
+      createPointerEvent('pointerup', { clientX: 200, clientY: 200 })
     )
 
-    mockOnPointerUp.mockClear()
-
-    // Test click (same position)
-    const samePos = { clientX: 100, clientY: 100 }
-    pointerHandlers.onPointerdown(createPointerEvent('pointerdown', samePos))
-
-    const clickUpEvent = createPointerEvent('pointerup', samePos)
-    pointerHandlers.onPointerup(clickUpEvent)
-
-    expect(mockOnPointerUp).toHaveBeenCalledWith(
-      clickUpEvent,
-      mockNodeData,
-      false
-    )
+    // onNodeSelect should not be called again on pointer up
+    expect(mockOnNodeSelect).not.toHaveBeenCalled()
   })
 
   it('should handle drag termination via cancel and context menu', async () => {
     const mockNodeData = createMockVueNodeData()
-    const mockOnPointerUp = vi.fn()
+    const mockOnNodeSelect = vi.fn()
 
     const { pointerHandlers } = useNodePointerInteractions(
       ref(mockNodeData),
-      mockOnPointerUp
+      mockOnNodeSelect
     )
 
-    // Test pointer cancel
+    // Test pointer cancel - selection happens on pointer down
     pointerHandlers.onPointerdown(createPointerEvent('pointerdown'))
+    expect(mockOnNodeSelect).toHaveBeenCalledTimes(1)
+
     pointerHandlers.onPointercancel(createPointerEvent('pointercancel'))
 
-    // Should not emit callback on cancel
-    expect(mockOnPointerUp).not.toHaveBeenCalled()
+    // Selection should have been called on pointer down only
+    expect(mockOnNodeSelect).toHaveBeenCalledTimes(1)
+
+    mockOnNodeSelect.mockClear()
 
     // Test context menu during drag prevents default
     pointerHandlers.onPointerdown(createPointerEvent('pointerdown'))
@@ -200,36 +160,35 @@ describe('useNodePointerInteractions', () => {
     expect(preventDefaultSpy).toHaveBeenCalled()
   })
 
-  it('should not emit callback when nodeData becomes null', async () => {
+  it('should not call onNodeSelect when nodeData is null', async () => {
     const mockNodeData = createMockVueNodeData()
-    const mockOnPointerUp = vi.fn()
+    const mockOnNodeSelect = vi.fn()
     const nodeDataRef = ref<VueNodeData | null>(mockNodeData)
 
     const { pointerHandlers } = useNodePointerInteractions(
       nodeDataRef,
-      mockOnPointerUp
+      mockOnNodeSelect
     )
 
+    // Clear nodeData before pointer down
+    nodeDataRef.value = null
+    await nextTick()
+
     pointerHandlers.onPointerdown(createPointerEvent('pointerdown'))
 
-    // Clear nodeData before pointerup
-    nodeDataRef.value = null
-
-    pointerHandlers.onPointerup(createPointerEvent('pointerup'))
-
-    expect(mockOnPointerUp).not.toHaveBeenCalled()
+    expect(mockOnNodeSelect).not.toHaveBeenCalled()
   })
 
   it('should integrate with layout store dragging state', async () => {
     const mockNodeData = createMockVueNodeData()
-    const mockOnPointerUp = vi.fn()
+    const mockOnNodeSelect = vi.fn()
     const { layoutStore } = await import(
       '@/renderer/core/layout/store/layoutStore'
     )
 
     const { pointerHandlers } = useNodePointerInteractions(
       ref(mockNodeData),
-      mockOnPointerUp
+      mockOnNodeSelect
     )
 
     // Start drag
@@ -242,4 +201,93 @@ describe('useNodePointerInteractions', () => {
     await nextTick()
     expect(layoutStore.isDraggingVueNodes.value).toBe(false)
   })
+
+  it('should select node on pointer down with ctrl key for multi-select', async () => {
+    const mockNodeData = createMockVueNodeData()
+    const mockOnNodeSelect = vi.fn()
+
+    const { pointerHandlers } = useNodePointerInteractions(
+      ref(mockNodeData),
+      mockOnNodeSelect
+    )
+
+    // Pointer down with ctrl key should pass the event with ctrl key set
+    const ctrlDownEvent = createPointerEvent('pointerdown', {
+      ctrlKey: true,
+      clientX: 100,
+      clientY: 100
+    })
+    pointerHandlers.onPointerdown(ctrlDownEvent)
+
+    expect(mockOnNodeSelect).toHaveBeenCalledWith(ctrlDownEvent, mockNodeData)
+    expect(mockOnNodeSelect).toHaveBeenCalledTimes(1)
+  })
+
+  it('should select pinned node on pointer down but not start drag', async () => {
+    const mockNodeData = createMockVueNodeData({
+      flags: { pinned: true }
+    })
+    const mockOnNodeSelect = vi.fn()
+    const { layoutStore } = await import(
+      '@/renderer/core/layout/store/layoutStore'
+    )
+
+    const { pointerHandlers } = useNodePointerInteractions(
+      ref(mockNodeData),
+      mockOnNodeSelect
+    )
+
+    // Pointer down on pinned node
+    const downEvent = createPointerEvent('pointerdown')
+    pointerHandlers.onPointerdown(downEvent)
+
+    // Should select the node
+    expect(mockOnNodeSelect).toHaveBeenCalledWith(downEvent, mockNodeData)
+
+    // But should not start dragging
+    expect(layoutStore.isDraggingVueNodes.value).toBe(false)
+  })
+
+  it('should select node immediately when drag starts', async () => {
+    const mockNodeData = createMockVueNodeData()
+    const mockOnNodeSelect = vi.fn()
+    const { layoutStore } = await import(
+      '@/renderer/core/layout/store/layoutStore'
+    )
+
+    const { pointerHandlers } = useNodePointerInteractions(
+      ref(mockNodeData),
+      mockOnNodeSelect
+    )
+
+    // Pointer down should select node immediately
+    const downEvent = createPointerEvent('pointerdown', {
+      clientX: 100,
+      clientY: 100
+    })
+    pointerHandlers.onPointerdown(downEvent)
+
+    // Selection should happen on pointer down (before move)
+    expect(mockOnNodeSelect).toHaveBeenCalledWith(downEvent, mockNodeData)
+    expect(mockOnNodeSelect).toHaveBeenCalledTimes(1)
+
+    // Dragging state should be active
+    expect(layoutStore.isDraggingVueNodes.value).toBe(true)
+
+    // Move the pointer (start dragging)
+    pointerHandlers.onPointermove(
+      createPointerEvent('pointermove', { clientX: 150, clientY: 150 })
+    )
+
+    // Selection should still only have been called once (on pointer down)
+    expect(mockOnNodeSelect).toHaveBeenCalledTimes(1)
+
+    // End drag
+    pointerHandlers.onPointerup(
+      createPointerEvent('pointerup', { clientX: 150, clientY: 150 })
+    )
+
+    // Selection should still only have been called once
+    expect(mockOnNodeSelect).toHaveBeenCalledTimes(1)
+  })
 })

src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts

@@ -7,16 +7,9 @@ import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteracti
 import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
 import { useNodeLayout } from '@/renderer/extensions/vueNodes/layout/useNodeLayout'
 
-// Treat tiny pointer jitter as a click, not a drag
-const DRAG_THRESHOLD_PX = 4
-
 export function useNodePointerInteractions(
   nodeDataMaybe: MaybeRefOrGetter<VueNodeData | null>,
-  onPointerUp: (
-    event: PointerEvent,
-    nodeData: VueNodeData,
-    wasDragging: boolean
-  ) => void
+  onNodeSelect: (event: PointerEvent, nodeData: VueNodeData) => void
 ) {
   const nodeData = computed(() => {
     const value = toValue(nodeDataMaybe)
@@ -84,8 +77,11 @@ export function useNodePointerInteractions(
       return
     }
 
-    // Don't allow dragging if node is pinned (but still record position for selection)
+    // Record position for drag threshold calculation
     startPosition.value = { x: event.clientX, y: event.clientY }
+
+    onNodeSelect(event, nodeData.value)
+
     if (nodeData.value.flags?.pinned) {
       return
     }
@@ -147,19 +143,11 @@ export function useNodePointerInteractions(
       handleDragTermination(event, 'drag end')
     }
 
-    // Don't emit node-click when canvas is in panning mode - forward to canvas instead
+    // Don't handle pointer events when canvas is in panning mode - forward to canvas instead
     if (!shouldHandleNodePointerEvents.value) {
       forwardEventToCanvas(event)
       return
     }
-
-    // Emit node-click for selection handling in GraphCanvas
-    const dx = event.clientX - startPosition.value.x
-    const dy = event.clientY - startPosition.value.y
-    const wasDragging = Math.hypot(dx, dy) > DRAG_THRESHOLD_PX
-
-    if (!nodeData?.value) return
-    onPointerUp(event, nodeData.value, wasDragging)
   }
 
   /**

tests-ui/tests/renderer/extensions/vueNodes/components/LGraphNode.test.ts

@@ -8,7 +8,6 @@ import { createI18n } from 'vue-i18n'
 import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
 import { TransformStateKey } from '@/renderer/core/layout/injectionKeys'
 import LGraphNode from '@/renderer/extensions/vueNodes/components/LGraphNode.vue'
-import { useNodeEventHandlers } from '@/renderer/extensions/vueNodes/composables/useNodeEventHandlers'
 import { useVueElementTracking } from '@/renderer/extensions/vueNodes/composables/useVueNodeResizeTracking'
 
 const mockData = vi.hoisted(() => ({
@@ -205,18 +204,4 @@ describe('LGraphNode', () => {
 
     expect(wrapper.classes()).toContain('animate-pulse')
   })
-
-  it('should emit node-click event on pointer up', async () => {
-    const { handleNodeSelect } = useNodeEventHandlers()
-    const wrapper = mountLGraphNode({ nodeData: mockNodeData })
-
-    await wrapper.trigger('pointerup')
-
-    expect(handleNodeSelect).toHaveBeenCalledOnce()
-    expect(handleNodeSelect).toHaveBeenCalledWith(
-      expect.any(PointerEvent),
-      mockNodeData,
-      expect.any(Boolean)
-    )
-  })
 })

tests-ui/tests/renderer/extensions/vueNodes/composables/useNodeEventHandlers.test.ts

@@ -102,7 +102,7 @@ describe('useNodeEventHandlers', () => {
         metaKey: false
       })
 
-      handleNodeSelect(event, testNodeData, false)
+      handleNodeSelect(event, testNodeData)
 
       expect(canvas?.deselectAll).toHaveBeenCalledOnce()
       expect(canvas?.select).toHaveBeenCalledWith(mockNode)
@@ -122,7 +122,7 @@ describe('useNodeEventHandlers', () => {
         metaKey: false
       })
 
-      handleNodeSelect(ctrlClickEvent, testNodeData, false)
+      handleNodeSelect(ctrlClickEvent, testNodeData)
 
       expect(canvas?.deselectAll).not.toHaveBeenCalled()
       expect(canvas?.select).toHaveBeenCalledWith(mockNode)
@@ -141,7 +141,7 @@ describe('useNodeEventHandlers', () => {
         metaKey: false
       })
 
-      handleNodeSelect(ctrlClickEvent, testNodeData, false)
+      handleNodeSelect(ctrlClickEvent, testNodeData)
 
       expect(canvas?.deselect).toHaveBeenCalledWith(mockNode)
       expect(canvas?.select).not.toHaveBeenCalled()
@@ -159,7 +159,7 @@ describe('useNodeEventHandlers', () => {
         metaKey: true
       })
 
-      handleNodeSelect(metaClickEvent, testNodeData, false)
+      handleNodeSelect(metaClickEvent, testNodeData)
 
       expect(canvas?.select).toHaveBeenCalledWith(mockNode)
       expect(canvas?.deselectAll).not.toHaveBeenCalled()
@@ -171,7 +171,7 @@ describe('useNodeEventHandlers', () => {
       mockNode!.flags.pinned = false
 
       const event = new PointerEvent('pointerdown')
-      handleNodeSelect(event, testNodeData, false)
+      handleNodeSelect(event, testNodeData)
 
       expect(mockLayoutMutations.bringNodeToFront).toHaveBeenCalledWith(
         'node-1'
@@ -184,7 +184,7 @@ describe('useNodeEventHandlers', () => {
       mockNode!.flags.pinned = true
 
       const event = new PointerEvent('pointerdown')
-      handleNodeSelect(event, testNodeData, false)
+      handleNodeSelect(event, testNodeData)
 
       expect(mockLayoutMutations.bringNodeToFront).not.toHaveBeenCalled()
     })