📝 CodeRabbit Chat: Implement requested code changes

browser_tests/fixtures/ComfyPage.ts

@@ -18,6 +18,7 @@ import { ComfyNodeSearchBoxV2 } from './components/ComfyNodeSearchBoxV2'
 import { ContextMenu } from './components/ContextMenu'
 import { SettingDialog } from './components/SettingDialog'
 import { BottomPanel } from './components/BottomPanel'
+import { ConfirmDialog } from './components/ConfirmDialog'
 import { QueuePanel } from './components/QueuePanel'
 import {
   NodeLibrarySidebarTab,
@@ -39,7 +40,6 @@ import { SubgraphHelper } from './helpers/SubgraphHelper'
 import { ToastHelper } from './helpers/ToastHelper'
 import { WorkflowHelper } from './helpers/WorkflowHelper'
 import type { NodeReference } from './utils/litegraphUtils'
-import type { WorkspaceStore } from '../types/globals'
 
 dotenvConfig()
 
@@ -112,48 +112,6 @@ class ComfyMenu {
   }
 }
 
-type KeysOfType<T, Match> = {
-  [K in keyof T]: T[K] extends Match ? K : never
-}[keyof T]
-
-class ConfirmDialog {
-  private readonly root: Locator
-  public readonly delete: Locator
-  public readonly overwrite: Locator
-  public readonly reject: Locator
-  public readonly confirm: Locator
-
-  constructor(public readonly page: Page) {
-    this.root = page.getByRole('dialog')
-    this.delete = this.root.getByRole('button', { name: 'Delete' })
-    this.overwrite = this.root.getByRole('button', { name: 'Overwrite' })
-    this.reject = this.root.getByRole('button', { name: 'Cancel' })
-    this.confirm = this.root.getByRole('button', { name: 'Confirm' })
-  }
-
-  async click(locator: KeysOfType<ConfirmDialog, Locator>) {
-    const loc = this[locator]
-    await loc.waitFor({ state: 'visible' })
-    await loc.click()
-
-    // Wait for the dialog mask to disappear after confirming
-    const mask = this.page.locator('.p-dialog-mask')
-    const count = await mask.count()
-    if (count > 0) {
-      await mask.first().waitFor({ state: 'hidden', timeout: 3000 })
-    }
-
-    // Wait for workflow service to finish if it's busy
-    await this.page.waitForFunction(
-      () =>
-        (window.app?.extensionManager as WorkspaceStore | undefined)?.workflow
-          ?.isBusy === false,
-      undefined,
-      { timeout: 3000 }
-    )
-  }
-}
-
 export class ComfyPage {
   public readonly url: string
   // All canvas position operations are based on default view of canvas.
@@ -513,4 +471,4 @@ export const comfyExpect = expect.extend({
       message: () => `Expected element to ${isFocused ? 'not ' : ''}be focused.`
     }
   }
-})
+})

browser_tests/fixtures/components/ConfirmDialog.ts

@@ -0,0 +1,64 @@
+import type { Locator, Page } from '@playwright/test'
+
+import type { WorkspaceStore } from '../../types/globals'
+
+type KeysOfType<T, Match> = {
+  [K in keyof T]: T[K] extends Match ? K : never
+}[keyof T]
+
+/**
+ * Page object for the generic confirm dialog shown via `dialogService.confirm()`.
+ *
+ * Accessible on `comfyPage.confirmDialog`.
+ */
+export class ConfirmDialog {
+  readonly root: Locator
+  readonly delete: Locator
+  readonly overwrite: Locator
+  /** Cancel / reject button */
+  readonly reject: Locator
+  /** Primary confirm button */
+  readonly confirm: Locator
+
+  constructor(public readonly page: Page) {
+    this.root = page.getByRole('dialog')
+    this.delete = this.root.getByRole('button', { name: 'Delete' })
+    this.overwrite = this.root.getByRole('button', { name: 'Overwrite' })
+    this.reject = this.root.getByRole('button', { name: 'Cancel' })
+    this.confirm = this.root.getByRole('button', { name: 'Confirm' })
+  }
+
+  async isVisible(): Promise<boolean> {
+    return this.root.isVisible()
+  }
+
+  async waitForVisible(): Promise<void> {
+    await this.root.waitFor({ state: 'visible' })
+  }
+
+  async waitForHidden(): Promise<void> {
+    await this.root.waitFor({ state: 'hidden' })
+  }
+
+  async click(locator: KeysOfType<ConfirmDialog, Locator>) {
+    const loc = this[locator]
+    await loc.waitFor({ state: 'visible' })
+    await loc.click()
+
+    // Wait for the dialog mask to disappear after confirming
+    const mask = this.page.locator('.p-dialog-mask')
+    const count = await mask.count()
+    if (count > 0) {
+      await mask.first().waitFor({ state: 'hidden', timeout: 3000 })
+    }
+
+    // Wait for workflow service to finish if it's busy
+    await this.page.waitForFunction(
+      () =>
+        (window.app?.extensionManager as WorkspaceStore | undefined)?.workflow
+          ?.isBusy === false,
+      undefined,
+      { timeout: 3000 }
+    )
+  }
+}

browser_tests/fixtures/components/QueuePanel.ts

@@ -3,13 +3,45 @@ import type { Locator, Page } from '@playwright/test'
 import { comfyExpect as expect } from '../ComfyPage'
 import { TestIds } from '../selectors'
 
+/**
+ * Page object for the "Clear queue history?" confirmation dialog that opens
+ * from the queue panel's history actions menu.
+ */
+export class QueueClearHistoryDialog {
+  readonly root: Locator
+  readonly cancelButton: Locator
+  readonly clearButton: Locator
+  readonly closeButton: Locator
+
+  constructor(public readonly page: Page) {
+    this.root = page.getByRole('dialog')
+    this.cancelButton = this.root.getByRole('button', { name: 'Cancel' })
+    this.clearButton = this.root.getByRole('button', { name: 'Clear' })
+    this.closeButton = this.root.getByLabel('Close')
+  }
+
+  async isVisible(): Promise<boolean> {
+    return this.root.isVisible()
+  }
+
+  async waitForVisible(): Promise<void> {
+    await this.root.waitFor({ state: 'visible' })
+  }
+
+  async waitForHidden(): Promise<void> {
+    await this.root.waitFor({ state: 'hidden' })
+  }
+}
+
 export class QueuePanel {
   readonly overlayToggle: Locator
   readonly moreOptionsButton: Locator
+  readonly clearHistoryDialog: QueueClearHistoryDialog
 
   constructor(readonly page: Page) {
     this.overlayToggle = page.getByTestId(TestIds.queue.overlayToggle)
     this.moreOptionsButton = page.getByLabel(/More options/i).first()
+    this.clearHistoryDialog = new QueueClearHistoryDialog(page)
   }
 
   async openClearHistoryDialog() {
@@ -21,4 +53,4 @@ export class QueuePanel {
     await expect(clearHistoryAction).toBeVisible()
     await clearHistoryAction.click()
   }
-}
+}

browser_tests/tests/confirmDialogTextWrap.spec.ts

@@ -18,15 +18,13 @@ test.describe('Confirm dialog text wrapping', { tag: ['@mobile'] }, () => {
         .catch(() => {})
     }, longFilename)
 
-    const dialog = comfyPage.page.getByRole('dialog')
-    await expect(dialog).toBeVisible()
+    const dialog = comfyPage.confirmDialog
+    await dialog.waitForVisible()
 
-    const confirmButton = dialog.getByRole('button', { name: 'Confirm' })
-    await expect(confirmButton).toBeVisible()
-    await expect(confirmButton).toBeInViewport()
+    await expect(dialog.confirm).toBeVisible()
+    await expect(dialog.confirm).toBeInViewport()
 
-    const cancelButton = dialog.getByRole('button', { name: 'Cancel' })
-    await expect(cancelButton).toBeVisible()
-    await expect(cancelButton).toBeInViewport()
+    await expect(dialog.reject).toBeVisible()
+    await expect(dialog.reject).toBeInViewport()
   })
-})
+})

browser_tests/tests/dialogs/queueClearHistory.spec.ts

@@ -16,8 +16,7 @@ test.describe('QueueClearHistoryDialog', { tag: '@ui' }, () => {
   }) => {
     await comfyPage.queuePanel.openClearHistoryDialog()
 
-    const dialog = comfyPage.page.getByRole('dialog')
-    await expect(dialog).toBeVisible()
+    await expect(comfyPage.queuePanel.clearHistoryDialog.root).toBeVisible()
   })
 
   test('Dialog shows confirmation message with title, description, and assets note', async ({
@@ -25,24 +24,24 @@ test.describe('QueueClearHistoryDialog', { tag: '@ui' }, () => {
   }) => {
     await comfyPage.queuePanel.openClearHistoryDialog()
 
-    const dialog = comfyPage.page.getByRole('dialog')
-    await expect(dialog).toBeVisible()
+    const dialog = comfyPage.queuePanel.clearHistoryDialog
+    await expect(dialog.root).toBeVisible()
 
     // Verify title
     await expect(
-      dialog.getByText('Clear your job queue history?')
+      dialog.root.getByText('Clear your job queue history?')
     ).toBeVisible()
 
     // Verify description
     await expect(
-      dialog.getByText(
+      dialog.root.getByText(
         'All the finished or failed jobs below will be removed from this Job queue panel.'
       )
     ).toBeVisible()
 
     // Verify assets note (locale uses Unicode RIGHT SINGLE QUOTATION MARK \u2019)
     await expect(
-      dialog.getByText(
+      dialog.root.getByText(
         'Assets generated by these jobs won\u2019t be deleted and can always be viewed from the assets panel.'
       )
     ).toBeVisible()
@@ -53,8 +52,8 @@ test.describe('QueueClearHistoryDialog', { tag: '@ui' }, () => {
   }) => {
     await comfyPage.queuePanel.openClearHistoryDialog()
 
-    const dialog = comfyPage.page.getByRole('dialog')
-    await expect(dialog).toBeVisible()
+    const dialog = comfyPage.queuePanel.clearHistoryDialog
+    await expect(dialog.root).toBeVisible()
 
     // Intercept the clear API call — it should NOT be called
     let clearCalled = false
@@ -65,13 +64,9 @@ test.describe('QueueClearHistoryDialog', { tag: '@ui' }, () => {
       return route.continue()
     })
 
-    // Click Cancel
-    await dialog.getByRole('button', { name: 'Cancel' }).click()
+    await dialog.cancelButton.click()
 
-    // Dialog should disappear
-    await expect(dialog).not.toBeVisible()
-
-    // Verify clear was not called
+    await expect(dialog.root).not.toBeVisible()
     expect(clearCalled).toBe(false)
 
     await comfyPage.page.unroute('**/api/history')
@@ -82,8 +77,8 @@ test.describe('QueueClearHistoryDialog', { tag: '@ui' }, () => {
   }) => {
     await comfyPage.queuePanel.openClearHistoryDialog()
 
-    const dialog = comfyPage.page.getByRole('dialog')
-    await expect(dialog).toBeVisible()
+    const dialog = comfyPage.queuePanel.clearHistoryDialog
+    await expect(dialog.root).toBeVisible()
 
     // Intercept the clear API call — it should NOT be called
     let clearCalled = false
@@ -94,13 +89,9 @@ test.describe('QueueClearHistoryDialog', { tag: '@ui' }, () => {
       return route.continue()
     })
 
-    // Click the X close button
-    await dialog.getByLabel('Close').click()
+    await dialog.closeButton.click()
 
-    // Dialog should disappear
-    await expect(dialog).not.toBeVisible()
-
-    // Verify clear was not called
+    await expect(dialog.root).not.toBeVisible()
     expect(clearCalled).toBe(false)
 
     await comfyPage.page.unroute('**/api/history')
@@ -111,39 +102,36 @@ test.describe('QueueClearHistoryDialog', { tag: '@ui' }, () => {
   }) => {
     await comfyPage.queuePanel.openClearHistoryDialog()
 
-    const dialog = comfyPage.page.getByRole('dialog')
-    await expect(dialog).toBeVisible()
+    const dialog = comfyPage.queuePanel.clearHistoryDialog
+    await expect(dialog.root).toBeVisible()
 
     // Intercept the clear API call to verify it is made
     const clearPromise = comfyPage.page.waitForRequest(
       (req) => req.url().includes('/api/history') && req.method() === 'POST'
     )
 
-    // Click Clear
-    await dialog.getByRole('button', { name: 'Clear' }).click()
+    await dialog.clearButton.click()
 
     // Verify the API call was made
     const request = await clearPromise
     expect(request.postDataJSON()).toEqual({ clear: true })
 
-    // Dialog should close after clearing
-    await expect(dialog).not.toBeVisible()
+    await expect(dialog.root).not.toBeVisible()
   })
 
   test('Dialog state resets after close and reopen', async ({ comfyPage }) => {
     // Open and cancel
     await comfyPage.queuePanel.openClearHistoryDialog()
-    const dialog = comfyPage.page.getByRole('dialog')
-    await expect(dialog).toBeVisible()
-    await dialog.getByRole('button', { name: 'Cancel' }).click()
-    await expect(dialog).not.toBeVisible()
+    const dialog = comfyPage.queuePanel.clearHistoryDialog
+    await expect(dialog.root).toBeVisible()
+    await dialog.cancelButton.click()
+    await expect(dialog.root).not.toBeVisible()
 
     // Reopen — dialog should be fresh (Clear button enabled, not stuck)
     await comfyPage.queuePanel.openClearHistoryDialog()
-    await expect(dialog).toBeVisible()
+    await expect(dialog.root).toBeVisible()
 
-    const clearButton = dialog.getByRole('button', { name: 'Clear' })
-    await expect(clearButton).toBeVisible()
-    await expect(clearButton).toBeEnabled()
+    await expect(dialog.clearButton).toBeVisible()
+    await expect(dialog.clearButton).toBeEnabled()
   })
-})
+})