mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-30 03:01:54 +00:00
Fix: In standard mode, don't stop when you hit a Vue node. (#5445)
* fix: Forward the scrolling events to the litegraph canvas. * prior-art: Use the existing event forwarding logic from useCanvasInteractions (h/t Ben) * fix: Get proper scaling from properties in the original event, fix browser zoom * tests: Fix missing property on mock * types: Cleanup type annotations in the test * cleanup: Initialize the mocks in place. * tests: extract createMockPointerEvent * tests: extract createMockWheelEvent * tests: extract createMockLGraphCanvas * tests: Add additional assertion for stopPropagation * tests: Comment pruning, test rename suggested by @arjansingh
This commit is contained in:
@@ -36,6 +36,7 @@
|
|||||||
v-if="isVueNodesEnabled && comfyApp.canvas && comfyAppReady"
|
v-if="isVueNodesEnabled && comfyApp.canvas && comfyAppReady"
|
||||||
:canvas="comfyApp.canvas"
|
:canvas="comfyApp.canvas"
|
||||||
@transform-update="handleTransformUpdate"
|
@transform-update="handleTransformUpdate"
|
||||||
|
@wheel.capture="canvasInteractions.forwardEventToCanvas"
|
||||||
>
|
>
|
||||||
<!-- Vue nodes rendered based on graph nodes -->
|
<!-- Vue nodes rendered based on graph nodes -->
|
||||||
<VueGraphNode
|
<VueGraphNode
|
||||||
@@ -96,6 +97,7 @@ import NodeSearchboxPopover from '@/components/searchbox/NodeSearchBoxPopover.vu
|
|||||||
import SideToolbar from '@/components/sidebar/SideToolbar.vue'
|
import SideToolbar from '@/components/sidebar/SideToolbar.vue'
|
||||||
import SecondRowWorkflowTabs from '@/components/topbar/SecondRowWorkflowTabs.vue'
|
import SecondRowWorkflowTabs from '@/components/topbar/SecondRowWorkflowTabs.vue'
|
||||||
import { useChainCallback } from '@/composables/functional/useChainCallback'
|
import { useChainCallback } from '@/composables/functional/useChainCallback'
|
||||||
|
import { useCanvasInteractions } from '@/composables/graph/useCanvasInteractions'
|
||||||
import { useViewportCulling } from '@/composables/graph/useViewportCulling'
|
import { useViewportCulling } from '@/composables/graph/useViewportCulling'
|
||||||
import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
|
import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
|
||||||
import { useNodeBadge } from '@/composables/node/useNodeBadge'
|
import { useNodeBadge } from '@/composables/node/useNodeBadge'
|
||||||
@@ -147,6 +149,8 @@ const workspaceStore = useWorkspaceStore()
|
|||||||
const canvasStore = useCanvasStore()
|
const canvasStore = useCanvasStore()
|
||||||
const executionStore = useExecutionStore()
|
const executionStore = useExecutionStore()
|
||||||
const toastStore = useToastStore()
|
const toastStore = useToastStore()
|
||||||
|
const canvasInteractions = useCanvasInteractions()
|
||||||
|
|
||||||
const betaMenuEnabled = computed(
|
const betaMenuEnabled = computed(
|
||||||
() => settingStore.get('Comfy.UseNewMenu') !== 'Disabled'
|
() => settingStore.get('Comfy.UseNewMenu') !== 'Disabled'
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -24,14 +24,12 @@ export function useCanvasInteractions() {
|
|||||||
const handleWheel = (event: WheelEvent) => {
|
const handleWheel = (event: WheelEvent) => {
|
||||||
// In standard mode, Ctrl+wheel should go to canvas for zoom
|
// In standard mode, Ctrl+wheel should go to canvas for zoom
|
||||||
if (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) {
|
if (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) {
|
||||||
event.preventDefault() // Prevent browser zoom
|
|
||||||
forwardEventToCanvas(event)
|
forwardEventToCanvas(event)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// In legacy mode, all wheel events go to canvas for zoom
|
// In legacy mode, all wheel events go to canvas for zoom
|
||||||
if (!isStandardNavMode.value) {
|
if (!isStandardNavMode.value) {
|
||||||
event.preventDefault()
|
|
||||||
forwardEventToCanvas(event)
|
forwardEventToCanvas(event)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -68,9 +66,30 @@ export function useCanvasInteractions() {
|
|||||||
) => {
|
) => {
|
||||||
const canvasEl = app.canvas?.canvas
|
const canvasEl = app.canvas?.canvas
|
||||||
if (!canvasEl) return
|
if (!canvasEl) return
|
||||||
|
event.preventDefault()
|
||||||
|
event.stopPropagation()
|
||||||
|
|
||||||
|
if (event instanceof WheelEvent) {
|
||||||
|
const { clientX, clientY, deltaX, deltaY, ctrlKey, metaKey, shiftKey } =
|
||||||
|
event
|
||||||
|
canvasEl.dispatchEvent(
|
||||||
|
new WheelEvent('wheel', {
|
||||||
|
clientX,
|
||||||
|
clientY,
|
||||||
|
deltaX,
|
||||||
|
deltaY,
|
||||||
|
ctrlKey,
|
||||||
|
metaKey,
|
||||||
|
shiftKey
|
||||||
|
})
|
||||||
|
)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Create new event with same properties
|
// Create new event with same properties
|
||||||
const EventConstructor = event.constructor as typeof WheelEvent
|
const EventConstructor = event.constructor as
|
||||||
|
| typeof MouseEvent
|
||||||
|
| typeof PointerEvent
|
||||||
const newEvent = new EventConstructor(event.type, event)
|
const newEvent = new EventConstructor(event.type, event)
|
||||||
canvasEl.dispatchEvent(newEvent)
|
canvasEl.dispatchEvent(newEvent)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,12 +1,19 @@
|
|||||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||||
|
|
||||||
import { useCanvasInteractions } from '@/composables/graph/useCanvasInteractions'
|
import { useCanvasInteractions } from '@/composables/graph/useCanvasInteractions'
|
||||||
|
import type { LGraphCanvas } from '@/lib/litegraph/src/litegraph'
|
||||||
import { useCanvasStore } from '@/stores/graphStore'
|
import { useCanvasStore } from '@/stores/graphStore'
|
||||||
import { useSettingStore } from '@/stores/settingStore'
|
import { useSettingStore } from '@/stores/settingStore'
|
||||||
|
|
||||||
// Mock stores
|
// Mock stores
|
||||||
vi.mock('@/stores/graphStore')
|
vi.mock('@/stores/graphStore', () => {
|
||||||
vi.mock('@/stores/settingStore')
|
const getCanvas = vi.fn()
|
||||||
|
return { useCanvasStore: vi.fn(() => ({ getCanvas })) }
|
||||||
|
})
|
||||||
|
vi.mock('@/stores/settingStore', () => {
|
||||||
|
const getFn = vi.fn()
|
||||||
|
return { useSettingStore: vi.fn(() => ({ get: getFn })) }
|
||||||
|
})
|
||||||
vi.mock('@/scripts/app', () => ({
|
vi.mock('@/scripts/app', () => ({
|
||||||
app: {
|
app: {
|
||||||
canvas: {
|
canvas: {
|
||||||
@@ -17,105 +24,86 @@ vi.mock('@/scripts/app', () => ({
|
|||||||
}
|
}
|
||||||
}))
|
}))
|
||||||
|
|
||||||
|
function createMockLGraphCanvas(read_only = true): LGraphCanvas {
|
||||||
|
const mockCanvas: Partial<LGraphCanvas> = { read_only }
|
||||||
|
return mockCanvas as LGraphCanvas
|
||||||
|
}
|
||||||
|
|
||||||
|
function createMockPointerEvent(
|
||||||
|
buttons: PointerEvent['buttons'] = 1
|
||||||
|
): PointerEvent {
|
||||||
|
const mockEvent: Partial<PointerEvent> = {
|
||||||
|
buttons,
|
||||||
|
preventDefault: vi.fn(),
|
||||||
|
stopPropagation: vi.fn()
|
||||||
|
}
|
||||||
|
return mockEvent as PointerEvent
|
||||||
|
}
|
||||||
|
|
||||||
|
function createMockWheelEvent(ctrlKey = false, metaKey = false): WheelEvent {
|
||||||
|
const mockEvent: Partial<WheelEvent> = {
|
||||||
|
ctrlKey,
|
||||||
|
metaKey,
|
||||||
|
preventDefault: vi.fn(),
|
||||||
|
stopPropagation: vi.fn()
|
||||||
|
}
|
||||||
|
return mockEvent as WheelEvent
|
||||||
|
}
|
||||||
|
|
||||||
describe('useCanvasInteractions', () => {
|
describe('useCanvasInteractions', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.clearAllMocks()
|
vi.resetAllMocks()
|
||||||
vi.mocked(useCanvasStore, { partial: true }).mockReturnValue({
|
|
||||||
getCanvas: vi.fn()
|
|
||||||
})
|
|
||||||
vi.mocked(useSettingStore, { partial: true }).mockReturnValue({
|
|
||||||
get: vi.fn()
|
|
||||||
})
|
|
||||||
})
|
})
|
||||||
|
|
||||||
describe('handlePointer', () => {
|
describe('handlePointer', () => {
|
||||||
it('should forward space+drag events to canvas when read_only is true', () => {
|
it('should intercept left mouse events when canvas is read_only to enable space+drag navigation', () => {
|
||||||
// Setup
|
|
||||||
const mockCanvas = { read_only: true }
|
|
||||||
const { getCanvas } = useCanvasStore()
|
const { getCanvas } = useCanvasStore()
|
||||||
vi.mocked(getCanvas).mockReturnValue(mockCanvas as any)
|
const mockCanvas = createMockLGraphCanvas(true)
|
||||||
|
vi.mocked(getCanvas).mockReturnValue(mockCanvas)
|
||||||
|
|
||||||
const { handlePointer } = useCanvasInteractions()
|
const { handlePointer } = useCanvasInteractions()
|
||||||
|
|
||||||
// Create mock pointer event
|
const mockEvent = createMockPointerEvent(1) // Left Mouse Button
|
||||||
const mockEvent = {
|
handlePointer(mockEvent)
|
||||||
buttons: 1, // Left mouse button
|
|
||||||
preventDefault: vi.fn(),
|
|
||||||
stopPropagation: vi.fn()
|
|
||||||
} satisfies Partial<PointerEvent>
|
|
||||||
|
|
||||||
// Test
|
|
||||||
handlePointer(mockEvent as unknown as PointerEvent)
|
|
||||||
|
|
||||||
// Verify
|
|
||||||
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
||||||
expect(mockEvent.stopPropagation).toHaveBeenCalled()
|
expect(mockEvent.stopPropagation).toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should forward middle mouse button events to canvas', () => {
|
it('should forward middle mouse button events to canvas', () => {
|
||||||
// Setup
|
|
||||||
const mockCanvas = { read_only: false }
|
|
||||||
const { getCanvas } = useCanvasStore()
|
const { getCanvas } = useCanvasStore()
|
||||||
vi.mocked(getCanvas).mockReturnValue(mockCanvas as any)
|
const mockCanvas = createMockLGraphCanvas(false)
|
||||||
|
vi.mocked(getCanvas).mockReturnValue(mockCanvas)
|
||||||
const { handlePointer } = useCanvasInteractions()
|
const { handlePointer } = useCanvasInteractions()
|
||||||
|
|
||||||
// Create mock pointer event with middle button
|
const mockEvent = createMockPointerEvent(4) // Middle mouse button
|
||||||
const mockEvent = {
|
handlePointer(mockEvent)
|
||||||
buttons: 4, // Middle mouse button
|
|
||||||
preventDefault: vi.fn(),
|
|
||||||
stopPropagation: vi.fn()
|
|
||||||
} satisfies Partial<PointerEvent>
|
|
||||||
|
|
||||||
// Test
|
|
||||||
handlePointer(mockEvent as unknown as PointerEvent)
|
|
||||||
|
|
||||||
// Verify
|
|
||||||
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
||||||
expect(mockEvent.stopPropagation).toHaveBeenCalled()
|
expect(mockEvent.stopPropagation).toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should not prevent default when canvas is not in read_only mode and not middle button', () => {
|
it('should not prevent default when canvas is not in read_only mode and not middle button', () => {
|
||||||
// Setup
|
|
||||||
const mockCanvas = { read_only: false }
|
|
||||||
const { getCanvas } = useCanvasStore()
|
const { getCanvas } = useCanvasStore()
|
||||||
vi.mocked(getCanvas).mockReturnValue(mockCanvas as any)
|
const mockCanvas = createMockLGraphCanvas(false)
|
||||||
|
vi.mocked(getCanvas).mockReturnValue(mockCanvas)
|
||||||
const { handlePointer } = useCanvasInteractions()
|
const { handlePointer } = useCanvasInteractions()
|
||||||
|
|
||||||
// Create mock pointer event
|
const mockEvent = createMockPointerEvent(1)
|
||||||
const mockEvent = {
|
handlePointer(mockEvent)
|
||||||
buttons: 1, // Left mouse button
|
|
||||||
preventDefault: vi.fn(),
|
|
||||||
stopPropagation: vi.fn()
|
|
||||||
} satisfies Partial<PointerEvent>
|
|
||||||
|
|
||||||
// Test
|
|
||||||
handlePointer(mockEvent as unknown as PointerEvent)
|
|
||||||
|
|
||||||
// Verify - should not prevent default (let media handle normally)
|
|
||||||
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
|
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
|
||||||
expect(mockEvent.stopPropagation).not.toHaveBeenCalled()
|
expect(mockEvent.stopPropagation).not.toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should return early when canvas is null', () => {
|
it('should return early when canvas is null', () => {
|
||||||
// Setup
|
|
||||||
const { getCanvas } = useCanvasStore()
|
const { getCanvas } = useCanvasStore()
|
||||||
vi.mocked(getCanvas).mockReturnValue(null as any)
|
vi.mocked(getCanvas).mockReturnValue(null as unknown as LGraphCanvas) // TODO: Fix misaligned types
|
||||||
|
|
||||||
const { handlePointer } = useCanvasInteractions()
|
const { handlePointer } = useCanvasInteractions()
|
||||||
|
|
||||||
// Create mock pointer event that would normally trigger forwarding
|
const mockEvent = createMockPointerEvent(1)
|
||||||
const mockEvent = {
|
handlePointer(mockEvent)
|
||||||
buttons: 1, // Left mouse button - would trigger space+drag if canvas had read_only=true
|
|
||||||
preventDefault: vi.fn(),
|
|
||||||
stopPropagation: vi.fn()
|
|
||||||
} satisfies Partial<PointerEvent>
|
|
||||||
|
|
||||||
// Test
|
|
||||||
handlePointer(mockEvent as unknown as PointerEvent)
|
|
||||||
|
|
||||||
// Verify early return - no event methods should be called at all
|
|
||||||
expect(getCanvas).toHaveBeenCalled()
|
expect(getCanvas).toHaveBeenCalled()
|
||||||
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
|
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
|
||||||
expect(mockEvent.stopPropagation).not.toHaveBeenCalled()
|
expect(mockEvent.stopPropagation).not.toHaveBeenCalled()
|
||||||
@@ -124,66 +112,42 @@ describe('useCanvasInteractions', () => {
|
|||||||
|
|
||||||
describe('handleWheel', () => {
|
describe('handleWheel', () => {
|
||||||
it('should forward ctrl+wheel events to canvas in standard nav mode', () => {
|
it('should forward ctrl+wheel events to canvas in standard nav mode', () => {
|
||||||
// Setup
|
|
||||||
const { get } = useSettingStore()
|
const { get } = useSettingStore()
|
||||||
vi.mocked(get).mockReturnValue('standard')
|
vi.mocked(get).mockReturnValue('standard')
|
||||||
|
|
||||||
const { handleWheel } = useCanvasInteractions()
|
const { handleWheel } = useCanvasInteractions()
|
||||||
|
|
||||||
// Create mock wheel event with ctrl key
|
// Ctrl key pressed
|
||||||
const mockEvent = {
|
const mockEvent = createMockWheelEvent(true)
|
||||||
ctrlKey: true,
|
|
||||||
metaKey: false,
|
|
||||||
preventDefault: vi.fn()
|
|
||||||
} satisfies Partial<WheelEvent>
|
|
||||||
|
|
||||||
// Test
|
handleWheel(mockEvent)
|
||||||
handleWheel(mockEvent as unknown as WheelEvent)
|
|
||||||
|
|
||||||
// Verify
|
|
||||||
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
||||||
|
expect(mockEvent.stopPropagation).toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should forward all wheel events to canvas in legacy nav mode', () => {
|
it('should forward all wheel events to canvas in legacy nav mode', () => {
|
||||||
// Setup
|
|
||||||
const { get } = useSettingStore()
|
const { get } = useSettingStore()
|
||||||
vi.mocked(get).mockReturnValue('legacy')
|
vi.mocked(get).mockReturnValue('legacy')
|
||||||
|
|
||||||
const { handleWheel } = useCanvasInteractions()
|
const { handleWheel } = useCanvasInteractions()
|
||||||
|
|
||||||
// Create mock wheel event without modifiers
|
const mockEvent = createMockWheelEvent()
|
||||||
const mockEvent = {
|
handleWheel(mockEvent)
|
||||||
ctrlKey: false,
|
|
||||||
metaKey: false,
|
|
||||||
preventDefault: vi.fn()
|
|
||||||
} satisfies Partial<WheelEvent>
|
|
||||||
|
|
||||||
// Test
|
|
||||||
handleWheel(mockEvent as unknown as WheelEvent)
|
|
||||||
|
|
||||||
// Verify
|
|
||||||
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
||||||
|
expect(mockEvent.stopPropagation).toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should not prevent default for regular wheel events in standard nav mode', () => {
|
it('should not prevent default for regular wheel events in standard nav mode', () => {
|
||||||
// Setup
|
|
||||||
const { get } = useSettingStore()
|
const { get } = useSettingStore()
|
||||||
vi.mocked(get).mockReturnValue('standard')
|
vi.mocked(get).mockReturnValue('standard')
|
||||||
|
|
||||||
const { handleWheel } = useCanvasInteractions()
|
const { handleWheel } = useCanvasInteractions()
|
||||||
|
|
||||||
// Create mock wheel event without modifiers
|
const mockEvent = createMockWheelEvent()
|
||||||
const mockEvent = {
|
handleWheel(mockEvent)
|
||||||
ctrlKey: false,
|
|
||||||
metaKey: false,
|
|
||||||
preventDefault: vi.fn()
|
|
||||||
} satisfies Partial<WheelEvent>
|
|
||||||
|
|
||||||
// Test
|
|
||||||
handleWheel(mockEvent as unknown as WheelEvent)
|
|
||||||
|
|
||||||
// Verify - should not prevent default (let component handle normally)
|
|
||||||
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
|
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
|
||||||
|
expect(mockEvent.stopPropagation).not.toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user