mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-02 03:30:04 +00:00
fix: make docked job history toggle persistence-safe (#9265)
## Summary Follow-up to #9215 to keep Docked Job History toggle behavior deterministic even when settings persistence fails. ## Changes - Close the actions popover immediately when toggling Docked Job History. - Use settingStore.setMany(...) when switching from docked to floating mode. - Set sidebarTabStore.activeSidebarTabId = 'job-history' before persisting when switching from floating to docked mode. - Wrap persistence calls in try/catch without rollback so locally-applied UI state remains deterministic. - Expand QueueOverlayHeader tests to cover setMany, popover close behavior, and persistence-failure paths. ## Testing - pnpm test:unit -- src/components/queue/QueueOverlayHeader.test.ts - pnpm typecheck - pnpm exec eslint src/components/queue/JobHistoryActionsMenu.vue src/components/queue/QueueOverlayHeader.test.ts - pnpm lint (fails in this branch due pre-existing stylelint errors in generated apps/desktop-ui/dist/**/*.css files, unrelated to this change) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9265-fix-make-docked-job-history-toggle-persistence-safe-3146d73d3650818f86c4dfdd57669abd) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -20,7 +20,7 @@
|
||||
class="w-full justify-between text-sm font-light"
|
||||
variant="textonly"
|
||||
size="sm"
|
||||
@click="onToggleDockedJobHistory"
|
||||
@click="onToggleDockedJobHistory(close)"
|
||||
>
|
||||
<span class="flex items-center gap-2">
|
||||
<i
|
||||
@@ -100,14 +100,22 @@ const onClearHistoryFromMenu = (close: () => void) => {
|
||||
emit('clearHistory')
|
||||
}
|
||||
|
||||
const onToggleDockedJobHistory = async () => {
|
||||
if (isQueuePanelV2Enabled.value) {
|
||||
await settingStore.set('Comfy.Queue.QPOV2', false)
|
||||
await settingStore.set('Comfy.Queue.History.Expanded', true)
|
||||
const onToggleDockedJobHistory = async (close: () => void) => {
|
||||
close()
|
||||
|
||||
try {
|
||||
if (isQueuePanelV2Enabled.value) {
|
||||
await settingStore.setMany({
|
||||
'Comfy.Queue.QPOV2': false,
|
||||
'Comfy.Queue.History.Expanded': true
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
sidebarTabStore.activeSidebarTabId = 'job-history'
|
||||
await settingStore.set('Comfy.Queue.QPOV2', true)
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
|
||||
await settingStore.set('Comfy.Queue.QPOV2', true)
|
||||
sidebarTabStore.activeSidebarTabId = 'job-history'
|
||||
}
|
||||
</script>
|
||||
|
||||
@@ -27,6 +27,7 @@ const mockGetSetting = vi.fn<(key: string) => boolean | undefined>((key) =>
|
||||
key === 'Comfy.Queue.QPOV2' ? true : undefined
|
||||
)
|
||||
const mockSetSetting = vi.fn()
|
||||
const mockSetMany = vi.fn()
|
||||
const mockSidebarTabStore = {
|
||||
activeSidebarTabId: null as string | null
|
||||
}
|
||||
@@ -34,7 +35,8 @@ const mockSidebarTabStore = {
|
||||
vi.mock('@/platform/settings/settingStore', () => ({
|
||||
useSettingStore: () => ({
|
||||
get: mockGetSetting,
|
||||
set: mockSetSetting
|
||||
set: mockSetSetting,
|
||||
setMany: mockSetMany
|
||||
})
|
||||
}))
|
||||
|
||||
@@ -88,6 +90,7 @@ describe('QueueOverlayHeader', () => {
|
||||
beforeEach(() => {
|
||||
popoverCloseSpy.mockClear()
|
||||
mockSetSetting.mockClear()
|
||||
mockSetMany.mockClear()
|
||||
mockSidebarTabStore.activeSidebarTabId = null
|
||||
mockGetSetting.mockImplementation((key: string) =>
|
||||
key === 'Comfy.Queue.QPOV2' ? true : undefined
|
||||
@@ -144,17 +147,13 @@ describe('QueueOverlayHeader', () => {
|
||||
)
|
||||
await dockedJobHistoryButton.trigger('click')
|
||||
|
||||
expect(mockSetSetting).toHaveBeenCalledTimes(2)
|
||||
expect(mockSetSetting).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
'Comfy.Queue.QPOV2',
|
||||
false
|
||||
)
|
||||
expect(mockSetSetting).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
'Comfy.Queue.History.Expanded',
|
||||
true
|
||||
)
|
||||
expect(popoverCloseSpy).toHaveBeenCalledTimes(1)
|
||||
expect(mockSetMany).toHaveBeenCalledTimes(1)
|
||||
expect(mockSetMany).toHaveBeenCalledWith({
|
||||
'Comfy.Queue.QPOV2': false,
|
||||
'Comfy.Queue.History.Expanded': true
|
||||
})
|
||||
expect(mockSetSetting).not.toHaveBeenCalled()
|
||||
expect(mockSidebarTabStore.activeSidebarTabId).toBe(null)
|
||||
})
|
||||
|
||||
@@ -169,8 +168,43 @@ describe('QueueOverlayHeader', () => {
|
||||
)
|
||||
await dockedJobHistoryButton.trigger('click')
|
||||
|
||||
expect(popoverCloseSpy).toHaveBeenCalledTimes(1)
|
||||
expect(mockSetSetting).toHaveBeenCalledTimes(1)
|
||||
expect(mockSetSetting).toHaveBeenCalledWith('Comfy.Queue.QPOV2', true)
|
||||
expect(mockSetMany).not.toHaveBeenCalled()
|
||||
expect(mockSidebarTabStore.activeSidebarTabId).toBe('job-history')
|
||||
})
|
||||
|
||||
it('keeps docked target open even when enabling persistence fails', async () => {
|
||||
mockGetSetting.mockImplementation((key: string) =>
|
||||
key === 'Comfy.Queue.QPOV2' ? false : undefined
|
||||
)
|
||||
mockSetSetting.mockRejectedValueOnce(new Error('persistence failed'))
|
||||
const wrapper = mountHeader()
|
||||
|
||||
const dockedJobHistoryButton = wrapper.get(
|
||||
'[data-testid="docked-job-history-action"]'
|
||||
)
|
||||
await dockedJobHistoryButton.trigger('click')
|
||||
|
||||
expect(popoverCloseSpy).toHaveBeenCalledTimes(1)
|
||||
expect(mockSetSetting).toHaveBeenCalledWith('Comfy.Queue.QPOV2', true)
|
||||
expect(mockSidebarTabStore.activeSidebarTabId).toBe('job-history')
|
||||
})
|
||||
|
||||
it('closes the menu when disabling persistence fails', async () => {
|
||||
mockSetMany.mockRejectedValueOnce(new Error('persistence failed'))
|
||||
const wrapper = mountHeader()
|
||||
|
||||
const dockedJobHistoryButton = wrapper.get(
|
||||
'[data-testid="docked-job-history-action"]'
|
||||
)
|
||||
await dockedJobHistoryButton.trigger('click')
|
||||
|
||||
expect(popoverCloseSpy).toHaveBeenCalledTimes(1)
|
||||
expect(mockSetMany).toHaveBeenCalledWith({
|
||||
'Comfy.Queue.QPOV2': false,
|
||||
'Comfy.Queue.History.Expanded': true
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user