Skip to content

Commit 7245213

Browse files
authored
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
1 parent b72e22f commit 7245213

File tree

3 files changed

+88
-101
lines changed

3 files changed

+88
-101
lines changed

src/components/graph/GraphCanvas.vue

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
v-if="isVueNodesEnabled && comfyApp.canvas && comfyAppReady"
3737
:canvas="comfyApp.canvas"
3838
@transform-update="handleTransformUpdate"
39+
@wheel.capture="canvasInteractions.forwardEventToCanvas"
3940
>
4041
<!-- Vue nodes rendered based on graph nodes -->
4142
<VueGraphNode
@@ -96,6 +97,7 @@ import NodeSearchboxPopover from '@/components/searchbox/NodeSearchBoxPopover.vu
9697
import SideToolbar from '@/components/sidebar/SideToolbar.vue'
9798
import SecondRowWorkflowTabs from '@/components/topbar/SecondRowWorkflowTabs.vue'
9899
import { useChainCallback } from '@/composables/functional/useChainCallback'
100+
import { useCanvasInteractions } from '@/composables/graph/useCanvasInteractions'
99101
import { useViewportCulling } from '@/composables/graph/useViewportCulling'
100102
import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
101103
import { useNodeBadge } from '@/composables/node/useNodeBadge'
@@ -147,6 +149,8 @@ const workspaceStore = useWorkspaceStore()
147149
const canvasStore = useCanvasStore()
148150
const executionStore = useExecutionStore()
149151
const toastStore = useToastStore()
152+
const canvasInteractions = useCanvasInteractions()
153+
150154
const betaMenuEnabled = computed(
151155
() => settingStore.get('Comfy.UseNewMenu') !== 'Disabled'
152156
)

src/composables/graph/useCanvasInteractions.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,12 @@ export function useCanvasInteractions() {
2424
const handleWheel = (event: WheelEvent) => {
2525
// In standard mode, Ctrl+wheel should go to canvas for zoom
2626
if (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) {
27-
event.preventDefault() // Prevent browser zoom
2827
forwardEventToCanvas(event)
2928
return
3029
}
3130

3231
// In legacy mode, all wheel events go to canvas for zoom
3332
if (!isStandardNavMode.value) {
34-
event.preventDefault()
3533
forwardEventToCanvas(event)
3634
return
3735
}
@@ -68,9 +66,30 @@ export function useCanvasInteractions() {
6866
) => {
6967
const canvasEl = app.canvas?.canvas
7068
if (!canvasEl) return
69+
event.preventDefault()
70+
event.stopPropagation()
71+
72+
if (event instanceof WheelEvent) {
73+
const { clientX, clientY, deltaX, deltaY, ctrlKey, metaKey, shiftKey } =
74+
event
75+
canvasEl.dispatchEvent(
76+
new WheelEvent('wheel', {
77+
clientX,
78+
clientY,
79+
deltaX,
80+
deltaY,
81+
ctrlKey,
82+
metaKey,
83+
shiftKey
84+
})
85+
)
86+
return
87+
}
7188

7289
// Create new event with same properties
73-
const EventConstructor = event.constructor as typeof WheelEvent
90+
const EventConstructor = event.constructor as
91+
| typeof MouseEvent
92+
| typeof PointerEvent
7493
const newEvent = new EventConstructor(event.type, event)
7594
canvasEl.dispatchEvent(newEvent)
7695
}
Lines changed: 62 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
import { beforeEach, describe, expect, it, vi } from 'vitest'
22

33
import { useCanvasInteractions } from '@/composables/graph/useCanvasInteractions'
4+
import type { LGraphCanvas } from '@/lib/litegraph/src/litegraph'
45
import { useCanvasStore } from '@/stores/graphStore'
56
import { useSettingStore } from '@/stores/settingStore'
67

78
// Mock stores
8-
vi.mock('@/stores/graphStore')
9-
vi.mock('@/stores/settingStore')
9+
vi.mock('@/stores/graphStore', () => {
10+
const getCanvas = vi.fn()
11+
return { useCanvasStore: vi.fn(() => ({ getCanvas })) }
12+
})
13+
vi.mock('@/stores/settingStore', () => {
14+
const getFn = vi.fn()
15+
return { useSettingStore: vi.fn(() => ({ get: getFn })) }
16+
})
1017
vi.mock('@/scripts/app', () => ({
1118
app: {
1219
canvas: {
@@ -17,105 +24,86 @@ vi.mock('@/scripts/app', () => ({
1724
}
1825
}))
1926

27+
function createMockLGraphCanvas(read_only = true): LGraphCanvas {
28+
const mockCanvas: Partial<LGraphCanvas> = { read_only }
29+
return mockCanvas as LGraphCanvas
30+
}
31+
32+
function createMockPointerEvent(
33+
buttons: PointerEvent['buttons'] = 1
34+
): PointerEvent {
35+
const mockEvent: Partial<PointerEvent> = {
36+
buttons,
37+
preventDefault: vi.fn(),
38+
stopPropagation: vi.fn()
39+
}
40+
return mockEvent as PointerEvent
41+
}
42+
43+
function createMockWheelEvent(ctrlKey = false, metaKey = false): WheelEvent {
44+
const mockEvent: Partial<WheelEvent> = {
45+
ctrlKey,
46+
metaKey,
47+
preventDefault: vi.fn(),
48+
stopPropagation: vi.fn()
49+
}
50+
return mockEvent as WheelEvent
51+
}
52+
2053
describe('useCanvasInteractions', () => {
2154
beforeEach(() => {
22-
vi.clearAllMocks()
23-
vi.mocked(useCanvasStore, { partial: true }).mockReturnValue({
24-
getCanvas: vi.fn()
25-
})
26-
vi.mocked(useSettingStore, { partial: true }).mockReturnValue({
27-
get: vi.fn()
28-
})
55+
vi.resetAllMocks()
2956
})
3057

3158
describe('handlePointer', () => {
32-
it('should forward space+drag events to canvas when read_only is true', () => {
33-
// Setup
34-
const mockCanvas = { read_only: true }
59+
it('should intercept left mouse events when canvas is read_only to enable space+drag navigation', () => {
3560
const { getCanvas } = useCanvasStore()
36-
vi.mocked(getCanvas).mockReturnValue(mockCanvas as any)
61+
const mockCanvas = createMockLGraphCanvas(true)
62+
vi.mocked(getCanvas).mockReturnValue(mockCanvas)
3763

3864
const { handlePointer } = useCanvasInteractions()
3965

40-
// Create mock pointer event
41-
const mockEvent = {
42-
buttons: 1, // Left mouse button
43-
preventDefault: vi.fn(),
44-
stopPropagation: vi.fn()
45-
} satisfies Partial<PointerEvent>
46-
47-
// Test
48-
handlePointer(mockEvent as unknown as PointerEvent)
66+
const mockEvent = createMockPointerEvent(1) // Left Mouse Button
67+
handlePointer(mockEvent)
4968

50-
// Verify
5169
expect(mockEvent.preventDefault).toHaveBeenCalled()
5270
expect(mockEvent.stopPropagation).toHaveBeenCalled()
5371
})
5472

5573
it('should forward middle mouse button events to canvas', () => {
56-
// Setup
57-
const mockCanvas = { read_only: false }
5874
const { getCanvas } = useCanvasStore()
59-
vi.mocked(getCanvas).mockReturnValue(mockCanvas as any)
60-
75+
const mockCanvas = createMockLGraphCanvas(false)
76+
vi.mocked(getCanvas).mockReturnValue(mockCanvas)
6177
const { handlePointer } = useCanvasInteractions()
6278

63-
// Create mock pointer event with middle button
64-
const mockEvent = {
65-
buttons: 4, // Middle mouse button
66-
preventDefault: vi.fn(),
67-
stopPropagation: vi.fn()
68-
} satisfies Partial<PointerEvent>
79+
const mockEvent = createMockPointerEvent(4) // Middle mouse button
80+
handlePointer(mockEvent)
6981

70-
// Test
71-
handlePointer(mockEvent as unknown as PointerEvent)
72-
73-
// Verify
7482
expect(mockEvent.preventDefault).toHaveBeenCalled()
7583
expect(mockEvent.stopPropagation).toHaveBeenCalled()
7684
})
7785

7886
it('should not prevent default when canvas is not in read_only mode and not middle button', () => {
79-
// Setup
80-
const mockCanvas = { read_only: false }
8187
const { getCanvas } = useCanvasStore()
82-
vi.mocked(getCanvas).mockReturnValue(mockCanvas as any)
83-
88+
const mockCanvas = createMockLGraphCanvas(false)
89+
vi.mocked(getCanvas).mockReturnValue(mockCanvas)
8490
const { handlePointer } = useCanvasInteractions()
8591

86-
// Create mock pointer event
87-
const mockEvent = {
88-
buttons: 1, // Left mouse button
89-
preventDefault: vi.fn(),
90-
stopPropagation: vi.fn()
91-
} satisfies Partial<PointerEvent>
92+
const mockEvent = createMockPointerEvent(1)
93+
handlePointer(mockEvent)
9294

93-
// Test
94-
handlePointer(mockEvent as unknown as PointerEvent)
95-
96-
// Verify - should not prevent default (let media handle normally)
9795
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
9896
expect(mockEvent.stopPropagation).not.toHaveBeenCalled()
9997
})
10098

10199
it('should return early when canvas is null', () => {
102-
// Setup
103100
const { getCanvas } = useCanvasStore()
104-
vi.mocked(getCanvas).mockReturnValue(null as any)
105-
101+
vi.mocked(getCanvas).mockReturnValue(null as unknown as LGraphCanvas) // TODO: Fix misaligned types
106102
const { handlePointer } = useCanvasInteractions()
107103

108-
// Create mock pointer event that would normally trigger forwarding
109-
const mockEvent = {
110-
buttons: 1, // Left mouse button - would trigger space+drag if canvas had read_only=true
111-
preventDefault: vi.fn(),
112-
stopPropagation: vi.fn()
113-
} satisfies Partial<PointerEvent>
114-
115-
// Test
116-
handlePointer(mockEvent as unknown as PointerEvent)
104+
const mockEvent = createMockPointerEvent(1)
105+
handlePointer(mockEvent)
117106

118-
// Verify early return - no event methods should be called at all
119107
expect(getCanvas).toHaveBeenCalled()
120108
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
121109
expect(mockEvent.stopPropagation).not.toHaveBeenCalled()
@@ -124,66 +112,42 @@ describe('useCanvasInteractions', () => {
124112

125113
describe('handleWheel', () => {
126114
it('should forward ctrl+wheel events to canvas in standard nav mode', () => {
127-
// Setup
128115
const { get } = useSettingStore()
129116
vi.mocked(get).mockReturnValue('standard')
130117

131118
const { handleWheel } = useCanvasInteractions()
132119

133-
// Create mock wheel event with ctrl key
134-
const mockEvent = {
135-
ctrlKey: true,
136-
metaKey: false,
137-
preventDefault: vi.fn()
138-
} satisfies Partial<WheelEvent>
120+
// Ctrl key pressed
121+
const mockEvent = createMockWheelEvent(true)
139122

140-
// Test
141-
handleWheel(mockEvent as unknown as WheelEvent)
123+
handleWheel(mockEvent)
142124

143-
// Verify
144125
expect(mockEvent.preventDefault).toHaveBeenCalled()
126+
expect(mockEvent.stopPropagation).toHaveBeenCalled()
145127
})
146128

147129
it('should forward all wheel events to canvas in legacy nav mode', () => {
148-
// Setup
149130
const { get } = useSettingStore()
150131
vi.mocked(get).mockReturnValue('legacy')
151-
152132
const { handleWheel } = useCanvasInteractions()
153133

154-
// Create mock wheel event without modifiers
155-
const mockEvent = {
156-
ctrlKey: false,
157-
metaKey: false,
158-
preventDefault: vi.fn()
159-
} satisfies Partial<WheelEvent>
134+
const mockEvent = createMockWheelEvent()
135+
handleWheel(mockEvent)
160136

161-
// Test
162-
handleWheel(mockEvent as unknown as WheelEvent)
163-
164-
// Verify
165137
expect(mockEvent.preventDefault).toHaveBeenCalled()
138+
expect(mockEvent.stopPropagation).toHaveBeenCalled()
166139
})
167140

168141
it('should not prevent default for regular wheel events in standard nav mode', () => {
169-
// Setup
170142
const { get } = useSettingStore()
171143
vi.mocked(get).mockReturnValue('standard')
172-
173144
const { handleWheel } = useCanvasInteractions()
174145

175-
// Create mock wheel event without modifiers
176-
const mockEvent = {
177-
ctrlKey: false,
178-
metaKey: false,
179-
preventDefault: vi.fn()
180-
} satisfies Partial<WheelEvent>
146+
const mockEvent = createMockWheelEvent()
147+
handleWheel(mockEvent)
181148

182-
// Test
183-
handleWheel(mockEvent as unknown as WheelEvent)
184-
185-
// Verify - should not prevent default (let component handle normally)
186149
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
150+
expect(mockEvent.stopPropagation).not.toHaveBeenCalled()
187151
})
188152
})
189153
})

0 commit comments

Comments
 (0)