Skip to content

Commit

Permalink
Migrate deprecated setting values (#954)
Browse files Browse the repository at this point in the history
  • Loading branch information
huchenlei committed Sep 24, 2024
1 parent db610ae commit 83c7fed
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 2 deletions.
19 changes: 19 additions & 0 deletions browser_tests/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,25 @@ test.describe('Menu', () => {
})
})

// Only test 'Top' to reduce test time.
// ['Bottom', 'Top']
;['Top'].forEach(async (position) => {
test(`Can migrate deprecated menu positions (${position})`, async ({
comfyPage
}) => {
await comfyPage.setSetting('Comfy.UseNewMenu', position)
expect(await comfyPage.getSetting('Comfy.UseNewMenu')).toBe('Floating')
})

test(`Can migrate deprecated menu positions on initial load (${position})`, async ({
comfyPage
}) => {
await comfyPage.setSetting('Comfy.UseNewMenu', position)
await comfyPage.setup()
expect(await comfyPage.getSetting('Comfy.UseNewMenu')).toBe('Floating')
})
})

test('Can change canvas zoom speed setting', async ({ comfyPage }) => {
const [defaultSpeed, maxSpeed] = [1.1, 2.5]
expect(await comfyPage.getSetting('Comfy.Graph.ZoomSpeed')).toBe(
Expand Down
20 changes: 19 additions & 1 deletion src/scripts/ui/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ export class ComfySettingsDialog extends ComfyDialog<HTMLDialogElement> {
return Object.values(this.settingsLookup)
}

private tryMigrateDeprecatedValue(id: string, value: any) {
if (this.app.vueAppReady) {
const settingStore = useSettingStore()
const setting = settingStore.settings[id]
if (setting?.migrateDeprecatedValue) {
return setting.migrateDeprecatedValue(value)
}
}
return value
}

#dispatchChange<T>(id: string, value: T, oldValue?: T) {
// Keep the settingStore updated. Not using `store.set` as it would trigger
// setSettingValue again.
Expand Down Expand Up @@ -86,7 +97,12 @@ export class ComfySettingsDialog extends ComfyDialog<HTMLDialogElement> {

// Trigger onChange for any settings added before load
for (const id in this.settingsLookup) {
const value = this.settingsValues[this.getId(id)]
const compatId = this.getId(id)
this.settingsValues[compatId] = this.tryMigrateDeprecatedValue(
id,
this.settingsValues[compatId]
)
const value = this.settingsValues[compatId]
this.settingsLookup[id].onChange?.(value)
this.#dispatchChange(id, value)
}
Expand Down Expand Up @@ -123,6 +139,8 @@ export class ComfySettingsDialog extends ComfyDialog<HTMLDialogElement> {
id: K,
value: Settings[K]
) {
value = this.tryMigrateDeprecatedValue(id, value)

const json = JSON.stringify(value)
localStorage['Comfy.Settings.' + id] = json // backwards compatibility for extensions keep setting in storage

Expand Down
8 changes: 7 additions & 1 deletion src/stores/coreSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,13 @@ export const CORE_SETTINGS: SettingParams[] = [
name: 'Use new menu and workflow management.',
experimental: true,
type: 'combo',
options: ['Disabled', 'Floating']
options: ['Disabled', 'Floating'],
migrateDeprecatedValue: (value: string) => {
if (['Top', 'Bottom'].includes(value)) {
return 'Floating'
}
return value
}
},
{
id: 'Comfy.Workflow.WorkflowTabsPosition',
Expand Down
2 changes: 2 additions & 0 deletions src/types/settingTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,6 @@ export interface SettingParams {
category?: string[]
experimental?: boolean
deprecated?: boolean
// Deprecated values are mapped to new values.
migrateDeprecatedValue?: (value: any) => any
}

0 comments on commit 83c7fed

Please sign in to comment.