Skip to content

Commit 14d94da

Browse files
authored
Fix Node Event Handlers for Shift Click (#6262)
This pull request refactors the node selection and pointer interaction logic in the Vue node graph editor to improve multi-selection behavior, clarify event handling, and enhance test coverage. The main change is to defer multi-select toggle actions (such as ctrl+click for selection/deselection) from pointer down to pointer up, preventing premature selection state changes and making drag interactions more robust. The drag initiation logic is also refined to only start dragging after the pointer moves beyond a threshold, and new composable methods are introduced for granular node selection control. **Node selection and pointer event handling improvements:** * Refactored multi-select (ctrl/cmd/shift+click) logic in `useNodeEventHandlersIndividual`: selection toggling is now deferred to pointer up, and pointer down only brings the node to front without changing selection state. The previous `hasMultipleNodesSelected` function and related logic were removed for clarity. [[1]](diffhunk://#diff-8d3820a1ca9c569bce00671fdd6290af81315ae11b8f3d6f29a5a9d30379d084L18-L35) [[2]](diffhunk://#diff-8d3820a1ca9c569bce00671fdd6290af81315ae11b8f3d6f29a5a9d30379d084L57-L73) [[3]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aL112-L116) [[4]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aR127-R143) * Added new composable methods `deselectNode` and `toggleNodeSelectionAfterPointerUp` to `useNodeEventHandlersIndividual` for more granular control over node selection, and exposed them in the returned API. [[1]](diffhunk://#diff-8d3820a1ca9c569bce00671fdd6290af81315ae11b8f3d6f29a5a9d30379d084R210-R245) [[2]](diffhunk://#diff-8d3820a1ca9c569bce00671fdd6290af81315ae11b8f3d6f29a5a9d30379d084L251-R259) **Pointer interaction and drag behavior changes:** * Updated `useNodePointerInteractions` to track pointer down/up state and only start dragging after the pointer moves beyond a pixel threshold. Multi-select toggling is now handled on pointer up, not pointer down, and selection state is read from the actual node manager for accuracy. [[1]](diffhunk://#diff-b50f38fec4f988dcbee7b7adf2b3425ae1e40a7ff10439ecbcb380dfa0a05ee1R6-R10) [[2]](diffhunk://#diff-b50f38fec4f988dcbee7b7adf2b3425ae1e40a7ff10439ecbcb380dfa0a05ee1R33-R34) [[3]](diffhunk://#diff-b50f38fec4f988dcbee7b7adf2b3425ae1e40a7ff10439ecbcb380dfa0a05ee1R44-R53) [[4]](diffhunk://#diff-b50f38fec4f988dcbee7b7adf2b3425ae1e40a7ff10439ecbcb380dfa0a05ee1R76-R110) [[5]](diffhunk://#diff-b50f38fec4f988dcbee7b7adf2b3425ae1e40a7ff10439ecbcb380dfa0a05ee1R122-R123) [[6]](diffhunk://#diff-b50f38fec4f988dcbee7b7adf2b3425ae1e40a7ff10439ecbcb380dfa0a05ee1L131-R175) **Test suite enhancements:** * Improved and expanded tests for pointer interactions and selection logic, including new cases for ctrl+click selection toggling on pointer up, drag threshold behavior, and mocking of new composable methods. [[1]](diffhunk://#diff-8d94b444c448b346f5863e859c75f67267439a56a02baf44b385af1c6945effdR9-R11) [[2]](diffhunk://#diff-8d94b444c448b346f5863e859c75f67267439a56a02baf44b385af1c6945effdR35-R56) [[3]](diffhunk://#diff-8d94b444c448b346f5863e859c75f67267439a56a02baf44b385af1c6945effdR100-R102) [[4]](diffhunk://#diff-8d94b444c448b346f5863e859c75f67267439a56a02baf44b385af1c6945effdL144-R181) [[5]](diffhunk://#diff-8d94b444c448b346f5863e859c75f67267439a56a02baf44b385af1c6945effdL155-R196) [[6]](diffhunk://#diff-8d94b444c448b346f5863e859c75f67267439a56a02baf44b385af1c6945effdL196-R247) [[7]](diffhunk://#diff-8d94b444c448b346f5863e859c75f67267439a56a02baf44b385af1c6945effdL276-R336) [[8]](diffhunk://#diff-8d94b444c448b346f5863e859c75f67267439a56a02baf44b385af1c6945effdR348-R423) * Updated test setup and assertions for node event handlers, ensuring selection changes are only triggered at the correct event phase and that drag and multi-select logic is covered. [[1]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aL4-R7) [[2]](diffhunk://#diff-89bfc2a05201c6ff7116578efa45f96097594eb346f18446c70aa7125ab1811aR92) These changes make node selection more predictable and user-friendly, and ensure drag and multi-select actions behave consistently in both the UI and the test suite. fix #6128 https://github.com/user-attachments/assets/582804d0-1d21-4ba0-a161-6582fb379352
1 parent a521066 commit 14d94da

File tree

6 files changed

+600
-75
lines changed

6 files changed

+600
-75
lines changed

src/composables/canvas/useSelectionToolboxPosition.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,14 @@ export function useSelectionToolboxPosition(
6666
lgCanvas.canvas
6767
)
6868

69+
// Unified dragging state - combines both LiteGraph and Vue node dragging
70+
const isDragging = computed((): boolean => {
71+
const litegraphDragging = canvasStore.canvas?.state?.draggingItems ?? false
72+
const vueNodeDragging =
73+
shouldRenderVueNodes.value && layoutStore.isDraggingVueNodes.value
74+
return litegraphDragging || vueNodeDragging
75+
})
76+
6977
/**
7078
* Update position based on selection
7179
*/
@@ -77,6 +85,12 @@ export function useSelectionToolboxPosition(
7785
return
7886
}
7987

88+
// Don't show toolbox while dragging
89+
if (isDragging.value) {
90+
visible.value = false
91+
return
92+
}
93+
8094
visible.value = true
8195

8296
// Get bounds for all selected items
@@ -241,14 +255,6 @@ export function useSelectionToolboxPosition(
241255
})
242256
}
243257

244-
// Unified dragging state - combines both LiteGraph and Vue node dragging
245-
const isDragging = computed((): boolean => {
246-
const litegraphDragging = canvasStore.canvas?.state?.draggingItems ?? false
247-
const vueNodeDragging =
248-
shouldRenderVueNodes.value && layoutStore.isDraggingVueNodes.value
249-
return litegraphDragging || vueNodeDragging
250-
})
251-
252258
watch(isDragging, handleDragStateChange)
253259

254260
onUnmounted(() => {

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

Lines changed: 87 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,7 @@ 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-
}
18+
import { isMultiSelectKey } from '@/renderer/extensions/vueNodes/utils/selectionUtils'
3619

3720
function useNodeEventHandlersIndividual() {
3821
const canvasStore = useCanvasStore()
@@ -52,24 +35,19 @@ function useNodeEventHandlersIndividual() {
5235
const node = nodeManager.value.getNode(nodeData.id)
5336
if (!node) return
5437

55-
const isMultiSelect = event.ctrlKey || event.metaKey || event.shiftKey
38+
const multiSelect = isMultiSelectKey(event)
39+
const selectedItemsCount = canvasStore.selectedItems.length
40+
const preserveExistingSelection =
41+
!multiSelect && node.selected && selectedItemsCount > 1
5642

57-
if (isMultiSelect) {
58-
// Ctrl/Cmd+click -> toggle selection
59-
if (node.selected) {
60-
canvasStore.canvas.deselect(node)
61-
} else {
62-
canvasStore.canvas.select(node)
63-
}
64-
} else {
65-
const selectedMultipleNodes = hasMultipleNodesSelected(
66-
canvasStore.selectedItems
67-
)
68-
if (!selectedMultipleNodes) {
69-
// Single-select the node
70-
canvasStore.canvas.deselectAll()
43+
if (multiSelect) {
44+
if (!node.selected) {
7145
canvasStore.canvas.select(node)
7246
}
47+
} else if (!preserveExistingSelection) {
48+
// Regular click -> single select
49+
canvasStore.canvas.deselectAll()
50+
canvasStore.canvas.select(node)
7351
}
7452

7553
// Bring node to front when clicked (similar to LiteGraph behavior)
@@ -219,6 +197,32 @@ function useNodeEventHandlersIndividual() {
219197
canvasStore.updateSelectedItems()
220198
}
221199

200+
/**
201+
* Ensure node is selected for shift-drag operations
202+
* Handles special logic for promoting a node to selection when shift-dragging
203+
* @param event - The pointer event (for multi-select key detection)
204+
* @param nodeData - The node data for the node being dragged
205+
* @param wasSelectedAtPointerDown - Whether the node was selected when pointer-down occurred
206+
*/
207+
const ensureNodeSelectedForShiftDrag = (
208+
event: PointerEvent,
209+
nodeData: VueNodeData,
210+
wasSelectedAtPointerDown: boolean
211+
) => {
212+
if (wasSelectedAtPointerDown) return
213+
214+
const multiSelectKeyPressed = isMultiSelectKey(event)
215+
if (!multiSelectKeyPressed) return
216+
217+
if (!canvasStore.canvas || !nodeManager.value) return
218+
const node = nodeManager.value.getNode(nodeData.id)
219+
if (!node || node.selected) return
220+
221+
const selectionCount = canvasStore.selectedItems.length
222+
const addToSelection = selectionCount > 0
223+
selectNodes([nodeData.id], addToSelection)
224+
}
225+
222226
/**
223227
* Deselect specific nodes
224228
*/
@@ -229,14 +233,58 @@ function useNodeEventHandlersIndividual() {
229233

230234
nodeIds.forEach((nodeId) => {
231235
const node = nodeManager.value?.getNode(nodeId)
232-
if (node) {
233-
node.selected = false
236+
if (node && canvasStore.canvas) {
237+
canvasStore.canvas.deselect(node)
234238
}
235239
})
236240

237241
canvasStore.updateSelectedItems()
238242
}
239243

244+
const deselectNode = (nodeId: string) => {
245+
const node = nodeManager.value?.getNode(nodeId)
246+
if (node) {
247+
canvasStore.canvas?.deselect(node)
248+
canvasStore.updateSelectedItems()
249+
}
250+
}
251+
252+
const toggleNodeSelectionAfterPointerUp = (
253+
nodeId: string,
254+
{
255+
wasSelectedAtPointerDown,
256+
multiSelect
257+
}: {
258+
wasSelectedAtPointerDown: boolean
259+
multiSelect: boolean
260+
}
261+
) => {
262+
if (!shouldHandleNodePointerEvents.value) return
263+
264+
if (!canvasStore.canvas || !nodeManager.value) return
265+
266+
const node = nodeManager.value.getNode(nodeId)
267+
if (!node) return
268+
269+
if (!multiSelect) {
270+
const multipleSelected = canvasStore.selectedItems.length > 1
271+
if (multipleSelected && wasSelectedAtPointerDown) {
272+
canvasStore.canvas.deselectAll()
273+
canvasStore.canvas.select(node)
274+
canvasStore.updateSelectedItems()
275+
}
276+
return
277+
}
278+
279+
if (wasSelectedAtPointerDown) {
280+
canvasStore.canvas.deselect(node)
281+
canvasStore.updateSelectedItems()
282+
}
283+
284+
// No action needed when the node was not previously selected since the pointer-down
285+
// handler already added it to the selection.
286+
}
287+
240288
return {
241289
// Core event handlers
242290
handleNodeSelect,
@@ -248,7 +296,10 @@ function useNodeEventHandlersIndividual() {
248296

249297
// Batch operations
250298
selectNodes,
251-
deselectNodes
299+
deselectNodes,
300+
deselectNode,
301+
ensureNodeSelectedForShiftDrag,
302+
toggleNodeSelectionAfterPointerUp
252303
}
253304
}
254305

0 commit comments

Comments
 (0)