Skip to content

Replace QPO with opening assets tab #8260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
christian-byrne merged 5 commits into from
Jan 24, 2026
Merged

Replace QPO with opening assets tab #8260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c74a03
fix: open assets tab when QPO V2 enabled
benceruleanlu Jan 23, 2026
9fb11a1
fix: toggle assets tab from queue button
benceruleanlu Jan 23, 2026
fd07245
Merge remote-tracking branch 'origin/main' into fix/qpo-v2-queue-butt…
benceruleanlu Jan 23, 2026
7609554
test: cover queue button behavior by QPO setting
benceruleanlu Jan 23, 2026
f8dfaa3
fix: rename misleading queue overlay flag to isQueueProgressOverlayEn…
Jan 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 70 additions & 2 deletions src/components/TopMenuSection.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import type {
JobListItem,
JobStatus
} from '@/platform/remote/comfyui/jobs/jobTypes'
import { useSettingStore } from '@/platform/settings/settingStore'
import { useCommandStore } from '@/stores/commandStore'
import { TaskItemImpl, useQueueStore } from '@/stores/queueStore'
import { useSidebarTabStore } from '@/stores/workspace/sidebarTabStore'
import { isElectron } from '@/utils/envUtil'

const mockData = vi.hoisted(() => ({ isLoggedIn: false }))
Expand All @@ -33,7 +36,7 @@ vi.mock('@/stores/firebaseAuthStore', () => ({
}))
}))

function createWrapper() {
function createWrapper(pinia = createTestingPinia({ createSpy: vi.fn })) {
const i18n = createI18n({
legacy: false,
locale: 'en',
Expand All @@ -53,7 +56,7 @@ function createWrapper() {

return mount(TopMenuSection, {
global: {
plugins: [createTestingPinia({ createSpy: vi.fn }), i18n],
plugins: [pinia, i18n],
stubs: {
SubgraphBreadcrumb: true,
QueueProgressOverlay: true,
Expand Down Expand Up @@ -142,6 +145,71 @@ describe('TopMenuSection', () => {
expect(queueButton.text()).toContain('3 active')
})

it('hides queue progress overlay when QPO V2 is enabled', async () => {
const pinia = createTestingPinia({ createSpy: vi.fn })
const settingStore = useSettingStore(pinia)
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? true : undefined
)
const wrapper = createWrapper(pinia)

await nextTick()

expect(wrapper.find('[data-testid="queue-overlay-toggle"]').exists()).toBe(
true
)
expect(
wrapper.findComponent({ name: 'QueueProgressOverlay' }).exists()
).toBe(false)
})

it('toggles the queue progress overlay when QPO V2 is disabled', async () => {
const pinia = createTestingPinia({ createSpy: vi.fn, stubActions: false })
const settingStore = useSettingStore(pinia)
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? false : undefined
)
const wrapper = createWrapper(pinia)
const commandStore = useCommandStore(pinia)

await wrapper.find('[data-testid="queue-overlay-toggle"]').trigger('click')

expect(commandStore.execute).toHaveBeenCalledWith(
'Comfy.Queue.ToggleOverlay'
)
})
Comment on lines +166 to +180
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using default stubbed actions for this test.

This test uses stubActions: false, but since you're only verifying that execute was called (not testing the command's actual behavior), the default stubbed actions would be sufficient and simpler. The current approach works because the component doesn't await commandStore.execute, but it could be cleaner.

Suggested simplification 
-  it('toggles the queue progress overlay when QPO V2 is disabled', async () => {
-    const pinia = createTestingPinia({ createSpy: vi.fn, stubActions: false })
+  it('executes toggle overlay command when QPO V2 is disabled', async () => {
+    const pinia = createTestingPinia({ createSpy: vi.fn })
     const settingStore = useSettingStore(pinia)
     vi.mocked(settingStore.get).mockImplementation((key) =>
       key === 'Comfy.Queue.QPOV2' ? false : undefined
     )
     const wrapper = createWrapper(pinia)
     const commandStore = useCommandStore(pinia)

     await wrapper.find('[data-testid="queue-overlay-toggle"]').trigger('click')

     expect(commandStore.execute).toHaveBeenCalledWith(
       'Comfy.Queue.ToggleOverlay'
     )
   })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('toggles the queue progress overlay when QPO V2 is disabled', async () => {
const pinia = createTestingPinia({ createSpy: vi.fn, stubActions: false })
const settingStore = useSettingStore(pinia)
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? false : undefined
)
const wrapper = createWrapper(pinia)
const commandStore = useCommandStore(pinia)
await wrapper.find('[data-testid="queue-overlay-toggle"]').trigger('click')
expect(commandStore.execute).toHaveBeenCalledWith(
'Comfy.Queue.ToggleOverlay'
)
})
it('executes toggle overlay command when QPO V2 is disabled', async () => {
const pinia = createTestingPinia({ createSpy: vi.fn })
const settingStore = useSettingStore(pinia)
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? false : undefined
)
const wrapper = createWrapper(pinia)
const commandStore = useCommandStore(pinia)
await wrapper.find('[data-testid="queue-overlay-toggle"]').trigger('click')
expect(commandStore.execute).toHaveBeenCalledWith(
'Comfy.Queue.ToggleOverlay'
)
})
🤖 Prompt for AI Agents 
In `@src/components/TopMenuSection.test.ts` around lines 166 - 180, The test
unnecessarily sets stubActions: false when it only asserts that
commandStore.execute was called; change createTestingPinia({ createSpy: vi.fn,
stubActions: false }) to use the default stubbed actions (remove or set
stubActions: true) so the commandStore.execute is a stubbed spy and you don't
need to mock or rely on real action behavior—update the call site where
createTestingPinia is used in this test (and keep usage of useSettingStore and
useCommandStore as-is) to simplify the test.


it('opens the assets sidebar tab when QPO V2 is enabled', async () => {
const pinia = createTestingPinia({ createSpy: vi.fn, stubActions: false })
const settingStore = useSettingStore(pinia)
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? true : undefined
)
const wrapper = createWrapper(pinia)
const sidebarTabStore = useSidebarTabStore(pinia)

await wrapper.find('[data-testid="queue-overlay-toggle"]').trigger('click')

expect(sidebarTabStore.activeSidebarTabId).toBe('assets')
})

it('toggles the assets sidebar tab when QPO V2 is enabled', async () => {
const pinia = createTestingPinia({ createSpy: vi.fn, stubActions: false })
const settingStore = useSettingStore(pinia)
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? true : undefined
)
const wrapper = createWrapper(pinia)
const sidebarTabStore = useSidebarTabStore(pinia)
const toggleButton = wrapper.find('[data-testid="queue-overlay-toggle"]')
Comment on lines +189 to +204
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While tests are added for the QPO V2 enabled behavior, there's no test coverage for the original behavior when QPO V2 is disabled. Add a test that verifies the command 'Comfy.Queue.ToggleOverlay' is executed when the button is clicked and QPO V2 is disabled.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@benceruleanlu benceruleanlu Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved


await toggleButton.trigger('click')
expect(sidebarTabStore.activeSidebarTabId).toBe('assets')

await toggleButton.trigger('click')
expect(sidebarTabStore.activeSidebarTabId).toBe(null)
})

it('disables the clear queue context menu item when no queued jobs exist', () => {
const wrapper = createWrapper()
const menu = wrapper.findComponent({ name: 'ContextMenu' })
Expand Down
28 changes: 26 additions & 2 deletions src/components/TopMenuSection.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@
v-tooltip.bottom="queueHistoryTooltipConfig"
type="destructive"
size="md"
:aria-pressed="isQueueOverlayExpanded"
:aria-pressed="
isQueuePanelV2Enabled
? activeSidebarTabId === 'assets'
: isQueueProgressOverlayEnabled
? isQueueOverlayExpanded
: undefined
"
class="px-3"
data-testid="queue-overlay-toggle"
@click="toggleQueueOverlay"
Expand All @@ -55,7 +61,11 @@
{{ activeJobsLabel }}
</span>
<span class="sr-only">
{{ t('sideToolbar.queueProgressOverlay.expandCollapsedQueue') }}
{{
isQueuePanelV2Enabled
? t('sideToolbar.queueProgressOverlay.viewJobHistory')
: t('sideToolbar.queueProgressOverlay.expandCollapsedQueue')
}}
</span>
</Button>
<ContextMenu ref="queueContextMenu" :model="queueContextMenuItems" />
Expand All @@ -77,6 +87,7 @@
</div>
</div>
<QueueProgressOverlay
v-if="isQueueProgressOverlayEnabled"
v-model:expanded="isQueueOverlayExpanded"
:menu-hovered="isTopMenuHovered"
/>
Expand Down Expand Up @@ -108,6 +119,7 @@ import { useCommandStore } from '@/stores/commandStore'
import { useExecutionStore } from '@/stores/executionStore'
import { useQueueStore, useQueueUIStore } from '@/stores/queueStore'
import { useRightSidePanelStore } from '@/stores/workspace/rightSidePanelStore'
import { useSidebarTabStore } from '@/stores/workspace/sidebarTabStore'
import { useWorkspaceStore } from '@/stores/workspaceStore'
import { isElectron } from '@/utils/envUtil'
import { useConflictAcknowledgment } from '@/workbench/extensions/manager/composables/useConflictAcknowledgment'
Expand All @@ -126,8 +138,10 @@ const commandStore = useCommandStore()
const queueStore = useQueueStore()
const executionStore = useExecutionStore()
const queueUIStore = useQueueUIStore()
const sidebarTabStore = useSidebarTabStore()
const { activeJobsCount } = storeToRefs(queueStore)
const { isOverlayExpanded: isQueueOverlayExpanded } = storeToRefs(queueUIStore)
const { activeSidebarTabId } = storeToRefs(sidebarTabStore)
const releaseStore = useReleaseStore()
const { shouldShowRedDot: showReleaseRedDot } = storeToRefs(releaseStore)
const { shouldShowRedDot: shouldShowConflictRedDot } =
Expand All @@ -144,6 +158,12 @@ const activeJobsLabel = computed(() => {
const isIntegratedTabBar = computed(
() => settingStore.get('Comfy.UI.TabBarLayout') === 'Integrated'
)
const isQueuePanelV2Enabled = computed(() =>
settingStore.get('Comfy.Queue.QPOV2')
)
const isQueueProgressOverlayEnabled = computed(
() => !isQueuePanelV2Enabled.value
)
const queueHistoryTooltipConfig = computed(() =>
buildTooltipConfig(t('sideToolbar.queueProgressOverlay.viewJobHistory'))
)
Expand Down Expand Up @@ -185,6 +205,10 @@ onMounted(() => {
})

const toggleQueueOverlay = () => {
if (isQueuePanelV2Enabled.value) {
sidebarTabStore.toggleSidebarTab('assets')
return
}
commandStore.execute('Comfy.Queue.ToggleOverlay')
}

Expand Down