Skip to content

Commit 53be9f2

Browse files
Myesterygithub-actions
authored andcommitted
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 <[email protected]>
1 parent 3e02535 commit 53be9f2

File tree

9 files changed

+206
-137
lines changed

9 files changed

+206
-137
lines changed
61 Bytes
Loading
223 Bytes
Loading
89 Bytes
Loading

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,36 @@ test.describe('Vue Node Selection', () => {
4949
expect(await comfyPage.vueNodes.getSelectedNodeCount()).toBe(0)
5050
})
5151
}
52+
53+
test('should select pinned node without dragging', async ({ comfyPage }) => {
54+
const PIN_HOTKEY = 'p'
55+
const PIN_INDICATOR = '[data-testid="node-pin-indicator"]'
56+
57+
// Select a node by clicking its title
58+
const checkpointNodeHeader = comfyPage.page.getByText('Load Checkpoint')
59+
await checkpointNodeHeader.click()
60+
61+
// Pin it using the hotkey (as a user would)
62+
await comfyPage.page.keyboard.press(PIN_HOTKEY)
63+
64+
const checkpointNode = comfyPage.vueNodes.getNodeByTitle('Load Checkpoint')
65+
const pinIndicator = checkpointNode.locator(PIN_INDICATOR)
66+
await expect(pinIndicator).toBeVisible()
67+
68+
expect(await comfyPage.vueNodes.getSelectedNodeCount()).toBe(1)
69+
70+
const initialPos = await checkpointNodeHeader.boundingBox()
71+
if (!initialPos) throw new Error('Failed to get header position')
72+
73+
await comfyPage.dragAndDrop(
74+
{ x: initialPos.x + 10, y: initialPos.y + 10 },
75+
{ x: initialPos.x + 100, y: initialPos.y + 100 }
76+
)
77+
78+
const finalPos = await checkpointNodeHeader.boundingBox()
79+
if (!finalPos) throw new Error('Failed to get header position after drag')
80+
expect(finalPos).toEqual(initialPos)
81+
82+
expect(await comfyPage.vueNodes.getSelectedNodeCount()).toBe(1)
83+
})
5284
})

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

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,24 @@ import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
1515
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
1616
import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
1717
import { useNodeZIndex } from '@/renderer/extensions/vueNodes/composables/useNodeZIndex'
18+
import { isLGraphNode } from '@/utils/litegraphUtil'
19+
20+
/**
21+
* Check if multiple nodes are selected
22+
* Optimized to return early when 2+ nodes found
23+
*/
24+
function hasMultipleNodesSelected(selectedItems: unknown[]): boolean {
25+
let count = 0
26+
for (let i = 0; i < selectedItems.length; i++) {
27+
if (isLGraphNode(selectedItems[i])) {
28+
count++
29+
if (count >= 2) {
30+
return true
31+
}
32+
}
33+
}
34+
return false
35+
}
1836

1937
function useNodeEventHandlersIndividual() {
2038
const canvasStore = useCanvasStore()
@@ -26,11 +44,7 @@ function useNodeEventHandlersIndividual() {
2644
* Handle node selection events
2745
* Supports single selection and multi-select with Ctrl/Cmd
2846
*/
29-
const handleNodeSelect = (
30-
event: PointerEvent,
31-
nodeData: VueNodeData,
32-
wasDragging: boolean
33-
) => {
47+
const handleNodeSelect = (event: PointerEvent, nodeData: VueNodeData) => {
3448
if (!shouldHandleNodePointerEvents.value) return
3549

3650
if (!canvasStore.canvas || !nodeManager.value) return
@@ -48,12 +62,14 @@ function useNodeEventHandlersIndividual() {
4862
canvasStore.canvas.select(node)
4963
}
5064
} else {
51-
// If it wasn't a drag: single-select the node
52-
if (!wasDragging) {
65+
const selectedMultipleNodes = hasMultipleNodesSelected(
66+
canvasStore.selectedItems
67+
)
68+
if (!selectedMultipleNodes) {
69+
// Single-select the node
5370
canvasStore.canvas.deselectAll()
5471
canvasStore.canvas.select(node)
5572
}
56-
// Regular click -> single select
5773
}
5874

5975
// Bring node to front when clicked (similar to LiteGraph behavior)
@@ -122,7 +138,7 @@ function useNodeEventHandlersIndividual() {
122138
// TODO: add custom double-click behavior here
123139
// For now, ensure node is selected
124140
if (!node.selected) {
125-
handleNodeSelect(event, nodeData, false)
141+
handleNodeSelect(event, nodeData)
126142
}
127143
}
128144

@@ -143,7 +159,7 @@ function useNodeEventHandlersIndividual() {
143159

144160
// Select the node if not already selected
145161
if (!node.selected) {
146-
handleNodeSelect(event, nodeData, false)
162+
handleNodeSelect(event, nodeData)
147163
}
148164

149165
// Let LiteGraph handle the context menu
@@ -170,7 +186,7 @@ function useNodeEventHandlersIndividual() {
170186
metaKey: event.metaKey,
171187
bubbles: true
172188
})
173-
handleNodeSelect(syntheticEvent, nodeData, false)
189+
handleNodeSelect(syntheticEvent, nodeData)
174190
}
175191

176192
// Set drag data for potential drop operations

0 commit comments

Comments
 (0)