Rework theme menu (#5161)

* Change theme "button" to sub menu of all themes

* Add test for theme menu

* Prevent separator being added before View

* Refactor test

* Update locales [skip ci]

* Fix has-text vs text-is change breaking other tests

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: bymyself <cbyrne@comfy.org>

browser_tests/fixtures/ComfyPage.ts

@@ -453,6 +453,32 @@ export class ComfyPage {
     await workflowsTab.close()
   }
 
+  /**
+   * Attach a screenshot to the test report.
+   * By default, screenshots are only taken in non-CI environments.
+   * @param name - Name for the screenshot attachment
+   * @param options - Optional configuration
+   * @param options.runInCI - Whether to take screenshot in CI (default: false)
+   * @param options.fullPage - Whether to capture full page (default: false)
+   */
+  async attachScreenshot(
+    name: string,
+    options: { runInCI?: boolean; fullPage?: boolean } = {}
+  ) {
+    const { runInCI = false, fullPage = false } = options
+
+    // Skip in CI unless explicitly requested
+    if (process.env.CI && !runInCI) {
+      return
+    }
+
+    const testInfo = comfyPageFixture.info()
+    await testInfo.attach(name, {
+      body: await this.page.screenshot({ fullPage }),
+      contentType: 'image/png'
+    })
+  }
+
   async resetView() {
     if (await this.resetViewButton.isVisible()) {
       await this.resetViewButton.click()

browser_tests/fixtures/components/Topbar.ts

@@ -1,7 +1,13 @@
-import { Locator, Page } from '@playwright/test'
+import { Locator, Page, expect } from '@playwright/test'
 
 export class Topbar {
-  constructor(public readonly page: Page) {}
+  private readonly menuLocator: Locator
+  private readonly menuTrigger: Locator
+
+  constructor(public readonly page: Page) {
+    this.menuLocator = page.locator('.comfy-command-menu')
+    this.menuTrigger = page.locator('.comfyui-logo-wrapper')
+  }
 
   async getTabNames(): Promise<string[]> {
     return await this.page
@@ -15,10 +21,33 @@ export class Topbar {
       .innerText()
   }
 
-  getMenuItem(itemLabel: string): Locator {
+  /**
+   * Get a menu item by its label, optionally within a specific parent container
+   */
+  getMenuItem(itemLabel: string, parent?: Locator): Locator {
+    if (parent) {
+      return parent.locator(`.p-tieredmenu-item:has-text("${itemLabel}")`)
+    }
+
     return this.page.locator(`.p-menubar-item-label:text-is("${itemLabel}")`)
   }
 
+  /**
+   * Get the visible submenu (last visible submenu in case of nested menus)
+   */
+  getVisibleSubmenu(): Locator {
+    return this.page.locator('.p-tieredmenu-submenu:visible').last()
+  }
+
+  /**
+   * Check if a menu item has an active checkmark
+   */
+  async isMenuItemActive(menuItem: Locator): Promise<boolean> {
+    const checkmark = menuItem.locator('.pi-check')
+    const classes = await checkmark.getAttribute('class')
+    return classes ? !classes.includes('invisible') : false
+  }
+
   getWorkflowTab(tabName: string): Locator {
     return this.page
       .locator(`.workflow-tabs .workflow-label:has-text("${tabName}")`)
@@ -66,10 +95,50 @@ export class Topbar {
 
   async openTopbarMenu() {
     await this.page.waitForTimeout(1000)
-    await this.page.locator('.comfyui-logo-wrapper').click()
-    const menu = this.page.locator('.comfy-command-menu')
-    await menu.waitFor({ state: 'visible' })
-    return menu
+    await this.menuTrigger.click()
+    await this.menuLocator.waitFor({ state: 'visible' })
+    return this.menuLocator
+  }
+
+  /**
+   * Close the topbar menu by clicking outside
+   */
+  async closeTopbarMenu() {
+    await this.page.locator('body').click({ position: { x: 10, y: 10 } })
+    await expect(this.menuLocator).not.toBeVisible()
+  }
+
+  /**
+   * Navigate to a submenu by hovering over a menu item
+   */
+  async openSubmenu(menuItemLabel: string): Promise<Locator> {
+    const menuItem = this.getMenuItem(menuItemLabel)
+    await menuItem.hover()
+    const submenu = this.getVisibleSubmenu()
+    await submenu.waitFor({ state: 'visible' })
+    return submenu
+  }
+
+  /**
+   * Get theme menu items and interact with theme switching
+   */
+  async getThemeMenuItems() {
+    const themeSubmenu = await this.openSubmenu('Theme')
+    return {
+      submenu: themeSubmenu,
+      darkTheme: this.getMenuItem('Dark (Default)', themeSubmenu),
+      lightTheme: this.getMenuItem('Light', themeSubmenu)
+    }
+  }
+
+  /**
+   * Switch to a specific theme
+   */
+  async switchTheme(theme: 'dark' | 'light') {
+    const { darkTheme, lightTheme } = await this.getThemeMenuItems()
+    const themeItem = theme === 'dark' ? darkTheme : lightTheme
+    const themeLabel = themeItem.locator('.p-menubar-item-label')
+    await themeLabel.click()
   }
 
   async triggerTopbarCommand(path: string[]) {
@@ -79,9 +148,7 @@ export class Topbar {
 
     const menu = await this.openTopbarMenu()
     const tabName = path[0]
-    const topLevelMenuItem = this.page.locator(
-      `.p-menubar-item-label:text-is("${tabName}")`
-    )
+    const topLevelMenuItem = this.getMenuItem(tabName)
     const topLevelMenu = menu
       .locator('.p-tieredmenu-item')
       .filter({ has: topLevelMenuItem })

browser_tests/tests/menu.spec.ts

@@ -178,6 +178,72 @@ test.describe('Menu', () => {
       await comfyPage.menu.topbar.triggerTopbarCommand(['ext', 'foo-command'])
       expect(await comfyPage.getVisibleToastCount()).toBe(1)
     })
+
+    test('Can navigate Theme menu and switch between Dark and Light themes', async ({
+      comfyPage
+    }) => {
+      const { topbar } = comfyPage.menu
+
+      // Take initial screenshot with default theme
+      await comfyPage.attachScreenshot('theme-initial')
+
+      // Open the topbar menu
+      const menu = await topbar.openTopbarMenu()
+      await expect(menu).toBeVisible()
+
+      // Get theme menu items
+      const {
+        submenu: themeSubmenu,
+        darkTheme: darkThemeItem,
+        lightTheme: lightThemeItem
+      } = await topbar.getThemeMenuItems()
+
+      await expect(darkThemeItem).toBeVisible()
+      await expect(lightThemeItem).toBeVisible()
+
+      // Switch to Light theme
+      await topbar.switchTheme('light')
+
+      // Verify menu stays open and Light theme shows as active
+      await expect(menu).toBeVisible()
+      await expect(themeSubmenu).toBeVisible()
+
+      // Check that Light theme is active
+      expect(await topbar.isMenuItemActive(lightThemeItem)).toBe(true)
+
+      // Screenshot with light theme active
+      await comfyPage.attachScreenshot('theme-menu-light-active')
+
+      // Verify ColorPalette setting is set to "light"
+      expect(await comfyPage.getSetting('Comfy.ColorPalette')).toBe('light')
+
+      // Close menu to see theme change
+      await topbar.closeTopbarMenu()
+
+      // Re-open menu and get theme items again
+      await topbar.openTopbarMenu()
+      const themeItems2 = await topbar.getThemeMenuItems()
+
+      // Switch back to Dark theme
+      await topbar.switchTheme('dark')
+
+      // Verify menu stays open and Dark theme shows as active
+      await expect(menu).toBeVisible()
+      await expect(themeItems2.submenu).toBeVisible()
+
+      // Check that Dark theme is active and Light theme is not
+      expect(await topbar.isMenuItemActive(themeItems2.darkTheme)).toBe(true)
+      expect(await topbar.isMenuItemActive(themeItems2.lightTheme)).toBe(false)
+
+      // Screenshot with dark theme active
+      await comfyPage.attachScreenshot('theme-menu-dark-active')
+
+      // Verify ColorPalette setting is set to "dark"
+      expect(await comfyPage.getSetting('Comfy.ColorPalette')).toBe('dark')
+
+      // Close menu
+      await topbar.closeTopbarMenu()
+    })
   })
 
   // Only test 'Top' to reduce test time.