Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
32 changes: 32 additions & 0 deletions browser_tests/tests/vueNodes/interactions/node/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[architecture] medium Priority

Issue: The new selection logic changes behavior - previously wasDragging prevented single selection after drag, now multiple selection status is checked instead
Context: This could cause unintended single-selection when clicking on a node with multiple nodes already selected, breaking existing UX patterns
Suggestion: Consider if this behavioral change is intentional or if the original drag-based logic should be preserved

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)
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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'))
Expand All @@ -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<VueNodeData | null>(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
Expand All @@ -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)
})
})
Loading