diff --git a/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.png index 6636b96720..d675f8d32a 100644 Binary files a/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.png b/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.png index 172c373fc8..d4ff354004 100644 Binary files a/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.png and b/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/node/select.spec.ts b/browser_tests/tests/vueNodes/interactions/node/select.spec.ts index 2af6765896..4402236d24 100644 --- a/browser_tests/tests/vueNodes/interactions/node/select.spec.ts +++ b/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) + }) }) diff --git a/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts b/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts index 6adee1e894..05373c5223 100644 --- a/src/renderer/extensions/vueNodes/composables/useNodeEventHandlers.ts +++ b/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 diff --git a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts index 7010d253ce..8dffa6c74a 100644 --- a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts +++ b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts @@ -69,38 +69,35 @@ const createMouseEvent = ( } describe('useNodePointerInteractions', () => { - beforeEach(() => { + beforeEach(async () => { vi.clearAllMocks() forwardEventToCanvasMock.mockClear() + 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', () => { @@ -131,63 +128,25 @@ describe('useNodePointerInteractions', () => { 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 - }) - pointerHandlers.onPointerup(dragUpEvent) - - expect(mockOnPointerUp).toHaveBeenCalledWith( - dragUpEvent, - mockNodeData, - true - ) - - 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 - ) - }) - 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 +159,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(mockNodeData) const { pointerHandlers } = useNodePointerInteractions( nodeDataRef, - mockOnPointerUp + mockOnNodeSelect ) - pointerHandlers.onPointerdown(createPointerEvent('pointerdown')) - - // Clear nodeData before pointerup + // Clear nodeData before pointer down nodeDataRef.value = null + await nextTick() - pointerHandlers.onPointerup(createPointerEvent('pointerup')) + pointerHandlers.onPointerdown(createPointerEvent('pointerdown')) - 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 +200,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) + }) }) diff --git a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts index ccbf845cbf..1f1fc8710e 100644 --- a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts +++ b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts @@ -6,16 +6,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, - onPointerUp: ( - event: PointerEvent, - nodeData: VueNodeData, - wasDragging: boolean - ) => void + onNodeSelect: (event: PointerEvent, nodeData: VueNodeData) => void ) { const nodeData = computed(() => { const value = toValue(nodeDataMaybe) @@ -83,8 +76,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 } @@ -146,19 +142,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) } /** diff --git a/tests-ui/tests/renderer/extensions/vueNodes/components/LGraphNode.test.ts b/tests-ui/tests/renderer/extensions/vueNodes/components/LGraphNode.test.ts index 2d10edb2d6..8aff459367 100644 --- a/tests-ui/tests/renderer/extensions/vueNodes/components/LGraphNode.test.ts +++ b/tests-ui/tests/renderer/extensions/vueNodes/components/LGraphNode.test.ts @@ -7,7 +7,6 @@ import { createI18n } from 'vue-i18n' import type { VueNodeData } from '@/composables/graph/useGraphNodeManager' 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(() => ({ @@ -181,18 +180,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) - ) - }) }) 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 dd08457eb7..f19104f0fa 100644 --- a/tests-ui/tests/renderer/extensions/vueNodes/composables/useNodeEventHandlers.test.ts +++ b/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() })