mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
fix: preset showing as modified after import due to auto-unset side effects
applyPreset now snapshots savedPresetData from the store's actual state after applying bindings, capturing auto-generated unsets from conflicting defaults. Also adds comprehensive unit and browser tests for preset lifecycle: switchPreset paths, promptAndSaveNewPreset, switchToDefaultPreset, loadPreset errors, export roundtrip, delete, and save-as-new.
This commit is contained in:
@@ -112,4 +112,177 @@ test.describe('Keybinding Presets', { tag: '@keyboard' }, () => {
|
||||
await page.keyboard.press('Escape')
|
||||
await comfyPage.settingDialog.waitForHidden()
|
||||
})
|
||||
|
||||
test('Can export a preset and re-import it', async ({ comfyPage }) => {
|
||||
test.setTimeout(30000)
|
||||
const { page } = comfyPage
|
||||
|
||||
// Open keybinding settings panel
|
||||
await comfyPage.settingDialog.open()
|
||||
await comfyPage.settingDialog.category('Keybinding').click()
|
||||
|
||||
// Import preset via ellipsis menu
|
||||
const ellipsisButton = page
|
||||
.locator('.icon-\\[lucide--ellipsis\\]')
|
||||
.locator('..')
|
||||
await ellipsisButton.click()
|
||||
|
||||
const fileChooserPromise = page.waitForEvent('filechooser')
|
||||
await page.getByRole('menuitem', { name: /Import preset/i }).click()
|
||||
const fileChooser = await fileChooserPromise
|
||||
|
||||
const presetPath = path.join(os.tmpdir(), 'test-preset.json')
|
||||
fs.writeFileSync(presetPath, JSON.stringify(TEST_PRESET))
|
||||
await fileChooser.setFiles(presetPath)
|
||||
|
||||
// Verify active preset switched to test-preset
|
||||
const presetTrigger = page
|
||||
.locator('#keybinding-panel-actions')
|
||||
.locator('button[role="combobox"]')
|
||||
await expect(presetTrigger).toContainText('test-preset')
|
||||
|
||||
// Wait for toast to auto-dismiss
|
||||
await expect(comfyPage.toast.visibleToasts).toHaveCount(0, {
|
||||
timeout: 5000
|
||||
})
|
||||
|
||||
// Export via ellipsis menu
|
||||
await ellipsisButton.click()
|
||||
const downloadPromise = page.waitForEvent('download')
|
||||
await page.getByRole('menuitem', { name: /Export preset/i }).click()
|
||||
const download = await downloadPromise
|
||||
|
||||
// Verify filename contains test-preset
|
||||
expect(download.suggestedFilename()).toContain('test-preset')
|
||||
|
||||
// Close settings
|
||||
await page.keyboard.press('Escape')
|
||||
await comfyPage.settingDialog.waitForHidden()
|
||||
|
||||
// Verify the downloaded file is valid JSON with correct structure
|
||||
const downloadPath = await download.path()
|
||||
expect(downloadPath).toBeTruthy()
|
||||
const content = fs.readFileSync(downloadPath!, 'utf-8')
|
||||
const parsed = JSON.parse(content) as {
|
||||
name: string
|
||||
newBindings: unknown[]
|
||||
unsetBindings: unknown[]
|
||||
}
|
||||
expect(parsed).toHaveProperty('name')
|
||||
expect(parsed).toHaveProperty('newBindings')
|
||||
expect(parsed).toHaveProperty('unsetBindings')
|
||||
expect(parsed.name).toBe('test-preset')
|
||||
})
|
||||
|
||||
test('Can delete an imported preset', async ({ comfyPage }) => {
|
||||
test.setTimeout(30000)
|
||||
const { page } = comfyPage
|
||||
|
||||
// Open keybinding settings panel
|
||||
await comfyPage.settingDialog.open()
|
||||
await comfyPage.settingDialog.category('Keybinding').click()
|
||||
|
||||
// Import preset via ellipsis menu
|
||||
const ellipsisButton = page
|
||||
.locator('.icon-\\[lucide--ellipsis\\]')
|
||||
.locator('..')
|
||||
await ellipsisButton.click()
|
||||
|
||||
const fileChooserPromise = page.waitForEvent('filechooser')
|
||||
await page.getByRole('menuitem', { name: /Import preset/i }).click()
|
||||
const fileChooser = await fileChooserPromise
|
||||
|
||||
const presetPath = path.join(os.tmpdir(), 'test-preset.json')
|
||||
fs.writeFileSync(presetPath, JSON.stringify(TEST_PRESET))
|
||||
await fileChooser.setFiles(presetPath)
|
||||
|
||||
// Verify active preset switched to test-preset
|
||||
const presetTrigger = page
|
||||
.locator('#keybinding-panel-actions')
|
||||
.locator('button[role="combobox"]')
|
||||
await expect(presetTrigger).toContainText('test-preset')
|
||||
|
||||
// Wait for toast to auto-dismiss
|
||||
await expect(comfyPage.toast.visibleToasts).toHaveCount(0, {
|
||||
timeout: 5000
|
||||
})
|
||||
|
||||
// Delete via ellipsis menu
|
||||
await ellipsisButton.click()
|
||||
await page.getByRole('menuitem', { name: /Delete preset/i }).click()
|
||||
|
||||
// Confirm deletion in the dialog
|
||||
const confirmDialog = page.getByRole('dialog', {
|
||||
name: /Delete the current preset/i
|
||||
})
|
||||
await confirmDialog.getByRole('button', { name: /Delete/i }).click()
|
||||
|
||||
// Verify preset trigger now shows Default Preset
|
||||
await expect(presetTrigger).toContainText('Default Preset')
|
||||
|
||||
// Close settings
|
||||
await page.keyboard.press('Escape')
|
||||
await comfyPage.settingDialog.waitForHidden()
|
||||
})
|
||||
|
||||
test('Can save modifications as a new preset', async ({ comfyPage }) => {
|
||||
test.setTimeout(30000)
|
||||
const { page } = comfyPage
|
||||
|
||||
// Open keybinding settings panel
|
||||
await comfyPage.settingDialog.open()
|
||||
await comfyPage.settingDialog.category('Keybinding').click()
|
||||
|
||||
// Import preset via ellipsis menu
|
||||
const ellipsisButton = page
|
||||
.locator('.icon-\\[lucide--ellipsis\\]')
|
||||
.locator('..')
|
||||
await ellipsisButton.click()
|
||||
|
||||
const fileChooserPromise = page.waitForEvent('filechooser')
|
||||
await page.getByRole('menuitem', { name: /Import preset/i }).click()
|
||||
const fileChooser = await fileChooserPromise
|
||||
|
||||
const presetPath = path.join(os.tmpdir(), 'test-preset.json')
|
||||
fs.writeFileSync(presetPath, JSON.stringify(TEST_PRESET))
|
||||
await fileChooser.setFiles(presetPath)
|
||||
|
||||
// Verify active preset switched to test-preset
|
||||
const presetTrigger = page
|
||||
.locator('#keybinding-panel-actions')
|
||||
.locator('button[role="combobox"]')
|
||||
await expect(presetTrigger).toContainText('test-preset')
|
||||
|
||||
// Wait for toast to auto-dismiss
|
||||
await expect(comfyPage.toast.visibleToasts).toHaveCount(0, {
|
||||
timeout: 5000
|
||||
})
|
||||
|
||||
// Save as new preset via ellipsis menu
|
||||
await ellipsisButton.click()
|
||||
await page.getByRole('menuitem', { name: /Save as new preset/i }).click()
|
||||
|
||||
// Fill in the preset name in the prompt dialog
|
||||
const promptInput = page.locator('.prompt-dialog-content input')
|
||||
await promptInput.fill('my-custom-preset')
|
||||
await promptInput.press('Enter')
|
||||
|
||||
// Wait for toast to auto-dismiss
|
||||
await expect(comfyPage.toast.visibleToasts).toHaveCount(0, {
|
||||
timeout: 5000
|
||||
})
|
||||
|
||||
// Verify preset trigger shows my-custom-preset
|
||||
await expect(presetTrigger).toContainText('my-custom-preset')
|
||||
|
||||
// Close settings
|
||||
await page.keyboard.press('Escape')
|
||||
await comfyPage.settingDialog.waitForHidden()
|
||||
|
||||
// Cleanup: delete the my-custom-preset file
|
||||
await comfyPage.request.fetch(
|
||||
`${comfyPage.url}/api/userdata/keybindings%2Fmy-custom-preset.json`,
|
||||
{ method: 'DELETE' }
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -16,6 +16,7 @@ const mockApi = vi.hoisted(() => ({
|
||||
const mockDownloadBlob = vi.hoisted(() => vi.fn())
|
||||
const mockUploadFile = vi.hoisted(() => vi.fn())
|
||||
const mockConfirm = vi.hoisted(() => vi.fn().mockResolvedValue(true))
|
||||
const mockPrompt = vi.hoisted(() => vi.fn().mockResolvedValue('test-preset'))
|
||||
const mockShowSmallLayoutDialog = vi.hoisted(() =>
|
||||
vi.fn().mockImplementation((options: Record<string, unknown>) => {
|
||||
const props = options.props as Record<string, unknown> | undefined
|
||||
@@ -44,7 +45,7 @@ vi.mock('@/scripts/utils', () => ({
|
||||
vi.mock('@/services/dialogService', () => ({
|
||||
useDialogService: () => ({
|
||||
confirm: mockConfirm,
|
||||
prompt: vi.fn().mockResolvedValue('test-preset'),
|
||||
prompt: mockPrompt,
|
||||
showSmallLayoutDialog: mockShowSmallLayoutDialog
|
||||
})
|
||||
}))
|
||||
@@ -387,10 +388,278 @@ describe('useKeybindingPresetService', () => {
|
||||
service.applyPreset(preset)
|
||||
|
||||
expect(store.currentPresetName).toBe('vim')
|
||||
expect(store.savedPresetData).toEqual(preset)
|
||||
expect(store.savedPresetData?.name).toBe('vim')
|
||||
expect(store.savedPresetData?.newBindings).toHaveLength(1)
|
||||
expect(store.savedPresetData?.newBindings[0].commandId).toBe('new.cmd')
|
||||
expect(Object.keys(store.getUserKeybindings())).toHaveLength(1)
|
||||
const bindings = Object.values(store.getUserKeybindings())
|
||||
expect(bindings[0].commandId).toBe('new.cmd')
|
||||
})
|
||||
})
|
||||
|
||||
describe('switchPreset', () => {
|
||||
it('discards unsaved changes when dialog returns false', async () => {
|
||||
store.addUserKeybinding(
|
||||
new KeybindingImpl({
|
||||
commandId: 'dirty.cmd',
|
||||
combo: { key: 'X', ctrl: true }
|
||||
})
|
||||
)
|
||||
|
||||
mockShowSmallLayoutDialog.mockImplementationOnce(
|
||||
(options: Record<string, unknown>) => {
|
||||
const props = options.props as Record<string, unknown> | undefined
|
||||
const onResult = props?.onResult as ((v: boolean) => void) | undefined
|
||||
onResult?.(false)
|
||||
}
|
||||
)
|
||||
|
||||
const targetPreset: KeybindingPreset = {
|
||||
name: 'vim',
|
||||
newBindings: [
|
||||
{ commandId: 'vim.cmd', combo: { key: 'J', ctrl: false } }
|
||||
],
|
||||
unsetBindings: []
|
||||
}
|
||||
mockApi.getUserData.mockResolvedValueOnce(
|
||||
new Response(JSON.stringify(targetPreset), { status: 200 })
|
||||
)
|
||||
|
||||
const service = await getPresetService()
|
||||
await service.switchPreset('vim')
|
||||
|
||||
expect(store.currentPresetName).toBe('vim')
|
||||
})
|
||||
|
||||
it('saves unsaved changes when dialog returns true on non-default preset', async () => {
|
||||
store.currentPresetName = 'my-preset'
|
||||
store.savedPresetData = {
|
||||
name: 'my-preset',
|
||||
newBindings: [],
|
||||
unsetBindings: []
|
||||
}
|
||||
store.addUserKeybinding(
|
||||
new KeybindingImpl({
|
||||
commandId: 'dirty.cmd',
|
||||
combo: { key: 'X', ctrl: true }
|
||||
})
|
||||
)
|
||||
|
||||
mockApi.storeUserData.mockResolvedValueOnce(new Response())
|
||||
|
||||
const targetPreset: KeybindingPreset = {
|
||||
name: 'vim',
|
||||
newBindings: [
|
||||
{ commandId: 'vim.cmd', combo: { key: 'J', ctrl: false } }
|
||||
],
|
||||
unsetBindings: []
|
||||
}
|
||||
mockApi.getUserData.mockResolvedValueOnce(
|
||||
new Response(JSON.stringify(targetPreset), { status: 200 })
|
||||
)
|
||||
|
||||
const service = await getPresetService()
|
||||
await service.switchPreset('vim')
|
||||
|
||||
expect(mockApi.storeUserData).toHaveBeenCalledWith(
|
||||
'keybindings/my-preset.json',
|
||||
expect.any(String),
|
||||
{ overwrite: true, stringify: false }
|
||||
)
|
||||
expect(store.currentPresetName).toBe('vim')
|
||||
})
|
||||
|
||||
it('cancels switch when dialog returns null', async () => {
|
||||
store.addUserKeybinding(
|
||||
new KeybindingImpl({
|
||||
commandId: 'dirty.cmd',
|
||||
combo: { key: 'X', ctrl: true }
|
||||
})
|
||||
)
|
||||
|
||||
mockShowSmallLayoutDialog.mockImplementationOnce(
|
||||
(options: Record<string, unknown>) => {
|
||||
const dialogComponentProps = options.dialogComponentProps as
|
||||
| Record<string, unknown>
|
||||
| undefined
|
||||
const onClose = dialogComponentProps?.onClose as
|
||||
| (() => void)
|
||||
| undefined
|
||||
onClose?.()
|
||||
}
|
||||
)
|
||||
|
||||
const service = await getPresetService()
|
||||
await service.switchPreset('vim')
|
||||
|
||||
expect(store.currentPresetName).toBe('default')
|
||||
expect(mockApi.getUserData).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('switches without dialog when preset is not modified', async () => {
|
||||
const targetPreset: KeybindingPreset = {
|
||||
name: 'vim',
|
||||
newBindings: [
|
||||
{ commandId: 'vim.cmd', combo: { key: 'J', ctrl: false } }
|
||||
],
|
||||
unsetBindings: []
|
||||
}
|
||||
mockApi.getUserData.mockResolvedValueOnce(
|
||||
new Response(JSON.stringify(targetPreset), { status: 200 })
|
||||
)
|
||||
|
||||
const service = await getPresetService()
|
||||
await service.switchPreset('vim')
|
||||
|
||||
expect(mockShowSmallLayoutDialog).not.toHaveBeenCalled()
|
||||
expect(store.currentPresetName).toBe('vim')
|
||||
})
|
||||
})
|
||||
|
||||
describe('promptAndSaveNewPreset', () => {
|
||||
it('returns false when user cancels prompt', async () => {
|
||||
mockPrompt.mockResolvedValueOnce(null)
|
||||
|
||||
const service = await getPresetService()
|
||||
const result = await service.promptAndSaveNewPreset()
|
||||
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false when user enters empty name', async () => {
|
||||
mockPrompt.mockResolvedValueOnce(' ')
|
||||
|
||||
const service = await getPresetService()
|
||||
const result = await service.promptAndSaveNewPreset()
|
||||
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it('saves successfully with valid name', async () => {
|
||||
mockApi.listUserDataFullInfo.mockResolvedValueOnce([])
|
||||
mockApi.storeUserData.mockResolvedValueOnce(new Response())
|
||||
|
||||
const service = await getPresetService()
|
||||
const result = await service.promptAndSaveNewPreset()
|
||||
|
||||
expect(result).toBe(true)
|
||||
expect(mockApi.storeUserData).toHaveBeenCalledWith(
|
||||
'keybindings/test-preset.json',
|
||||
expect.any(String),
|
||||
{ overwrite: true, stringify: false }
|
||||
)
|
||||
})
|
||||
|
||||
it('confirms overwrite when preset name already exists', async () => {
|
||||
mockApi.listUserDataFullInfo.mockResolvedValueOnce([
|
||||
{ path: 'test-preset.json', size: 100, modified: 123 }
|
||||
])
|
||||
mockApi.storeUserData.mockResolvedValueOnce(new Response())
|
||||
|
||||
const service = await getPresetService()
|
||||
const result = await service.promptAndSaveNewPreset()
|
||||
|
||||
expect(result).toBe(true)
|
||||
expect(mockConfirm).toHaveBeenCalled()
|
||||
expect(mockApi.storeUserData).toHaveBeenCalledWith(
|
||||
'keybindings/test-preset.json',
|
||||
expect.any(String),
|
||||
{ overwrite: true, stringify: false }
|
||||
)
|
||||
})
|
||||
|
||||
it('returns false when user rejects overwrite', async () => {
|
||||
mockApi.listUserDataFullInfo.mockResolvedValueOnce([
|
||||
{ path: 'test-preset.json', size: 100, modified: 123 }
|
||||
])
|
||||
mockConfirm.mockResolvedValueOnce(false)
|
||||
|
||||
const service = await getPresetService()
|
||||
const result = await service.promptAndSaveNewPreset()
|
||||
|
||||
expect(result).toBe(false)
|
||||
expect(mockApi.storeUserData).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('switchToDefaultPreset', () => {
|
||||
it('resets bindings and updates store and settings', async () => {
|
||||
store.addUserKeybinding(
|
||||
new KeybindingImpl({
|
||||
commandId: 'test.cmd',
|
||||
combo: { key: 'A', ctrl: true }
|
||||
})
|
||||
)
|
||||
store.currentPresetName = 'vim'
|
||||
store.savedPresetData = {
|
||||
name: 'vim',
|
||||
newBindings: [],
|
||||
unsetBindings: []
|
||||
}
|
||||
|
||||
const service = await getPresetService()
|
||||
await service.switchToDefaultPreset()
|
||||
|
||||
expect(Object.keys(store.getUserKeybindings())).toHaveLength(0)
|
||||
expect(store.currentPresetName).toBe('default')
|
||||
expect(store.savedPresetData).toBeNull()
|
||||
expect(mockPersistUserKeybindings).toHaveBeenCalled()
|
||||
expect(mockSettingSet).toHaveBeenCalledWith(
|
||||
'Comfy.Keybinding.CurrentPreset',
|
||||
'default'
|
||||
)
|
||||
})
|
||||
|
||||
it('does not reset bindings when resetBindings is false', async () => {
|
||||
store.addUserKeybinding(
|
||||
new KeybindingImpl({
|
||||
commandId: 'test.cmd',
|
||||
combo: { key: 'A', ctrl: true }
|
||||
})
|
||||
)
|
||||
store.currentPresetName = 'vim'
|
||||
|
||||
const service = await getPresetService()
|
||||
await service.switchToDefaultPreset({ resetBindings: false })
|
||||
|
||||
expect(Object.keys(store.getUserKeybindings())).toHaveLength(1)
|
||||
expect(store.currentPresetName).toBe('default')
|
||||
expect(store.savedPresetData).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('loadPreset error handling', () => {
|
||||
it('throws when API returns non-ok response', async () => {
|
||||
mockApi.getUserData.mockResolvedValueOnce(
|
||||
new Response(null, { status: 404 })
|
||||
)
|
||||
|
||||
const service = await getPresetService()
|
||||
await expect(service.loadPreset('missing')).rejects.toThrow(
|
||||
'g.keybindingPresets.loadPresetFailed'
|
||||
)
|
||||
})
|
||||
|
||||
it('throws when response contains invalid JSON', async () => {
|
||||
mockApi.getUserData.mockResolvedValueOnce(
|
||||
new Response('not-json{{{', { status: 200 })
|
||||
)
|
||||
|
||||
const service = await getPresetService()
|
||||
await expect(service.loadPreset('bad-json')).rejects.toThrow()
|
||||
})
|
||||
|
||||
it('throws when Zod validation fails', async () => {
|
||||
mockApi.getUserData.mockResolvedValueOnce(
|
||||
new Response(JSON.stringify({ name: 'valid', wrongField: true }), {
|
||||
status: 200
|
||||
})
|
||||
)
|
||||
|
||||
const service = await getPresetService()
|
||||
await expect(service.loadPreset('bad-schema')).rejects.toThrow(
|
||||
'g.keybindingPresets.invalidPresetFile'
|
||||
)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -121,7 +121,13 @@ export function useKeybindingPresetService() {
|
||||
for (const binding of preset.newBindings) {
|
||||
keybindingStore.addUserKeybinding(new KeybindingImpl(binding))
|
||||
}
|
||||
keybindingStore.savedPresetData = preset
|
||||
// Snapshot savedPresetData from the store's actual state after applying,
|
||||
// because addUserKeybinding may auto-unset conflicting defaults beyond
|
||||
// what the raw preset specifies.
|
||||
keybindingStore.savedPresetData = buildPresetFromStore(
|
||||
preset.name,
|
||||
keybindingStore
|
||||
)
|
||||
keybindingStore.currentPresetName = preset.name
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user