Compare commits

...

5 Commits

Author SHA1 Message Date
Kelly Yang
2f8e26fa7a fix: remove Escape special-casing from keybinding propagation
Escape no longer needs custom handling — the existing targetElementId
and dialogStack guards handle priority correctly. Revert the isBareEscape
stopPropagation exemption and the ExitSubgraph ghost placement guard
that were introduced alongside the capture-phase fix.
2026-05-11 15:37:15 -07:00
Kelly Yang
86b1e93685 Merge branch 'main' into fix/dropdown-expands-on-run-keybind 2026-05-11 15:14:34 -07:00
Kelly Yang
2e6ec6c1fe fix: allow Escape to propagate so menus and dialogs can close
stopPropagation in capture phase for Escape was blocking BuilderFooterToolbar's
window-bubble listener and Reka UI / PrimeVue element-level handlers, causing
E2E tests to fail when pressing Escape to close menus, popovers, and dialogs.

Only suppress propagation for non-Escape keybindings; bare Escape must reach
all downstream handlers. Adds tests for the new propagation behaviour and for
the ghost-placement guards in DeleteSelectedItems and ExitSubgraph.
2026-05-04 15:53:17 -07:00
Kelly Yang
f7ca59dda4 fix: prevent dropdown from opening on run keybind (Ctrl+Enter)
Move keybindHandler to window capture phase and call stopPropagation()
on keybinding match so the event never reaches Reka UI dropdown triggers.
Add ghost-node guards in ExitSubgraph and DeleteSelectedItems so Escape
and Delete cancel ghost placement instead of bypassing the _ghostKeyHandler
that can no longer intercept in document capture.
2026-05-04 13:44:57 -07:00
Kelly Yang
35d171c28a test: add failing test for keybinding stopPropagation when dropdown is focused 2026-05-03 22:29:51 -07:00
5 changed files with 143 additions and 2 deletions

View File

@@ -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,32 @@ 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()
})
})
})
})

View File

@@ -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 })

View File

@@ -0,0 +1,101 @@
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<typeof useKeybindingService>
beforeEach(() => {
vi.clearAllMocks()
setActivePinia(createTestingPinia({ stubActions: false }))
const commandStore = useCommandStore()
commandStore.execute = vi.fn()
vi.mocked(useDialogStore).mockReturnValue({
dialogStack: []
} as Partial<ReturnType<typeof useDialogStore>> 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()
})
})

View File

@@ -56,6 +56,7 @@ export function useKeybindingService() {
}
event.preventDefault()
event.stopPropagation()
const runCommandIds = new Set([
'Comfy.QueuePrompt',
'Comfy.QueuePromptFront',

View File

@@ -276,7 +276,9 @@ onBeforeUnmount(() => {
executionStore.unbindExecutionEvents()
})
useEventListener(window, 'keydown', useKeybindingService().keybindHandler)
useEventListener(window, 'keydown', useKeybindingService().keybindHandler, {
capture: true
})
const { wrapWithErrorHandling, wrapWithErrorHandlingAsync } = useErrorHandling()