From 44482e017a2b3f92d2e7435b692ff9a23a7655ab Mon Sep 17 00:00:00 2001 From: Tristan Sommer Date: Mon, 9 Dec 2024 15:20:30 +0100 Subject: [PATCH] initial implementation, copy from pr #1820 + migration + comments --- .../content/setting/KeybindingPanel.vue | 47 ++- .../setting/keybinding/KeyComboDisplay.vue | 17 +- .../setting/keybinding/KeyContextSelect.vue | 58 +++ src/components/topbar/CommandMenubar.vue | 2 +- src/constants/coreKeybindings.ts | 17 + src/constants/coreSettings.ts | 10 + src/extensions/core/keybinds.ts | 137 ++++--- src/scripts/app.ts | 2 +- src/scripts/changeTracker.ts | 17 +- src/stores/commandStore.ts | 2 +- src/stores/extensionStore.ts | 1 + src/stores/keybindingStore.ts | 387 ++++++++++-------- src/types/keyBindingTypes.ts | 14 +- .../tests/slow/store/keybindingStore.test.ts | 23 +- 14 files changed, 469 insertions(+), 265 deletions(-) create mode 100644 src/components/dialog/content/setting/keybinding/KeyContextSelect.vue diff --git a/src/components/dialog/content/setting/KeybindingPanel.vue b/src/components/dialog/content/setting/KeybindingPanel.vue index 6b3ebb8f1..6ed4e86a1 100644 --- a/src/components/dialog/content/setting/KeybindingPanel.vue +++ b/src/components/dialog/content/setting/KeybindingPanel.vue @@ -5,6 +5,10 @@ v-model="filters['global'].value" :placeholder="$t('g.searchKeybindings') + '...'" /> + (() => { - return Object.values(commandStore.commands).map((command) => ({ - id: command.id, - keybinding: keybindingStore.getKeybindingByCommandId(command.id) - })) + return Object.values(commandStore.commands) + .map((command) => ({ + id: command.id, + keybinding: keybindingStore.getKeybindingByCommandId(command.id) + })) + .filter((data) => { + // If keybinding is null, treat as global context + if (!data.keybinding) { + return selectedContext.value === 'global' + } + // Show commands that match the selected context + return data.keybinding.context === selectedContext.value + }) }) const selectedCommandData = ref(null) @@ -158,6 +174,7 @@ const editDialogVisible = ref(false) const newBindingKeyCombo = ref(null) const currentEditingCommand = ref(null) const keybindingInput = ref(null) +const keybindingContexts = keybindingStore.keybindingContexts const existingKeybindingOnCombo = computed(() => { if (!currentEditingCommand.value) { @@ -177,14 +194,16 @@ const existingKeybindingOnCombo = computed(() => { return null } - return keybindingStore.getKeybinding(newBindingKeyCombo.value) + return keybindingStore.getKeybinding( + newBindingKeyCombo.value, + currentEditingCommand.value.keybinding?.context + ) }) function editKeybinding(commandData: ICommandData) { currentEditingCommand.value = commandData - newBindingKeyCombo.value = commandData.keybinding - ? commandData.keybinding.combo - : null + newBindingKeyCombo.value = commandData.keybinding?.effectiveCombo ?? null + editDialogVisible.value = true } @@ -199,7 +218,7 @@ watchEffect(() => { function removeKeybinding(commandData: ICommandData) { if (commandData.keybinding) { - keybindingStore.unsetKeybinding(commandData.keybinding) + keybindingStore.unsetKeybinding(commandData.id) keybindingStore.persistUserKeybindings() } } @@ -220,7 +239,8 @@ function saveKeybinding() { const updated = keybindingStore.updateKeybindingOnCommand( new KeybindingImpl({ commandId: currentEditingCommand.value.id, - combo: newBindingKeyCombo.value + combo: newBindingKeyCombo.value, + context: currentEditingCommand.value.keybinding?.context }) ) if (updated) { @@ -230,6 +250,11 @@ function saveKeybinding() { cancelEdit() } +function handleContextChange(contextId: string) { + selectedContext.value = contextId + console.log('Context changed to', contextId) +} + const toast = useToast() async function resetKeybindings() { keybindingStore.resetKeybindings() diff --git a/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue b/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue index 25ab289a6..cc5c8dba4 100644 --- a/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue +++ b/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue @@ -1,11 +1,14 @@ diff --git a/src/constants/coreKeybindings.ts b/src/constants/coreKeybindings.ts index 023a63ac9..4a9973432 100644 --- a/src/constants/coreKeybindings.ts +++ b/src/constants/coreKeybindings.ts @@ -173,5 +173,22 @@ export const CORE_KEYBINDINGS: Keybinding[] = [ key: 'f' }, commandId: 'Workspace.ToggleFocusMode' + }, + { + combo: { + key: 'z', + ctrl: true + }, + commandId: 'Comfy.Undo', + targetSelector: '#graph-canvas' + }, + { + combo: { + key: 'z', + ctrl: true, + shift: true + }, + commandId: 'Comfy.Redo', + targetSelector: '#graph-canvas' } ] diff --git a/src/constants/coreSettings.ts b/src/constants/coreSettings.ts index 96e0fde6e..1cc9a0a74 100644 --- a/src/constants/coreSettings.ts +++ b/src/constants/coreSettings.ts @@ -416,6 +416,15 @@ export const CORE_SETTINGS: SettingParams[] = [ versionAdded: '1.3.5' }, { + //new array that saves all modified keybindings, no distinction between modified and unset -> unified in object + id: 'Comfy.Keybinding.ModifiedBindings', + name: 'Keybindings changed by the user', + type: 'hidden', + defaultValue: [] as Keybinding[], + versionAdded: '1.5.7' + }, + { + //deprecated id: 'Comfy.Keybinding.UnsetBindings', name: 'Keybindings unset by the user', type: 'hidden', @@ -423,6 +432,7 @@ export const CORE_SETTINGS: SettingParams[] = [ versionAdded: '1.3.7' }, { + //deprecated id: 'Comfy.Keybinding.NewBindings', name: 'Keybindings set by the user', type: 'hidden', diff --git a/src/extensions/core/keybinds.ts b/src/extensions/core/keybinds.ts index 0505773d2..980469bd0 100644 --- a/src/extensions/core/keybinds.ts +++ b/src/extensions/core/keybinds.ts @@ -2,64 +2,99 @@ import { app } from '../../scripts/app' import { KeyComboImpl, useKeybindingStore } from '@/stores/keybindingStore' import { useCommandStore } from '@/stores/commandStore' -app.registerExtension({ - name: 'Comfy.Keybinds', - init() { - const keybindListener = async function (event: KeyboardEvent) { - // Ignore keybindings for legacy jest tests as jest tests don't have - // a Vue app instance or pinia stores. - if (!app.vueAppReady) return +//this class is responsible for handling key events and executing commands +class KeyboardManager { + private modifiers: string[] = [] + private context: string = 'global' - const keyCombo = KeyComboImpl.fromEvent(event) - if (keyCombo.isModifier) { - return - } + constructor() { + this.addListeners() + } - // Ignore non-modifier keybindings if typing in input fields - const target = event.composedPath()[0] as HTMLElement + addListeners() { + window.addEventListener('keydown', (event) => this.handleKeyDown(event)) + window.addEventListener('keyup', (event) => this.handleKeyUp(event)) + window.addEventListener('blur', () => this.clearKeys()) + + app.extensionManager.setting.set('Comfy.KeybindContext', 'global') + } + + private clearKeys() { + this.modifiers = [] + } + + private handleKeyUp(event: KeyboardEvent) { + this.modifiers = this.modifiers.filter((key) => key !== event.key) + } + + private setContext(event?: KeyboardEvent) { + if (!event) return + event.preventDefault() + const context = app.extensionManager.setting.get('Comfy.KeybindContext') + this.context = context + } + + private async handleKeyDown(event: KeyboardEvent) { + if (!app.vueAppReady) return + + if (event.key === 'Escape' && this.modifiers.length === 0) { + this.handleEscapeKey() + return + } + + if (event.key === 'F12') return // prevent opening dev tools + + this.setContext(event) + + const target = event.composedPath()[0] as HTMLElement + const excludedTags = ['TEXTAREA', 'INPUT', 'SPAN'] + + if (this.context === 'global') { if ( - !keyCombo.hasModifier && - (target.tagName === 'TEXTAREA' || - target.tagName === 'INPUT' || - (target.tagName === 'SPAN' && - target.classList.contains('property_value'))) + excludedTags.includes(target.tagName) || + target.classList.contains('property_value') ) { return } - - const keybindingStore = useKeybindingStore() - const commandStore = useCommandStore() - const keybinding = keybindingStore.getKeybinding(keyCombo) - if (keybinding && keybinding.targetSelector !== '#graph-canvas') { - // Prevent default browser behavior first, then execute the command - event.preventDefault() - await commandStore.execute(keybinding.commandId) - 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) { - const modalDisplay = window - .getComputedStyle(modal) - .getPropertyValue('display') - - if (modalDisplay !== 'none') { - modal.style.display = 'none' - break - } - } - - for (const d of document.querySelectorAll('dialog')) d.close() - } } - window.addEventListener('keydown', keybindListener) + const keyCombo = KeyComboImpl.fromEvent(event) + if (keyCombo.isModifier) return + const keybindingStore = useKeybindingStore() + const commandStore = useCommandStore() + const keybinding = keybindingStore.getKeybinding(keyCombo, this.context) + console.log(keyCombo, keybinding) + if (keybinding) { + console.log('executing command', keybinding.commandId) + event.preventDefault() + await commandStore.execute(keybinding.commandId) + return + } + } + + private handleEscapeKey() { + const modals = document.querySelectorAll('.comfy-modal') + const modal = Array.from(modals).find( + (modal) => + window.getComputedStyle(modal).getPropertyValue('display') !== 'none' + ) + if (modal) { + modal.style.display = 'none' + } + + ;[...document.querySelectorAll('dialog')].forEach((d) => { + d.close() + }) + } + + setKeybindingContext(context: string) { + this.context = context + } +} + +app.registerExtension({ + name: 'Comfy.Keybinds', + init() { + const manager = new KeyboardManager() } }) diff --git a/src/scripts/app.ts b/src/scripts/app.ts index e986cc5e9..95c2b0452 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -1706,7 +1706,7 @@ export class ComfyApp { await this.#loadExtensions() this.#addProcessMouseHandler() - this.#addProcessKeyHandler() + //this.#addProcessKeyHandler() //removes another key handler this.#addConfigureHandler() this.#addApiUpdateHandlers() this.#addRestoreWorkflowView() diff --git a/src/scripts/changeTracker.ts b/src/scripts/changeTracker.ts index 5f21cddef..f19c2adf8 100644 --- a/src/scripts/changeTracker.ts +++ b/src/scripts/changeTracker.ts @@ -162,20 +162,6 @@ export class ChangeTracker { ) } - async undoRedo(e: KeyboardEvent) { - if ((e.ctrlKey || e.metaKey) && !e.altKey) { - const key = e.key.toUpperCase() - // Redo: Ctrl + Y, or Ctrl + Shift + Z - if ((key === 'Y' && !e.shiftKey) || (key == 'Z' && e.shiftKey)) { - await this.redo() - return true - } else if (key === 'Z' && !e.shiftKey) { - await this.undo() - return true - } - } - } - beforeChange() { this.changeCount++ } @@ -194,6 +180,7 @@ export class ChangeTracker { ChangeTracker.app = app let keyIgnored = false + /* window.addEventListener( 'keydown', (e: KeyboardEvent) => { @@ -237,7 +224,7 @@ export class ChangeTracker { }, true ) - + */ window.addEventListener('keyup', (e) => { if (keyIgnored) { keyIgnored = false diff --git a/src/stores/commandStore.ts b/src/stores/commandStore.ts index 49835ed98..c3ceb3194 100644 --- a/src/stores/commandStore.ts +++ b/src/stores/commandStore.ts @@ -57,7 +57,7 @@ export class ComfyCommandImpl implements ComfyCommand { : this._menubarLabel } - get keybinding(): KeybindingImpl | null { + get keybinding(): KeybindingImpl | undefined { return useKeybindingStore().getKeybindingByCommandId(this.id) } } diff --git a/src/stores/extensionStore.ts b/src/stores/extensionStore.ts index 9269dff4e..88edaf81d 100644 --- a/src/stores/extensionStore.ts +++ b/src/stores/extensionStore.ts @@ -46,6 +46,7 @@ export const useExtensionStore = defineStore('extension', () => { } extensionByName.value[extension.name] = markRaw(extension) + useKeybindingStore().loadExtensionKeybindingContexts(extension) useKeybindingStore().loadExtensionKeybindings(extension) useCommandStore().loadExtensionCommands(extension) useMenuItemStore().loadExtensionMenuCommands(extension) diff --git a/src/stores/keybindingStore.ts b/src/stores/keybindingStore.ts index 570ded477..be8ab768d 100644 --- a/src/stores/keybindingStore.ts +++ b/src/stores/keybindingStore.ts @@ -1,29 +1,70 @@ +// @ts-strict-ignore - will remove in the future import { defineStore } from 'pinia' -import { computed, Ref, ref, toRaw } from 'vue' -import { Keybinding, KeyCombo } from '@/types/keyBindingTypes' +import { ref, toRaw } from 'vue' +import { + Keybinding, + KeyCombo, + KeyBindingContext +} from '@/types/keyBindingTypes' import { useSettingStore } from './settingStore' import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings' import type { ComfyExtension } from '@/types/comfy' export class KeybindingImpl implements Keybinding { - commandId: string - combo: KeyComboImpl - targetSelector?: string + readonly commandId: string + readonly combo?: KeyComboImpl | null + currentCombo?: KeyComboImpl | null = null + readonly targetSelector?: string + readonly context?: string + //the new system saves the state of the keybindings in the object instead of the store constructor(obj: Keybinding) { this.commandId = obj.commandId - this.combo = new KeyComboImpl(obj.combo) + this.combo = obj.combo ? new KeyComboImpl(obj.combo) : null //this is the default combo set by comfyUI/the extension, can also be null for unset keybindings + this.currentCombo = this.combo //this is the current combo set by the user, can also be null for user unset keybindings + if (obj.currentCombo) this.currentCombo = new KeyComboImpl(obj.currentCombo) this.targetSelector = obj.targetSelector + this.context = obj.context ?? 'global' + } + + get defaultCombo(): KeyComboImpl { + return this.combo + } + + get effectiveCombo(): KeyComboImpl { + return this.currentCombo // null = unset, currentCombo != combo = user set combo + } + + overwriteCombo(combo: KeyComboImpl) { + this.currentCombo = combo // user set combo + } + + unsetCombo() { + this.currentCombo = null // unset + } + + resetCombo() { + this.currentCombo = this.combo //resets the combo to the default combo + } + + isModified(): boolean { + return this.currentCombo !== this.combo + } + + getContext(): string { + return this.context } equals(other: unknown): boolean { const raw = toRaw(other) - - return raw instanceof KeybindingImpl - ? this.commandId === raw.commandId && - this.combo.equals(raw.combo) && - this.targetSelector === raw.targetSelector - : false + if (!(raw instanceof KeybindingImpl)) return false + //the new system compares keybindingsd by the object properties instead of a serialized string + return ( + this.commandId === raw.commandId && + this.combo.equals(raw.combo) && + this.targetSelector === raw.targetSelector && + this.context === raw.context + ) } } @@ -50,6 +91,8 @@ export class KeyComboImpl implements KeyCombo { }) } + //removed the serialization function because the new system compares keybindings using the object properties + equals(other: unknown): boolean { const raw = toRaw(other) @@ -61,10 +104,6 @@ export class KeyComboImpl implements KeyCombo { : false } - serialize(): string { - return `${this.key.toUpperCase()}:${this.ctrl}:${this.alt}:${this.shift}` - } - toString(): string { return this.getKeySequences().join(' + ') } @@ -93,182 +132,182 @@ export class KeyComboImpl implements KeyCombo { } } +//used in the vue settings header to select between keybinding contexts +export class KeyBindingContextImpl implements KeyBindingContext { + id: string + name: string + + constructor(obj: KeyBindingContext) { + this.id = obj.id + this.name = obj.name + } +} + 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>({}) - - const keybindingByKeyCombo = computed>(() => { - const result: Record = { - ...defaultKeybindings.value + const keybindings = ref([]) + const keybindingContexts = ref([ + { + id: 'global', + name: 'Global' } - - for (const keybinding of Object.values(userUnsetKeybindings.value)) { - const serializedCombo = keybinding.combo.serialize() - if (result[serializedCombo]?.equals(keybinding)) { - delete result[serializedCombo] - } - } - - return { - ...result, - ...userKeybindings.value - } - }) - - const keybindings = computed(() => - Object.values(keybindingByKeyCombo.value) - ) - - function getKeybinding(combo: KeyComboImpl) { - return keybindingByKeyCombo.value[combo.serialize()] + ]) // global is default + function getKeybinding( + combo: KeyCombo, + context: string = 'global' + ): KeybindingImpl | undefined { + return keybindings.value.find((keybinding) => { + return ( + keybinding.effectiveCombo && + keybinding.effectiveCombo.equals(combo) && + keybinding.context === context + ) + }) } - function createKeybindingsByCommandId(keybindings: KeybindingImpl[]) { - const result: Record = {} - for (const keybinding of keybindings) { - if (!(keybinding.commandId in result)) { - result[keybinding.commandId] = [] - } - result[keybinding.commandId].push(keybinding) - } - return result + function getKeybindingsByCommandId(commandId: string): KeybindingImpl[] { + return keybindings.value.filter( + (keybinding) => keybinding.commandId === commandId + ) } - const keybindingsByCommandId = computed>( - () => { - return createKeybindingsByCommandId(keybindings.value) - } - ) - - function getKeybindingsByCommandId(commandId: string) { - return keybindingsByCommandId.value[commandId] ?? [] - } - - const defaultKeybindingsByCommandId = computed< - Record - >(() => { - return createKeybindingsByCommandId(Object.values(defaultKeybindings.value)) - }) - - function getKeybindingByCommandId(commandId: string) { + function getKeybindingByCommandId( + commandId: string + ): KeybindingImpl | undefined { return getKeybindingsByCommandId(commandId)[0] } - function addKeybinding( - target: Ref>, - keybinding: KeybindingImpl, - { existOk = false }: { existOk: boolean } - ) { - if (!existOk && keybinding.combo.serialize() in target.value) { - throw new Error( - `Keybinding on ${keybinding.combo} already exists on ${ - target.value[keybinding.combo.serialize()].commandId - }` - ) + function addKeybinding(keybinding: KeybindingImpl) { + if ( + getKeybinding(keybinding.effectiveCombo, keybinding.context) !== undefined + ) { + throw new Error(`Keybinding ${keybinding.commandId} already exists.`) } - target.value[keybinding.combo.serialize()] = keybinding + keybindings.value.push(keybinding) } + //kept for backwards compatibility function addDefaultKeybinding(keybinding: KeybindingImpl) { - addKeybinding(defaultKeybindings, keybinding, { existOk: false }) + addKeybinding(keybinding) } function addUserKeybinding(keybinding: KeybindingImpl) { - const defaultKeybinding = - defaultKeybindings.value[keybinding.combo.serialize()] - const userUnsetKeybinding = - userUnsetKeybindings.value[keybinding.combo.serialize()] + const effectiveCombo = keybinding.effectiveCombo + const context = keybinding.context ?? 'global' - // User is adding back a keybinding that was an unsetted default keybinding. - if ( - keybinding.equals(defaultKeybinding) && - keybinding.equals(userUnsetKeybinding) - ) { - delete userUnsetKeybindings.value[keybinding.combo.serialize()] - return + const existingKeybinding = getKeybinding(effectiveCombo, context) + if (existingKeybinding) { + existingKeybinding.overwriteCombo(effectiveCombo) + } else { + addKeybinding(keybinding) } - - // Unset keybinding on default keybinding if it exists and is not the same as userUnsetKeybinding - if (defaultKeybinding && !defaultKeybinding.equals(userUnsetKeybinding)) { - unsetKeybinding(defaultKeybinding) - } - - addKeybinding(userKeybindings, keybinding, { existOk: true }) } - function unsetKeybinding(keybinding: KeybindingImpl) { - const serializedCombo = keybinding.combo.serialize() - if (!(serializedCombo in keybindingByKeyCombo.value)) { - console.warn( - `Trying to unset non-exist keybinding: ${JSON.stringify(keybinding)}` - ) - return - } + function unsetKeybinding(commandId: string) { + const existingKeybinding = getKeybindingByCommandId(commandId) - if (userKeybindings.value[serializedCombo]?.equals(keybinding)) { - delete userKeybindings.value[serializedCombo] - return + if (existingKeybinding) { + existingKeybinding.unsetCombo() } - - if (defaultKeybindings.value[serializedCombo]?.equals(keybinding)) { - addKeybinding(userUnsetKeybindings, keybinding, { existOk: false }) - return - } - - throw new Error(`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)) { - return false + const existingKeybinding1 = getKeybindingByCommandId(keybinding.commandId) + const existingKeybinding2 = getKeybinding( + keybinding.effectiveCombo, + keybinding.context + ) + + if (existingKeybinding1) { + existingKeybinding1.overwriteCombo(keybinding.effectiveCombo) + return true } - if (currentKeybinding) { - unsetKeybinding(currentKeybinding) + if (existingKeybinding2) { + existingKeybinding2.overwriteCombo(keybinding.effectiveCombo) + return true } - addUserKeybinding(keybinding) + + addKeybinding(keybinding) return true } - function loadUserKeybindings() { + async function loadUserKeybindings() { + await migrateKeybindings() + const settingStore = useSettingStore() - // Unset bindings first as new bindings might conflict with default bindings. - const unsetBindings = settingStore.get('Comfy.Keybinding.UnsetBindings') - for (const keybinding of unsetBindings) { - unsetKeybinding(new KeybindingImpl(keybinding)) - } - const newBindings = settingStore.get('Comfy.Keybinding.NewBindings') - for (const keybinding of newBindings) { - addUserKeybinding(new KeybindingImpl(keybinding)) + + // Load modified bindings from settings + const modifiedBindings = + settingStore.get('Comfy.Keybinding.ModifiedBindings') ?? [] + for (const binding of modifiedBindings) { + const existing = getKeybindingByCommandId(binding.commandId) + if (existing != null && binding) { + const keyCombo = binding.currentCombo + ? new KeyComboImpl(binding.currentCombo) + : null + existing.overwriteCombo(keyCombo) + } } } + async function migrateKeybindings() { + const settingStore = useSettingStore() + const changedKeybindings = + settingStore.get('Comfy.Keybinding.NewBindings') ?? [] + const unsetKeybindings = + settingStore.get('Comfy.Keybinding.UnsetBindings') ?? [] + + console.log('Migrating keybindings', changedKeybindings, unsetKeybindings) + + for (const keybinding of changedKeybindings) { + const existing = getKeybindingByCommandId(keybinding.commandId) + if (existing != null) { + existing.overwriteCombo(new KeyComboImpl(keybinding.combo)) + } else { + const { combo: currentCombo, ...rest } = keybinding + const newKeybinding = { + ...rest, + currentCombo, + combo: null + } + addUserKeybinding(new KeybindingImpl(newKeybinding)) + } + } + + for (const keybinding of unsetKeybindings) { + unsetKeybinding(keybinding.commandId) + } + + //await settingStore.set('Comfy.Keybinding.NewBindings', []) + //await settingStore.set('Comfy.Keybinding.UnsetBindings', []) + + /* + clearing the old keybinding arrays can be done in a future version because the new keybinding + system is loaded after this migration function is called. This means that the old keybindings + will be overwritten if the user sets a new keybinding for the same command. + */ + } + function loadCoreKeybindings() { + // Simply load core keybindings as defaults for (const keybinding of CORE_KEYBINDINGS) { addDefaultKeybinding(new KeybindingImpl(keybinding)) } } + function loadExtensionKeybindingContexts(extension: ComfyExtension) { + if (extension.keybindingContexts) { + for (const context of extension.keybindingContexts) { + if (!keybindingContexts.value.includes(context)) { + keybindingContexts.value.push(context) + } + } + } + } function loadExtensionKeybindings(extension: ComfyExtension) { if (extension.keybindings) { for (const keybinding of extension.keybindings) { try { - addDefaultKeybinding(new KeybindingImpl(keybinding)) + addUserKeybinding(new KeybindingImpl(keybinding)) } catch (error) { console.warn( `Failed to load keybinding for extension ${extension.name}`, @@ -281,37 +320,48 @@ export const useKeybindingStore = defineStore('keybinding', () => { async function persistUserKeybindings() { const settingStore = useSettingStore() - // TODO(https://github.com/Comfy-Org/ComfyUI_frontend/issues/1079): - // Allow setting multiple values at once in settingStore + + // Only save modified keybindings + const modifiedBindings = keybindings.value + .filter((kb) => kb.isModified()) + .map((kb) => ({ + //only the relevant properties are saved to the settings + commandId: kb.commandId, + currentCombo: kb.currentCombo + })) + await settingStore.set( - 'Comfy.Keybinding.NewBindings', - Object.values(userKeybindings.value) - ) - await settingStore.set( - 'Comfy.Keybinding.UnsetBindings', - Object.values(userUnsetKeybindings.value) + 'Comfy.Keybinding.ModifiedBindings', + modifiedBindings ) } - function resetKeybindings() { - userKeybindings.value = {} - userUnsetKeybindings.value = {} + /** + * Resets keybindings for a specific context or all keybindings if no context is provided + * @param context Optional context to reset keybindings for + */ + function resetKeybindings(context?: string): void { + if (!keybindings.value?.length) { + return + } + + keybindings.value.forEach((keybinding) => { + if (!context || keybinding.getContext() === context) { + keybinding.resetCombo() + } + }) } function isCommandKeybindingModified(commandId: string): boolean { - const currentKeybinding: KeybindingImpl | undefined = - getKeybindingByCommandId(commandId) - const defaultKeybinding: KeybindingImpl | undefined = - defaultKeybindingsByCommandId.value[commandId]?.[0] - - return !( - (currentKeybinding === undefined && defaultKeybinding === undefined) || - currentKeybinding?.equals(defaultKeybinding) - ) + const keybinding = getKeybindingByCommandId(commandId) + if (!keybinding) + throw new Error(`Keybinding for command ${commandId} not found.`) + return keybinding.isModified() } return { keybindings, + keybindingContexts, getKeybinding, getKeybindingsByCommandId, getKeybindingByCommandId, @@ -322,6 +372,7 @@ export const useKeybindingStore = defineStore('keybinding', () => { loadUserKeybindings, loadCoreKeybindings, loadExtensionKeybindings, + loadExtensionKeybindingContexts, persistUserKeybindings, resetKeybindings, isCommandKeybindingModified diff --git a/src/types/keyBindingTypes.ts b/src/types/keyBindingTypes.ts index 863cae0dd..eab387d76 100644 --- a/src/types/keyBindingTypes.ts +++ b/src/types/keyBindingTypes.ts @@ -13,13 +13,25 @@ export const zKeyCombo = z.object({ export const zKeybinding = z.object({ commandId: z.string(), combo: zKeyCombo, + currentCombo: zKeyCombo.optional(), // Optional target element CSS selector 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. - targetSelector: z.string().optional() + targetSelector: z.string().optional(), + //context is used to distinguish between global keybindings and keybinds that only work in designated contexts. + // keybindngs without a set context are presumed to be global keybindngs. + // Extensions can create their own contexts to reuse keybindngs already set in the global context like crtl+z for undo. + context: z.string().optional() +}) + +// KeyBindingContext schema +export const zKeyBindingContext = z.object({ + id: z.string(), + name: z.string() }) // Infer types from schemas export type KeyCombo = z.infer export type Keybinding = z.infer +export type KeyBindingContext = z.infer diff --git a/tests-ui/tests/slow/store/keybindingStore.test.ts b/tests-ui/tests/slow/store/keybindingStore.test.ts index 35aee13fe..7f02c7fbe 100644 --- a/tests-ui/tests/slow/store/keybindingStore.test.ts +++ b/tests-ui/tests/slow/store/keybindingStore.test.ts @@ -1,3 +1,4 @@ +// @ts-strict-ignore import { setActivePinia, createPinia } from 'pinia' import { useKeybindingStore, KeybindingImpl } from '@/stores/keybindingStore' @@ -16,7 +17,7 @@ describe('useKeybindingStore', () => { store.addDefaultKeybinding(keybinding) expect(store.keybindings).toHaveLength(1) - expect(store.getKeybinding(keybinding.combo)).toEqual(keybinding) + expect(store.getKeybinding(keybinding!.combo)).toEqual(keybinding) }) it('should add and retrieve user keybindings', () => { @@ -57,7 +58,7 @@ describe('useKeybindingStore', () => { combo: { key: 'C', ctrl: true } }) store.addDefaultKeybinding(defaultKeybinding) - store.unsetKeybinding(defaultKeybinding) + store.unsetKeybinding(defaultKeybinding.commandId) const userKeybinding = new KeybindingImpl({ commandId: 'test.command2', @@ -79,8 +80,10 @@ describe('useKeybindingStore', () => { store.addUserKeybinding(keybinding) expect(store.keybindings).toHaveLength(1) - store.unsetKeybinding(keybinding) - expect(store.keybindings).toHaveLength(0) + store.unsetKeybinding(keybinding.commandId) + expect( + store.getKeybindingByCommandId(keybinding.commandId).currentCombo + ).toBeNull() }) it('should unset default keybindings', () => { @@ -93,8 +96,10 @@ describe('useKeybindingStore', () => { store.addDefaultKeybinding(keybinding) expect(store.keybindings).toHaveLength(1) - store.unsetKeybinding(keybinding) - expect(store.keybindings).toHaveLength(0) + store.unsetKeybinding(keybinding.commandId) + expect( + store.getKeybindingByCommandId(keybinding.commandId).currentCombo + ).toBeNull() }) it('should throw an error when adding duplicate default keybindings', () => { @@ -133,7 +138,7 @@ describe('useKeybindingStore', () => { combo: { key: 'H', alt: true, shift: true } }) - expect(() => store.unsetKeybinding(keybinding)).not.toThrow() + expect(() => store.unsetKeybinding(keybinding.commandId)).not.toThrow() }) it('should remove unset keybinding when adding back a default keybinding', () => { @@ -148,7 +153,7 @@ describe('useKeybindingStore', () => { expect(store.keybindings).toHaveLength(1) // Unset the default keybinding - store.unsetKeybinding(defaultKeybinding) + store.unsetKeybinding(defaultKeybinding.commandId) expect(store.keybindings).toHaveLength(0) // Add the same keybinding as a user keybinding @@ -215,7 +220,7 @@ describe('useKeybindingStore', () => { ) for (const keybinding of userUnsetKeybindings) { - store.unsetKeybinding(keybinding) + store.unsetKeybinding(keybinding.commandId) } expect(store.keybindings).toHaveLength(1)