mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-06 13:40:25 +00:00
[fix] do not drag on right-click + fix refs (#5784)
## Summary Fixes drag handling logic. ## Changes Only check for drag on left-click. Adds handler logic for following pointer events: 1. drag termination 2. context menu 3. pointer cancel Adds tests. Consolidates cleanup tasks. ## Screenshots Fixed State: Ignore first failed drag, browser window didn't have context. https://github.com/user-attachments/assets/00ec685a-1ef7-4102-b19b-4cdb9b201d22 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5784-fix-do-not-drag-on-right-click-fix-refs-27a6d73d3650812ea797fccf14022568) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -38,9 +38,7 @@
|
||||
},
|
||||
dragStyle
|
||||
]"
|
||||
@pointerdown="handlePointerDown"
|
||||
@pointermove="handlePointerMove"
|
||||
@pointerup="handlePointerUp"
|
||||
v-bind="pointerHandlers"
|
||||
@wheel="handleWheel"
|
||||
>
|
||||
<div class="flex items-center">
|
||||
@@ -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) {
|
||||
|
||||
@@ -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> = {}
|
||||
): 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<PointerEventInit> = {}
|
||||
): PointerEvent => {
|
||||
return new PointerEvent(eventType, {
|
||||
pointerId: 1,
|
||||
button: 0,
|
||||
clientX: 100,
|
||||
clientY: 100,
|
||||
...overrides
|
||||
})
|
||||
}
|
||||
|
||||
const createMouseEvent = (
|
||||
eventType: string,
|
||||
overrides: Partial<MouseEventInit> = {}
|
||||
): MouseEvent => {
|
||||
return new MouseEvent(eventType, {
|
||||
button: 2, // Right click
|
||||
clientX: 100,
|
||||
clientY: 100,
|
||||
...overrides
|
||||
})
|
||||
}
|
||||
|
||||
describe('useNodePointerInteractions', () => {
|
||||
let mockNodeData: VueNodeData
|
||||
let mockOnPointerUp: ReturnType<typeof vi.fn>
|
||||
|
||||
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<VueNodeData | null>(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)
|
||||
})
|
||||
})
|
||||
@@ -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<VueNodeData>,
|
||||
nodeDataMaybe: MaybeRefOrGetter<VueNodeData | null>,
|
||||
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<void> => {
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
type MaybeRefOrGetter,
|
||||
computed,
|
||||
inject,
|
||||
ref,
|
||||
toValue
|
||||
} from 'vue'
|
||||
|
||||
@@ -50,7 +51,7 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter<string>) {
|
||||
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<string, Point> | null = null
|
||||
@@ -59,9 +60,9 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter<string>) {
|
||||
* 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<string>) {
|
||||
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<string>) {
|
||||
* 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<string>) {
|
||||
bounds,
|
||||
isVisible,
|
||||
zIndex,
|
||||
isDragging,
|
||||
|
||||
// Mutations
|
||||
moveTo,
|
||||
@@ -196,7 +203,7 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter<string>) {
|
||||
width: `${size.value.width}px`,
|
||||
height: `${size.value.height}px`,
|
||||
zIndex: zIndex.value,
|
||||
cursor: isDragging ? 'grabbing' : 'grab'
|
||||
cursor: isDragging.value ? 'grabbing' : 'grab'
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user