Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-enable keybinding jest test #2090

Merged
merged 5 commits into from
Dec 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/components/dialog/content/setting/KeybindingPanel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -204,7 +206,7 @@ watchEffect(() => {
function removeKeybinding(commandData: ICommandData) {
if (commandData.keybinding) {
keybindingStore.unsetKeybinding(commandData.keybinding)
keybindingStore.persistUserKeybindings()
keybindingService.persistUserKeybindings()
}
}

Expand All @@ -228,7 +230,7 @@ function saveKeybinding() {
})
)
if (updated) {
keybindingStore.persistUserKeybindings()
keybindingService.persistUserKeybindings()
}
}
cancelEdit()
Expand All @@ -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',
Expand Down
18 changes: 13 additions & 5 deletions src/services/extensionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
45 changes: 43 additions & 2 deletions src/services/keybindingService.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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
}
}
57 changes: 2 additions & 55 deletions src/stores/keybindingStore.ts
Original file line number Diff line number Diff line change
@@ -1,9 +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 {
commandId: string
Expand Down Expand Up @@ -245,54 +242,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) {
try {
addDefaultKeybinding(new KeybindingImpl(keybinding))
} catch (error) {
console.warn(
`Failed to load keybinding for extension ${extension.name}`,
error
)
}
}
}
}

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 = {}
Expand All @@ -312,17 +261,15 @@ export const useKeybindingStore = defineStore('keybinding', () => {

return {
keybindings,
userKeybindings,
userUnsetKeybindings,
getKeybinding,
getKeybindingsByCommandId,
getKeybindingByCommandId,
addDefaultKeybinding,
addUserKeybinding,
unsetKeybinding,
updateKeybindingOnCommand,
loadUserKeybindings,
loadCoreKeybindings,
loadExtensionKeybindings,
persistUserKeybindings,
resetKeybindings,
isCommandKeybindingModified
}
Expand Down
5 changes: 2 additions & 3 deletions src/views/GraphView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -111,7 +110,7 @@ const init = () => {
const coreCommands = useCoreCommands()
useCommandStore().registerCommands(coreCommands)
useMenuItemStore().registerCoreMenuCommands()
useKeybindingStore().loadCoreKeybindings()
useKeybindingService().registerCoreKeybindings()
useSidebarTabStore().registerCoreSidebarTabs()
useBottomPanelStore().registerCoreBottomPanelTabs()
app.extensionManager = useWorkspaceStore()
Expand Down Expand Up @@ -168,7 +167,7 @@ const onGraphReady = () => {
() => {
// Setting values now available after comfyApp.setup.
// Load keybindings.
useKeybindingStore().loadUserKeybindings()
useKeybindingService().registerUserKeybindings()

// Load server config
useServerConfigStore().loadServerConfig(
Expand Down
Loading