mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
fix: clean up orphaned keybindings on load
Filter out stored keybindings referencing commands that are no longer registered (e.g. from removed extensions) during registerUserKeybindings(). Persist the cleanup to remove orphans from storage.
This commit is contained in:
130
src/platform/keybindings/keybindingService.test.ts
Normal file
130
src/platform/keybindings/keybindingService.test.ts
Normal file
@@ -0,0 +1,130 @@
|
||||
import { createPinia, setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { useKeybindingService } from '@/platform/keybindings/keybindingService'
|
||||
import { useKeybindingStore } from '@/platform/keybindings/keybindingStore'
|
||||
import type { Keybinding } from '@/platform/keybindings/types'
|
||||
import { useCommandStore } from '@/stores/commandStore'
|
||||
|
||||
const mockSettingState = vi.hoisted(() => ({
|
||||
newBindings: [] as Keybinding[],
|
||||
unsetBindings: [] as Keybinding[],
|
||||
setMany: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@/platform/settings/settingStore', () => ({
|
||||
useSettingStore: vi.fn(() => ({
|
||||
get: vi.fn((key: string) => {
|
||||
if (key === 'Comfy.Keybinding.NewBindings')
|
||||
return mockSettingState.newBindings
|
||||
if (key === 'Comfy.Keybinding.UnsetBindings')
|
||||
return mockSettingState.unsetBindings
|
||||
return []
|
||||
}),
|
||||
setMany: mockSettingState.setMany
|
||||
}))
|
||||
}))
|
||||
|
||||
vi.mock('@/scripts/app', () => ({
|
||||
app: { canvas: null }
|
||||
}))
|
||||
|
||||
vi.mock('@/stores/dialogStore', () => ({
|
||||
useDialogStore: vi.fn(() => ({ dialogStack: [] }))
|
||||
}))
|
||||
|
||||
describe('keybindingService - orphaned keybinding cleanup', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
setActivePinia(createPinia())
|
||||
mockSettingState.newBindings = []
|
||||
mockSettingState.unsetBindings = []
|
||||
})
|
||||
|
||||
function registerCommand(commandId: string) {
|
||||
useCommandStore().registerCommand({ id: commandId, function: vi.fn() })
|
||||
}
|
||||
|
||||
it('should skip orphaned new bindings referencing unregistered commands', () => {
|
||||
registerCommand('Registered.Command')
|
||||
|
||||
mockSettingState.newBindings = [
|
||||
{ commandId: 'Registered.Command', combo: { key: 'A', ctrl: true } },
|
||||
{
|
||||
commandId: 'Removed.Extension.Command',
|
||||
combo: { key: 'B', alt: true }
|
||||
}
|
||||
]
|
||||
|
||||
const service = useKeybindingService()
|
||||
service.registerCoreKeybindings()
|
||||
service.registerUserKeybindings()
|
||||
|
||||
const keybindingStore = useKeybindingStore()
|
||||
expect(
|
||||
keybindingStore.getKeybindingsByCommandId('Registered.Command')
|
||||
).toHaveLength(1)
|
||||
expect(
|
||||
keybindingStore.getKeybindingsByCommandId('Removed.Extension.Command')
|
||||
).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('should skip orphaned unset bindings referencing unregistered commands', () => {
|
||||
registerCommand('Registered.Command')
|
||||
|
||||
const registeredBinding: Keybinding = {
|
||||
commandId: 'Registered.Command',
|
||||
combo: { key: 'A', ctrl: true }
|
||||
}
|
||||
|
||||
mockSettingState.unsetBindings = [
|
||||
registeredBinding,
|
||||
{
|
||||
commandId: 'Removed.Extension.Command',
|
||||
combo: { key: 'B', alt: true }
|
||||
}
|
||||
]
|
||||
|
||||
const keybindingStore = useKeybindingStore()
|
||||
const unsetSpy = vi.spyOn(keybindingStore, 'unsetKeybinding')
|
||||
|
||||
const service = useKeybindingService()
|
||||
service.registerCoreKeybindings()
|
||||
service.registerUserKeybindings()
|
||||
|
||||
expect(unsetSpy).toHaveBeenCalledTimes(1)
|
||||
expect(unsetSpy.mock.calls[0][0].commandId).toBe('Registered.Command')
|
||||
})
|
||||
|
||||
it('should persist cleanup when orphaned bindings are found', () => {
|
||||
registerCommand('Registered.Command')
|
||||
|
||||
mockSettingState.newBindings = [
|
||||
{ commandId: 'Registered.Command', combo: { key: 'A', ctrl: true } },
|
||||
{
|
||||
commandId: 'Removed.Extension.Command',
|
||||
combo: { key: 'B', alt: true }
|
||||
}
|
||||
]
|
||||
|
||||
const service = useKeybindingService()
|
||||
service.registerCoreKeybindings()
|
||||
service.registerUserKeybindings()
|
||||
|
||||
expect(mockSettingState.setMany).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('should not persist when no orphaned bindings exist', () => {
|
||||
registerCommand('Registered.Command')
|
||||
|
||||
mockSettingState.newBindings = [
|
||||
{ commandId: 'Registered.Command', combo: { key: 'A', ctrl: true } }
|
||||
]
|
||||
|
||||
const service = useKeybindingService()
|
||||
service.registerCoreKeybindings()
|
||||
service.registerUserKeybindings()
|
||||
|
||||
expect(mockSettingState.setMany).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -121,11 +121,20 @@ export function useKeybindingService() {
|
||||
|
||||
function registerUserKeybindings() {
|
||||
const unsetBindings = settingStore.get('Comfy.Keybinding.UnsetBindings')
|
||||
for (const keybinding of unsetBindings) {
|
||||
const registeredUnsetBindings = unsetBindings.filter((kb) =>
|
||||
commandStore.isRegistered(kb.commandId)
|
||||
)
|
||||
|
||||
for (const keybinding of registeredUnsetBindings) {
|
||||
keybindingStore.unsetKeybinding(new KeybindingImpl(keybinding))
|
||||
}
|
||||
|
||||
const newBindings = settingStore.get('Comfy.Keybinding.NewBindings')
|
||||
for (const keybinding of newBindings) {
|
||||
const registeredNewBindings = newBindings.filter((kb) =>
|
||||
commandStore.isRegistered(kb.commandId)
|
||||
)
|
||||
|
||||
for (const keybinding of registeredNewBindings) {
|
||||
if (
|
||||
isCloud &&
|
||||
keybinding.commandId === 'Workspace.ToggleBottomPanelTab.logs-terminal'
|
||||
@@ -134,6 +143,13 @@ export function useKeybindingService() {
|
||||
}
|
||||
keybindingStore.addUserKeybinding(new KeybindingImpl(keybinding))
|
||||
}
|
||||
|
||||
const hadOrphans =
|
||||
registeredUnsetBindings.length < unsetBindings.length ||
|
||||
registeredNewBindings.length < newBindings.length
|
||||
if (hadOrphans) {
|
||||
void persistUserKeybindings()
|
||||
}
|
||||
}
|
||||
|
||||
async function persistUserKeybindings() {
|
||||
|
||||
Reference in New Issue
Block a user