From 05aedffbcde72a14695b37c288f403b9e8296989 Mon Sep 17 00:00:00 2001 From: christian-byrne Date: Mon, 6 Jan 2025 12:40:26 -0700 Subject: [PATCH 1/2] replace direct access to user keybindings with getter methods --- src/services/keybindingService.ts | 4 ++-- src/stores/keybindingStore.ts | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/services/keybindingService.ts b/src/services/keybindingService.ts index 5a0b19816..20e293798 100644 --- a/src/services/keybindingService.ts +++ b/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()) ) } diff --git a/src/stores/keybindingStore.ts b/src/stores/keybindingStore.ts index 759ac95aa..ec9da6a0f 100644 --- a/src/stores/keybindingStore.ts +++ b/src/stores/keybindingStore.ts @@ -105,6 +105,20 @@ export const useKeybindingStore = defineStore('keybinding', () => { */ const userUnsetKeybindings = ref>({}) + /** + * 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>(() => { const result: Record = { ...defaultKeybindings.value @@ -262,8 +276,8 @@ export const useKeybindingStore = defineStore('keybinding', () => { return { keybindings, - userKeybindings, - userUnsetKeybindings, + getUserKeybindings, + getUserUnsetKeybindings, getKeybinding, getKeybindingsByCommandId, getKeybindingByCommandId, From d26ca4944f0bd700e24a29ffaf40fa0fd5c27a6b Mon Sep 17 00:00:00 2001 From: christian-byrne Date: Mon, 6 Jan 2025 12:42:33 -0700 Subject: [PATCH 2/2] Add browser test --- browser_tests/dialog.spec.ts | 49 ++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/browser_tests/dialog.spec.ts b/browser_tests/dialog.spec.ts index d7dcfda2e..ea856091d 100644 --- a/browser_tests/dialog.spec.ts +++ b/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)) + }) })