diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index fe439af20..2d5df88b0 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -5,7 +5,7 @@ import * as fs from 'fs' import type { LGraphNode, LGraph } from '../../src/lib/litegraph/src/litegraph' import type { NodeId } from '../../src/platform/workflow/validation/schemas/workflowSchema' -import type { KeyCombo } from '../../src/schemas/keyBindingSchema' +import type { KeyCombo } from '../../src/platform/keybindings' import type { useWorkspaceStore } from '../../src/stores/workspaceStore' import { NodeBadgeMode } from '../../src/types/nodeSource' import { ComfyActionbar } from '../helpers/actionbar' diff --git a/browser_tests/tests/dialog.spec.ts b/browser_tests/tests/dialog.spec.ts index dd190f2a5..cfa1c8cd2 100644 --- a/browser_tests/tests/dialog.spec.ts +++ b/browser_tests/tests/dialog.spec.ts @@ -1,7 +1,7 @@ import type { Locator } from '@playwright/test' import { expect } from '@playwright/test' -import type { Keybinding } from '../../src/schemas/keyBindingSchema' +import type { Keybinding } from '../../src/platform/keybindings' import { comfyPageFixture as test } from '../fixtures/ComfyPage' test.beforeEach(async ({ comfyPage }) => { diff --git a/src/components/dialog/content/setting/KeybindingPanel.vue b/src/components/dialog/content/setting/KeybindingPanel.vue index 4ef7c3e95..0bc15b9cf 100644 --- a/src/components/dialog/content/setting/KeybindingPanel.vue +++ b/src/components/dialog/content/setting/KeybindingPanel.vue @@ -150,13 +150,11 @@ import { useI18n } from 'vue-i18n' import SearchBox from '@/components/common/SearchBox.vue' import Button from '@/components/ui/button/Button.vue' -import { useKeybindingService } from '@/services/keybindingService' +import { KeyComboImpl } from '@/platform/keybindings/keyCombo' +import { KeybindingImpl } from '@/platform/keybindings/keybinding' +import { useKeybindingService } from '@/platform/keybindings/keybindingService' +import { useKeybindingStore } from '@/platform/keybindings/keybindingStore' import { useCommandStore } from '@/stores/commandStore' -import { - KeyComboImpl, - KeybindingImpl, - useKeybindingStore -} from '@/stores/keybindingStore' import { normalizeI18nKey } from '@/utils/formatUtil' import PanelTemplate from './PanelTemplate.vue' diff --git a/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue b/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue index ded8cb18b..911a0f41b 100644 --- a/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue +++ b/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue @@ -13,7 +13,7 @@ import Tag from 'primevue/tag' import { computed } from 'vue' -import type { KeyComboImpl } from '@/stores/keybindingStore' +import type { KeyComboImpl } from '@/platform/keybindings/keyCombo' const { keyCombo, isModified = false } = defineProps<{ keyCombo: KeyComboImpl diff --git a/src/components/sidebar/SideToolbar.vue b/src/components/sidebar/SideToolbar.vue index e7f3ac2be..b69736327 100644 --- a/src/components/sidebar/SideToolbar.vue +++ b/src/components/sidebar/SideToolbar.vue @@ -70,7 +70,7 @@ import { useSettingStore } from '@/platform/settings/settingStore' import { useTelemetry } from '@/platform/telemetry' import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' import { useCommandStore } from '@/stores/commandStore' -import { useKeybindingStore } from '@/stores/keybindingStore' +import { useKeybindingStore } from '@/platform/keybindings/keybindingStore' import { useMenuItemStore } from '@/stores/menuItemStore' import { useUserStore } from '@/stores/userStore' import { useWorkspaceStore } from '@/stores/workspaceStore' diff --git a/src/constants/coreKeybindings.ts b/src/platform/keybindings/defaults.ts similarity index 95% rename from src/constants/coreKeybindings.ts rename to src/platform/keybindings/defaults.ts index cd8f3e5bb..f791d4246 100644 --- a/src/constants/coreKeybindings.ts +++ b/src/platform/keybindings/defaults.ts @@ -1,4 +1,4 @@ -import type { Keybinding } from '@/schemas/keyBindingSchema' +import type { Keybinding } from './types' export const CORE_KEYBINDINGS: Keybinding[] = [ { @@ -76,7 +76,6 @@ export const CORE_KEYBINDINGS: Keybinding[] = [ }, commandId: 'Comfy.ShowSettingsDialog' }, - // For '=' both holding shift and not holding shift { combo: { key: '=', @@ -94,7 +93,6 @@ export const CORE_KEYBINDINGS: Keybinding[] = [ commandId: 'Comfy.Canvas.ZoomIn', targetElementId: 'graph-canvas' }, - // For number pad '+' { combo: { key: '+', diff --git a/src/platform/keybindings/keyCombo.ts b/src/platform/keybindings/keyCombo.ts new file mode 100644 index 000000000..2a61c0fc7 --- /dev/null +++ b/src/platform/keybindings/keyCombo.ts @@ -0,0 +1,86 @@ +import { toRaw } from 'vue' + +import { RESERVED_BY_TEXT_INPUT } from './reserved' +import type { KeyCombo } from './types' + +export class KeyComboImpl implements KeyCombo { + key: string + ctrl: boolean + alt: boolean + shift: boolean + + constructor(obj: KeyCombo) { + this.key = obj.key + this.ctrl = obj.ctrl ?? false + this.alt = obj.alt ?? false + this.shift = obj.shift ?? false + } + + static fromEvent(event: KeyboardEvent) { + return new KeyComboImpl({ + key: event.key, + ctrl: event.ctrlKey || event.metaKey, + alt: event.altKey, + shift: event.shiftKey + }) + } + + equals(other: unknown): boolean { + const raw = toRaw(other) + + return raw instanceof KeyComboImpl + ? this.key.toUpperCase() === raw.key.toUpperCase() && + this.ctrl === raw.ctrl && + this.alt === raw.alt && + this.shift === raw.shift + : false + } + + serialize(): string { + return `${this.key.toUpperCase()}:${this.ctrl}:${this.alt}:${this.shift}` + } + + toString(): string { + return this.getKeySequences().join(' + ') + } + + get hasModifier(): boolean { + return this.ctrl || this.alt || this.shift + } + + get isModifier(): boolean { + return ['Control', 'Meta', 'Alt', 'Shift'].includes(this.key) + } + + get modifierCount(): number { + const modifiers = [this.ctrl, this.alt, this.shift] + return modifiers.reduce((acc, cur) => acc + Number(cur), 0) + } + + get isShiftOnly(): boolean { + return this.shift && this.modifierCount === 1 + } + + get isReservedByTextInput(): boolean { + return ( + !this.hasModifier || + this.isShiftOnly || + RESERVED_BY_TEXT_INPUT.has(this.toString()) + ) + } + + getKeySequences(): string[] { + const sequences: string[] = [] + if (this.ctrl) { + sequences.push('Ctrl') + } + if (this.alt) { + sequences.push('Alt') + } + if (this.shift) { + sequences.push('Shift') + } + sequences.push(this.key) + return sequences + } +} diff --git a/src/platform/keybindings/keybinding.ts b/src/platform/keybindings/keybinding.ts new file mode 100644 index 000000000..9f879c92e --- /dev/null +++ b/src/platform/keybindings/keybinding.ts @@ -0,0 +1,26 @@ +import { toRaw } from 'vue' + +import { KeyComboImpl } from './keyCombo' +import type { Keybinding } from './types' + +export class KeybindingImpl implements Keybinding { + commandId: string + combo: KeyComboImpl + targetElementId?: string + + constructor(obj: Keybinding) { + this.commandId = obj.commandId + this.combo = new KeyComboImpl(obj.combo) + this.targetElementId = obj.targetElementId + } + + equals(other: unknown): boolean { + const raw = toRaw(other) + + return raw instanceof KeybindingImpl + ? this.commandId === raw.commandId && + this.combo.equals(raw.combo) && + this.targetElementId === raw.targetElementId + : false + } +} diff --git a/src/platform/keybindings/keybindingService.escape.test.ts b/src/platform/keybindings/keybindingService.escape.test.ts new file mode 100644 index 000000000..962c5b3a2 --- /dev/null +++ b/src/platform/keybindings/keybindingService.escape.test.ts @@ -0,0 +1,179 @@ +import { createPinia, setActivePinia } from 'pinia' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { CORE_KEYBINDINGS } from '@/platform/keybindings/defaults' +import { KeyComboImpl } from '@/platform/keybindings/keyCombo' +import { KeybindingImpl } from '@/platform/keybindings/keybinding' +import { useKeybindingService } from '@/platform/keybindings/keybindingService' +import { useKeybindingStore } from '@/platform/keybindings/keybindingStore' +import { useCommandStore } from '@/stores/commandStore' +import type { DialogInstance } from '@/stores/dialogStore' +import { useDialogStore } from '@/stores/dialogStore' + +vi.mock('@/platform/settings/settingStore', () => ({ + useSettingStore: vi.fn(() => ({ + get: vi.fn(() => []) + })) +})) + +vi.mock('@/stores/dialogStore', () => ({ + useDialogStore: vi.fn(() => ({ + dialogStack: [] + })) +})) + +vi.mock('@/scripts/app', () => ({ + app: { + canvas: null + } +})) + +describe('keybindingService - Escape key handling', () => { + let keybindingService: ReturnType + let mockCommandExecute: ReturnType['execute'] + + beforeEach(() => { + vi.clearAllMocks() + setActivePinia(createPinia()) + + const commandStore = useCommandStore() + mockCommandExecute = vi.fn() + commandStore.execute = mockCommandExecute + + vi.mocked(useDialogStore).mockReturnValue({ + dialogStack: [] as DialogInstance[] + } as Partial> as ReturnType< + typeof useDialogStore + >) + + keybindingService = useKeybindingService() + keybindingService.registerCoreKeybindings() + }) + + function createKeyboardEvent( + key: string, + options: { + ctrlKey?: boolean + altKey?: boolean + metaKey?: boolean + shiftKey?: boolean + } = {} + ): KeyboardEvent { + const event = new KeyboardEvent('keydown', { + key, + ctrlKey: options.ctrlKey ?? false, + altKey: options.altKey ?? false, + metaKey: options.metaKey ?? false, + shiftKey: options.shiftKey ?? false, + bubbles: true, + cancelable: true + }) + + event.preventDefault = vi.fn() + event.composedPath = vi.fn(() => [document.body]) + + return event + } + + it('should execute Escape keybinding when no dialogs are open', async () => { + vi.mocked(useDialogStore).mockReturnValue({ + dialogStack: [] as DialogInstance[] + } as Partial> as ReturnType< + typeof useDialogStore + >) + + const event = createKeyboardEvent('Escape') + await keybindingService.keybindHandler(event) + + expect(mockCommandExecute).toHaveBeenCalledWith('Comfy.Graph.ExitSubgraph') + }) + + it('should NOT execute Escape keybinding when dialogs are open', async () => { + vi.mocked(useDialogStore).mockReturnValue({ + dialogStack: [{ key: 'test-dialog' } as DialogInstance] + } as Partial> as ReturnType< + typeof useDialogStore + >) + + keybindingService = useKeybindingService() + + const event = createKeyboardEvent('Escape') + await keybindingService.keybindHandler(event) + + expect(mockCommandExecute).not.toHaveBeenCalled() + }) + + it('should execute Escape keybinding with modifiers regardless of dialog state', async () => { + vi.mocked(useDialogStore).mockReturnValue({ + dialogStack: [{ key: 'test-dialog' } as DialogInstance] + } as Partial> as ReturnType< + typeof useDialogStore + >) + + const keybindingStore = useKeybindingStore() + keybindingStore.addDefaultKeybinding( + new KeybindingImpl({ + commandId: 'Test.CtrlEscape', + combo: { key: 'Escape', ctrl: true } + }) + ) + + keybindingService = useKeybindingService() + + const event = createKeyboardEvent('Escape', { ctrlKey: true }) + await keybindingService.keybindHandler(event) + + expect(mockCommandExecute).toHaveBeenCalledWith('Test.CtrlEscape') + }) + + it('should verify Escape keybinding exists in CORE_KEYBINDINGS', () => { + const escapeBinding = CORE_KEYBINDINGS.find( + (kb) => kb.combo.key === 'Escape' && !kb.combo.ctrl && !kb.combo.alt + ) + + expect(escapeBinding).toBeDefined() + expect(escapeBinding?.commandId).toBe('Comfy.Graph.ExitSubgraph') + }) + + it('should create correct KeyComboImpl from Escape event', () => { + const event = new KeyboardEvent('keydown', { + key: 'Escape', + ctrlKey: false, + altKey: false, + metaKey: false, + shiftKey: false + }) + + const keyCombo = KeyComboImpl.fromEvent(event) + + expect(keyCombo.key).toBe('Escape') + expect(keyCombo.ctrl).toBe(false) + expect(keyCombo.alt).toBe(false) + expect(keyCombo.shift).toBe(false) + }) + + it('should still close legacy modals on Escape when no keybinding matched', async () => { + setActivePinia(createPinia()) + keybindingService = useKeybindingService() + + const mockModal = document.createElement('div') + mockModal.className = 'comfy-modal' + mockModal.style.display = 'block' + document.body.appendChild(mockModal) + + const originalGetComputedStyle = window.getComputedStyle + window.getComputedStyle = vi.fn().mockReturnValue({ + getPropertyValue: vi.fn().mockReturnValue('block') + }) + + try { + const event = createKeyboardEvent('Escape') + await keybindingService.keybindHandler(event) + + expect(mockModal.style.display).toBe('none') + } finally { + document.body.removeChild(mockModal) + window.getComputedStyle = originalGetComputedStyle + } + }) +}) diff --git a/src/services/keybindingService.forwarding.test.ts b/src/platform/keybindings/keybindingService.forwarding.test.ts similarity index 89% rename from src/services/keybindingService.forwarding.test.ts rename to src/platform/keybindings/keybindingService.forwarding.test.ts index eb6d614fd..8656d7ed8 100644 --- a/src/services/keybindingService.forwarding.test.ts +++ b/src/platform/keybindings/keybindingService.forwarding.test.ts @@ -2,12 +2,11 @@ import { createTestingPinia } from '@pinia/testing' import { setActivePinia } from 'pinia' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { useKeybindingService } from '@/platform/keybindings/keybindingService' import { app } from '@/scripts/app' -import { useKeybindingService } from '@/services/keybindingService' import { useCommandStore } from '@/stores/commandStore' import { useDialogStore } from '@/stores/dialogStore' -// Mock the app and canvas using factory functions vi.mock('@/scripts/app', () => { return { app: { @@ -18,7 +17,6 @@ vi.mock('@/scripts/app', () => { } }) -// Mock stores vi.mock('@/platform/settings/settingStore', () => ({ useSettingStore: vi.fn(() => ({ get: vi.fn(() => []) @@ -31,7 +29,6 @@ vi.mock('@/stores/dialogStore', () => ({ })) })) -// Test utility for creating keyboard events with mocked methods function createTestKeyboardEvent( key: string, options: { @@ -57,7 +54,6 @@ function createTestKeyboardEvent( cancelable: true }) - // Mock event methods event.preventDefault = vi.fn() event.composedPath = vi.fn(() => [target]) @@ -71,11 +67,9 @@ describe('keybindingService - Event Forwarding', () => { vi.clearAllMocks() setActivePinia(createTestingPinia({ stubActions: false })) - // Mock command store execute const commandStore = useCommandStore() commandStore.execute = vi.fn() - // Reset dialog store mock to empty vi.mocked(useDialogStore).mockReturnValue({ dialogStack: [] } as Partial> as ReturnType< @@ -91,9 +85,7 @@ describe('keybindingService - Event Forwarding', () => { await keybindingService.keybindHandler(event) - // Should forward to canvas processKey expect(vi.mocked(app.canvas.processKey)).toHaveBeenCalledWith(event) - // Should not execute any command expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled() }) @@ -112,7 +104,6 @@ describe('keybindingService - Event Forwarding', () => { await keybindingService.keybindHandler(event) - // Should not forward to canvas when in input field expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled() expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled() }) @@ -144,7 +135,6 @@ describe('keybindingService - Event Forwarding', () => { }) it('should not forward Delete key when canvas is not available', async () => { - // Temporarily set canvas to null const originalCanvas = vi.mocked(app).canvas vi.mocked(app).canvas = null! @@ -164,7 +154,6 @@ describe('keybindingService - Event Forwarding', () => { await keybindingService.keybindHandler(event) - // Should not forward Enter key expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled() expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled() }) @@ -174,7 +163,6 @@ describe('keybindingService - Event Forwarding', () => { await keybindingService.keybindHandler(event) - // Should not forward when modifiers are pressed expect(vi.mocked(app.canvas.processKey)).not.toHaveBeenCalled() expect(vi.mocked(useCommandStore().execute)).not.toHaveBeenCalled() }) diff --git a/src/services/keybindingService.ts b/src/platform/keybindings/keybindingService.ts similarity index 73% rename from src/services/keybindingService.ts rename to src/platform/keybindings/keybindingService.ts index d2f45e005..31552ed43 100644 --- a/src/services/keybindingService.ts +++ b/src/platform/keybindings/keybindingService.ts @@ -1,40 +1,35 @@ -import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings' import { useSettingStore } from '@/platform/settings/settingStore' import { app } from '@/scripts/app' import { useCommandStore } from '@/stores/commandStore' import { useDialogStore } from '@/stores/dialogStore' -import { - KeyComboImpl, - KeybindingImpl, - useKeybindingStore -} from '@/stores/keybindingStore' -export const useKeybindingService = () => { +import { CORE_KEYBINDINGS } from './defaults' +import { KeyComboImpl } from './keyCombo' +import { KeybindingImpl } from './keybinding' +import { useKeybindingStore } from './keybindingStore' + +export function useKeybindingService() { const keybindingStore = useKeybindingStore() const commandStore = useCommandStore() const settingStore = useSettingStore() const dialogStore = useDialogStore() - // Helper function to determine if an event should be forwarded to canvas - const shouldForwardToCanvas = (event: KeyboardEvent): boolean => { - // Don't forward if modifier keys are pressed (except shift) + function shouldForwardToCanvas(event: KeyboardEvent): boolean { if (event.ctrlKey || event.altKey || event.metaKey) { return false } - // Keys that LiteGraph handles but aren't in core keybindings const canvasKeys = ['Delete', 'Backspace'] return canvasKeys.includes(event.key) } - const keybindHandler = async function (event: KeyboardEvent) { + async function keybindHandler(event: KeyboardEvent) { const keyCombo = KeyComboImpl.fromEvent(event) if (keyCombo.isModifier) { return } - // Ignore reserved or non-modifier keybindings if typing in input fields const target = event.composedPath()[0] as HTMLElement if ( keyCombo.isReservedByTextInput && @@ -49,20 +44,17 @@ export const useKeybindingService = () => { const keybinding = keybindingStore.getKeybinding(keyCombo) if (keybinding && keybinding.targetElementId !== 'graph-canvas') { - // Special handling for Escape key - let dialogs handle it first if ( event.key === 'Escape' && !event.ctrlKey && !event.altKey && !event.metaKey ) { - // If dialogs are open, don't execute the keybinding - let the dialog handle it if (dialogStore.dialogStack.length > 0) { return } } - // Prevent default browser behavior first, then execute the command event.preventDefault() const runCommandIds = new Set([ 'Comfy.QueuePrompt', @@ -81,7 +73,6 @@ export const useKeybindingService = () => { return } - // Forward unhandled canvas-targeted events to LiteGraph if (!keybinding && shouldForwardToCanvas(event)) { const canvas = app.canvas if ( @@ -89,18 +80,15 @@ export const useKeybindingService = () => { canvas.processKey && typeof canvas.processKey === 'function' ) { - // Let LiteGraph handle the event canvas.processKey(event) return } } - // Only clear dialogs if not using modifiers if (event.ctrlKey || event.altKey || event.metaKey) { return } - // Escape key: close the first open modal found, and all dialogs if (event.key === 'Escape') { const modals = document.querySelectorAll('.comfy-modal') for (const modal of modals) { @@ -118,14 +106,13 @@ export const useKeybindingService = () => { } } - const registerCoreKeybindings = () => { + function registerCoreKeybindings() { for (const keybinding of CORE_KEYBINDINGS) { keybindingStore.addDefaultKeybinding(new KeybindingImpl(keybinding)) } } function registerUserKeybindings() { - // Unset bindings first as new bindings might conflict with default bindings. const unsetBindings = settingStore.get('Comfy.Keybinding.UnsetBindings') for (const keybinding of unsetBindings) { keybindingStore.unsetKeybinding(new KeybindingImpl(keybinding)) @@ -137,8 +124,6 @@ export const useKeybindingService = () => { } async function persistUserKeybindings() { - // TODO(https://github.com/Comfy-Org/ComfyUI_frontend/issues/1079): - // Allow setting multiple values at once in settingStore await settingStore.set( 'Comfy.Keybinding.NewBindings', Object.values(keybindingStore.getUserKeybindings()) diff --git a/src/stores/keybindingStore.test.ts b/src/platform/keybindings/keybindingStore.test.ts similarity index 94% rename from src/stores/keybindingStore.test.ts rename to src/platform/keybindings/keybindingStore.test.ts index 1f7e1c6e2..15f16444f 100644 --- a/src/stores/keybindingStore.test.ts +++ b/src/platform/keybindings/keybindingStore.test.ts @@ -2,7 +2,8 @@ import { createTestingPinia } from '@pinia/testing' import { setActivePinia } from 'pinia' import { beforeEach, describe, expect, it } from 'vitest' -import { KeybindingImpl, useKeybindingStore } from '@/stores/keybindingStore' +import { KeybindingImpl } from '@/platform/keybindings/keybinding' +import { useKeybindingStore } from '@/platform/keybindings/keybindingStore' describe('useKeybindingStore', () => { beforeEach(() => { @@ -176,18 +177,14 @@ describe('useKeybindingStore', () => { combo: { key: 'I', ctrl: true } }) - // Add default keybinding store.addDefaultKeybinding(defaultKeybinding) expect(store.keybindings).toHaveLength(1) - // Unset the default keybinding store.unsetKeybinding(defaultKeybinding) expect(store.keybindings).toHaveLength(0) - // Add the same keybinding as a user keybinding store.addUserKeybinding(defaultKeybinding) - // Check that the keybinding is back and not in the unset list expect(store.keybindings).toHaveLength(1) expect(store.getKeybinding(defaultKeybinding.combo)).toEqual( defaultKeybinding @@ -200,10 +197,7 @@ describe('useKeybindingStore', () => { commandId: 'test.command', combo: { key: 'J', ctrl: true } }) - // Add default keybinding. - // This can happen when we change default keybindings. store.addDefaultKeybinding(keybinding) - // Add user keybinding. store.addUserKeybinding(keybinding) expect(store.keybindings).toHaveLength(1) @@ -211,10 +205,6 @@ describe('useKeybindingStore', () => { }) it('Should keep previously customized keybindings after default keybindings change', () => { - // Initially command 'foo' was bound to 'K, Ctrl'. User unset it and bound the - // command to 'A, Ctrl'. - // Now we change the default keybindings of 'foo' to 'A, Ctrl'. - // The user customized keybinding should be kept. const store = useKeybindingStore() const userUnsetKeybindings = [ @@ -391,18 +381,15 @@ describe('useKeybindingStore', () => { it('should handle complex scenario with both unset and user keybindings', () => { const store = useKeybindingStore() - // Create default keybinding const defaultKeybinding = new KeybindingImpl({ commandId: 'test.command', combo: { key: 'Q', ctrl: true } }) store.addDefaultKeybinding(defaultKeybinding) - // Unset default keybinding store.unsetKeybinding(defaultKeybinding) expect(store.keybindings).toHaveLength(0) - // Add user keybinding with different combo const userKeybinding = new KeybindingImpl({ commandId: 'test.command', combo: { key: 'R', alt: true } @@ -413,7 +400,6 @@ describe('useKeybindingStore', () => { userKeybinding ) - // Reset keybinding to default const result = store.resetKeybindingForCommand('test.command') expect(result).toBe(true) diff --git a/src/stores/keybindingStore.ts b/src/platform/keybindings/keybindingStore.ts similarity index 55% rename from src/stores/keybindingStore.ts rename to src/platform/keybindings/keybindingStore.ts index 76a21120c..84b0f271b 100644 --- a/src/stores/keybindingStore.ts +++ b/src/platform/keybindings/keybindingStore.ts @@ -1,140 +1,20 @@ -import _ from 'es-toolkit/compat' +import { groupBy } from 'es-toolkit/compat' import { defineStore } from 'pinia' import type { Ref } from 'vue' -import { computed, ref, toRaw } from 'vue' +import { computed, ref } from 'vue' -import { RESERVED_BY_TEXT_INPUT } from '@/constants/reservedKeyCombos' -import type { KeyCombo, Keybinding } from '@/schemas/keyBindingSchema' - -export class KeybindingImpl implements Keybinding { - commandId: string - combo: KeyComboImpl - targetElementId?: string - - constructor(obj: Keybinding) { - this.commandId = obj.commandId - this.combo = new KeyComboImpl(obj.combo) - this.targetElementId = obj.targetElementId - } - - equals(other: unknown): boolean { - const raw = toRaw(other) - - return raw instanceof KeybindingImpl - ? this.commandId === raw.commandId && - this.combo.equals(raw.combo) && - this.targetElementId === raw.targetElementId - : false - } -} - -export class KeyComboImpl implements KeyCombo { - key: string - // ctrl or meta(cmd on mac) - ctrl: boolean - alt: boolean - shift: boolean - - constructor(obj: KeyCombo) { - this.key = obj.key - this.ctrl = obj.ctrl ?? false - this.alt = obj.alt ?? false - this.shift = obj.shift ?? false - } - - static fromEvent(event: KeyboardEvent) { - return new KeyComboImpl({ - key: event.key, - ctrl: event.ctrlKey || event.metaKey, - alt: event.altKey, - shift: event.shiftKey - }) - } - - equals(other: unknown): boolean { - const raw = toRaw(other) - - return raw instanceof KeyComboImpl - ? this.key.toUpperCase() === raw.key.toUpperCase() && - this.ctrl === raw.ctrl && - this.alt === raw.alt && - this.shift === raw.shift - : false - } - - serialize(): string { - return `${this.key.toUpperCase()}:${this.ctrl}:${this.alt}:${this.shift}` - } - - toString(): string { - return this.getKeySequences().join(' + ') - } - - get hasModifier(): boolean { - return this.ctrl || this.alt || this.shift - } - - get isModifier(): boolean { - return ['Control', 'Meta', 'Alt', 'Shift'].includes(this.key) - } - - get modifierCount(): number { - const modifiers = [this.ctrl, this.alt, this.shift] - return modifiers.reduce((acc, cur) => acc + Number(cur), 0) - } - - get isShiftOnly(): boolean { - return this.shift && this.modifierCount === 1 - } - - get isReservedByTextInput(): boolean { - return ( - !this.hasModifier || - this.isShiftOnly || - RESERVED_BY_TEXT_INPUT.has(this.toString()) - ) - } - - getKeySequences(): string[] { - const sequences: string[] = [] - if (this.ctrl) { - sequences.push('Ctrl') - } - if (this.alt) { - sequences.push('Alt') - } - if (this.shift) { - sequences.push('Shift') - } - sequences.push(this.key) - return sequences - } -} +import type { KeyComboImpl } from './keyCombo' +import type { KeybindingImpl } from './keybinding' export const useKeybindingStore = defineStore('keybinding', () => { - /** - * Default keybindings provided by core and extensions. - */ const defaultKeybindings = ref>({}) - /** - * User-defined keybindings. - */ const userKeybindings = ref>({}) - /** - * User-defined keybindings that unset default keybindings. - */ const userUnsetKeybindings = ref>({}) - /** - * Get user-defined keybindings. - */ function getUserKeybindings() { return userKeybindings.value } - /** - * Get user-defined keybindings that unset default keybindings. - */ function getUserUnsetKeybindings() { return userUnsetKeybindings.value } @@ -167,7 +47,7 @@ export const useKeybindingStore = defineStore('keybinding', () => { const keybindingsByCommandId = computed>( () => { - return _.groupBy(keybindings.value, 'commandId') + return groupBy(keybindings.value, 'commandId') } ) @@ -178,26 +58,13 @@ export const useKeybindingStore = defineStore('keybinding', () => { const defaultKeybindingsByCommandId = computed< Record >(() => { - return _.groupBy(Object.values(defaultKeybindings.value), 'commandId') + return groupBy(Object.values(defaultKeybindings.value), 'commandId') }) function getKeybindingByCommandId(commandId: string) { return getKeybindingsByCommandId(commandId)[0] } - /** - * Adds a keybinding to the specified target reference. - * - * @param target - A ref that holds a record of keybindings. The keys represent - * serialized key combos, and the values are `KeybindingImpl` objects. - * @param keybinding - The keybinding to add, represented as a `KeybindingImpl` object. - * @param options - An options object. - * @param options.existOk - If true, allows overwriting an existing keybinding with the - * same combo. Defaults to false. - * - * @throws {Error} Throws an error if a keybinding with the same combo already exists in - * the target and `existOk` is false. - */ function addKeybinding( target: Ref>, keybinding: KeybindingImpl, @@ -223,7 +90,6 @@ export const useKeybindingStore = defineStore('keybinding', () => { const userUnsetKeybinding = userUnsetKeybindings.value[keybinding.combo.serialize()] - // User is adding back a keybinding that was an unsetted default keybinding. if ( keybinding.equals(defaultKeybinding) && keybinding.equals(userUnsetKeybinding) @@ -232,7 +98,6 @@ export const useKeybindingStore = defineStore('keybinding', () => { return } - // Unset keybinding on default keybinding if it exists and is not the same as userUnsetKeybinding if (defaultKeybinding && !defaultKeybinding.equals(userUnsetKeybinding)) { unsetKeybinding(defaultKeybinding) } @@ -262,11 +127,6 @@ export const useKeybindingStore = defineStore('keybinding', () => { console.warn(`Unset unknown keybinding: ${JSON.stringify(keybinding)}`) } - /** - * Update the keybinding on given command if it is different from the current keybinding. - * - * @returns true if the keybinding is updated, false otherwise. - */ function updateKeybindingOnCommand(keybinding: KeybindingImpl): boolean { const currentKeybinding = getKeybindingByCommandId(keybinding.commandId) if (currentKeybinding?.equals(keybinding)) { @@ -284,18 +144,11 @@ export const useKeybindingStore = defineStore('keybinding', () => { userUnsetKeybindings.value = {} } - /** - * Resets the keybinding for a given command to its default value. - * - * @param commandId - The commandId of the keybind to be reset - * @returns `true` if changes were made, `false` if not - */ function resetKeybindingForCommand(commandId: string): boolean { const currentKeybinding = getKeybindingByCommandId(commandId) const defaultKeybinding = defaultKeybindingsByCommandId.value[commandId]?.[0] - // No default keybinding exists, need to remove any user binding if (!defaultKeybinding) { if (currentKeybinding) { unsetKeybinding(currentKeybinding) @@ -304,17 +157,14 @@ export const useKeybindingStore = defineStore('keybinding', () => { return false } - // Current binding equals default binding, no changes needed if (currentKeybinding?.equals(defaultKeybinding)) { return false } - // Unset current keybinding if exists if (currentKeybinding) { unsetKeybinding(currentKeybinding) } - // Remove the unset record if it exists const serializedCombo = defaultKeybinding.combo.serialize() if ( userUnsetKeybindings.value[serializedCombo]?.equals(defaultKeybinding) diff --git a/src/constants/reservedKeyCombos.ts b/src/platform/keybindings/reserved.ts similarity index 100% rename from src/constants/reservedKeyCombos.ts rename to src/platform/keybindings/reserved.ts diff --git a/src/schemas/keyBindingSchema.ts b/src/platform/keybindings/types.ts similarity index 59% rename from src/schemas/keyBindingSchema.ts rename to src/platform/keybindings/types.ts index 04cbe319a..670101b0b 100644 --- a/src/schemas/keyBindingSchema.ts +++ b/src/platform/keybindings/types.ts @@ -1,6 +1,5 @@ import { z } from 'zod' -// KeyCombo schema const zKeyCombo = z.object({ key: z.string(), ctrl: z.boolean().optional(), @@ -9,17 +8,11 @@ const zKeyCombo = z.object({ meta: z.boolean().optional() }) -// Keybinding schema export const zKeybinding = z.object({ commandId: z.string(), combo: zKeyCombo, - // Optional target element ID to limit keybinding to. - // Note: Currently only used to distinguish between global keybindings - // and litegraph canvas keybindings. - // Do NOT use this field in extensions as it has no effect. targetElementId: z.string().optional() }) -// Infer types from schemas export type KeyCombo = z.infer export type Keybinding = z.infer diff --git a/src/platform/settings/constants/coreSettings.ts b/src/platform/settings/constants/coreSettings.ts index a4f5179b3..75fcefbf5 100644 --- a/src/platform/settings/constants/coreSettings.ts +++ b/src/platform/settings/constants/coreSettings.ts @@ -3,7 +3,7 @@ import { isCloud } from '@/platform/distribution/types' import { useSettingStore } from '@/platform/settings/settingStore' import type { SettingParams } from '@/platform/settings/types' import type { ColorPalettes } from '@/schemas/colorPaletteSchema' -import type { Keybinding } from '@/schemas/keyBindingSchema' +import type { Keybinding } from '@/platform/keybindings/types' import { NodeBadgeMode } from '@/types/nodeSource' import { LinkReleaseTriggerAction } from '@/types/searchBoxTypes' import { breakpointsTailwind } from '@vueuse/core' diff --git a/src/schemas/apiSchema.ts b/src/schemas/apiSchema.ts index e9ac9e454..b75942a55 100644 --- a/src/schemas/apiSchema.ts +++ b/src/schemas/apiSchema.ts @@ -3,7 +3,7 @@ import { z } from 'zod' import { LinkMarkerShape } from '@/lib/litegraph/src/litegraph' import { zNodeId } from '@/platform/workflow/validation/schemas/workflowSchema' import { colorPalettesSchema } from '@/schemas/colorPaletteSchema' -import { zKeybinding } from '@/schemas/keyBindingSchema' +import { zKeybinding } from '@/platform/keybindings/types' import { NodeBadgeMode } from '@/types/nodeSource' import { LinkReleaseTriggerAction } from '@/types/searchBoxTypes' diff --git a/src/scripts/app.ts b/src/scripts/app.ts index 0c985860c..067b6177d 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -59,7 +59,8 @@ import { useExecutionStore } from '@/stores/executionStore' import { useExtensionStore } from '@/stores/extensionStore' import { useFirebaseAuthStore } from '@/stores/firebaseAuthStore' import { useNodeOutputStore } from '@/stores/imagePreviewStore' -import { KeyComboImpl, useKeybindingStore } from '@/stores/keybindingStore' +import { KeyComboImpl } from '@/platform/keybindings/keyCombo' +import { useKeybindingStore } from '@/platform/keybindings/keybindingStore' import { useModelStore } from '@/stores/modelStore' import { SYSTEM_NODE_DEFS, useNodeDefStore } from '@/stores/nodeDefStore' import { useSubgraphStore } from '@/stores/subgraphStore' diff --git a/src/services/README.md b/src/services/README.md index b058efe9f..f01a6e8da 100644 --- a/src/services/README.md +++ b/src/services/README.md @@ -71,7 +71,6 @@ The following table lists ALL services in the system as of 2025-09-01: | customerEventsService.ts | Handles customer event tracking and audit logs | Analytics | | dialogService.ts | Provides dialog and modal management | UI | | extensionService.ts | Manages extension registration and lifecycle | Extensions | -| keybindingService.ts | Handles keyboard shortcuts and keybindings | Input | | litegraphService.ts | Provides utilities for working with the LiteGraph library | Graph | | load3dService.ts | Manages 3D model loading and visualization | 3D | | mediaCacheService.ts | Manages media file caching with blob storage and cleanup | Media | diff --git a/src/services/extensionService.ts b/src/services/extensionService.ts index b926ca2a6..d21b576ae 100644 --- a/src/services/extensionService.ts +++ b/src/services/extensionService.ts @@ -5,7 +5,8 @@ import { useSettingStore } from '@/platform/settings/settingStore' import { api } from '@/scripts/api' import { useCommandStore } from '@/stores/commandStore' import { useExtensionStore } from '@/stores/extensionStore' -import { KeybindingImpl, useKeybindingStore } from '@/stores/keybindingStore' +import { KeybindingImpl } from '@/platform/keybindings/keybinding' +import { useKeybindingStore } from '@/platform/keybindings/keybindingStore' import { useMenuItemStore } from '@/stores/menuItemStore' import { useWidgetStore } from '@/stores/widgetStore' import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore' diff --git a/src/services/keybindingService.escape.test.ts b/src/services/keybindingService.escape.test.ts deleted file mode 100644 index cc1059abc..000000000 --- a/src/services/keybindingService.escape.test.ts +++ /dev/null @@ -1,209 +0,0 @@ -import { createTestingPinia } from '@pinia/testing' -import { setActivePinia } from 'pinia' -import { beforeEach, describe, expect, it, vi } from 'vitest' - -import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings' -import { useKeybindingService } from '@/services/keybindingService' -import { useCommandStore } from '@/stores/commandStore' -import type { DialogInstance } from '@/stores/dialogStore' -import { useDialogStore } from '@/stores/dialogStore' -import { - KeyComboImpl, - KeybindingImpl, - useKeybindingStore -} from '@/stores/keybindingStore' - -// Mock stores -vi.mock('@/platform/settings/settingStore', () => ({ - useSettingStore: vi.fn(() => ({ - get: vi.fn(() => []) - })) -})) - -vi.mock('@/stores/dialogStore', () => ({ - useDialogStore: vi.fn(() => ({ - dialogStack: [] - })) -})) - -describe('keybindingService - Escape key handling', () => { - let keybindingService: ReturnType - let mockCommandExecute: ReturnType['execute'] - - beforeEach(() => { - vi.clearAllMocks() - setActivePinia(createTestingPinia({ stubActions: false })) - - // Mock command store execute - const commandStore = useCommandStore() - mockCommandExecute = vi.fn() - commandStore.execute = mockCommandExecute - - // Reset dialog store mock to empty - only mock the properties we need for testing - vi.mocked(useDialogStore).mockReturnValue({ - dialogStack: [], - // Add other required properties as undefined/default values to satisfy the type - // but they won't be used in these tests - ...({} as Omit, 'dialogStack'>) - }) - - keybindingService = useKeybindingService() - keybindingService.registerCoreKeybindings() - }) - - it('should register Escape key for ExitSubgraph command', () => { - const keybindingStore = useKeybindingStore() - - // Check that the Escape keybinding exists in core keybindings - const escapeKeybinding = CORE_KEYBINDINGS.find( - (kb) => - kb.combo.key === 'Escape' && kb.commandId === 'Comfy.Graph.ExitSubgraph' - ) - expect(escapeKeybinding).toBeDefined() - - // Check that it was registered in the store - const registeredBinding = keybindingStore.getKeybinding( - new KeyComboImpl({ key: 'Escape' }) - ) - expect(registeredBinding).toBeDefined() - expect(registeredBinding?.commandId).toBe('Comfy.Graph.ExitSubgraph') - }) - - it('should execute ExitSubgraph command when Escape is pressed', async () => { - const event = new KeyboardEvent('keydown', { - key: 'Escape', - bubbles: true, - cancelable: true - }) - - // Mock event methods - event.preventDefault = vi.fn() - event.composedPath = vi.fn(() => [document.body]) - - await keybindingService.keybindHandler(event) - - expect(event.preventDefault).toHaveBeenCalled() - expect(mockCommandExecute).toHaveBeenCalledWith('Comfy.Graph.ExitSubgraph') - }) - - it('should not execute command when Escape is pressed with modifiers', async () => { - const event = new KeyboardEvent('keydown', { - key: 'Escape', - ctrlKey: true, - bubbles: true, - cancelable: true - }) - - event.preventDefault = vi.fn() - event.composedPath = vi.fn(() => [document.body]) - - await keybindingService.keybindHandler(event) - - expect(mockCommandExecute).not.toHaveBeenCalled() - }) - - it('should not execute command when typing in input field', async () => { - const inputElement = document.createElement('input') - const event = new KeyboardEvent('keydown', { - key: 'Escape', - bubbles: true, - cancelable: true - }) - - event.preventDefault = vi.fn() - event.composedPath = vi.fn(() => [inputElement]) - - await keybindingService.keybindHandler(event) - - expect(mockCommandExecute).not.toHaveBeenCalled() - }) - - it('should close dialogs when no keybinding is registered', async () => { - // Remove the Escape keybinding - const keybindingStore = useKeybindingStore() - keybindingStore.unsetKeybinding( - new KeybindingImpl({ - commandId: 'Comfy.Graph.ExitSubgraph', - combo: { key: 'Escape' } - }) - ) - - // Create a mock dialog - const dialog = document.createElement('dialog') - dialog.open = true - dialog.close = vi.fn() - document.body.appendChild(dialog) - - const event = new KeyboardEvent('keydown', { - key: 'Escape', - bubbles: true, - cancelable: true - }) - - event.composedPath = vi.fn(() => [document.body]) - - await keybindingService.keybindHandler(event) - - expect(dialog.close).toHaveBeenCalled() - expect(mockCommandExecute).not.toHaveBeenCalled() - - // Cleanup - document.body.removeChild(dialog) - }) - - it('should allow user to rebind Escape key to different command', async () => { - const keybindingStore = useKeybindingStore() - - // Add a user keybinding for Escape to a different command - keybindingStore.addUserKeybinding( - new KeybindingImpl({ - commandId: 'Custom.Command', - combo: { key: 'Escape' } - }) - ) - - const event = new KeyboardEvent('keydown', { - key: 'Escape', - bubbles: true, - cancelable: true - }) - - event.preventDefault = vi.fn() - event.composedPath = vi.fn(() => [document.body]) - - await keybindingService.keybindHandler(event) - - expect(event.preventDefault).toHaveBeenCalled() - expect(mockCommandExecute).toHaveBeenCalledWith('Custom.Command') - expect(mockCommandExecute).not.toHaveBeenCalledWith( - 'Comfy.Graph.ExitSubgraph' - ) - }) - - it('should not execute Escape keybinding when dialogs are open', async () => { - // Mock dialog store to have open dialogs - vi.mocked(useDialogStore).mockReturnValue({ - dialogStack: [{ key: 'test-dialog' } as DialogInstance], - // Add other required properties as undefined/default values to satisfy the type - ...({} as Omit, 'dialogStack'>) - }) - - // Re-create keybinding service to pick up new mock - keybindingService = useKeybindingService() - - const event = new KeyboardEvent('keydown', { - key: 'Escape', - bubbles: true, - cancelable: true - }) - - event.preventDefault = vi.fn() - event.composedPath = vi.fn(() => [document.body]) - - await keybindingService.keybindHandler(event) - - // Should not call preventDefault or execute command - expect(event.preventDefault).not.toHaveBeenCalled() - expect(mockCommandExecute).not.toHaveBeenCalled() - }) -}) diff --git a/src/stores/README.md b/src/stores/README.md index 5c0261737..e037403c2 100644 --- a/src/stores/README.md +++ b/src/stores/README.md @@ -123,7 +123,6 @@ The following table lists ALL 46 store instances in the system as of 2025-09-01: | graphStore.ts | useCanvasStore | Manages the graph canvas state and interactions | Core | | helpCenterStore.ts | useHelpCenterStore | Manages help center visibility and state | UI | | imagePreviewStore.ts | useNodeOutputStore | Manages node outputs and execution results | Media | -| keybindingStore.ts | useKeybindingStore | Manages keyboard shortcuts | Input | | maintenanceTaskStore.ts | useMaintenanceTaskStore | Handles system maintenance tasks | System | | menuItemStore.ts | useMenuItemStore | Handles menu items and their state | UI | | modelStore.ts | useModelStore | Manages AI models information | Models | diff --git a/src/stores/commandStore.ts b/src/stores/commandStore.ts index ade180d37..27e032114 100644 --- a/src/stores/commandStore.ts +++ b/src/stores/commandStore.ts @@ -2,11 +2,10 @@ import { defineStore } from 'pinia' import { computed, ref } from 'vue' import { useErrorHandling } from '@/composables/useErrorHandling' +import type { KeybindingImpl } from '@/platform/keybindings/keybinding' +import { useKeybindingStore } from '@/platform/keybindings/keybindingStore' import type { ComfyExtension } from '@/types/comfy' -import { useKeybindingStore } from './keybindingStore' -import type { KeybindingImpl } from './keybindingStore' - export interface ComfyCommand { id: string function: (metadata?: Record) => void | Promise diff --git a/src/types/comfy.ts b/src/types/comfy.ts index 57ba2e971..e9c283263 100644 --- a/src/types/comfy.ts +++ b/src/types/comfy.ts @@ -5,7 +5,7 @@ import type { import type { LGraphCanvas, LGraphNode } from '@/lib/litegraph/src/litegraph' import type { SettingParams } from '@/platform/settings/types' import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' -import type { Keybinding } from '@/schemas/keyBindingSchema' +import type { Keybinding } from '@/platform/keybindings/types' import type { ComfyNodeDef } from '@/schemas/nodeDefSchema' import type { ComfyApp } from '@/scripts/app' import type { ComfyWidgetConstructor } from '@/scripts/widgets' diff --git a/src/views/GraphView.vue b/src/views/GraphView.vue index ca03f22a6..bbe22d1a1 100644 --- a/src/views/GraphView.vue +++ b/src/views/GraphView.vue @@ -62,7 +62,7 @@ import type { StatusWsMessageStatus } from '@/schemas/apiSchema' import { api } from '@/scripts/api' import { app } from '@/scripts/app' import { setupAutoQueueHandler } from '@/services/autoQueueService' -import { useKeybindingService } from '@/services/keybindingService' +import { useKeybindingService } from '@/platform/keybindings/keybindingService' import { useAssetsStore } from '@/stores/assetsStore' import { useCommandStore } from '@/stores/commandStore' import { useExecutionStore } from '@/stores/executionStore'