From 93c7ba8e5f1648bc9d4697e12572ab0a863fa457 Mon Sep 17 00:00:00 2001 From: huchenlei Date: Sun, 29 Dec 2024 14:08:11 -0500 Subject: [PATCH 1/5] Re-enable keybinding tests --- tests-ui/tests/{slow => fast}/store/keybindingStore.test.ts | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests-ui/tests/{slow => fast}/store/keybindingStore.test.ts (100%) diff --git a/tests-ui/tests/slow/store/keybindingStore.test.ts b/tests-ui/tests/fast/store/keybindingStore.test.ts similarity index 100% rename from tests-ui/tests/slow/store/keybindingStore.test.ts rename to tests-ui/tests/fast/store/keybindingStore.test.ts From 396b2c2317483a63c94edc3f2ecd19bd9b2c3c57 Mon Sep 17 00:00:00 2001 From: huchenlei Date: Sun, 29 Dec 2024 14:18:08 -0500 Subject: [PATCH 2/5] Refactor keybinding store func to keybinding service --- .../content/setting/KeybindingPanel.vue | 8 ++-- src/services/keybindingService.ts | 45 ++++++++++++++++++- src/stores/keybindingStore.ts | 40 +---------------- src/views/GraphView.vue | 5 ++- 4 files changed, 53 insertions(+), 45 deletions(-) diff --git a/src/components/dialog/content/setting/KeybindingPanel.vue b/src/components/dialog/content/setting/KeybindingPanel.vue index 83edba7ef..f1d323a1f 100644 --- a/src/components/dialog/content/setting/KeybindingPanel.vue +++ b/src/components/dialog/content/setting/KeybindingPanel.vue @@ -135,12 +135,14 @@ import { useToast } from 'primevue/usetoast' import { FilterMatchMode } from '@primevue/core/api' import { useI18n } from 'vue-i18n' import { normalizeI18nKey } from '@/utils/formatUtil' +import { useKeybindingService } from '@/services/keybindingService' const filters = ref({ global: { value: '', matchMode: FilterMatchMode.CONTAINS } }) const keybindingStore = useKeybindingStore() +const keybindingService = useKeybindingService() const commandStore = useCommandStore() const { t } = useI18n() @@ -204,7 +206,7 @@ watchEffect(() => { function removeKeybinding(commandData: ICommandData) { if (commandData.keybinding) { keybindingStore.unsetKeybinding(commandData.keybinding) - keybindingStore.persistUserKeybindings() + keybindingService.persistUserKeybindings() } } @@ -228,7 +230,7 @@ function saveKeybinding() { }) ) if (updated) { - keybindingStore.persistUserKeybindings() + keybindingService.persistUserKeybindings() } } cancelEdit() @@ -237,7 +239,7 @@ function saveKeybinding() { const toast = useToast() async function resetKeybindings() { keybindingStore.resetKeybindings() - await keybindingStore.persistUserKeybindings() + await keybindingService.persistUserKeybindings() toast.add({ severity: 'info', summary: 'Info', diff --git a/src/services/keybindingService.ts b/src/services/keybindingService.ts index 30d92e491..830dbc024 100644 --- a/src/services/keybindingService.ts +++ b/src/services/keybindingService.ts @@ -1,9 +1,16 @@ +import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings' import { useCommandStore } from '@/stores/commandStore' -import { KeyComboImpl, useKeybindingStore } from '@/stores/keybindingStore' +import { + KeybindingImpl, + KeyComboImpl, + useKeybindingStore +} from '@/stores/keybindingStore' +import { useSettingStore } from '@/stores/settingStore' export const useKeybindingService = () => { const keybindingStore = useKeybindingStore() const commandStore = useCommandStore() + const settingStore = useSettingStore() const keybindHandler = async function (event: KeyboardEvent) { const keyCombo = KeyComboImpl.fromEvent(event) @@ -54,7 +61,41 @@ export const useKeybindingService = () => { } } + const 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)) + } + const newBindings = settingStore.get('Comfy.Keybinding.NewBindings') + for (const keybinding of newBindings) { + keybindingStore.addUserKeybinding(new KeybindingImpl(keybinding)) + } + } + + 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.userKeybindings.value) + ) + await settingStore.set( + 'Comfy.Keybinding.UnsetBindings', + Object.values(keybindingStore.userUnsetKeybindings.value) + ) + } + return { - keybindHandler + keybindHandler, + registerCoreKeybindings, + registerUserKeybindings, + persistUserKeybindings } } diff --git a/src/stores/keybindingStore.ts b/src/stores/keybindingStore.ts index 570ded477..678386b31 100644 --- a/src/stores/keybindingStore.ts +++ b/src/stores/keybindingStore.ts @@ -1,8 +1,6 @@ import { defineStore } from 'pinia' import { computed, Ref, ref, toRaw } from 'vue' import { Keybinding, KeyCombo } from '@/types/keyBindingTypes' -import { useSettingStore } from './settingStore' -import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings' import type { ComfyExtension } from '@/types/comfy' export class KeybindingImpl implements Keybinding { @@ -245,25 +243,6 @@ export const useKeybindingStore = defineStore('keybinding', () => { return true } - function loadUserKeybindings() { - 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)) - } - } - - function loadCoreKeybindings() { - for (const keybinding of CORE_KEYBINDINGS) { - addDefaultKeybinding(new KeybindingImpl(keybinding)) - } - } - function loadExtensionKeybindings(extension: ComfyExtension) { if (extension.keybindings) { for (const keybinding of extension.keybindings) { @@ -279,20 +258,6 @@ 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 - await settingStore.set( - 'Comfy.Keybinding.NewBindings', - Object.values(userKeybindings.value) - ) - await settingStore.set( - 'Comfy.Keybinding.UnsetBindings', - Object.values(userUnsetKeybindings.value) - ) - } - function resetKeybindings() { userKeybindings.value = {} userUnsetKeybindings.value = {} @@ -312,6 +277,8 @@ export const useKeybindingStore = defineStore('keybinding', () => { return { keybindings, + userKeybindings, + userUnsetKeybindings, getKeybinding, getKeybindingsByCommandId, getKeybindingByCommandId, @@ -319,10 +286,7 @@ export const useKeybindingStore = defineStore('keybinding', () => { addUserKeybinding, unsetKeybinding, updateKeybindingOnCommand, - loadUserKeybindings, - loadCoreKeybindings, loadExtensionKeybindings, - persistUserKeybindings, resetKeybindings, isCommandKeybindingModified } diff --git a/src/views/GraphView.vue b/src/views/GraphView.vue index 165a7973b..1d971ad6b 100644 --- a/src/views/GraphView.vue +++ b/src/views/GraphView.vue @@ -44,6 +44,7 @@ import { useCommandStore } from '@/stores/commandStore' import { useCoreCommands } from '@/hooks/coreCommandHooks' import { useEventListener } from '@vueuse/core' import { useKeybindingService } from '@/services/keybindingService' +import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings' setupAutoQueueHandler() @@ -111,7 +112,7 @@ const init = () => { const coreCommands = useCoreCommands() useCommandStore().registerCommands(coreCommands) useMenuItemStore().registerCoreMenuCommands() - useKeybindingStore().loadCoreKeybindings() + useKeybindingService().registerCoreKeybindings() useSidebarTabStore().registerCoreSidebarTabs() useBottomPanelStore().registerCoreBottomPanelTabs() app.extensionManager = useWorkspaceStore() @@ -168,7 +169,7 @@ const onGraphReady = () => { () => { // Setting values now available after comfyApp.setup. // Load keybindings. - useKeybindingStore().loadUserKeybindings() + useKeybindingService().registerUserKeybindings() // Load server config useServerConfigStore().loadServerConfig( From 20dbd183e0a2240767943657a95200ad7559872f Mon Sep 17 00:00:00 2001 From: huchenlei Date: Sun, 29 Dec 2024 14:23:06 -0500 Subject: [PATCH 3/5] Move extension keybinding register to extension service --- src/hooks/errorHooks.ts | 3 +-- src/services/extensionService.ts | 18 +++++++++++++----- src/stores/keybindingStore.ts | 17 ----------------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/hooks/errorHooks.ts b/src/hooks/errorHooks.ts index feddbec99..7098db14d 100644 --- a/src/hooks/errorHooks.ts +++ b/src/hooks/errorHooks.ts @@ -8,8 +8,7 @@ export function useErrorHandling() { toast.add({ severity: 'error', summary: t('g.error'), - detail: error.message, - life: 3000 + detail: error.message }) } diff --git a/src/services/extensionService.ts b/src/services/extensionService.ts index 6cc7900e2..0a6f4712a 100644 --- a/src/services/extensionService.ts +++ b/src/services/extensionService.ts @@ -2,16 +2,19 @@ import { app } from '@/scripts/app' import { api } from '@/scripts/api' import { useCommandStore } from '@/stores/commandStore' import { useExtensionStore } from '@/stores/extensionStore' -import { useKeybindingStore } from '@/stores/keybindingStore' +import { KeybindingImpl, useKeybindingStore } from '@/stores/keybindingStore' import { useMenuItemStore } from '@/stores/menuItemStore' import { useSettingStore } from '@/stores/settingStore' import { useWidgetStore } from '@/stores/widgetStore' import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore' import type { ComfyExtension } from '@/types/comfy' +import { useErrorHandling } from '@/hooks/errorHooks' export const useExtensionService = () => { const extensionStore = useExtensionStore() const settingStore = useSettingStore() + const keybindingStore = useKeybindingStore() + const { wrapWithErrorHandling } = useErrorHandling() /** * Loads all extensions from the API into the window in parallel @@ -47,12 +50,17 @@ export const useExtensionService = () => { const registerExtension = (extension: ComfyExtension) => { extensionStore.registerExtension(extension) - useKeybindingStore().loadExtensionKeybindings(extension) + const addKeybinding = wrapWithErrorHandling( + keybindingStore.addDefaultKeybinding + ) + const addSetting = wrapWithErrorHandling(settingStore.addSetting) + + extension.keybindings?.forEach((keybinding) => { + addKeybinding(new KeybindingImpl(keybinding)) + }) useCommandStore().loadExtensionCommands(extension) useMenuItemStore().loadExtensionMenuCommands(extension) - extension.settings?.forEach((setting) => { - settingStore.addSetting(setting) - }) + extension.settings?.forEach(addSetting) useBottomPanelStore().registerExtensionBottomPanelTabs(extension) if (extension.getCustomWidgets) { // TODO(huchenlei): We should deprecate the async return value of diff --git a/src/stores/keybindingStore.ts b/src/stores/keybindingStore.ts index 678386b31..7d909887a 100644 --- a/src/stores/keybindingStore.ts +++ b/src/stores/keybindingStore.ts @@ -1,7 +1,6 @@ import { defineStore } from 'pinia' import { computed, Ref, ref, toRaw } from 'vue' import { Keybinding, KeyCombo } from '@/types/keyBindingTypes' -import type { ComfyExtension } from '@/types/comfy' export class KeybindingImpl implements Keybinding { commandId: string @@ -243,21 +242,6 @@ export const useKeybindingStore = defineStore('keybinding', () => { return true } - function loadExtensionKeybindings(extension: ComfyExtension) { - if (extension.keybindings) { - for (const keybinding of extension.keybindings) { - try { - addDefaultKeybinding(new KeybindingImpl(keybinding)) - } catch (error) { - console.warn( - `Failed to load keybinding for extension ${extension.name}`, - error - ) - } - } - } - } - function resetKeybindings() { userKeybindings.value = {} userUnsetKeybindings.value = {} @@ -286,7 +270,6 @@ export const useKeybindingStore = defineStore('keybinding', () => { addUserKeybinding, unsetKeybinding, updateKeybindingOnCommand, - loadExtensionKeybindings, resetKeybindings, isCommandKeybindingModified } From a4d3d2c3f3c27adfca993da3ba238623bdf6dca8 Mon Sep 17 00:00:00 2001 From: huchenlei Date: Sun, 29 Dec 2024 14:26:48 -0500 Subject: [PATCH 4/5] nit --- src/views/GraphView.vue | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/views/GraphView.vue b/src/views/GraphView.vue index 1d971ad6b..a109c375d 100644 --- a/src/views/GraphView.vue +++ b/src/views/GraphView.vue @@ -32,7 +32,6 @@ import UnloadWindowConfirmDialog from '@/components/dialog/UnloadWindowConfirmDi import BrowserTabTitle from '@/components/BrowserTabTitle.vue' import TopMenubar from '@/components/topbar/TopMenubar.vue' import { setupAutoQueueHandler } from '@/services/autoQueueService' -import { useKeybindingStore } from '@/stores/keybindingStore' import { useSidebarTabStore } from '@/stores/workspace/sidebarTabStore' import { useNodeDefStore, useNodeFrequencyStore } from '@/stores/nodeDefStore' import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore' @@ -44,7 +43,6 @@ import { useCommandStore } from '@/stores/commandStore' import { useCoreCommands } from '@/hooks/coreCommandHooks' import { useEventListener } from '@vueuse/core' import { useKeybindingService } from '@/services/keybindingService' -import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings' setupAutoQueueHandler() From ecb8820f1ccce7ad1049dcddf4f7e8eec5c6e78b Mon Sep 17 00:00:00 2001 From: huchenlei Date: Sun, 29 Dec 2024 14:27:45 -0500 Subject: [PATCH 5/5] nit --- src/hooks/errorHooks.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hooks/errorHooks.ts b/src/hooks/errorHooks.ts index 7098db14d..feddbec99 100644 --- a/src/hooks/errorHooks.ts +++ b/src/hooks/errorHooks.ts @@ -8,7 +8,8 @@ export function useErrorHandling() { toast.add({ severity: 'error', summary: t('g.error'), - detail: error.message + detail: error.message, + life: 3000 }) }