V3 UI - Tabs & Menu rework (#4374)

Co-authored-by: github-actions <[email protected]>

Author: pythongosssss

browser_tests/fixtures/ComfyPage.ts

@@ -168,7 +168,7 @@ export class ComfyPage {
     this.menu = new ComfyMenu(page)
     this.actionbar = new ComfyActionbar(page)
     this.templates = new ComfyTemplates(page)
-    this.settingDialog = new SettingDialog(page)
+    this.settingDialog = new SettingDialog(page, this)
     this.confirmDialog = new ConfirmDialog(page)
   }
 

browser_tests/fixtures/components/SettingDialog.ts

@@ -1,15 +1,19 @@
 import { Page } from '@playwright/test'
 
+import { ComfyPage } from '../ComfyPage'
+
 export class SettingDialog {
-  constructor(public readonly page: Page) {}
+  constructor(
+    public readonly page: Page,
+    public readonly comfyPage: ComfyPage
+  ) {}
 
   get root() {
     return this.page.locator('div.settings-container')
   }
 
   async open() {
-    const button = this.page.locator('button.comfy-settings-btn:visible')
-    await button.click()
+    await this.comfyPage.executeCommand('Comfy.ShowSettingsDialog')
     await this.page.waitForSelector('div.settings-container')
   }
 

browser_tests/fixtures/components/Topbar.ts

@@ -15,10 +15,6 @@ export class Topbar {
       .innerText()
   }
 
-  async openSubmenuMobile() {
-    await this.page.locator('.p-menubar-mobile .p-menubar-button').click()
-  }
-
   getMenuItem(itemLabel: string): Locator {
     return this.page.locator(`.p-menubar-item-label:text-is("${itemLabel}")`)
   }
@@ -68,31 +64,41 @@ export class Topbar {
     await this.getSaveDialog().waitFor({ state: 'hidden', timeout: 500 })
   }
 
+  async openTopbarMenu() {
+    await this.page.locator('.comfyui-logo-wrapper').click()
+    const menu = this.page.locator('.comfy-command-menu')
+    await menu.waitFor({ state: 'visible' })
+    return menu
+  }
+
   async triggerTopbarCommand(path: string[]) {
     if (path.length < 2) {
       throw new Error('Path is too short')
     }
 
+    const menu = await this.openTopbarMenu()
     const tabName = path[0]
-    const topLevelMenu = this.page.locator(
-      `.top-menubar .p-menubar-item-label:text-is("${tabName}")`
+    const topLevelMenuItem = this.page.locator(
+      `.p-menubar-item-label:text-is("${tabName}")`
     )
+    const topLevelMenu = menu
+      .locator('.p-tieredmenu-item')
+      .filter({ has: topLevelMenuItem })
     await topLevelMenu.waitFor({ state: 'visible' })
-    await topLevelMenu.click()
+    await topLevelMenu.hover()
 
+    let currentMenu = topLevelMenu
     for (let i = 1; i < path.length; i++) {
       const commandName = path[i]
-      const menuItem = this.page
+      const menuItem = currentMenu
         .locator(
-          `.top-menubar .p-menubar-submenu .p-menubar-item:has-text("${commandName}")`
+          `.p-tieredmenu-submenu .p-tieredmenu-item:has-text("${commandName}")`
         )
         .first()
       await menuItem.waitFor({ state: 'visible' })
       await menuItem.hover()
-
-      if (i === path.length - 1) {
-        await menuItem.click()
-      }
+      currentMenu = menuItem
     }
+    await currentMenu.click()
   }
 }

browser_tests/tests/interaction.spec.ts

@@ -769,27 +769,37 @@ test.describe('Viewport settings', () => {
   }) => {
     // Screenshot the canvas element
     await comfyPage.menu.topbar.saveWorkflow('Workflow A')
-    await expect(comfyPage.canvas).toHaveScreenshot('viewport-workflow-a.png')
+    await comfyPage.nextFrame()
+    const screenshotA = (await comfyPage.canvas.screenshot()).toString('base64')
 
     // Save workflow as a new file, then zoom out before screen shot
     await comfyPage.menu.topbar.saveWorkflowAs('Workflow B')
     await comfyMouse.move(comfyPage.emptySpace)
     for (let i = 0; i < 4; i++) {
       await comfyMouse.wheel(0, 60)
     }
-    await expect(comfyPage.canvas).toHaveScreenshot('viewport-workflow-b.png')
+
+    await comfyPage.nextFrame()
+    const screenshotB = (await comfyPage.canvas.screenshot()).toString('base64')
+
+    // Ensure that the screenshots are different due to zoom level
+    expect(screenshotB).not.toBe(screenshotA)
 
     const tabA = comfyPage.menu.topbar.getWorkflowTab('Workflow A')
     const tabB = comfyPage.menu.topbar.getWorkflowTab('Workflow B')
 
     // Go back to Workflow A
     await tabA.click()
     await comfyPage.nextFrame()
-    await expect(comfyPage.canvas).toHaveScreenshot('viewport-workflow-a.png')
+    expect((await comfyPage.canvas.screenshot()).toString('base64')).toBe(
+      screenshotA
+    )
 
     // And back to Workflow B
     await tabB.click()
     await comfyPage.nextFrame()
-    await expect(comfyPage.canvas).toHaveScreenshot('viewport-workflow-b.png')
+    expect((await comfyPage.canvas.screenshot()).toString('base64')).toBe(
+      screenshotB
+    )
   })
 })

browser_tests/tests/interaction.spec.ts-snapshots/viewport-workflow-a-chromium-linux.png

@@ -63,7 +63,7 @@ test.describe('Menu', () => {
     test('@mobile Items fully visible on mobile screen width', async ({
       comfyPage
     }) => {
-      await comfyPage.menu.topbar.openSubmenuMobile()
+      await comfyPage.menu.topbar.openTopbarMenu()
       const topLevelMenuItem = comfyPage.page
         .locator('a.p-menubar-item-link')
         .first()
@@ -74,8 +74,9 @@ test.describe('Menu', () => {
     })
 
     test('Displays keybinding next to item', async ({ comfyPage }) => {
+      await comfyPage.menu.topbar.openTopbarMenu()
       const workflowMenuItem = comfyPage.menu.topbar.getMenuItem('Workflow')
-      await workflowMenuItem.click()
+      await workflowMenuItem.hover()
       const exportTag = comfyPage.page.locator('.keybinding-tag', {
         hasText: 'Ctrl + s'
       })

browser_tests/tests/interaction.spec.ts-snapshots/viewport-workflow-b-chromium-linux.png

@@ -30,10 +30,11 @@ import ComfyQueueButton from './ComfyQueueButton.vue'
 
 const settingsStore = useSettingStore()
 
-const visible = computed(
-  () => settingsStore.get('Comfy.UseNewMenu') !== 'Disabled'
-)
+const position = computed(() => settingsStore.get('Comfy.UseNewMenu'))
+
+const visible = computed(() => position.value !== 'Disabled')
 
+const topMenuRef = inject<Ref<HTMLDivElement | null>>('topMenuRef')
 const panelRef = ref<HTMLElement | null>(null)
 const dragHandleRef = ref<HTMLElement | null>(null)
 const isDocked = useLocalStorage('Comfy.MenuPosition.Docked', false)
@@ -49,7 +50,16 @@ const {
 } = useDraggable(panelRef, {
   initialValue: { x: 0, y: 0 },
   handle: dragHandleRef,
-  containerElement: document.body
+  containerElement: document.body,
+  onMove: (event) => {
+    // Prevent dragging the menu over the top of the tabs
+    if (position.value === 'Top') {
+      const minY = topMenuRef?.value?.getBoundingClientRect().top ?? 40
+      if (event.y < minY) {
+        event.y = minY
+      }
+    }
+  }
 })
 
 // Update storedPosition when x or y changes
@@ -182,7 +192,6 @@ const adjustMenuPosition = () => {
 
 useEventListener(window, 'resize', adjustMenuPosition)
 
-const topMenuRef = inject<Ref<HTMLDivElement | null>>('topMenuRef')
 const topMenuBounds = useElementBounding(topMenuRef)
 const overlapThreshold = 20 // pixels
 const isOverlappingWithTopMenu = computed(() => {