mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 19:09:52 +00:00
[fix] Enable mouse gestures over video previews (#5279)
* [fix] Enable mouse gestures over video previews - fixes ctrl+scroll zoom and space+drag panning 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * [improve] Enhance code clarity in media gesture handling with descriptive constants * [clean] Remove dangling comment in handlePointer method * [improve] Use satisfies and partial mocking for better type safety in tests - Replace 'as any' with vi.mocked({partial: true}) for store mocks - Use 'satisfies Partial<Event>' for better event type checking - Remove 'not.toThrow' test as it tests default behavior - Improve test readability and type safety per review feedback * [improve] Test actual canvas existence check instead of side effects - Replace vague 'graceful handling' test with specific 'early return' test - Verify that getCanvas() is actually called to check canvas existence - Test early return behavior when canvas is null rather than just preventDefault side effect - More robust test that validates the intended logic flow * [refactor] Use localized mocking instead of global mock functions - Replace global mockGetCanvas and mockGet with in-situ vi.mocked() calls - Extract store functions directly in test cases for better locality - Follow DrJKL's suggestion for cleaner test structure - Maintains same test coverage with improved readability --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { computed } from 'vue'
|
||||
|
||||
import { app } from '@/scripts/app'
|
||||
import { useCanvasStore } from '@/stores/graphStore'
|
||||
import { useSettingStore } from '@/stores/settingStore'
|
||||
|
||||
/**
|
||||
@@ -10,6 +11,7 @@ import { useSettingStore } from '@/stores/settingStore'
|
||||
*/
|
||||
export function useCanvasInteractions() {
|
||||
const settingStore = useSettingStore()
|
||||
const { getCanvas } = useCanvasStore()
|
||||
|
||||
const isStandardNavMode = computed(
|
||||
() => settingStore.get('Comfy.Canvas.NavigationMode') === 'standard'
|
||||
@@ -37,6 +39,27 @@ export function useCanvasInteractions() {
|
||||
// Otherwise, let the component handle it normally
|
||||
}
|
||||
|
||||
/**
|
||||
* Handles pointer events from media elements that should potentially
|
||||
* be forwarded to canvas (e.g., space+drag for panning)
|
||||
*/
|
||||
const handlePointer = (event: PointerEvent) => {
|
||||
// Check if canvas exists using established pattern
|
||||
const canvas = getCanvas()
|
||||
if (!canvas) return
|
||||
|
||||
// Check conditions for forwarding events to canvas
|
||||
const isSpacePanningDrag = canvas.read_only && event.buttons === 1 // Space key pressed + left mouse drag
|
||||
const isMiddleMousePanning = event.buttons === 4 // Middle mouse button for panning
|
||||
|
||||
if (isSpacePanningDrag || isMiddleMousePanning) {
|
||||
event.preventDefault()
|
||||
event.stopPropagation()
|
||||
forwardEventToCanvas(event)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Forwards an event to the LiteGraph canvas
|
||||
*/
|
||||
@@ -54,6 +77,7 @@ export function useCanvasInteractions() {
|
||||
|
||||
return {
|
||||
handleWheel,
|
||||
handlePointer,
|
||||
forwardEventToCanvas
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { useCanvasInteractions } from '@/composables/graph/useCanvasInteractions'
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
import { fitDimensionsToNodeWidth } from '@/utils/imageUtil'
|
||||
@@ -130,6 +131,8 @@ export const useNodeVideo = (node: LGraphNode) => {
|
||||
let minHeight = DEFAULT_VIDEO_SIZE
|
||||
let minWidth = DEFAULT_VIDEO_SIZE
|
||||
|
||||
const { handleWheel, handlePointer } = useCanvasInteractions()
|
||||
|
||||
const setMinDimensions = (video: HTMLVideoElement) => {
|
||||
const { minHeight: calculatedHeight, minWidth: calculatedWidth } =
|
||||
fitDimensionsToNodeWidth(
|
||||
@@ -146,6 +149,12 @@ export const useNodeVideo = (node: LGraphNode) => {
|
||||
new Promise((resolve) => {
|
||||
const video = document.createElement('video')
|
||||
Object.assign(video, VIDEO_DEFAULT_OPTIONS)
|
||||
|
||||
// Add event listeners for canvas interactions
|
||||
video.addEventListener('wheel', handleWheel)
|
||||
video.addEventListener('pointermove', handlePointer)
|
||||
video.addEventListener('pointerdown', handlePointer)
|
||||
|
||||
video.onloadeddata = () => {
|
||||
setMinDimensions(video)
|
||||
resolve(video)
|
||||
|
||||
@@ -1,126 +1,189 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { useCanvasInteractions } from '@/composables/graph/useCanvasInteractions'
|
||||
import { app } from '@/scripts/app'
|
||||
import * as settingStore from '@/stores/settingStore'
|
||||
import { useCanvasStore } from '@/stores/graphStore'
|
||||
import { useSettingStore } from '@/stores/settingStore'
|
||||
|
||||
// Mock the app and canvas
|
||||
// Mock stores
|
||||
vi.mock('@/stores/graphStore')
|
||||
vi.mock('@/stores/settingStore')
|
||||
vi.mock('@/scripts/app', () => ({
|
||||
app: {
|
||||
canvas: {
|
||||
canvas: null as HTMLCanvasElement | null
|
||||
canvas: {
|
||||
dispatchEvent: vi.fn()
|
||||
}
|
||||
}
|
||||
}
|
||||
}))
|
||||
|
||||
// Mock the setting store
|
||||
vi.mock('@/stores/settingStore', () => ({
|
||||
useSettingStore: vi.fn()
|
||||
}))
|
||||
|
||||
describe('useCanvasInteractions', () => {
|
||||
let mockCanvas: HTMLCanvasElement
|
||||
let mockSettingStore: { get: ReturnType<typeof vi.fn> }
|
||||
let canvasInteractions: ReturnType<typeof useCanvasInteractions>
|
||||
|
||||
beforeEach(() => {
|
||||
// Clear mocks
|
||||
vi.clearAllMocks()
|
||||
vi.mocked(useCanvasStore, { partial: true }).mockReturnValue({
|
||||
getCanvas: vi.fn()
|
||||
})
|
||||
vi.mocked(useSettingStore, { partial: true }).mockReturnValue({
|
||||
get: vi.fn()
|
||||
})
|
||||
})
|
||||
|
||||
// Create mock canvas element
|
||||
mockCanvas = document.createElement('canvas')
|
||||
mockCanvas.dispatchEvent = vi.fn()
|
||||
app.canvas!.canvas = mockCanvas
|
||||
describe('handlePointer', () => {
|
||||
it('should forward space+drag events to canvas when read_only is true', () => {
|
||||
// Setup
|
||||
const mockCanvas = { read_only: true }
|
||||
const { getCanvas } = useCanvasStore()
|
||||
vi.mocked(getCanvas).mockReturnValue(mockCanvas as any)
|
||||
|
||||
// Mock setting store
|
||||
mockSettingStore = { get: vi.fn() }
|
||||
vi.mocked(settingStore.useSettingStore).mockReturnValue(
|
||||
mockSettingStore as any
|
||||
)
|
||||
const { handlePointer } = useCanvasInteractions()
|
||||
|
||||
canvasInteractions = useCanvasInteractions()
|
||||
// Create mock pointer event
|
||||
const 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.stopPropagation).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should forward middle mouse button events to canvas', () => {
|
||||
// Setup
|
||||
const mockCanvas = { read_only: false }
|
||||
const { getCanvas } = useCanvasStore()
|
||||
vi.mocked(getCanvas).mockReturnValue(mockCanvas as any)
|
||||
|
||||
const { handlePointer } = useCanvasInteractions()
|
||||
|
||||
// Create mock pointer event with middle button
|
||||
const 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.stopPropagation).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
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()
|
||||
vi.mocked(getCanvas).mockReturnValue(mockCanvas as any)
|
||||
|
||||
const { handlePointer } = useCanvasInteractions()
|
||||
|
||||
// Create mock pointer event
|
||||
const 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.stopPropagation).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should return early when canvas is null', () => {
|
||||
// Setup
|
||||
const { getCanvas } = useCanvasStore()
|
||||
vi.mocked(getCanvas).mockReturnValue(null as any)
|
||||
|
||||
const { handlePointer } = useCanvasInteractions()
|
||||
|
||||
// Create mock pointer event that would normally trigger forwarding
|
||||
const 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(mockEvent.preventDefault).not.toHaveBeenCalled()
|
||||
expect(mockEvent.stopPropagation).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('handleWheel', () => {
|
||||
it('should check navigation mode from settings', () => {
|
||||
mockSettingStore.get.mockReturnValue('standard')
|
||||
it('should forward ctrl+wheel events to canvas in standard nav mode', () => {
|
||||
// Setup
|
||||
const { get } = useSettingStore()
|
||||
vi.mocked(get).mockReturnValue('standard')
|
||||
|
||||
const wheelEvent = new WheelEvent('wheel', {
|
||||
const { handleWheel } = useCanvasInteractions()
|
||||
|
||||
// Create mock wheel event with ctrl key
|
||||
const mockEvent = {
|
||||
ctrlKey: true,
|
||||
deltaY: -100
|
||||
})
|
||||
metaKey: false,
|
||||
preventDefault: vi.fn()
|
||||
} satisfies Partial<WheelEvent>
|
||||
|
||||
canvasInteractions.handleWheel(wheelEvent)
|
||||
// Test
|
||||
handleWheel(mockEvent as unknown as WheelEvent)
|
||||
|
||||
expect(mockSettingStore.get).toHaveBeenCalledWith(
|
||||
'Comfy.Canvas.NavigationMode'
|
||||
)
|
||||
// Verify
|
||||
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should not forward regular wheel events in standard mode', () => {
|
||||
mockSettingStore.get.mockReturnValue('standard')
|
||||
it('should forward all wheel events to canvas in legacy nav mode', () => {
|
||||
// Setup
|
||||
const { get } = useSettingStore()
|
||||
vi.mocked(get).mockReturnValue('legacy')
|
||||
|
||||
const wheelEvent = new WheelEvent('wheel', {
|
||||
deltaY: -100
|
||||
})
|
||||
const { handleWheel } = useCanvasInteractions()
|
||||
|
||||
canvasInteractions.handleWheel(wheelEvent)
|
||||
// Create mock wheel event without modifiers
|
||||
const mockEvent = {
|
||||
ctrlKey: false,
|
||||
metaKey: false,
|
||||
preventDefault: vi.fn()
|
||||
} satisfies Partial<WheelEvent>
|
||||
|
||||
expect(mockCanvas.dispatchEvent).not.toHaveBeenCalled()
|
||||
// Test
|
||||
handleWheel(mockEvent as unknown as WheelEvent)
|
||||
|
||||
// Verify
|
||||
expect(mockEvent.preventDefault).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should forward all wheel events to canvas in legacy mode', () => {
|
||||
mockSettingStore.get.mockReturnValue('legacy')
|
||||
it('should not prevent default for regular wheel events in standard nav mode', () => {
|
||||
// Setup
|
||||
const { get } = useSettingStore()
|
||||
vi.mocked(get).mockReturnValue('standard')
|
||||
|
||||
const wheelEvent = new WheelEvent('wheel', {
|
||||
deltaY: -100,
|
||||
cancelable: true
|
||||
})
|
||||
const { handleWheel } = useCanvasInteractions()
|
||||
|
||||
canvasInteractions.handleWheel(wheelEvent)
|
||||
// Create mock wheel event without modifiers
|
||||
const mockEvent = {
|
||||
ctrlKey: false,
|
||||
metaKey: false,
|
||||
preventDefault: vi.fn()
|
||||
} satisfies Partial<WheelEvent>
|
||||
|
||||
expect(mockCanvas.dispatchEvent).toHaveBeenCalled()
|
||||
})
|
||||
// Test
|
||||
handleWheel(mockEvent as unknown as WheelEvent)
|
||||
|
||||
it('should handle missing canvas gracefully', () => {
|
||||
;(app.canvas as any).canvas = null
|
||||
mockSettingStore.get.mockReturnValue('standard')
|
||||
|
||||
const wheelEvent = new WheelEvent('wheel', {
|
||||
ctrlKey: true,
|
||||
deltaY: -100
|
||||
})
|
||||
|
||||
expect(() => {
|
||||
canvasInteractions.handleWheel(wheelEvent)
|
||||
}).not.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
describe('forwardEventToCanvas', () => {
|
||||
it('should dispatch event to canvas element', () => {
|
||||
const wheelEvent = new WheelEvent('wheel', {
|
||||
deltaY: -100,
|
||||
ctrlKey: true
|
||||
})
|
||||
|
||||
canvasInteractions.forwardEventToCanvas(wheelEvent)
|
||||
|
||||
expect(mockCanvas.dispatchEvent).toHaveBeenCalledWith(
|
||||
expect.any(WheelEvent)
|
||||
)
|
||||
})
|
||||
|
||||
it('should handle missing canvas gracefully', () => {
|
||||
;(app.canvas as any).canvas = null
|
||||
|
||||
const wheelEvent = new WheelEvent('wheel', {
|
||||
deltaY: -100
|
||||
})
|
||||
|
||||
expect(() => {
|
||||
canvasInteractions.forwardEventToCanvas(wheelEvent)
|
||||
}).not.toThrow()
|
||||
// Verify - should not prevent default (let component handle normally)
|
||||
expect(mockEvent.preventDefault).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user