-
Notifications
You must be signed in to change notification settings - Fork 448
Refactor Queue UI: Inline Progress & Cleanups #7230
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.
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
Closed
Closed
Changes from 37 commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
71c6da2
Readd cancel button
benceruleanlu b790aaf
Add new N queued toggle
benceruleanlu e65fc56
Remove active state
benceruleanlu ca084c3
Fix text size
benceruleanlu 5c106bd
Add progress bar embedded into top menu section
benceruleanlu da4b241
Remove unused QueueOverlayActive files
benceruleanlu 5e1dc91
Extract to own component file
benceruleanlu d965fb0
Add progress bars and progress text
benceruleanlu 05d2cae
Add close button back to queue progress overlay
benceruleanlu 99019cf
Align expanded QPO to new design
benceruleanlu 0a5fc8b
Delete unused file
benceruleanlu c4b53c1
Make job list button visual seperation change
benceruleanlu 48aa3d9
Remove unused event
benceruleanlu 87abea9
Merge remote-tracking branch 'origin/main' into bl-semantic-jellyfish
benceruleanlu ad44241
[automated] Update test expectations
invalid-email-address 73741df
fix: lint issues and unused exports
benceruleanlu 8b4f174
refactor(actionbar): move cancel and queue buttons into draggable con…
benceruleanlu f0b9d28
fix(queue): rename clearQueued translation key to clearQueue
benceruleanlu b428892
Add tests
benceruleanlu ab820bf
Readd missing DTID
benceruleanlu f6ffb1f
Fix actionbar digit shifting
benceruleanlu db80292
Fix text expectation after digit shift change
benceruleanlu a5b5fe8
Merge remote-tracking branch 'origin/main' into bl-semantic-jellyfish
benceruleanlu a48360d
nit
benceruleanlu 96893f7
nit
benceruleanlu cbbf3eb
[automated] Update test expectations
invalid-email-address f7d3a8c
nit
benceruleanlu 8fdf355
nit
benceruleanlu 38b589c
nit
benceruleanlu 3ce8cca
nit
benceruleanlu 3fe2d39
nit
benceruleanlu ae1d6f9
nit
benceruleanlu b277cbf
nit
benceruleanlu b21cf86
Merge branch 'bl-semantic-jellyfish' of https://github.com/Comfy-Org/…
benceruleanlu 8bec2df
nit
benceruleanlu 268ab1d
Implement progress bar on actionbar
benceruleanlu 36fda56
Implement progress text on actionbar
benceruleanlu 33626a5
Fix menu position docked issue
benceruleanlu d963b8e
Widen drop zone
benceruleanlu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import type { Page } from '@playwright/test' | ||
| import { expect } from '@playwright/test' | ||
|
|
||
| export class QueueList { | ||
| constructor(public readonly page: Page) {} | ||
|
|
||
| get toggleButton() { | ||
| return this.page.getByTestId('queue-toggle-button') | ||
| } | ||
|
|
||
| get inlineProgress() { | ||
| return this.page.getByTestId('queue-inline-progress') | ||
| } | ||
|
|
||
| get overlay() { | ||
| return this.page.getByTestId('queue-overlay') | ||
| } | ||
|
|
||
| get closeButton() { | ||
| return this.page.getByTestId('queue-overlay-close-button') | ||
| } | ||
|
|
||
| get jobItems() { | ||
| return this.page.getByTestId('queue-job-item') | ||
| } | ||
|
|
||
| get clearHistoryButton() { | ||
| return this.page.getByRole('button', { name: /Clear History/i }) | ||
| } | ||
|
|
||
| async open() { | ||
| if (!(await this.overlay.isVisible())) { | ||
| await this.toggleButton.click() | ||
| await expect(this.overlay).toBeVisible() | ||
| } | ||
| } | ||
|
|
||
| async close() { | ||
| if (await this.overlay.isVisible()) { | ||
| await this.closeButton.click() | ||
| await expect(this.overlay).not.toBeVisible() | ||
| } | ||
| } | ||
|
|
||
| async getJobCount(state?: string) { | ||
| if (state) { | ||
| return await this.page | ||
| .locator(`[data-testid="queue-job-item"][data-job-state="${state}"]`) | ||
| .count() | ||
| } | ||
| return await this.jobItems.count() | ||
| } | ||
|
|
||
| getJobAction(actionKey: string) { | ||
| return this.page.getByTestId(`job-action-${actionKey}`) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file modified
BIN
+3.5 KB
(100%)
...sts/tests/interaction.spec.ts-snapshots/prompt-dialog-closed-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| import { expect, mergeTests } from '@playwright/test' | ||
|
|
||
| import type { ComfyPage } from '../../fixtures/ComfyPage' | ||
| import { comfyPageFixture } from '../../fixtures/ComfyPage' | ||
| import { webSocketFixture } from '../../fixtures/ws' | ||
| import type { WsMessage } from '../../fixtures/ws' | ||
|
|
||
| const test = mergeTests(comfyPageFixture, webSocketFixture) | ||
|
|
||
| type QueueState = { | ||
| running: QueueJob[] | ||
| pending: QueueJob[] | ||
| } | ||
|
|
||
| type QueueJob = [ | ||
| string, | ||
| string, | ||
| Record<string, unknown>, | ||
| Record<string, unknown>, | ||
| string[] | ||
| ] | ||
|
|
||
| type QueueController = { | ||
| state: QueueState | ||
| sync: ( | ||
| ws: { trigger(data: WsMessage, url?: string): Promise<void> }, | ||
| nextState: Partial<QueueState> | ||
| ) => Promise<void> | ||
| } | ||
|
|
||
| test.describe('Queue UI', () => { | ||
| let queue: QueueController | ||
|
|
||
| test.beforeEach(async ({ comfyPage }) => { | ||
| await comfyPage.page.route('**/api/prompt', async (route) => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ | ||
| prompt_id: 'mock-prompt-id', | ||
| number: 1, | ||
| node_errors: {} | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| // Mock history to avoid pulling real data | ||
| await comfyPage.page.route('**/api/history**', async (route) => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ History: [] }) | ||
| }) | ||
| }) | ||
|
|
||
| queue = await createQueueController(comfyPage) | ||
| }) | ||
|
|
||
| test('toggles overlay and updates count from status events', async ({ | ||
| comfyPage, | ||
| ws | ||
| }) => { | ||
| await queue.sync(ws, { running: [], pending: [] }) | ||
|
|
||
| await expect(comfyPage.queueList.toggleButton).toContainText('0') | ||
| await expect(comfyPage.queueList.toggleButton).toContainText(/queued/i) | ||
| await expect(comfyPage.queueList.overlay).toBeHidden() | ||
|
|
||
| await queue.sync(ws, { | ||
| pending: [queueJob('1', 'mock-pending', 'client-a')] | ||
| }) | ||
|
|
||
| await expect(comfyPage.queueList.toggleButton).toContainText('1') | ||
| await expect(comfyPage.queueList.toggleButton).toContainText(/queued/i) | ||
|
|
||
| await comfyPage.queueList.open() | ||
| await expect(comfyPage.queueList.overlay).toBeVisible() | ||
| await expect(comfyPage.queueList.jobItems).toHaveCount(1) | ||
|
|
||
| await comfyPage.queueList.close() | ||
| await expect(comfyPage.queueList.overlay).toBeHidden() | ||
| }) | ||
|
|
||
| test('displays running and pending jobs via status updates', async ({ | ||
| comfyPage, | ||
| ws | ||
| }) => { | ||
| await queue.sync(ws, { | ||
| running: [queueJob('2', 'mock-running', 'client-b')], | ||
| pending: [queueJob('3', 'mock-pending', 'client-c')] | ||
| }) | ||
|
|
||
| await comfyPage.queueList.open() | ||
| await expect(comfyPage.queueList.jobItems).toHaveCount(2) | ||
|
|
||
| const firstJob = comfyPage.queueList.jobItems.first() | ||
| await firstJob.hover() | ||
|
|
||
| const cancelAction = firstJob | ||
| .getByTestId('job-action-cancel-running') | ||
| .or(firstJob.getByTestId('job-action-cancel-hover')) | ||
|
|
||
| await expect(cancelAction).toBeVisible() | ||
| }) | ||
| }) | ||
|
|
||
| const queueJob = ( | ||
| queueIndex: string, | ||
| promptId: string, | ||
| clientId: string | ||
| ): QueueJob => [ | ||
| queueIndex, | ||
| promptId, | ||
| { client_id: clientId }, | ||
| { class_type: 'Note' }, | ||
| ['output'] | ||
| ] | ||
|
|
||
| const createQueueController = async ( | ||
| comfyPage: ComfyPage | ||
| ): Promise<QueueController> => { | ||
| const state: QueueState = { running: [], pending: [] } | ||
|
|
||
| // Single queue handler reads the latest in-memory state | ||
| await comfyPage.page.route('**/api/queue', async (route) => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ | ||
| queue_running: state.running, | ||
| queue_pending: state.pending | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| const sync = async ( | ||
| ws: { trigger(data: WsMessage, url?: string): Promise<void> }, | ||
| nextState: Partial<QueueState> | ||
| ) => { | ||
| if (nextState.running) state.running = nextState.running | ||
| if (nextState.pending) state.pending = nextState.pending | ||
|
|
||
| const total = state.running.length + state.pending.length | ||
| const queueResponse = comfyPage.page.waitForResponse('**/api/queue') | ||
|
|
||
| await ws.trigger({ | ||
| type: 'status', | ||
| data: { | ||
| status: { exec_info: { queue_remaining: total } } | ||
| } | ||
| }) | ||
|
|
||
| await queueResponse | ||
| } | ||
|
|
||
| return { state, sync } | ||
| } | ||
Binary file modified
BIN
+831 Bytes
(100%)
...odes/groups/groups.spec.ts-snapshots/vue-groups-create-group-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+1.03 KB
(100%)
...s/groups/groups.spec.ts-snapshots/vue-groups-fit-to-contents-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+17 Bytes
(100%)
...canvas/pan.spec.ts-snapshots/vue-nodes-paned-with-touch-mobile-chrome-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+935 Bytes
(100%)
...eractions/canvas/zoom.spec.ts-snapshots/zoomed-in-ctrl-shift-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+720 Bytes
(100%)
...nks/linkInteraction.spec.ts-snapshots/vue-node-dragging-link-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+892 Bytes
(100%)
...nkInteraction.spec.ts-snapshots/vue-node-input-drag-ctrl-alt-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+919 Bytes
(100%)
...eraction.spec.ts-snapshots/vue-node-input-drag-reuses-origin-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+878 Bytes
(100%)
...inkInteraction.spec.ts-snapshots/vue-node-reroute-input-drag-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+802 Bytes
(100%)
...raction.spec.ts-snapshots/vue-node-reroute-output-shift-drag-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+896 Bytes
(100%)
...teraction.spec.ts-snapshots/vue-node-shift-output-multi-link-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+840 Bytes
(100%)
...inks/linkInteraction.spec.ts-snapshots/vue-node-snap-to-node-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+913 Bytes
(100%)
...inks/linkInteraction.spec.ts-snapshots/vue-node-snap-to-slot-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+932 Bytes
(100%)
...interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+22 Bytes
(100%)
...s/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+920 Bytes
(100%)
.../nodeStates/bypass.spec.ts-snapshots/vue-node-bypassed-state-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+945 Bytes
(100%)
...deStates/colors.spec.ts-snapshots/vue-node-custom-color-blue-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+854 Bytes
(100%)
...ors.spec.ts-snapshots/vue-node-custom-colors-dark-all-colors-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+927 Bytes
(100%)
...rs.spec.ts-snapshots/vue-node-custom-colors-light-all-colors-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+876 Bytes
(100%)
...Nodes/nodeStates/mute.spec.ts-snapshots/vue-node-muted-state-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+633 Bytes
(100%)
...oad/uploadWidgets.spec.ts-snapshots/vue-nodes-upload-widgets-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Good coverage of overlay toggle and count; consider asserting inline progress as well
This test validates the count text, overlay visibility, and item rendering end‑to‑end via mocked WS +
/api/queue, which is great. To more directly cover the new inline progress indicator you might also assert on whatever elementqueueList.inlineProgressexposes (e.g., its visible count or label) so regressions in the top‑menu indicator itself are caught, not just the toggle button text.🤖 Prompt for AI Agents