From 0df7a53ead1e271bf0d14b0049c74d0959aa41db Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Sat, 6 Sep 2025 11:09:58 +0100 Subject: [PATCH] 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 Co-authored-by: bymyself --- browser_tests/fixtures/ComfyPage.ts | 26 ++++++ browser_tests/fixtures/components/Topbar.ts | 87 ++++++++++++++++++--- browser_tests/tests/menu.spec.ts | 66 ++++++++++++++++ src/components/topbar/CommandMenubar.vue | 76 +++++++++--------- src/constants/coreMenuCommands.ts | 1 + src/locales/ar/main.json | 1 + src/locales/en/main.json | 1 + src/locales/es/main.json | 4 + src/locales/fr/main.json | 4 + src/locales/ja/main.json | 4 + src/locales/ko/main.json | 4 + src/locales/ru/main.json | 4 + src/locales/zh-TW/main.json | 4 + src/locales/zh/main.json | 1 + 14 files changed, 234 insertions(+), 49 deletions(-) diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index f64ca5c94..c32dd3937 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/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() diff --git a/browser_tests/fixtures/components/Topbar.ts b/browser_tests/fixtures/components/Topbar.ts index f2c9dfa16..04a9117ce 100644 --- a/browser_tests/fixtures/components/Topbar.ts +++ b/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 { 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 { + 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 { + 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 }) diff --git a/browser_tests/tests/menu.spec.ts b/browser_tests/tests/menu.spec.ts index 355acb590..fa46778a4 100644 --- a/browser_tests/tests/menu.spec.ts +++ b/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. diff --git a/src/components/topbar/CommandMenubar.vue b/src/components/topbar/CommandMenubar.vue index a64b2e4ba..4e01965f8 100644 --- a/src/components/topbar/CommandMenubar.vue +++ b/src/components/topbar/CommandMenubar.vue @@ -28,29 +28,7 @@ @show="onMenuShow" >