diff --git a/src/composables/useCoreCommands.test.ts b/src/composables/useCoreCommands.test.ts index 8a37e2e7e16..8ec94f1edb6 100644 --- a/src/composables/useCoreCommands.test.ts +++ b/src/composables/useCoreCommands.test.ts @@ -38,8 +38,12 @@ vi.mock('@/scripts/app', () => { copyToClipboard: vi.fn(), pasteFromClipboard: vi.fn(), selectItems: vi.fn(), + deleteSelected: vi.fn(), + finalizeGhostPlacement: vi.fn(), ds: mockDs, - setDirty: vi.fn() + setDirty: vi.fn(), + state: { ghostNodeId: null as number | null }, + canvas: { dispatchEvent: vi.fn() } } return { @@ -269,6 +273,7 @@ describe('useCoreCommands', () => { // Reset app state app.canvas.subgraph = undefined + app.canvas.state.ghostNodeId = null // Mock settings store vi.mocked(useSettingStore).mockReturnValue(createMockSettingStore(false)) @@ -607,4 +612,42 @@ describe('useCoreCommands', () => { expect(mockShowAbout).toHaveBeenCalled() }) }) + + describe('Ghost placement guards', () => { + function findCommand(id: string) { + return useCoreCommands().find((cmd) => cmd.id === id)! + } + + describe('DeleteSelectedItems', () => { + it('cancels ghost placement when active and skips deletion', async () => { + app.canvas.state.ghostNodeId = 42 + + await findCommand('Comfy.Canvas.DeleteSelectedItems').function() + + expect(app.canvas.finalizeGhostPlacement).toHaveBeenCalledWith(true) + expect(app.canvas.deleteSelected).not.toHaveBeenCalled() + }) + + it('deletes selected items when no ghost is active', async () => { + app.canvas.selectedItems = new Set([ + {} + ]) as typeof app.canvas.selectedItems + + await findCommand('Comfy.Canvas.DeleteSelectedItems').function() + + expect(app.canvas.finalizeGhostPlacement).not.toHaveBeenCalled() + expect(app.canvas.deleteSelected).toHaveBeenCalled() + }) + }) + + describe('ExitSubgraph', () => { + it('cancels ghost placement when active and skips graph navigation', async () => { + app.canvas.state.ghostNodeId = 7 + + await findCommand('Comfy.Graph.ExitSubgraph').function() + + expect(app.canvas.finalizeGhostPlacement).toHaveBeenCalledWith(true) + }) + }) + }) }) diff --git a/src/composables/useCoreCommands.ts b/src/composables/useCoreCommands.ts index 9eb5b9a3acd..0a949d1a049 100644 --- a/src/composables/useCoreCommands.ts +++ b/src/composables/useCoreCommands.ts @@ -923,6 +923,10 @@ export function useCoreCommands(): ComfyCommand[] { label: 'Delete Selected Items', versionAdded: '1.10.5', function: () => { + if (app.canvas.state.ghostNodeId != null) { + app.canvas.finalizeGhostPlacement(true) + return + } if (app.canvas.selectedItems.size === 0) { app.canvas.canvas.dispatchEvent( new CustomEvent('litegraph:no-items-selected', { bubbles: true }) @@ -1127,6 +1131,10 @@ export function useCoreCommands(): ComfyCommand[] { versionAdded: '1.20.1', function: () => { const canvas = useCanvasStore().getCanvas() + if (canvas.state.ghostNodeId != null) { + canvas.finalizeGhostPlacement(true) + return + } const navigationStore = useSubgraphNavigationStore() if (!canvas.graph) return diff --git a/src/platform/keybindings/keybindingService.propagation.test.ts b/src/platform/keybindings/keybindingService.propagation.test.ts new file mode 100644 index 00000000000..f4fd16a2e7b --- /dev/null +++ b/src/platform/keybindings/keybindingService.propagation.test.ts @@ -0,0 +1,115 @@ +import { createTestingPinia } from '@pinia/testing' +import { setActivePinia } from 'pinia' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { useKeybindingService } from '@/platform/keybindings/keybindingService' +import { useCommandStore } from '@/stores/commandStore' +import { useDialogStore } from '@/stores/dialogStore' + +vi.mock('@/platform/settings/settingStore', () => ({ + useSettingStore: vi.fn(() => ({ + get: vi.fn(() => []) + })) +})) + +vi.mock('@/stores/dialogStore', () => ({ + useDialogStore: vi.fn(() => ({ + dialogStack: [] + })) +})) + +function createKeyboardEvent( + key: string, + options: { + target?: Element + ctrlKey?: boolean + metaKey?: boolean + shiftKey?: boolean + altKey?: boolean + } = {} +): KeyboardEvent { + const { target = document.body, ...modifiers } = options + const event = new KeyboardEvent('keydown', { + key, + code: key === 'Enter' ? 'Enter' : key, + bubbles: true, + cancelable: true, + ...modifiers + }) + event.preventDefault = vi.fn() + event.stopPropagation = vi.fn() + event.composedPath = vi.fn(() => [target]) + return event +} + +describe('keybindingService - event propagation', () => { + let keybindingService: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + setActivePinia(createTestingPinia({ stubActions: false })) + + const commandStore = useCommandStore() + commandStore.execute = vi.fn() + + vi.mocked(useDialogStore).mockReturnValue({ + dialogStack: [] + } as Partial> as ReturnType< + typeof useDialogStore + >) + + keybindingService = useKeybindingService() + keybindingService.registerCoreKeybindings() + }) + + it('stops propagation when Ctrl+Enter fires with a non-input element focused', async () => { + // Simulates a dropdown (combobox div) being focused when Ctrl+Enter is pressed. + // Without stopPropagation the event reaches the dropdown handler and expands it. + const dropdown = document.createElement('div') + dropdown.setAttribute('role', 'combobox') + + const event = createKeyboardEvent('Enter', { + ctrlKey: true, + target: dropdown + }) + + await keybindingService.keybindHandler(event) + + expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith( + 'Comfy.QueuePrompt', + expect.any(Object) + ) + expect(event.stopPropagation).toHaveBeenCalled() + }) + + it('does not stop propagation when no keybinding matches', async () => { + const event = createKeyboardEvent('F13') // no binding for this key + + await keybindingService.keybindHandler(event) + + expect(event.stopPropagation).not.toHaveBeenCalled() + }) + + it('does not stop propagation when key is reserved by text input and target is textarea', async () => { + const textarea = document.createElement('textarea') + const event = createKeyboardEvent('Enter', { target: textarea }) + + await keybindingService.keybindHandler(event) + + expect(event.stopPropagation).not.toHaveBeenCalled() + }) + + it('does not stop propagation for bare Escape even when a keybinding matches', async () => { + // ExitSubgraph is bound to bare Escape. Element-level handlers (Reka UI + // menus, PrimeVue dialogs, BuilderFooterToolbar) need the event to reach + // them so they can close — stopPropagation would block them. + const event = createKeyboardEvent('Escape') + + await keybindingService.keybindHandler(event) + + expect(vi.mocked(useCommandStore().execute)).toHaveBeenCalledWith( + 'Comfy.Graph.ExitSubgraph' + ) + expect(event.stopPropagation).not.toHaveBeenCalled() + }) +}) diff --git a/src/platform/keybindings/keybindingService.ts b/src/platform/keybindings/keybindingService.ts index b11d6831a7d..4b40cd1961a 100644 --- a/src/platform/keybindings/keybindingService.ts +++ b/src/platform/keybindings/keybindingService.ts @@ -56,6 +56,16 @@ export function useKeybindingService() { } event.preventDefault() + // Bare Escape must keep propagating so element-level handlers (Reka UI + // menus, PrimeVue dialogs, BuilderFooterToolbar) can still close themselves. + const isBareEscape = + event.key === 'Escape' && + !event.ctrlKey && + !event.altKey && + !event.metaKey + if (!isBareEscape) { + event.stopPropagation() + } const runCommandIds = new Set([ 'Comfy.QueuePrompt', 'Comfy.QueuePromptFront', diff --git a/src/views/GraphView.vue b/src/views/GraphView.vue index f9f42823c76..c51c55b9438 100644 --- a/src/views/GraphView.vue +++ b/src/views/GraphView.vue @@ -269,7 +269,9 @@ onBeforeUnmount(() => { executionStore.unbindExecutionEvents() }) -useEventListener(window, 'keydown', useKeybindingService().keybindHandler) +useEventListener(window, 'keydown', useKeybindingService().keybindHandler, { + capture: true +}) const { wrapWithErrorHandling, wrapWithErrorHandlingAsync } = useErrorHandling()