Implement selection state management in Vue Nodes (#5421)

* let canvas continue to own selection state management

* fix merge error

* refactor: use computed instead of watcher for selectedNodeIds

Replace watcher pattern with computed for better Vue idioms:
- More reactive and efficient
- Automatically recomputes when dependencies change
- Simpler, more declarative code

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: improve injection error handling for selectedNodeIds

Replace silent fallback with explicit error when SelectedNodeIds
is not provided:
- Fail fast instead of silently using empty Set
- Clear error message for debugging
- Prevents nodes appearing unselected due to missing provider

Addresses DrJKL's concern about injection default behavior.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: improve mocking patterns using vi.mockObject

Replace manual mock interfaces with vi.mockObject for better type safety:
- Use Vitest's built-in mocking utilities instead of manual interfaces
- Properly configure mock return values
- Remove unnecessary type assertions

Addresses DrJKL's feedback on test mocking patterns.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: extract repeated nodeData for clarity

Extract common test nodeData object to reduce duplication:
- Move repeated VueNodeData object to describe scope
- Replace 6 instances of identical nodeData declarations
- Maintain different nodeData for specific test cases

Addresses DrJKL's suggestion to extract repeated test data.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* add type safety to mocks

---------

Co-authored-by: Claude <noreply@anthropic.com>

src/components/graph/GraphCanvas.vue

@@ -44,7 +44,6 @@
       :node-data="nodeData"
       :position="nodePositions.get(nodeData.id)"
       :size="nodeSizes.get(nodeData.id)"
-      :selected="nodeData.selected"
       :readonly="false"
       :executing="executionStore.executingNodeId === nodeData.id"
       :error="
@@ -79,6 +78,7 @@ import {
   computed,
   onMounted,
   onUnmounted,
+  provide,
   ref,
   shallowRef,
   watch,
@@ -112,6 +112,7 @@ import { useWorkflowPersistence } from '@/composables/useWorkflowPersistence'
 import { CORE_SETTINGS } from '@/constants/coreSettings'
 import { i18n, t } from '@/i18n'
 import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
+import { SelectedNodeIdsKey } from '@/renderer/core/canvas/injectionKeys'
 import TransformPane from '@/renderer/core/layout/TransformPane.vue'
 import MiniMap from '@/renderer/extensions/minimap/MiniMap.vue'
 import VueGraphNode from '@/renderer/extensions/vueNodes/components/LGraphNode.vue'
@@ -189,6 +190,17 @@ const handleNodeSelect = nodeEventHandlers.handleNodeSelect
 const handleNodeCollapse = nodeEventHandlers.handleNodeCollapse
 const handleNodeTitleUpdate = nodeEventHandlers.handleNodeTitleUpdate
 
+// Provide selection state to all Vue nodes
+const selectedNodeIds = computed(
+  () =>
+    new Set(
+      canvasStore.selectedItems
+        .filter((item) => item.id !== undefined)
+        .map((item) => String(item.id))
+    )
+)
+provide(SelectedNodeIdsKey, selectedNodeIds)
+
 watchEffect(() => {
   nodeDefStore.showDeprecated = settingStore.get('Comfy.Node.ShowDeprecated')
 })

src/composables/graph/useNodeEventHandlers.ts

@@ -33,12 +33,20 @@ export function useNodeEventHandlers(nodeManager: Ref<NodeManager | null>) {
     const node = nodeManager.value.getNode(nodeData.id)
     if (!node) return
 
-    // Handle multi-select with Ctrl/Cmd key
-    if (!event.ctrlKey && !event.metaKey) {
-      canvasStore.canvas.deselectAllNodes()
-    }
+    const isMultiSelect = event.ctrlKey || event.metaKey
 
-    canvasStore.canvas.selectNode(node)
+    if (isMultiSelect) {
+      // Ctrl/Cmd+click -> toggle selection
+      if (node.selected) {
+        canvasStore.canvas.deselect(node)
+      } else {
+        canvasStore.canvas.select(node)
+      }
+    } else {
+      // Regular click -> single select
+      canvasStore.canvas.deselectAll()
+      canvasStore.canvas.select(node)
+    }
 
     // Bring node to front when clicked (similar to LiteGraph behavior)
     // Skip if node is pinned to avoid unwanted movement
@@ -47,9 +55,6 @@ export function useNodeEventHandlers(nodeManager: Ref<NodeManager | null>) {
       layoutMutations.bringNodeToFront(nodeData.id)
     }
 
-    // Ensure node selection state is set
-    node.selected = true
-
     // Update canvas selection tracking
     canvasStore.updateSelectedItems()
   }

src/renderer/core/canvas/injectionKeys.ts

@@ -0,0 +1,8 @@
+import type { InjectionKey, Ref } from 'vue'
+
+/**
+ * Injection key for providing selected node IDs to Vue node components.
+ * Contains a reactive Set of selected node IDs (as strings).
+ */
+export const SelectedNodeIdsKey: InjectionKey<Ref<Set<string>>> =
+  Symbol('selectedNodeIds')

src/renderer/extensions/vueNodes/components/LGraphNode.vue

@@ -10,10 +10,13 @@
         'bg-white dark-theme:bg-[#15161A]',
         'min-w-[445px]',
         'lg-node absolute border border-solid rounded-2xl',
-        'outline outline-transparent outline-2 hover:outline-black dark-theme:hover:outline-white',
+        'outline outline-transparent outline-2',
         {
-          'border-blue-500 ring-2 ring-blue-300': selected,
-          'border-[#e1ded5] dark-theme:border-[#292A30]': !selected,
+          'outline-black dark-theme:outline-white': isSelected
+        },
+        {
+          'border-blue-500 ring-2 ring-blue-300': isSelected,
+          'border-[#e1ded5] dark-theme:border-[#292A30]': !isSelected,
           'animate-pulse': executing,
           'opacity-50': nodeData.mode === 4,
           'border-red-500 bg-red-50': error,
@@ -107,12 +110,13 @@
 </template>
 
 <script setup lang="ts">
-import { computed, onErrorCaptured, ref, toRef, watch } from 'vue'
+import { computed, inject, onErrorCaptured, ref, toRef, watch } from 'vue'
 
 // Import the VueNodeData type
 import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
 import { useErrorHandling } from '@/composables/useErrorHandling'
 import { LiteGraph } from '@/lib/litegraph/src/litegraph'
+import { SelectedNodeIdsKey } from '@/renderer/core/canvas/injectionKeys'
 import { useNodeLayout } from '@/renderer/extensions/vueNodes/layout/useNodeLayout'
 import { LODLevel, useLOD } from '@/renderer/extensions/vueNodes/lod/useLOD'
 import { cn } from '@/utils/tailwindUtil'
@@ -129,7 +133,6 @@ interface LGraphNodeProps {
   position?: { x: number; y: number }
   size?: { width: number; height: number }
   readonly?: boolean
-  selected?: boolean
   executing?: boolean
   progress?: number
   error?: string | null
@@ -150,6 +153,19 @@ const emit = defineEmits<{
   'update:title': [nodeId: string, newTitle: string]
 }>()
 
+// Inject selection state from parent
+const selectedNodeIds = inject(SelectedNodeIdsKey)
+if (!selectedNodeIds) {
+  throw new Error(
+    'SelectedNodeIds not provided - LGraphNode must be used within a component that provides selection state'
+  )
+}
+
+// Computed selection state - only this node re-evaluates when its selection changes
+const isSelected = computed(() => {
+  return selectedNodeIds.value.has(props.nodeData.id)
+})
+
 // LOD (Level of Detail) system based on zoom level
 const zoomRef = toRef(() => props.zoomLevel ?? 1)
 const {
@@ -193,7 +209,7 @@ const isCollapsed = ref(props.nodeData.flags?.collapsed ?? false)
 // Watch for external changes to the collapsed state
 watch(
   () => props.nodeData.flags?.collapsed,
-  (newCollapsed) => {
+  (newCollapsed: boolean | undefined) => {
     if (newCollapsed !== undefined && newCollapsed !== isCollapsed.value) {
       isCollapsed.value = newCollapsed
     }

tests-ui/tests/composables/graph/useNodeEventHandlers.test.ts

@@ -0,0 +1,236 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { ref } from 'vue'
+
+import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
+import { useGraphNodeManager } from '@/composables/graph/useGraphNodeManager'
+import { useNodeEventHandlers } from '@/composables/graph/useNodeEventHandlers'
+import type { LGraphCanvas, LGraphNode } from '@/lib/litegraph/src/litegraph'
+import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations'
+import { useCanvasStore } from '@/stores/graphStore'
+
+vi.mock('@/stores/graphStore', () => ({
+  useCanvasStore: vi.fn()
+}))
+
+vi.mock('@/renderer/core/layout/operations/layoutMutations', () => ({
+  useLayoutMutations: vi.fn()
+}))
+
+function createMockCanvas(): Pick<
+  LGraphCanvas,
+  'select' | 'deselect' | 'deselectAll'
+> {
+  return {
+    select: vi.fn(),
+    deselect: vi.fn(),
+    deselectAll: vi.fn()
+  }
+}
+
+function createMockNode(): Pick<LGraphNode, 'id' | 'selected' | 'flags'> {
+  return {
+    id: 'node-1',
+    selected: false,
+    flags: { pinned: false }
+  }
+}
+
+function createMockNodeManager(
+  node: Pick<LGraphNode, 'id' | 'selected' | 'flags'>
+) {
+  return {
+    getNode: vi.fn().mockReturnValue(node) as ReturnType<
+      typeof useGraphNodeManager
+    >['getNode']
+  }
+}
+
+function createMockCanvasStore(
+  canvas: Pick<LGraphCanvas, 'select' | 'deselect' | 'deselectAll'>
+): Pick<
+  ReturnType<typeof useCanvasStore>,
+  'canvas' | 'selectedItems' | 'updateSelectedItems'
+> {
+  return {
+    canvas: canvas as LGraphCanvas,
+    selectedItems: [],
+    updateSelectedItems: vi.fn()
+  }
+}
+
+function createMockLayoutMutations(): Pick<
+  ReturnType<typeof useLayoutMutations>,
+  'setSource' | 'bringNodeToFront'
+> {
+  return {
+    setSource: vi.fn(),
+    bringNodeToFront: vi.fn()
+  }
+}
+
+describe('useNodeEventHandlers', () => {
+  let mockCanvas: ReturnType<typeof createMockCanvas>
+  let mockNode: ReturnType<typeof createMockNode>
+  let mockNodeManager: ReturnType<typeof createMockNodeManager>
+  let mockCanvasStore: ReturnType<typeof createMockCanvasStore>
+  let mockLayoutMutations: ReturnType<typeof createMockLayoutMutations>
+
+  const testNodeData: VueNodeData = {
+    id: 'node-1',
+    title: 'Test Node',
+    type: 'test',
+    mode: 0,
+    selected: false,
+    executing: false
+  }
+
+  beforeEach(async () => {
+    mockNode = createMockNode()
+    mockCanvas = createMockCanvas()
+    mockNodeManager = createMockNodeManager(mockNode)
+    mockCanvasStore = createMockCanvasStore(mockCanvas)
+    mockLayoutMutations = createMockLayoutMutations()
+
+    vi.mocked(useCanvasStore).mockReturnValue(
+      mockCanvasStore as ReturnType<typeof useCanvasStore>
+    )
+    vi.mocked(useLayoutMutations).mockReturnValue(
+      mockLayoutMutations as ReturnType<typeof useLayoutMutations>
+    )
+  })
+
+  describe('handleNodeSelect', () => {
+    it('should select single node on regular click', () => {
+      const nodeManager = ref(mockNodeManager)
+      const { handleNodeSelect } = useNodeEventHandlers(nodeManager)
+
+      const event = new PointerEvent('pointerdown', {
+        bubbles: true,
+        ctrlKey: false,
+        metaKey: false
+      })
+
+      handleNodeSelect(event, testNodeData)
+
+      expect(mockCanvas.deselectAll).toHaveBeenCalledOnce()
+      expect(mockCanvas.select).toHaveBeenCalledWith(mockNode)
+      expect(mockCanvasStore.updateSelectedItems).toHaveBeenCalledOnce()
+    })
+
+    it('should toggle selection on ctrl+click', () => {
+      const nodeManager = ref(mockNodeManager)
+      const { handleNodeSelect } = useNodeEventHandlers(nodeManager)
+
+      // Test selecting unselected node with ctrl
+      mockNode.selected = false
+
+      const ctrlClickEvent = new PointerEvent('pointerdown', {
+        bubbles: true,
+        ctrlKey: true,
+        metaKey: false
+      })
+
+      handleNodeSelect(ctrlClickEvent, testNodeData)
+
+      expect(mockCanvas.deselectAll).not.toHaveBeenCalled()
+      expect(mockCanvas.select).toHaveBeenCalledWith(mockNode)
+    })
+
+    it('should deselect on ctrl+click of selected node', () => {
+      const nodeManager = ref(mockNodeManager)
+      const { handleNodeSelect } = useNodeEventHandlers(nodeManager)
+
+      // Test deselecting selected node with ctrl
+      mockNode.selected = true
+
+      const ctrlClickEvent = new PointerEvent('pointerdown', {
+        bubbles: true,
+        ctrlKey: true,
+        metaKey: false
+      })
+
+      handleNodeSelect(ctrlClickEvent, testNodeData)
+
+      expect(mockCanvas.deselect).toHaveBeenCalledWith(mockNode)
+      expect(mockCanvas.select).not.toHaveBeenCalled()
+    })
+
+    it('should handle meta key (Cmd) on Mac', () => {
+      const nodeManager = ref(mockNodeManager)
+      const { handleNodeSelect } = useNodeEventHandlers(nodeManager)
+
+      mockNode.selected = false
+
+      const metaClickEvent = new PointerEvent('pointerdown', {
+        bubbles: true,
+        ctrlKey: false,
+        metaKey: true
+      })
+
+      handleNodeSelect(metaClickEvent, testNodeData)
+
+      expect(mockCanvas.select).toHaveBeenCalledWith(mockNode)
+      expect(mockCanvas.deselectAll).not.toHaveBeenCalled()
+    })
+
+    it('should bring node to front when not pinned', () => {
+      const nodeManager = ref(mockNodeManager)
+      const { handleNodeSelect } = useNodeEventHandlers(nodeManager)
+
+      mockNode.flags.pinned = false
+
+      const event = new PointerEvent('pointerdown')
+      handleNodeSelect(event, testNodeData)
+
+      expect(mockLayoutMutations.bringNodeToFront).toHaveBeenCalledWith(
+        'node-1'
+      )
+    })
+
+    it('should not bring pinned node to front', () => {
+      const nodeManager = ref(mockNodeManager)
+      const { handleNodeSelect } = useNodeEventHandlers(nodeManager)
+
+      mockNode.flags.pinned = true
+
+      const event = new PointerEvent('pointerdown')
+      handleNodeSelect(event, testNodeData)
+
+      expect(mockLayoutMutations.bringNodeToFront).not.toHaveBeenCalled()
+    })
+
+    it('should handle missing canvas gracefully', () => {
+      const nodeManager = ref(mockNodeManager)
+      const { handleNodeSelect } = useNodeEventHandlers(nodeManager)
+
+      mockCanvasStore.canvas = null
+
+      const event = new PointerEvent('pointerdown')
+      expect(() => {
+        handleNodeSelect(event, testNodeData)
+      }).not.toThrow()
+
+      expect(mockCanvas.select).not.toHaveBeenCalled()
+    })
+
+    it('should handle missing node gracefully', () => {
+      const nodeManager = ref(mockNodeManager)
+      const { handleNodeSelect } = useNodeEventHandlers(nodeManager)
+
+      vi.mocked(mockNodeManager.getNode).mockReturnValue(undefined)
+
+      const event = new PointerEvent('pointerdown')
+      const nodeData = {
+        id: 'missing-node',
+        title: 'Missing Node',
+        type: 'test'
+      } as any
+
+      expect(() => {
+        handleNodeSelect(event, nodeData)
+      }).not.toThrow()
+
+      expect(mockCanvas.select).not.toHaveBeenCalled()
+    })
+  })
+})