mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-30 03:01:54 +00:00
refactor: address review comments - use useMagicKeys and derive button state
- Refactored spacebar forwarding to use useMagicKeys from VueUse instead of raw event listeners - Added filtering for editable elements (input, textarea, contentEditable) - Changed panning check in useSlotLinkInteraction to derive button state from event.buttons instead of storing canvas.pointer.isDown statefully - Updated tests to work with the new useMagicKeys approach using vi.hoisted pattern
This commit is contained in:
@@ -17,7 +17,26 @@ const mockCanvas = vi.hoisted(() => {
|
|||||||
const canvasElement = document.createElement('canvas')
|
const canvasElement = document.createElement('canvas')
|
||||||
return {
|
return {
|
||||||
canvas: canvasElement,
|
canvas: canvasElement,
|
||||||
processKey: vi.fn()
|
read_only: false,
|
||||||
|
dragging_canvas: false,
|
||||||
|
pointer: { isDown: false }
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
// Mock useMagicKeys and useActiveElement from VueUse
|
||||||
|
// Use vi.hoisted to store refs in an object that's available during mock hoisting
|
||||||
|
const vueUseMocks = vi.hoisted(() => ({
|
||||||
|
spaceKey: null as { value: boolean } | null,
|
||||||
|
activeElement: null as { value: Element | null } | null
|
||||||
|
}))
|
||||||
|
|
||||||
|
vi.mock('@vueuse/core', async () => {
|
||||||
|
const { ref: vueRef } = await import('vue')
|
||||||
|
vueUseMocks.spaceKey = vueRef(false)
|
||||||
|
vueUseMocks.activeElement = vueRef<Element | null>(null)
|
||||||
|
return {
|
||||||
|
useMagicKeys: () => ({ space: vueUseMocks.spaceKey }),
|
||||||
|
useActiveElement: () => vueUseMocks.activeElement
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -342,41 +361,79 @@ describe('useNodePointerInteractions', () => {
|
|||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|
||||||
describe('keydown forwarding for spacebar panning', () => {
|
describe('spacebar panning via useMagicKeys', () => {
|
||||||
it('forwards keydown events to canvas.processKey when target is not the canvas', () => {
|
beforeEach(() => {
|
||||||
// Initialize the composable to set up keydown forwarding
|
mockCanvas.read_only = false
|
||||||
useNodePointerInteractions('test-node-123')
|
mockCanvas.dragging_canvas = false
|
||||||
|
vueUseMocks.spaceKey!.value = false
|
||||||
// Create a div to simulate a Vue node element
|
vueUseMocks.activeElement!.value = null
|
||||||
const vueNodeElement = document.createElement('div')
|
|
||||||
document.body.appendChild(vueNodeElement)
|
|
||||||
|
|
||||||
// Dispatch keydown event on the Vue node element (not the canvas)
|
|
||||||
const keydownEvent = new KeyboardEvent('keydown', {
|
|
||||||
key: ' ',
|
|
||||||
bubbles: true
|
|
||||||
})
|
|
||||||
vueNodeElement.dispatchEvent(keydownEvent)
|
|
||||||
|
|
||||||
// Should forward to canvas.processKey
|
|
||||||
expect(mockCanvas.processKey).toHaveBeenCalledWith(keydownEvent)
|
|
||||||
|
|
||||||
document.body.removeChild(vueNodeElement)
|
|
||||||
})
|
})
|
||||||
|
|
||||||
it('does not forward keydown events when target is the canvas itself', () => {
|
it('sets read_only=true when spacebar is pressed on non-canvas element', async () => {
|
||||||
|
const vueNodeElement = document.createElement('div')
|
||||||
|
vueUseMocks.activeElement!.value = vueNodeElement
|
||||||
|
|
||||||
useNodePointerInteractions('test-node-123')
|
useNodePointerInteractions('test-node-123')
|
||||||
mockCanvas.processKey.mockClear()
|
|
||||||
|
|
||||||
// Dispatch keydown event directly on the canvas element
|
// Simulate spacebar press
|
||||||
const keydownEvent = new KeyboardEvent('keydown', {
|
vueUseMocks.spaceKey!.value = true
|
||||||
key: ' ',
|
await nextTick()
|
||||||
bubbles: true
|
|
||||||
})
|
|
||||||
mockCanvas.canvas.dispatchEvent(keydownEvent)
|
|
||||||
|
|
||||||
// Should NOT forward (canvas handles it directly)
|
expect(mockCanvas.read_only).toBe(true)
|
||||||
expect(mockCanvas.processKey).not.toHaveBeenCalled()
|
})
|
||||||
|
|
||||||
|
it('resets read_only=false when spacebar is released', async () => {
|
||||||
|
const vueNodeElement = document.createElement('div')
|
||||||
|
vueUseMocks.activeElement!.value = vueNodeElement
|
||||||
|
|
||||||
|
useNodePointerInteractions('test-node-123')
|
||||||
|
|
||||||
|
// Press and release spacebar
|
||||||
|
vueUseMocks.spaceKey!.value = true
|
||||||
|
await nextTick()
|
||||||
|
vueUseMocks.spaceKey!.value = false
|
||||||
|
await nextTick()
|
||||||
|
|
||||||
|
expect(mockCanvas.read_only).toBe(false)
|
||||||
|
expect(mockCanvas.dragging_canvas).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('does not set read_only when canvas has focus', async () => {
|
||||||
|
vueUseMocks.activeElement!.value = mockCanvas.canvas
|
||||||
|
|
||||||
|
useNodePointerInteractions('test-node-123')
|
||||||
|
|
||||||
|
vueUseMocks.spaceKey!.value = true
|
||||||
|
await nextTick()
|
||||||
|
|
||||||
|
// Should NOT change read_only (litegraph handles it directly)
|
||||||
|
expect(mockCanvas.read_only).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('does not set read_only when input element has focus', async () => {
|
||||||
|
const inputElement = document.createElement('input')
|
||||||
|
vueUseMocks.activeElement!.value = inputElement
|
||||||
|
|
||||||
|
useNodePointerInteractions('test-node-123')
|
||||||
|
|
||||||
|
vueUseMocks.spaceKey!.value = true
|
||||||
|
await nextTick()
|
||||||
|
|
||||||
|
// Should NOT change read_only (avoid interfering with text input)
|
||||||
|
expect(mockCanvas.read_only).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('does not set read_only when textarea element has focus', async () => {
|
||||||
|
const textareaElement = document.createElement('textarea')
|
||||||
|
vueUseMocks.activeElement!.value = textareaElement
|
||||||
|
|
||||||
|
useNodePointerInteractions('test-node-123')
|
||||||
|
|
||||||
|
vueUseMocks.spaceKey!.value = true
|
||||||
|
await nextTick()
|
||||||
|
|
||||||
|
// Should NOT change read_only (avoid interfering with text input)
|
||||||
|
expect(mockCanvas.read_only).toBe(false)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
import { onScopeDispose, ref, toValue } from 'vue'
|
import { onScopeDispose, ref, toValue, watch } from 'vue'
|
||||||
import type { MaybeRefOrGetter } from 'vue'
|
import type { MaybeRefOrGetter } from 'vue'
|
||||||
|
|
||||||
|
import { useActiveElement, useMagicKeys } from '@vueuse/core'
|
||||||
|
|
||||||
import { isMiddlePointerInput } from '@/base/pointerUtils'
|
import { isMiddlePointerInput } from '@/base/pointerUtils'
|
||||||
import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
|
import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
|
||||||
import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
|
import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
|
||||||
@@ -10,29 +12,51 @@ import { useNodeDrag } from '@/renderer/extensions/vueNodes/layout/useNodeDrag'
|
|||||||
import { isMultiSelectKey } from '@/renderer/extensions/vueNodes/utils/selectionUtils'
|
import { isMultiSelectKey } from '@/renderer/extensions/vueNodes/utils/selectionUtils'
|
||||||
import { app } from '@/scripts/app'
|
import { app } from '@/scripts/app'
|
||||||
|
|
||||||
// Forward keydown events to litegraph's processKey when Vue nodes have focus
|
// Forward spacebar key events to litegraph for panning when Vue nodes have focus
|
||||||
let keydownForwardingInitialized = false
|
let spacebarForwardingInitialized = false
|
||||||
|
|
||||||
function initKeydownForwarding() {
|
function isEditableElement(el: Element | null): boolean {
|
||||||
if (keydownForwardingInitialized) return
|
if (!el) return false
|
||||||
keydownForwardingInitialized = true
|
const tag = el.tagName.toUpperCase()
|
||||||
|
if (tag === 'INPUT' || tag === 'TEXTAREA') return true
|
||||||
|
if (el instanceof HTMLElement && el.isContentEditable) return true
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
document.addEventListener(
|
function initSpacebarForwarding() {
|
||||||
'keydown',
|
if (spacebarForwardingInitialized) return
|
||||||
(e) => {
|
spacebarForwardingInitialized = true
|
||||||
const canvas = app.canvas
|
|
||||||
if (!canvas) return
|
const { space } = useMagicKeys()
|
||||||
if (e.target === canvas.canvas) return
|
const activeElement = useActiveElement()
|
||||||
canvas.processKey(e)
|
|
||||||
},
|
watch(space, (isPressed) => {
|
||||||
true
|
const canvas = app.canvas
|
||||||
)
|
if (!canvas) return
|
||||||
|
|
||||||
|
// Skip if canvas has focus (litegraph handles it) or if in editable element
|
||||||
|
if (activeElement.value === canvas.canvas) return
|
||||||
|
if (isEditableElement(activeElement.value!)) return
|
||||||
|
|
||||||
|
// Mirror litegraph's processKey behavior for spacebar
|
||||||
|
if (isPressed) {
|
||||||
|
canvas.read_only = true
|
||||||
|
// Set dragging_canvas based on current pointer state
|
||||||
|
if (canvas.pointer?.isDown) {
|
||||||
|
canvas.dragging_canvas = true
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
canvas.read_only = false
|
||||||
|
canvas.dragging_canvas = false
|
||||||
|
}
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
export function useNodePointerInteractions(
|
export function useNodePointerInteractions(
|
||||||
nodeIdRef: MaybeRefOrGetter<string>
|
nodeIdRef: MaybeRefOrGetter<string>
|
||||||
) {
|
) {
|
||||||
initKeydownForwarding()
|
// Initialize spacebar forwarding for panning when Vue nodes have focus
|
||||||
|
initSpacebarForwarding()
|
||||||
|
|
||||||
const { startDrag, endDrag, handleDrag } = useNodeDrag()
|
const { startDrag, endDrag, handleDrag } = useNodeDrag()
|
||||||
// Use canvas interactions for proper wheel event handling and pointer event capture control
|
// Use canvas interactions for proper wheel event handling and pointer event capture control
|
||||||
|
|||||||
@@ -293,9 +293,6 @@ export function useSlotLinkInteraction({
|
|||||||
raf.cancel()
|
raf.cancel()
|
||||||
dragContext.dispose()
|
dragContext.dispose()
|
||||||
clearCompatible()
|
clearCompatible()
|
||||||
if (app.canvas?.pointer) {
|
|
||||||
app.canvas.pointer.isDown = false
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const updatePointerState = (event: PointerEvent) => {
|
const updatePointerState = (event: PointerEvent) => {
|
||||||
@@ -414,7 +411,10 @@ export function useSlotLinkInteraction({
|
|||||||
if (!pointerSession.matches(event)) return
|
if (!pointerSession.matches(event)) return
|
||||||
|
|
||||||
const canvas = app.canvas
|
const canvas = app.canvas
|
||||||
if (canvas?.read_only && canvas.dragging_canvas) {
|
// When spacebar is held (read_only=true) and left mouse button is down,
|
||||||
|
// delegate to litegraph's processMouseMove for panning
|
||||||
|
const isLeftButtonDown = (event.buttons & 1) !== 0
|
||||||
|
if (canvas?.read_only && isLeftButtonDown) {
|
||||||
canvas.processMouseMove(event)
|
canvas.processMouseMove(event)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -712,9 +712,7 @@ export function useSlotLinkInteraction({
|
|||||||
)
|
)
|
||||||
|
|
||||||
pointerSession.begin(event.pointerId)
|
pointerSession.begin(event.pointerId)
|
||||||
if (canvas.pointer) {
|
// Update last_mouse so panning delta calculations are correct
|
||||||
canvas.pointer.isDown = true
|
|
||||||
}
|
|
||||||
canvas.last_mouse = [event.clientX, event.clientY]
|
canvas.last_mouse = [event.clientX, event.clientY]
|
||||||
|
|
||||||
toCanvasPointerEvent(event)
|
toCanvasPointerEvent(event)
|
||||||
|
|||||||
Reference in New Issue
Block a user