fix: select node after adding from library (#12404)

## Summary

When adding a node from the library sidebar, the node was not correctly
selected upon placing it. This was due to the canvas capturing the node
under the cursor on mouse down, however the node had not yet been
comitted to the graph at that point, and so selection was then cleared
on mouse up.

## Changes

- **What**: 
- add `blockCommitPointerDown` so if the cursor is over the canvas stop
propagation to prevent LiteGraph adding the mouse handler to clear the
selection

## Review Focus

Alternative approaches considered were blocking the event in endDrag
however this then required manual cleanup of LiteGraph handlers or
overriding the `pointer.onClick` function to force selection of our
node, both felt worse than this approach.

## Screenshots (if applicable)


https://github.com/user-attachments/assets/a2eb154e-5178-4a1e-b5c7-884efd7a10c6

browser_tests/tests/sidebar/nodeLibraryV2.spec.ts

@@ -129,4 +129,26 @@ test.describe('Node library sidebar V2', () => {
     await expect(tab.nodePreview, 'Preview displays on hover').toBeVisible()
     await expect(tab.nodePreview).toContainText('Inverts the image')
   })
+
+  test('Click-to-place from sidebar selects the newly added node', async ({
+    comfyPage
+  }) => {
+    const tab = comfyPage.menu.nodeLibraryTabV2
+    await comfyPage.nodeOps.clearGraph()
+    await tab.expandFolder('sampling')
+
+    const canvasBox = (await comfyPage.canvas.boundingBox())!
+    const target = {
+      x: canvasBox.width / 2,
+      y: canvasBox.height / 2
+    }
+
+    await tab.getNode('KSampler (Advanced)').click()
+    await comfyPage.canvas.click({ position: target })
+
+    await expect.poll(() => comfyPage.nodeOps.getGraphNodesCount()).toBe(1)
+    await expect
+      .poll(() => comfyPage.nodeOps.getSelectedGraphNodesCount())
+      .toBe(1)
+  })
 })

src/composables/node/useNodeDragToCanvas.test.ts

@@ -3,20 +3,27 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import type { ComfyNodeDefImpl } from '@/stores/nodeDefStore'
 import type { useNodeDragToCanvas as UseNodeDragToCanvasType } from './useNodeDragToCanvas'
 
-const { mockAddNodeOnGraph, mockConvertEventToCanvasOffset, mockCanvas } =
-  vi.hoisted(() => {
-    const mockConvertEventToCanvasOffset = vi.fn()
-    return {
-      mockAddNodeOnGraph: vi.fn(),
-      mockConvertEventToCanvasOffset,
-      mockCanvas: {
-        canvas: {
-          getBoundingClientRect: vi.fn()
-        },
-        convertEventToCanvasOffset: mockConvertEventToCanvasOffset
-      }
+const {
+  mockAddNodeOnGraph,
+  mockConvertEventToCanvasOffset,
+  mockSelectItems,
+  mockCanvas
+} = vi.hoisted(() => {
+  const mockConvertEventToCanvasOffset = vi.fn()
+  const mockSelectItems = vi.fn()
+  return {
+    mockAddNodeOnGraph: vi.fn(),
+    mockConvertEventToCanvasOffset,
+    mockSelectItems,
+    mockCanvas: {
+      canvas: {
+        getBoundingClientRect: vi.fn()
+      },
+      convertEventToCanvasOffset: mockConvertEventToCanvasOffset,
+      selectItems: mockSelectItems
     }
-  })
+  }
+})
 
 vi.mock('@/renderer/core/canvas/canvasStore', () => ({
   useCanvasStore: vi.fn(() => ({
@@ -119,6 +126,11 @@ describe('useNodeDragToCanvas', () => {
         'pointermove',
         expect.any(Function)
       )
+      expect(addEventListenerSpy).toHaveBeenCalledWith(
+        'pointerdown',
+        expect.any(Function),
+        true
+      )
       expect(addEventListenerSpy).toHaveBeenCalledWith(
         'pointerup',
         expect.any(Function),
@@ -239,6 +251,57 @@ describe('useNodeDragToCanvas', () => {
       expect(isDragging.value).toBe(true)
     })
 
+    it('should select the placed node when one is returned from the graph', () => {
+      mockCanvas.canvas.getBoundingClientRect.mockReturnValue({
+        left: 0,
+        right: 500,
+        top: 0,
+        bottom: 500
+      })
+      mockConvertEventToCanvasOffset.mockReturnValue([150, 150])
+      const placedNode = { id: 1 }
+      mockAddNodeOnGraph.mockReturnValue(placedNode)
+
+      const { startDrag, setupGlobalListeners } = useNodeDragToCanvas()
+      setupGlobalListeners()
+      startDrag(mockNodeDef)
+
+      document.dispatchEvent(
+        new PointerEvent('pointerup', {
+          clientX: 250,
+          clientY: 250,
+          bubbles: true
+        })
+      )
+
+      expect(mockSelectItems).toHaveBeenCalledWith([placedNode])
+    })
+
+    it('should not call selectItems when graph returns no node', () => {
+      mockCanvas.canvas.getBoundingClientRect.mockReturnValue({
+        left: 0,
+        right: 500,
+        top: 0,
+        bottom: 500
+      })
+      mockConvertEventToCanvasOffset.mockReturnValue([150, 150])
+      mockAddNodeOnGraph.mockReturnValue(null)
+
+      const { startDrag, setupGlobalListeners } = useNodeDragToCanvas()
+      setupGlobalListeners()
+      startDrag(mockNodeDef)
+
+      document.dispatchEvent(
+        new PointerEvent('pointerup', {
+          clientX: 250,
+          clientY: 250,
+          bubbles: true
+        })
+      )
+
+      expect(mockSelectItems).not.toHaveBeenCalled()
+    })
+
     it('should not add node on pointerup when in native drag mode', () => {
       mockCanvas.canvas.getBoundingClientRect.mockReturnValue({
         left: 0,
@@ -339,4 +402,58 @@ describe('useNodeDragToCanvas', () => {
       expect(dragMode.value).toBe('click')
     })
   })
+
+  describe('blockCommitPointerDown', () => {
+    function dispatchPointerDown(x: number, y: number) {
+      const event = new PointerEvent('pointerdown', {
+        clientX: x,
+        clientY: y,
+        bubbles: true,
+        cancelable: true
+      })
+      const stopSpy = vi.spyOn(event, 'stopImmediatePropagation')
+      document.dispatchEvent(event)
+      return stopSpy
+    }
+
+    beforeEach(() => {
+      mockCanvas.canvas.getBoundingClientRect.mockReturnValue({
+        left: 0,
+        right: 500,
+        top: 0,
+        bottom: 500
+      })
+    })
+
+    it('should stop propagation when in click-drag mode over canvas', () => {
+      const { startDrag, setupGlobalListeners } = useNodeDragToCanvas()
+      setupGlobalListeners()
+      startDrag(mockNodeDef)
+
+      expect(dispatchPointerDown(250, 250)).toHaveBeenCalled()
+    })
+
+    it('should not stop propagation when not dragging', () => {
+      const { setupGlobalListeners } = useNodeDragToCanvas()
+      setupGlobalListeners()
+
+      expect(dispatchPointerDown(250, 250)).not.toHaveBeenCalled()
+    })
+
+    it('should not stop propagation in native drag mode', () => {
+      const { startDrag, setupGlobalListeners } = useNodeDragToCanvas()
+      setupGlobalListeners()
+      startDrag(mockNodeDef, 'native')
+
+      expect(dispatchPointerDown(250, 250)).not.toHaveBeenCalled()
+    })
+
+    it('should not stop propagation when pointer is outside canvas', () => {
+      const { startDrag, setupGlobalListeners } = useNodeDragToCanvas()
+      setupGlobalListeners()
+      startDrag(mockNodeDef)
+
+      expect(dispatchPointerDown(600, 250)).not.toHaveBeenCalled()
+    })
+  })
 })

src/composables/node/useNodeDragToCanvas.ts

@@ -22,31 +22,33 @@ function cancelDrag() {
   dragMode.value = 'click'
 }
 
-function addNodeAtPosition(clientX: number, clientY: number): boolean {
-  if (!draggedNode.value) return false
-
-  const canvasStore = useCanvasStore()
-  const canvas = canvasStore.canvas
-  if (!canvas) return false
-
-  const canvasElement = canvas.canvas as HTMLCanvasElement
+function isOverCanvas(clientX: number, clientY: number): boolean {
+  const canvasElement = useCanvasStore().canvas?.canvas as
+    | HTMLCanvasElement
+    | undefined
+  if (!canvasElement) return false
   const rect = canvasElement.getBoundingClientRect()
-  const isOverCanvas =
+  return (
     clientX >= rect.left &&
     clientX <= rect.right &&
     clientY >= rect.top &&
     clientY <= rect.bottom
+  )
+}
 
-  if (isOverCanvas) {
-    const pos = canvas.convertEventToCanvasOffset({
-      clientX,
-      clientY
-    } as PointerEvent)
-    const litegraphService = useLitegraphService()
-    litegraphService.addNodeOnGraph(draggedNode.value, { pos })
-    return true
-  }
-  return false
+function addNodeAtPosition(clientX: number, clientY: number): boolean {
+  if (!draggedNode.value) return false
+  const canvas = useCanvasStore().canvas
+  if (!canvas) return false
+  if (!isOverCanvas(clientX, clientY)) return false
+
+  const pos = canvas.convertEventToCanvasOffset({
+    clientX,
+    clientY
+  } as PointerEvent)
+  const node = useLitegraphService().addNodeOnGraph(draggedNode.value, { pos })
+  if (node) canvas.selectItems([node])
+  return true
 }
 
 function endDrag(e: PointerEvent) {
@@ -64,11 +66,19 @@ function handleKeydown(e: KeyboardEvent) {
   if (e.key === 'Escape') cancelDrag()
 }
 
+// Prevent LiteGraph's empty-canvas hit-test from deselecting the placed node on pointerup.
+function blockCommitPointerDown(e: PointerEvent) {
+  if (!isDragging.value || dragMode.value !== 'click') return
+  if (!isOverCanvas(e.clientX, e.clientY)) return
+  e.stopImmediatePropagation()
+}
+
 function setupGlobalListeners() {
   if (listenersSetup) return
   listenersSetup = true
 
   document.addEventListener('pointermove', updatePosition)
+  document.addEventListener('pointerdown', blockCommitPointerDown, true)
   document.addEventListener('pointerup', endDrag, true)
   document.addEventListener('keydown', handleKeydown)
 }
@@ -78,6 +88,7 @@ function cleanupGlobalListeners() {
   listenersSetup = false
 
   document.removeEventListener('pointermove', updatePosition)
+  document.removeEventListener('pointerdown', blockCommitPointerDown, true)
   document.removeEventListener('pointerup', endDrag, true)
   document.removeEventListener('keydown', handleKeydown)