diff --git a/src/renderer/extensions/vueNodes/components/LGraphNode.vue b/src/renderer/extensions/vueNodes/components/LGraphNode.vue index 828ee3c8cd..402469324c 100644 --- a/src/renderer/extensions/vueNodes/components/LGraphNode.vue +++ b/src/renderer/extensions/vueNodes/components/LGraphNode.vue @@ -38,9 +38,7 @@ }, dragStyle ]" - @pointerdown="handlePointerDown" - @pointermove="handlePointerMove" - @pointerup="handlePointerUp" + v-bind="pointerHandlers" @wheel="handleWheel" >
@@ -232,13 +230,10 @@ onErrorCaptured((error) => { // Use layout system for node position and dragging const { position, size, zIndex, resize } = useNodeLayout(() => nodeData.id) -const { - handlePointerDown, - handlePointerUp, - handlePointerMove, - isDragging, - dragStyle -} = useNodePointerInteractions(() => nodeData, handleNodeSelect) +const { pointerHandlers, isDragging, dragStyle } = useNodePointerInteractions( + () => nodeData, + handleNodeSelect +) onMounted(() => { if (size.value && transformState?.camera) { diff --git a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts new file mode 100644 index 0000000000..7e4f30c42b --- /dev/null +++ b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts @@ -0,0 +1,222 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { nextTick, ref } from 'vue' + +import type { VueNodeData } from '@/composables/graph/useGraphNodeManager' +import { useNodePointerInteractions } from '@/renderer/extensions/vueNodes/composables/useNodePointerInteractions' + +// Mock the dependencies +vi.mock('@/renderer/core/canvas/useCanvasInteractions', () => ({ + useCanvasInteractions: () => ({ + forwardEventToCanvas: vi.fn(), + shouldHandleNodePointerEvents: ref(true) + }) +})) + +vi.mock('@/renderer/extensions/vueNodes/layout/useNodeLayout', () => ({ + useNodeLayout: () => ({ + startDrag: vi.fn(), + endDrag: vi.fn().mockResolvedValue(undefined), + handleDrag: vi.fn().mockResolvedValue(undefined) + }) +})) + +vi.mock('@/renderer/core/layout/store/layoutStore', () => ({ + layoutStore: { + isDraggingVueNodes: ref(false) + } +})) + +const createMockVueNodeData = ( + overrides: Partial = {} +): VueNodeData => ({ + id: 'test-node-123', + title: 'Test Node', + type: 'TestNodeType', + mode: 0, + selected: false, + executing: false, + inputs: [], + outputs: [], + widgets: [], + ...overrides +}) + +const createPointerEvent = ( + eventType: string, + overrides: Partial = {} +): PointerEvent => { + return new PointerEvent(eventType, { + pointerId: 1, + button: 0, + clientX: 100, + clientY: 100, + ...overrides + }) +} + +const createMouseEvent = ( + eventType: string, + overrides: Partial = {} +): MouseEvent => { + return new MouseEvent(eventType, { + button: 2, // Right click + clientX: 100, + clientY: 100, + ...overrides + }) +} + +describe('useNodePointerInteractions', () => { + let mockNodeData: VueNodeData + let mockOnPointerUp: ReturnType + + beforeEach(() => { + mockNodeData = createMockVueNodeData() + mockOnPointerUp = vi.fn() + vi.clearAllMocks() + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('should only start drag on left-click', async () => { + const { pointerHandlers } = useNodePointerInteractions( + ref(mockNodeData), + mockOnPointerUp + ) + + // Right-click should not start drag + const rightClickEvent = createPointerEvent('pointerdown', { button: 2 }) + pointerHandlers.onPointerdown(rightClickEvent) + await nextTick() + + expect(mockOnPointerUp).not.toHaveBeenCalled() + + // Left-click should start drag and emit callback + const leftClickEvent = createPointerEvent('pointerdown', { button: 0 }) + pointerHandlers.onPointerdown(leftClickEvent) + await nextTick() + + const pointerUpEvent = createPointerEvent('pointerup') + pointerHandlers.onPointerup(pointerUpEvent) + await nextTick() + + expect(mockOnPointerUp).toHaveBeenCalledWith( + pointerUpEvent, + mockNodeData, + false // wasDragging = false (same position) + ) + }) + + it('should distinguish drag from click based on distance threshold', async () => { + const { pointerHandlers } = useNodePointerInteractions( + ref(mockNodeData), + mockOnPointerUp + ) + + // Test drag (distance > 4px) + pointerHandlers.onPointerdown( + createPointerEvent('pointerdown', { clientX: 100, clientY: 100 }) + ) + await nextTick() + + const dragUpEvent = createPointerEvent('pointerup', { + clientX: 200, + clientY: 200 + }) + pointerHandlers.onPointerup(dragUpEvent) + await nextTick() + + expect(mockOnPointerUp).toHaveBeenCalledWith( + dragUpEvent, + mockNodeData, + true + ) + + mockOnPointerUp.mockClear() + + // Test click (same position) + const samePos = { clientX: 100, clientY: 100 } + pointerHandlers.onPointerdown(createPointerEvent('pointerdown', samePos)) + await nextTick() + + const clickUpEvent = createPointerEvent('pointerup', samePos) + pointerHandlers.onPointerup(clickUpEvent) + await nextTick() + + expect(mockOnPointerUp).toHaveBeenCalledWith( + clickUpEvent, + mockNodeData, + false + ) + }) + + it('should handle drag termination via cancel and context menu', async () => { + const { pointerHandlers } = useNodePointerInteractions( + ref(mockNodeData), + mockOnPointerUp + ) + + // Test pointer cancel + pointerHandlers.onPointerdown(createPointerEvent('pointerdown')) + await nextTick() + pointerHandlers.onPointercancel(createPointerEvent('pointercancel')) + await nextTick() + + // Should not emit callback on cancel + expect(mockOnPointerUp).not.toHaveBeenCalled() + + // Test context menu during drag prevents default + pointerHandlers.onPointerdown(createPointerEvent('pointerdown')) + await nextTick() + + const contextMenuEvent = createMouseEvent('contextmenu') + const preventDefaultSpy = vi.spyOn(contextMenuEvent, 'preventDefault') + + pointerHandlers.onContextmenu(contextMenuEvent) + await nextTick() + + expect(preventDefaultSpy).toHaveBeenCalled() + }) + + it('should not emit callback when nodeData becomes null', async () => { + const nodeDataRef = ref(mockNodeData) + const { pointerHandlers } = useNodePointerInteractions( + nodeDataRef, + mockOnPointerUp + ) + + pointerHandlers.onPointerdown(createPointerEvent('pointerdown')) + await nextTick() + + // Clear nodeData before pointerup + nodeDataRef.value = null + await nextTick() + + pointerHandlers.onPointerup(createPointerEvent('pointerup')) + await nextTick() + + expect(mockOnPointerUp).not.toHaveBeenCalled() + }) + + it('should integrate with layout store dragging state', async () => { + const { layoutStore } = await import( + '@/renderer/core/layout/store/layoutStore' + ) + const { pointerHandlers } = useNodePointerInteractions( + ref(mockNodeData), + mockOnPointerUp + ) + + // Start drag + pointerHandlers.onPointerdown(createPointerEvent('pointerdown')) + await nextTick() + expect(layoutStore.isDraggingVueNodes.value).toBe(true) + + // End drag + pointerHandlers.onPointercancel(createPointerEvent('pointercancel')) + await nextTick() + expect(layoutStore.isDraggingVueNodes.value).toBe(false) + }) +}) diff --git a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts index f5ba083742..ce63dcadd8 100644 --- a/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts +++ b/src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts @@ -1,4 +1,4 @@ -import { type MaybeRefOrGetter, computed, ref, toValue } from 'vue' +import { type MaybeRefOrGetter, computed, onUnmounted, ref, toValue } from 'vue' import type { VueNodeData } from '@/composables/graph/useGraphNodeManager' import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions' @@ -9,16 +9,27 @@ import { useNodeLayout } from '@/renderer/extensions/vueNodes/layout/useNodeLayo const DRAG_THRESHOLD_PX = 4 export function useNodePointerInteractions( - nodeDataMaybe: MaybeRefOrGetter, + nodeDataMaybe: MaybeRefOrGetter, onPointerUp: ( event: PointerEvent, nodeData: VueNodeData, wasDragging: boolean ) => void ) { - const nodeData = toValue(nodeDataMaybe) + const nodeData = computed(() => { + const value = toValue(nodeDataMaybe) + if (!value) { + console.warn( + 'useNodePointerInteractions: nodeDataMaybe resolved to null/undefined' + ) + return null + } + return value + }) - const { startDrag, endDrag, handleDrag } = useNodeLayout(nodeData.id) + // Avoid potential null access during component initialization + const nodeIdComputed = computed(() => nodeData.value?.id ?? '') + const { startDrag, endDrag, handleDrag } = useNodeLayout(nodeIdComputed) // Use canvas interactions for proper wheel event handling and pointer event capture control const { forwardEventToCanvas, shouldHandleNodePointerEvents } = useCanvasInteractions() @@ -28,17 +39,21 @@ export function useNodePointerInteractions( const dragStyle = computed(() => ({ cursor: isDragging.value ? 'grabbing' : 'grab' })) - const lastX = ref(0) - const lastY = ref(0) + const startPosition = ref({ x: 0, y: 0 }) const handlePointerDown = (event: PointerEvent) => { - if (!nodeData) { + if (!nodeData.value) { console.warn( 'LGraphNode: nodeData is null/undefined in handlePointerDown' ) return } + // Only start drag on left-click (button 0) + if (event.button !== 0) { + return + } + // Don't handle pointer events when canvas is in panning mode - forward to canvas instead if (!shouldHandleNodePointerEvents.value) { forwardEventToCanvas(event) @@ -52,8 +67,7 @@ export function useNodePointerInteractions( layoutStore.isDraggingVueNodes.value = true startDrag(event) - lastY.value = event.clientY - lastX.value = event.clientX + startPosition.value = { x: event.clientX, y: event.clientY } } const handlePointerMove = (event: PointerEvent) => { @@ -62,13 +76,42 @@ export function useNodePointerInteractions( } } + /** + * Centralized cleanup function for drag state + * Ensures consistent cleanup across all drag termination scenarios + */ + const cleanupDragState = () => { + isDragging.value = false + layoutStore.isDraggingVueNodes.value = false + } + + /** + * Safely ends drag operation with proper error handling + * @param event - PointerEvent to end the drag with + */ + const safeDragEnd = async (event: PointerEvent): Promise => { + try { + await endDrag(event) + } catch (error) { + console.error('Error during endDrag:', error) + } finally { + cleanupDragState() + } + } + + /** + * Common drag termination handler with fallback cleanup + */ + const handleDragTermination = (event: PointerEvent, errorContext: string) => { + safeDragEnd(event).catch((error) => { + console.error(`Failed to complete ${errorContext}:`, error) + cleanupDragState() // Fallback cleanup + }) + } + const handlePointerUp = (event: PointerEvent) => { if (isDragging.value) { - isDragging.value = false - void endDrag(event) - - // Clear Vue node dragging state for selection toolbox - layoutStore.isDraggingVueNodes.value = false + handleDragTermination(event, 'drag end') } // Don't emit node-click when canvas is in panning mode - forward to canvas instead @@ -78,16 +121,52 @@ export function useNodePointerInteractions( } // Emit node-click for selection handling in GraphCanvas - const dx = event.clientX - lastX.value - const dy = event.clientY - lastY.value + const dx = event.clientX - startPosition.value.x + const dy = event.clientY - startPosition.value.y const wasDragging = Math.hypot(dx, dy) > DRAG_THRESHOLD_PX - onPointerUp(event, nodeData, wasDragging) + + if (!nodeData?.value) return + onPointerUp(event, nodeData.value, wasDragging) + } + + /** + * Handles pointer cancellation events (e.g., touch cancelled by browser) + * Ensures drag state is properly cleaned up when pointer interaction is interrupted + */ + const handlePointerCancel = (event: PointerEvent) => { + if (!isDragging.value) return + handleDragTermination(event, 'drag cancellation') + } + + /** + * Handles right-click during drag operations + * Cancels the current drag to prevent context menu from appearing while dragging + */ + const handleContextMenu = (event: MouseEvent) => { + if (!isDragging.value) return + + event.preventDefault() + // Simply cleanup state without calling endDrag to avoid synthetic event creation + cleanupDragState() + } + + // Cleanup on unmount to prevent resource leaks + onUnmounted(() => { + if (!isDragging.value) return + cleanupDragState() + }) + + const pointerHandlers = { + onPointerdown: handlePointerDown, + onPointermove: handlePointerMove, + onPointerup: handlePointerUp, + onPointercancel: handlePointerCancel, + onContextmenu: handleContextMenu } + return { isDragging, dragStyle, - handlePointerMove, - handlePointerDown, - handlePointerUp + pointerHandlers } } diff --git a/src/renderer/extensions/vueNodes/layout/useNodeLayout.ts b/src/renderer/extensions/vueNodes/layout/useNodeLayout.ts index 60e5a7fd8d..052c7c1b0a 100644 --- a/src/renderer/extensions/vueNodes/layout/useNodeLayout.ts +++ b/src/renderer/extensions/vueNodes/layout/useNodeLayout.ts @@ -4,6 +4,7 @@ import { type MaybeRefOrGetter, computed, inject, + ref, toValue } from 'vue' @@ -50,7 +51,7 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter) { const zIndex = computed(() => layoutRef.value?.zIndex ?? 0) // Drag state - let isDragging = false + const isDragging = ref(false) let dragStartPos: Point | null = null let dragStartMouse: Point | null = null let otherSelectedNodesStartPositions: Map | null = null @@ -59,9 +60,9 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter) { * Start dragging the node */ function startDrag(event: PointerEvent) { - if (!layoutRef.value) return + if (!layoutRef.value || !transformState) return - isDragging = true + isDragging.value = true dragStartPos = { ...position.value } dragStartMouse = { x: event.clientX, y: event.clientY } @@ -87,15 +88,20 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter) { mutations.setSource(LayoutSource.Vue) // Capture pointer - const target = event.target as HTMLElement - target.setPointerCapture(event.pointerId) + if (!(event.target instanceof HTMLElement)) return + event.target.setPointerCapture(event.pointerId) } /** * Handle drag movement */ const handleDrag = (event: PointerEvent) => { - if (!isDragging || !dragStartPos || !dragStartMouse || !transformState) { + if ( + !isDragging.value || + !dragStartPos || + !dragStartMouse || + !transformState + ) { return } @@ -141,16 +147,16 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter) { * End dragging */ function endDrag(event: PointerEvent) { - if (!isDragging) return + if (!isDragging.value) return - isDragging = false + isDragging.value = false dragStartPos = null dragStartMouse = null otherSelectedNodesStartPositions = null // Release pointer - const target = event.target as HTMLElement - target.releasePointerCapture(event.pointerId) + if (!(event.target instanceof HTMLElement)) return + event.target.releasePointerCapture(event.pointerId) } /** @@ -177,6 +183,7 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter) { bounds, isVisible, zIndex, + isDragging, // Mutations moveTo, @@ -196,7 +203,7 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter) { width: `${size.value.width}px`, height: `${size.value.height}px`, zIndex: zIndex.value, - cursor: isDragging ? 'grabbing' : 'grab' + cursor: isDragging.value ? 'grabbing' : 'grab' }) ) }