Fix saved keybinding persistence (#2176)

Author: christian-byrne

browser_tests/dialog.spec.ts

@@ -1,5 +1,6 @@
 import { expect } from '@playwright/test'
 
+import { Keybinding } from '../src/types/keyBindingTypes'
 import { comfyPageFixture as test } from './fixtures/ComfyPage'
 
 test.describe('Load workflow warning', () => {
@@ -103,4 +104,52 @@ test.describe('Settings', () => {
       expect(await comfyPage.getSetting('Comfy.Graph.ZoomSpeed')).toBe(maxSpeed)
     })
   })
+
+  test('Should persist keybinding setting', async ({ comfyPage }) => {
+    // Open the settings dialog
+    await comfyPage.page.keyboard.press('Control+,')
+    await comfyPage.page.waitForSelector('.settings-container')
+
+    // Open the keybinding tab
+    await comfyPage.page.getByLabel('Keybinding').click()
+    await comfyPage.page.waitForSelector(
+      '[placeholder="Search Keybindings..."]'
+    )
+
+    // Focus the 'New Blank Workflow' row
+    const newBlankWorkflowRow = comfyPage.page.locator('tr', {
+      has: comfyPage.page.getByRole('cell', { name: 'New Blank Workflow' })
+    })
+    await newBlankWorkflowRow.click()
+
+    // Click edit button
+    const editKeybindingButton = newBlankWorkflowRow.locator('.pi-pencil')
+    await editKeybindingButton.click()
+
+    // Set new keybinding
+    const input = comfyPage.page.getByPlaceholder('Press keys for new binding')
+    await input.press('Alt+n')
+
+    const requestPromise = comfyPage.page.waitForRequest(
+      '**/api/settings/Comfy.Keybinding.NewBindings'
+    )
+
+    // Save keybinding
+    const saveButton = comfyPage.page
+      .getByLabel('Comfy.NewBlankWorkflow')
+      .getByLabel('Save')
+    await saveButton.click()
+
+    const request = await requestPromise
+    const expectedSetting: Keybinding = {
+      commandId: 'Comfy.NewBlankWorkflow',
+      combo: {
+        key: 'n',
+        ctrl: false,
+        alt: true,
+        shift: false
+      }
+    }
+    expect(request.postData()).toContain(JSON.stringify(expectedSetting))
+  })
 })

src/services/keybindingService.ts

@@ -84,11 +84,11 @@ export const useKeybindingService = () => {
     // Allow setting multiple values at once in settingStore
     await settingStore.set(
       'Comfy.Keybinding.NewBindings',
-      Object.values(keybindingStore.userKeybindings.value)
+      Object.values(keybindingStore.getUserKeybindings())
     )
     await settingStore.set(
       'Comfy.Keybinding.UnsetBindings',
-      Object.values(keybindingStore.userUnsetKeybindings.value)
+      Object.values(keybindingStore.getUserUnsetKeybindings())
     )
   }
 

src/stores/keybindingStore.ts

@@ -105,6 +105,20 @@ export const useKeybindingStore = defineStore('keybinding', () => {
    */
   const userUnsetKeybindings = ref<Record<string, KeybindingImpl>>({})
 
+  /**
+   * Get user-defined keybindings.
+   */
+  function getUserKeybindings() {
+    return userKeybindings.value
+  }
+
+  /**
+   * Get user-defined keybindings that unset default keybindings.
+   */
+  function getUserUnsetKeybindings() {
+    return userUnsetKeybindings.value
+  }
+
   const keybindingByKeyCombo = computed<Record<string, KeybindingImpl>>(() => {
     const result: Record<string, KeybindingImpl> = {
       ...defaultKeybindings.value
@@ -262,8 +276,8 @@ export const useKeybindingStore = defineStore('keybinding', () => {
 
   return {
     keybindings,
-    userKeybindings,
-    userUnsetKeybindings,
+    getUserKeybindings,
+    getUserUnsetKeybindings,
     getKeybinding,
     getKeybindingsByCommandId,
     getKeybindingByCommandId,