Compare commits

..

1 Commits

Author SHA1 Message Date
CodeRabbit Fixer
69bc33eef5 fix: Add unit tests for resolveSubgraphInputLink (#9293)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-06 17:04:25 +01:00
9 changed files with 198 additions and 196 deletions

View File

@@ -905,14 +905,6 @@ export function useCoreCommands(): ComfyCommand[] {
app.canvas.pasteFromClipboard() app.canvas.pasteFromClipboard()
} }
}, },
{
id: 'Comfy.Canvas.PasteFromClipboardWithConnect',
icon: 'icon-[lucide--clipboard-paste]',
label: () => t('Paste with Connect'),
function: () => {
app.canvas.pasteFromClipboard({ connectInputs: true })
}
},
{ {
id: 'Comfy.Canvas.SelectAll', id: 'Comfy.Canvas.SelectAll',
icon: 'icon-[lucide--lasso-select]', icon: 'icon-[lucide--lasso-select]',
@@ -927,12 +919,6 @@ export function useCoreCommands(): ComfyCommand[] {
label: 'Delete Selected Items', label: 'Delete Selected Items',
versionAdded: '1.10.5', versionAdded: '1.10.5',
function: () => { function: () => {
if (app.canvas.selectedItems.size === 0) {
app.canvas.canvas.dispatchEvent(
new CustomEvent('litegraph:no-items-selected', { bubbles: true })
)
return
}
app.canvas.deleteSelected() app.canvas.deleteSelected()
app.canvas.setDirty(true, true) app.canvas.setDirty(true, true)
} }

View File

@@ -12,7 +12,6 @@ interface UseCurveEditorOptions {
export function useCurveEditor({ svgRef, modelValue }: UseCurveEditorOptions) { export function useCurveEditor({ svgRef, modelValue }: UseCurveEditorOptions) {
const dragIndex = ref(-1) const dragIndex = ref(-1)
let cleanupDrag: (() => void) | null = null let cleanupDrag: (() => void) | null = null
let cachedInverseCTM: DOMMatrix | null = null
const curvePath = computed(() => { const curvePath = computed(() => {
const points = modelValue.value const points = modelValue.value
@@ -32,14 +31,16 @@ export function useCurveEditor({ svgRef, modelValue }: UseCurveEditorOptions) {
return parts.join('') return parts.join('')
}) })
function svgCoords( function svgCoords(e: PointerEvent): [number, number] {
e: PointerEvent, const svg = svgRef.value
inverseCTM?: DOMMatrix | null if (!svg) return [0, 0]
): [number, number] {
const inv = inverseCTM ?? svgRef.value?.getScreenCTM()?.inverse()
if (!inv) return [0, 0]
const svgPt = new DOMPoint(e.clientX, e.clientY).matrixTransform(inv) const ctm = svg.getScreenCTM()
if (!ctm) return [0, 0]
const svgPt = new DOMPoint(e.clientX, e.clientY).matrixTransform(
ctm.inverse()
)
return [ return [
Math.max(0, Math.min(1, svgPt.x)), Math.max(0, Math.min(1, svgPt.x)),
Math.max(0, Math.min(1, 1 - svgPt.y)) Math.max(0, Math.min(1, 1 - svgPt.y))
@@ -99,12 +100,11 @@ export function useCurveEditor({ svgRef, modelValue }: UseCurveEditorOptions) {
const svg = svgRef.value const svg = svgRef.value
if (!svg) return if (!svg) return
cachedInverseCTM = svg.getScreenCTM()?.inverse() ?? null
svg.setPointerCapture(e.pointerId) svg.setPointerCapture(e.pointerId)
const onMove = (ev: PointerEvent) => { const onMove = (ev: PointerEvent) => {
if (dragIndex.value < 0) return if (dragIndex.value < 0) return
const [x, y] = svgCoords(ev, cachedInverseCTM) const [x, y] = svgCoords(ev)
const movedPoint: CurvePoint = [x, y] const movedPoint: CurvePoint = [x, y]
const newPoints = [...modelValue.value] const newPoints = [...modelValue.value]
newPoints[dragIndex.value] = movedPoint newPoints[dragIndex.value] = movedPoint
@@ -116,7 +116,6 @@ export function useCurveEditor({ svgRef, modelValue }: UseCurveEditorOptions) {
const endDrag = () => { const endDrag = () => {
if (dragIndex.value < 0) return if (dragIndex.value < 0) return
dragIndex.value = -1 dragIndex.value = -1
cachedInverseCTM = null
svg.removeEventListener('pointermove', onMove) svg.removeEventListener('pointermove', onMove)
svg.removeEventListener('pointerup', endDrag) svg.removeEventListener('pointerup', endDrag)
svg.removeEventListener('lostpointercapture', endDrag) svg.removeEventListener('lostpointercapture', endDrag)

View File

@@ -121,6 +121,68 @@ describe('resolveSubgraphInputLink', () => {
expect(result).toBe('seed_input') expect(result).toBe('seed_input')
}) })
test('skips broken links where getLink returns undefined', () => {
const { subgraph, subgraphNode } = createSubgraphSetup('prompt')
addLinkedInteriorInput(subgraph, 'prompt', 'valid_input', 'valid')
const broken = addLinkedInteriorInput(
subgraph,
'prompt',
'broken_input',
'broken'
)
const originalGetLink = subgraph.getLink.bind(subgraph)
vi.spyOn(subgraph, 'getLink').mockImplementation((linkId) => {
if (typeof linkId !== 'number') return originalGetLink(linkId)
if (linkId === broken.linkId) return undefined
return originalGetLink(linkId)
})
const result = resolveSubgraphInputLink(
subgraphNode,
'prompt',
({ targetInput }) => targetInput.name
)
expect(result).toBe('valid_input')
})
test('returns result from latest connection when multiple links resolve', () => {
const { subgraph, subgraphNode } = createSubgraphSetup('prompt')
addLinkedInteriorInput(subgraph, 'prompt', 'older_input', 'older')
addLinkedInteriorInput(subgraph, 'prompt', 'newer_input', 'newer')
const result = resolveSubgraphInputLink(
subgraphNode,
'prompt',
({ targetInput }) => targetInput.name
)
expect(result).toBe('newer_input')
})
test('falls back to earlier link when latest resolve callback returns undefined', () => {
const { subgraph, subgraphNode } = createSubgraphSetup('prompt')
addLinkedInteriorInput(subgraph, 'prompt', 'fallback_input', 'fallback')
const newer = addLinkedInteriorInput(
subgraph,
'prompt',
'skipped_input',
'skipped'
)
const result = resolveSubgraphInputLink(
subgraphNode,
'prompt',
({ targetInput }) => {
if (targetInput.link === newer.linkId) return undefined
return targetInput.name
}
)
expect(result).toBe('fallback_input')
})
test('caches getTargetWidget result within the same callback evaluation', () => { test('caches getTargetWidget result within the same callback evaluation', () => {
const { subgraph, subgraphNode } = createSubgraphSetup('model') const { subgraph, subgraphNode } = createSubgraphSetup('model')
const linked = addLinkedInteriorInput( const linked = addLinkedInteriorInput(

View File

@@ -3791,6 +3791,13 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
return return
} }
private _noItemsSelected(): void {
const event = new CustomEvent('litegraph:no-items-selected', {
bubbles: true
})
this.canvas.dispatchEvent(event)
}
/** /**
* process a key event * process a key event
*/ */
@@ -3835,6 +3842,31 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
this.node_panel?.close() this.node_panel?.close()
this.options_panel?.close() this.options_panel?.close()
if (this.node_panel || this.options_panel) block_default = true if (this.node_panel || this.options_panel) block_default = true
} else if (e.keyCode === 65 && e.ctrlKey) {
// select all Control A
this.selectItems()
block_default = true
} else if (e.keyCode === 67 && (e.metaKey || e.ctrlKey) && !e.shiftKey) {
// copy
if (this.selected_nodes) {
this.copyToClipboard()
block_default = true
}
} else if (e.keyCode === 86 && (e.metaKey || e.ctrlKey)) {
// paste
this.pasteFromClipboard({ connectInputs: e.shiftKey })
} else if (e.key === 'Delete' || e.key === 'Backspace') {
// delete or backspace
// @ts-expect-error EventTarget.localName is not in standard types
if (e.target.localName != 'input' && e.target.localName != 'textarea') {
if (this.selectedItems.size === 0) {
this._noItemsSelected()
return
}
this.deleteSelected()
block_default = true
}
} }
// TODO // TODO

View File

@@ -1262,7 +1262,6 @@
"Move Selected Nodes Right": "Move Selected Nodes Right", "Move Selected Nodes Right": "Move Selected Nodes Right",
"Move Selected Nodes Up": "Move Selected Nodes Up", "Move Selected Nodes Up": "Move Selected Nodes Up",
"Paste": "Paste", "Paste": "Paste",
"Paste with Connect": "Paste with Connect",
"Reset View": "Reset View", "Reset View": "Reset View",
"Resize Selected Nodes": "Resize Selected Nodes", "Resize Selected Nodes": "Resize Selected Nodes",
"Select All": "Select All", "Select All": "Select All",

View File

@@ -208,52 +208,5 @@ export const CORE_KEYBINDINGS: Keybinding[] = [
key: 'Escape' key: 'Escape'
}, },
commandId: 'Comfy.Graph.ExitSubgraph' commandId: 'Comfy.Graph.ExitSubgraph'
},
{
combo: {
ctrl: true,
key: 'a'
},
commandId: 'Comfy.Canvas.SelectAll',
targetElementId: 'graph-canvas-container'
},
{
combo: {
ctrl: true,
key: 'c'
},
commandId: 'Comfy.Canvas.CopySelected',
targetElementId: 'graph-canvas-container'
},
{
combo: {
ctrl: true,
key: 'v'
},
commandId: 'Comfy.Canvas.PasteFromClipboard',
targetElementId: 'graph-canvas-container'
},
{
combo: {
ctrl: true,
shift: true,
key: 'v'
},
commandId: 'Comfy.Canvas.PasteFromClipboardWithConnect',
targetElementId: 'graph-canvas-container'
},
{
combo: {
key: 'Delete'
},
commandId: 'Comfy.Canvas.DeleteSelectedItems',
targetElementId: 'graph-canvas-container'
},
{
combo: {
key: 'Backspace'
},
commandId: 'Comfy.Canvas.DeleteSelectedItems',
targetElementId: 'graph-canvas-container'
} }
] ]

View File

@@ -1,11 +1,22 @@
import { createTestingPinia } from '@pinia/testing' import { createTestingPinia } from '@pinia/testing'
import { setActivePinia } from 'pinia' import { setActivePinia } from 'pinia'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { beforeEach, describe, expect, it, vi } from 'vitest'
import { useKeybindingService } from '@/platform/keybindings/keybindingService' import { useKeybindingService } from '@/platform/keybindings/keybindingService'
import { app } from '@/scripts/app'
import { useCommandStore } from '@/stores/commandStore' import { useCommandStore } from '@/stores/commandStore'
import { useDialogStore } from '@/stores/dialogStore' import { useDialogStore } from '@/stores/dialogStore'
vi.mock('@/scripts/app', () => {
return {
app: {
canvas: {
processKey: vi.fn()
}
}
}
})
vi.mock('@/platform/settings/settingStore', () => ({ vi.mock('@/platform/settings/settingStore', () => ({
useSettingStore: vi.fn(() => ({ useSettingStore: vi.fn(() => ({
get: vi.fn(() => []) get: vi.fn(() => [])
@@ -25,15 +36,13 @@ function createTestKeyboardEvent(
ctrlKey?: boolean ctrlKey?: boolean
altKey?: boolean altKey?: boolean
metaKey?: boolean metaKey?: boolean
shiftKey?: boolean
} = {} } = {}
): KeyboardEvent { ): KeyboardEvent {
const { const {
target = document.body, target = document.body,
ctrlKey = false, ctrlKey = false,
altKey = false, altKey = false,
metaKey = false, metaKey = false
shiftKey = false
} = options } = options
const event = new KeyboardEvent('keydown', { const event = new KeyboardEvent('keydown', {
@@ -41,7 +50,6 @@ function createTestKeyboardEvent(
ctrlKey, ctrlKey,
altKey, altKey,
metaKey, metaKey,
shiftKey,
bubbles: true, bubbles: true,
cancelable: true cancelable: true
}) })
@@ -52,10 +60,8 @@ function createTestKeyboardEvent(
return event return event
} }
describe('keybindingService - Canvas Keybindings', () => { describe('keybindingService - Event Forwarding', () => {
let keybindingService: ReturnType<typeof useKeybindingService> let keybindingService: ReturnType<typeof useKeybindingService>
let canvasContainer: HTMLDivElement
let canvasChild: HTMLCanvasElement
beforeEach(() => { beforeEach(() => {
vi.clearAllMocks() vi.clearAllMocks()
@@ -70,156 +76,94 @@ describe('keybindingService - Canvas Keybindings', () => {
typeof useDialogStore typeof useDialogStore
>) >)
canvasContainer = document.createElement('div')
canvasContainer.id = 'graph-canvas-container'
canvasChild = document.createElement('canvas')
canvasContainer.appendChild(canvasChild)
document.body.appendChild(canvasContainer)
keybindingService = useKeybindingService() keybindingService = useKeybindingService()
keybindingService.registerCoreKeybindings() keybindingService.registerCoreKeybindings()
}) })
afterEach(() => { it('should forward Delete key to canvas when no keybinding exists', async () => {
canvasContainer.remove() const event = createTestKeyboardEvent('Delete')
})
it('should execute DeleteSelectedItems for Delete key on canvas', async () => {
const event = createTestKeyboardEvent('Delete', {
target: canvasChild
})
await keybindingService.keybindHandler(event) await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith( expect(vi.mocked(app.canvas.processKey)).toHaveBeenCalledWith(event)
'Comfy.Canvas.DeleteSelectedItems' expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
)
}) })
it('should execute DeleteSelectedItems for Backspace key on canvas', async () => { it('should forward Backspace key to canvas when no keybinding exists', async () => {
const event = createTestKeyboardEvent('Backspace', { const event = createTestKeyboardEvent('Backspace')
target: canvasChild
})
await keybindingService.keybindHandler(event) await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith( expect(vi.mocked(app.canvas.processKey)).toHaveBeenCalledWith(event)
'Comfy.Canvas.DeleteSelectedItems' expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
)
}) })
it('should not execute DeleteSelectedItems when typing in input field', async () => { it('should not forward Delete key when typing in input field', async () => {
const inputElement = document.createElement('input') const inputElement = document.createElement('input')
const event = createTestKeyboardEvent('Delete', { target: inputElement }) const event = createTestKeyboardEvent('Delete', { target: inputElement })
await keybindingService.keybindHandler(event) await keybindingService.keybindHandler(event)
expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled()
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled() expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
}) })
it('should not execute DeleteSelectedItems when typing in textarea', async () => { it('should not forward Delete key when typing in textarea', async () => {
const textareaElement = document.createElement('textarea') const textareaElement = document.createElement('textarea')
const event = createTestKeyboardEvent('Delete', { const event = createTestKeyboardEvent('Delete', { target: textareaElement })
target: textareaElement
})
await keybindingService.keybindHandler(event) await keybindingService.keybindHandler(event)
expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled()
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled() expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
}) })
it('should execute SelectAll for Ctrl+A on canvas', async () => { it('should not forward Delete key when canvas processKey is not available', async () => {
const event = createTestKeyboardEvent('a', { // Temporarily replace processKey with undefined - testing edge case
ctrlKey: true, const originalProcessKey = vi.mocked(app.canvas).processKey
target: canvasChild vi.mocked(app.canvas).processKey = undefined!
})
await keybindingService.keybindHandler(event) const event = createTestKeyboardEvent('Delete')
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith( try {
'Comfy.Canvas.SelectAll' await keybindingService.keybindHandler(event)
) expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
} finally {
// Restore processKey for other tests
vi.mocked(app.canvas).processKey = originalProcessKey
}
}) })
it('should execute CopySelected for Ctrl+C on canvas', async () => { it('should not forward Delete key when canvas is not available', async () => {
const event = createTestKeyboardEvent('c', { const originalCanvas = vi.mocked(app).canvas
ctrlKey: true, vi.mocked(app).canvas = null!
target: canvasChild
})
await keybindingService.keybindHandler(event) const event = createTestKeyboardEvent('Delete')
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith( try {
'Comfy.Canvas.CopySelected' await keybindingService.keybindHandler(event)
) expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
} finally {
// Restore canvas for other tests
vi.mocked(app).canvas = originalCanvas
}
}) })
it('should execute PasteFromClipboard for Ctrl+V on canvas', async () => { it('should not forward non-canvas keys', async () => {
const event = createTestKeyboardEvent('v', { const event = createTestKeyboardEvent('Enter')
ctrlKey: true,
target: canvasChild
})
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.PasteFromClipboard'
)
})
it('should execute PasteFromClipboardWithConnect for Ctrl+Shift+V on canvas', async () => {
const event = createTestKeyboardEvent('v', {
ctrlKey: true,
shiftKey: true,
target: canvasChild
})
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.PasteFromClipboardWithConnect'
)
})
it('should execute graph-canvas bindings by normalizing to graph-canvas-container', async () => {
const event = createTestKeyboardEvent('=', {
altKey: true,
target: canvasChild
})
await keybindingService.keybindHandler(event)
expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith(
'Comfy.Canvas.ZoomIn'
)
})
it('should not execute graph-canvas bindings when target is outside canvas', async () => {
const outsideDiv = document.createElement('div')
document.body.appendChild(outsideDiv)
const event = createTestKeyboardEvent('=', {
altKey: true,
target: outsideDiv
})
await keybindingService.keybindHandler(event) await keybindingService.keybindHandler(event)
expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled()
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled() expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
outsideDiv.remove()
}) })
it('should not execute canvas commands when target is outside canvas container', async () => { it('should not forward when modifier keys are pressed', async () => {
const outsideDiv = document.createElement('div') const event = createTestKeyboardEvent('Delete', { ctrlKey: true })
document.body.appendChild(outsideDiv)
const event = createTestKeyboardEvent('Delete', {
target: outsideDiv
})
await keybindingService.keybindHandler(event) await keybindingService.keybindHandler(event)
expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled()
expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled() expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled()
outsideDiv.remove()
}) })
}) })

View File

@@ -1,5 +1,6 @@
import { isCloud } from '@/platform/distribution/types' import { isCloud } from '@/platform/distribution/types'
import { useSettingStore } from '@/platform/settings/settingStore' import { useSettingStore } from '@/platform/settings/settingStore'
import { app } from '@/scripts/app'
import { useCommandStore } from '@/stores/commandStore' import { useCommandStore } from '@/stores/commandStore'
import { useDialogStore } from '@/stores/dialogStore' import { useDialogStore } from '@/stores/dialogStore'
@@ -14,6 +15,16 @@ export function useKeybindingService() {
const settingStore = useSettingStore() const settingStore = useSettingStore()
const dialogStore = useDialogStore() const dialogStore = useDialogStore()
function shouldForwardToCanvas(event: KeyboardEvent): boolean {
if (event.ctrlKey || event.altKey || event.metaKey) {
return false
}
const canvasKeys = ['Delete', 'Backspace']
return canvasKeys.includes(event.key)
}
async function keybindHandler(event: KeyboardEvent) { async function keybindHandler(event: KeyboardEvent) {
const keyCombo = KeyComboImpl.fromEvent(event) const keyCombo = KeyComboImpl.fromEvent(event)
if (keyCombo.isModifier) { if (keyCombo.isModifier) {
@@ -33,17 +44,7 @@ export function useKeybindingService() {
} }
const keybinding = keybindingStore.getKeybinding(keyCombo) const keybinding = keybindingStore.getKeybinding(keyCombo)
if (keybinding) { if (keybinding && keybinding.targetElementId !== 'graph-canvas') {
const targetElementId =
keybinding.targetElementId === 'graph-canvas'
? 'graph-canvas-container'
: keybinding.targetElementId
if (targetElementId) {
const container = document.getElementById(targetElementId)
if (!container?.contains(target)) {
return
}
}
if ( if (
event.key === 'Escape' && event.key === 'Escape' &&
!event.ctrlKey && !event.ctrlKey &&
@@ -73,6 +74,18 @@ export function useKeybindingService() {
return return
} }
if (!keybinding && shouldForwardToCanvas(event)) {
const canvas = app.canvas
if (
canvas &&
canvas.processKey &&
typeof canvas.processKey === 'function'
) {
canvas.processKey(event)
return
}
}
if (event.ctrlKey || event.altKey || event.metaKey) { if (event.ctrlKey || event.altKey || event.metaKey) {
return return
} }

View File

@@ -676,6 +676,20 @@ export class ComfyApp {
e.stopImmediatePropagation() e.stopImmediatePropagation()
return return
} }
// Ctrl+C Copy
if (e.key === 'c' && (e.metaKey || e.ctrlKey)) {
return
}
// Ctrl+V Paste
if (
(e.key === 'v' || e.key == 'V') &&
(e.metaKey || e.ctrlKey) &&
!e.shiftKey
) {
return
}
} }
// Fall through to Litegraph defaults // Fall through to Litegraph defaults