diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index fe439af20..a0901b5b5 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -13,6 +13,7 @@ import { ComfyTemplates } from '../helpers/templates' import { ComfyMouse } from './ComfyMouse' import { VueNodeHelpers } from './VueNodeHelpers' import { ComfyNodeSearchBox } from './components/ComfyNodeSearchBox' +import { PropertiesPanel } from './components/PropertiesPanel' import { SettingDialog } from './components/SettingDialog' import { NodeLibrarySidebarTab, @@ -26,32 +27,20 @@ dotenv.config() type WorkspaceStore = ReturnType -class ComfyPropertiesPanel { - readonly root: Locator - readonly panelTitle: Locator - readonly searchBox: Locator - - constructor(readonly page: Page) { - this.root = page.getByTestId('properties-panel') - this.panelTitle = this.root.locator('h3') - this.searchBox = this.root.getByPlaceholder('Search...') - } -} - class ComfyMenu { private _nodeLibraryTab: NodeLibrarySidebarTab | null = null private _workflowsTab: WorkflowsSidebarTab | null = null private _topbar: Topbar | null = null public readonly sideToolbar: Locator - public readonly propertiesPanel: ComfyPropertiesPanel + public readonly propertiesPanel: PropertiesPanel public readonly themeToggleButton: Locator public readonly saveButton: Locator constructor(public readonly page: Page) { this.sideToolbar = page.locator('.side-tool-bar-container') this.themeToggleButton = page.locator('.comfy-vue-theme-toggle') - this.propertiesPanel = new ComfyPropertiesPanel(page) + this.propertiesPanel = new PropertiesPanel(page) this.saveButton = page .locator('button[title="Save the current workflow"]') .nth(0) @@ -1583,6 +1572,31 @@ export class ComfyPage { return window['app'].graph.nodes }) } + + async isInSubgraph(): Promise { + return await this.page.evaluate(() => { + const graph = window['app'].canvas.graph + return graph?.constructor?.name === 'Subgraph' + }) + } + + async createNode( + nodeType: string, + position: Position = { x: 200, y: 200 } + ): Promise { + const nodeId = await this.page.evaluate( + ({ nodeType, pos }) => { + const node = window['LiteGraph'].createNode(nodeType) + if (!node) throw new Error(`Failed to create node: ${nodeType}`) + window['app'].graph.add(node) + node.pos = [pos.x, pos.y] + return node.id + }, + { nodeType, pos: position } + ) + await this.nextFrame() + return this.getNodeRefById(nodeId) + } async waitForGraphNodes(count: number) { await this.page.waitForFunction((count) => { return window['app']?.canvas.graph?.nodes?.length === count diff --git a/browser_tests/fixtures/components/PropertiesPanel.ts b/browser_tests/fixtures/components/PropertiesPanel.ts new file mode 100644 index 000000000..05782495e --- /dev/null +++ b/browser_tests/fixtures/components/PropertiesPanel.ts @@ -0,0 +1,97 @@ +import type { Locator, Page } from '@playwright/test' + +export class PropertiesPanel { + readonly root: Locator + readonly panelTitle: Locator + readonly searchBox: Locator + + constructor(readonly page: Page) { + this.root = page.getByTestId('properties-panel') + this.panelTitle = this.root.locator('h3') + this.searchBox = this.root.getByPlaceholder('Search...') + } + + async ensureOpen() { + const isOpen = await this.root.isVisible() + if (!isOpen) { + await this.page.getByLabel('Toggle properties panel').click() + await this.root.waitFor({ state: 'visible' }) + } + } + + async close() { + const isOpen = await this.root.isVisible() + if (isOpen) { + await this.page.getByLabel('Toggle properties panel').click() + await this.root.waitFor({ state: 'hidden' }) + } + } + + async promoteWidget(widgetName: string) { + await this.ensureOpen() + + // Click on Advanced Inputs to expand it + const advancedInputsButton = this.root + .getByRole('button') + .filter({ hasText: /advanced inputs/i }) + await advancedInputsButton.click() + + // Find the widget row and click the more options button + const widgetRow = this.root + .locator('[class*="widget-item"], [class*="input-item"]') + .filter({ hasText: widgetName }) + .first() + + const moreButton = widgetRow.locator('button').filter({ + has: this.page.locator('[class*="lucide--more-vertical"]') + }) + await moreButton.click() + + // Click "Show input" to promote the widget + await this.page.getByText('Show input').click() + + // Close and reopen panel to refresh the UI state + await this.page.getByLabel('Toggle properties panel').click() + await this.page.getByLabel('Toggle properties panel').click() + } + + async demoteWidget(widgetName: string) { + await this.ensureOpen() + + // Check if INPUTS section content is already visible + const inputsContent = this.root.locator('div').filter({ + hasText: new RegExp(`^${widgetName}$`) + }) + const isInputsExpanded = await inputsContent.first().isVisible() + + if (!isInputsExpanded) { + // Click on INPUTS section to expand it (where promoted widgets appear) + const inputsButton = this.root + .getByRole('button') + .filter({ hasText: /^inputs$/i }) + await inputsButton.click() + } + + // Find the widget row and click the more options button + const widgetRow = this.root + .locator('div') + .filter({ hasText: new RegExp(`^${widgetName}$`) }) + .first() + + await widgetRow.waitFor({ state: 'visible', timeout: 5000 }) + + // Find the more options button (the vertical dots icon button) + const moreButton = widgetRow + .locator('..') + .locator('button') + .filter({ + has: this.page.locator('[class*="more-vertical"], [class*="lucide"]') + }) + .first() + + await moreButton.click() + + // Click "Hide input" to demote the widget + await this.page.getByText('Hide input').click() + } +} diff --git a/browser_tests/fixtures/components/Topbar.ts b/browser_tests/fixtures/components/Topbar.ts index c5e7c8155..49758b172 100644 --- a/browser_tests/fixtures/components/Topbar.ts +++ b/browser_tests/fixtures/components/Topbar.ts @@ -60,6 +60,11 @@ export class Topbar { await tab.locator('.close-button').click({ force: true }) } + async switchToTab(index: number) { + const tabs = this.page.locator('.workflow-tabs button') + await tabs.nth(index).click() + } + getSaveDialog(): Locator { return this.page.locator('.p-dialog-content input') } diff --git a/browser_tests/fixtures/utils/litegraphUtils.ts b/browser_tests/fixtures/utils/litegraphUtils.ts index ea5a0b78f..d804f7720 100644 --- a/browser_tests/fixtures/utils/litegraphUtils.ts +++ b/browser_tests/fixtures/utils/litegraphUtils.ts @@ -263,6 +263,26 @@ class NodeWidgetReference { [this.node.id, this.index] as const ) } + + async setValue(value: unknown, useCanvasGraph = false) { + await this.node.comfyPage.page.evaluate( + ([id, index, val, useCanvas]) => { + const graph = useCanvas + ? window['app'].canvas.graph + : window['app'].graph + const node = graph.getNodeById(id) + if (!node) throw new Error(`Node ${id} not found.`) + const widget = node.widgets[index] + if (!widget) throw new Error(`Widget ${index} not found.`) + widget.value = val + if (widget.callback) { + widget.callback(val, window['app'].canvas, node, null, null) + } + }, + [this.node.id, this.index, value, useCanvasGraph] as const + ) + await this.node.comfyPage.nextFrame() + } } export class NodeReference { constructor( @@ -339,8 +359,43 @@ export class NodeReference { async getWidget(index: number) { return new NodeWidgetReference(index, this) } + + async getWidgetByName( + name: string, + useCanvasGraph = false + ): Promise { + const index = await this.comfyPage.page.evaluate( + ([id, widgetName, useCanvas]) => { + const graph = useCanvas + ? window['app'].canvas.graph + : window['app'].graph + const node = graph.getNodeById(id) + if (!node?.widgets) return -1 + return node.widgets.findIndex( + (w: { name: string }) => w.name === widgetName + ) + }, + [this.id, name, useCanvasGraph] as const + ) + if (index === -1) return null + return new NodeWidgetReference(index, this) + } + + async getWidgets(): Promise< + Array<{ name: string; visible: boolean; value: unknown }> + > { + return await this.comfyPage.page.evaluate((id) => { + const node = window['app'].graph.getNodeById(id) + if (!node?.widgets) return [] + + return node.widgets.map((w) => { + const isHidden = w.hidden === true || w.options?.hidden === true + return { name: w.name, visible: !isHidden, value: w.value } + }) + }, this.id) + } async click( - position: 'title' | 'collapse', + position: 'title' | 'collapse' | 'subgraph', options?: Parameters[1] & { moveMouseToEmptyArea?: boolean } ) { const nodePos = await this.getPosition() @@ -353,6 +408,9 @@ export class NodeReference { case 'collapse': clickPos = { x: nodePos.x + 5, y: nodePos.y - 10 } break + case 'subgraph': + clickPos = { x: nodePos.x + nodeSize.width - 15, y: nodePos.y - 15 } + break default: throw new Error(`Invalid click position ${position}`) } diff --git a/browser_tests/tests/dynamicWidgetsSubgraph.spec.ts b/browser_tests/tests/dynamicWidgetsSubgraph.spec.ts new file mode 100644 index 000000000..b4f992143 --- /dev/null +++ b/browser_tests/tests/dynamicWidgetsSubgraph.spec.ts @@ -0,0 +1,322 @@ +import { expect } from '@playwright/test' + +import type { ComfyPage } from '../fixtures/ComfyPage' +import { comfyPageFixture as test } from '../fixtures/ComfyPage' + +test.describe('Dynamic Combo Widgets in Subgraphs', () => { + const TEST_NODE_TYPE = 'TestDynamicComboNode' + + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.setSetting('Comfy.UseNewMenu', 'Top') + await comfyPage.setSetting('Comfy.Workflow.WorkflowTabsPosition', 'Topbar') + await comfyPage.setSetting('Comfy.ConfirmClear', false) + }) + + function subgraphWidgetName(widgetName: string): string { + return `1: ${widgetName}` + } + + function widget(name: string, visible: boolean, value: unknown) { + return { + name: subgraphWidgetName(name), + visible, + value + } + } + + async function clearGraph(comfyPage: ComfyPage) { + await comfyPage.executeCommand('Comfy.ClearWorkflow') + await comfyPage.nextFrame() + } + + async function getSubgraphNode(comfyPage: ComfyPage) { + const nodes = await comfyPage.getNodeRefsByTitle('New Subgraph') + return nodes[0] + } + + async function createTestNodeAsSubgraph( + comfyPage: ComfyPage, + mode: 'none' | 'one' | 'two' | 'three' = 'none' + ) { + const testNode = await comfyPage.createNode(TEST_NODE_TYPE) + + if (mode !== 'none') { + const widget = await testNode.getWidgetByName('dynamic_combo') + if (widget) await widget.setValue(mode) + } + + await testNode.click('title') + await comfyPage.nextFrame() + + return await testNode.convertToSubgraph() + } + + test('Promoted dynamic combo promotes all children with it', async ({ + comfyPage + }) => { + await clearGraph(comfyPage) + + const subgraphNode = await createTestNodeAsSubgraph(comfyPage, 'two') + + await subgraphNode.click('title') + await comfyPage.nextFrame() + await comfyPage.menu.propertiesPanel.promoteWidget('dynamic_combo') + + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'two'), + widget('dynamic_combo.w1', true, 0), + widget('dynamic_combo.w2', true, 0) + ]) + }) + + test('Demoted dynamic combo unpromotes all children with it', async ({ + comfyPage + }) => { + await clearGraph(comfyPage) + + const subgraphNode = await createTestNodeAsSubgraph(comfyPage, 'two') + + await subgraphNode.click('title') + await comfyPage.nextFrame() + await comfyPage.menu.propertiesPanel.promoteWidget('dynamic_combo') + + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'two'), + widget('dynamic_combo.w1', true, 0), + widget('dynamic_combo.w2', true, 0) + ]) + + await comfyPage.menu.propertiesPanel.demoteWidget('dynamic_combo') + + const widgets = await subgraphNode.getWidgets() + const visibleWidgets = widgets.filter((w) => w.visible) + expect(visibleWidgets).toEqual([]) + }) + + test('Promoted combo widgets hide and show based on combo value', async ({ + comfyPage + }) => { + await clearGraph(comfyPage) + + const subgraphNode = await createTestNodeAsSubgraph(comfyPage, 'none') + + await subgraphNode.click('title') + await comfyPage.nextFrame() + await comfyPage.menu.propertiesPanel.promoteWidget('dynamic_combo') + + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'none') + ]) + + const comboWidget = await subgraphNode.getWidgetByName( + subgraphWidgetName('dynamic_combo') + ) + + await comboWidget!.setValue('one') + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'one'), + widget('dynamic_combo.w1', true, 0) + ]) + + await comboWidget!.setValue('two') + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'two'), + widget('dynamic_combo.w1', true, 0), + widget('dynamic_combo.w2', true, 0) + ]) + + await comboWidget!.setValue('three') + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'three'), + widget('dynamic_combo.w1', true, 0), + widget('dynamic_combo.w2', true, 0), + widget('dynamic_combo.w3', true, 0) + ]) + + await comboWidget!.setValue('two') + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'two'), + widget('dynamic_combo.w1', true, 0), + widget('dynamic_combo.w2', true, 0), + widget('dynamic_combo.w3', false, undefined) + ]) + + await comboWidget!.setValue('one') + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'one'), + widget('dynamic_combo.w1', true, 0), + widget('dynamic_combo.w2', false, undefined), + widget('dynamic_combo.w3', false, undefined) + ]) + + await comboWidget!.setValue('none') + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'none'), + widget('dynamic_combo.w1', false, undefined), + widget('dynamic_combo.w2', false, undefined), + widget('dynamic_combo.w3', false, undefined) + ]) + }) + + test('Promoted combo maintains state after workflow reload', async ({ + comfyPage + }) => { + await clearGraph(comfyPage) + + const subgraphNode = await createTestNodeAsSubgraph(comfyPage, 'two') + + await subgraphNode.click('title') + await comfyPage.nextFrame() + + await comfyPage.menu.propertiesPanel.promoteWidget('dynamic_combo') + + const w1 = await subgraphNode.getWidgetByName( + subgraphWidgetName('dynamic_combo.w1') + ) + const w2 = await subgraphNode.getWidgetByName( + subgraphWidgetName('dynamic_combo.w2') + ) + await w1!.setValue(123) + await w2!.setValue(456) + + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'two'), + widget('dynamic_combo.w1', true, 123), + widget('dynamic_combo.w2', true, 456) + ]) + + // Click on node to ensure changes are committed before switching + await subgraphNode.click('title') + await comfyPage.nextFrame() + + await comfyPage.executeCommand('Comfy.NewBlankWorkflow') + await comfyPage.nextFrame() + await comfyPage.menu.topbar.switchToTab(0) + await comfyPage.nextFrame() + + const reloadedSubgraph = await getSubgraphNode(comfyPage) + expect(await reloadedSubgraph.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'two'), + widget('dynamic_combo.w1', true, 123), + widget('dynamic_combo.w2', true, 456) + ]) + }) + + test('Hidden children remain hidden after workflow reload when combo is none', async ({ + comfyPage + }) => { + await clearGraph(comfyPage) + + const subgraphNode = await createTestNodeAsSubgraph(comfyPage, 'two') + + await subgraphNode.click('title') + await comfyPage.nextFrame() + await comfyPage.menu.propertiesPanel.promoteWidget('dynamic_combo') + + const comboWidget = await subgraphNode.getWidgetByName( + subgraphWidgetName('dynamic_combo') + ) + await comboWidget!.setValue('none') + + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'none'), + widget('dynamic_combo.w1', false, undefined), + widget('dynamic_combo.w2', false, undefined) + ]) + + // Click on node to ensure changes are committed before switching + await subgraphNode.click('title') + await comfyPage.nextFrame() + + await comfyPage.executeCommand('Comfy.NewBlankWorkflow') + await comfyPage.nextFrame() + await comfyPage.menu.topbar.switchToTab(0) + await comfyPage.nextFrame() + + const reloadedSubgraph = await getSubgraphNode(comfyPage) + expect(await reloadedSubgraph.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'none'), + widget('dynamic_combo.w1', false, undefined), + widget('dynamic_combo.w2', false, undefined) + ]) + }) + + test('Children appear when combo changes after workflow reload', async ({ + comfyPage + }) => { + await clearGraph(comfyPage) + + const subgraphNode = await createTestNodeAsSubgraph(comfyPage, 'none') + + await subgraphNode.click('title') + await comfyPage.nextFrame() + await comfyPage.menu.propertiesPanel.promoteWidget('dynamic_combo') + + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'none') + ]) + + await comfyPage.executeCommand('Comfy.NewBlankWorkflow') + await comfyPage.nextFrame() + await comfyPage.menu.topbar.switchToTab(0) + await comfyPage.nextFrame() + + const reloadedSubgraph = await getSubgraphNode(comfyPage) + const comboWidget = await reloadedSubgraph.getWidgetByName( + subgraphWidgetName('dynamic_combo') + ) + await comboWidget!.setValue('two') + await comfyPage.page.waitForTimeout(500) + + expect(await reloadedSubgraph.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'two'), + widget('dynamic_combo.w1', true, 0), + widget('dynamic_combo.w2', true, 0) + ]) + }) + + test('Dynamic combo children created inside subgraph are auto-promoted', async ({ + comfyPage + }) => { + await clearGraph(comfyPage) + + const testNode = await comfyPage.createNode(TEST_NODE_TYPE) + await testNode.click('title') + await comfyPage.nextFrame() + + const subgraphNode = await testNode.convertToSubgraph() + await comfyPage.page.waitForTimeout(500) + + await subgraphNode.click('title') + await comfyPage.nextFrame() + await comfyPage.menu.propertiesPanel.promoteWidget('dynamic_combo') + + expect(await subgraphNode.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'none') + ]) + + await subgraphNode.click('subgraph') + await expect + .poll(() => comfyPage.isInSubgraph(), { timeout: 5000 }) + .toBe(true) + + const innerNodes = await comfyPage.getNodeRefsByType(TEST_NODE_TYPE, true) + const innerNode = innerNodes[0] + const innerComboWidget = await innerNode.getWidgetByName( + 'dynamic_combo', + true + ) + await innerComboWidget!.setValue('two', true) + + await comfyPage.page.keyboard.press('Escape') + await comfyPage.nextFrame() + expect(await comfyPage.isInSubgraph()).toBe(false) + + const outerSubgraph = await getSubgraphNode(comfyPage) + expect(await outerSubgraph.getWidgets()).toEqual([ + widget('dynamic_combo', true, 'two'), + widget('dynamic_combo.w1', true, 0), + widget('dynamic_combo.w2', true, 0) + ]) + }) +}) diff --git a/src/core/graph/subgraph/proxyWidgetUtils.ts b/src/core/graph/subgraph/proxyWidgetUtils.ts index 9f50a8840..2a901aaf3 100644 --- a/src/core/graph/subgraph/proxyWidgetUtils.ts +++ b/src/core/graph/subgraph/proxyWidgetUtils.ts @@ -184,13 +184,24 @@ export function autoPromoteDynamicChildren( node: LGraphNode, parentWidget: IBaseWidget ) { - if (!parentWidget.promoted) return - const parents = getSubgraphParents(node) if (!parents.length) return + // Check if the parent widget is actually promoted on any parent SubgraphNode. + // This is more reliable than checking parentWidget.promoted, which may not + // be set after workflow reload (the flag is only synced when navigating into + // the subgraph). + const nodeId = String(node.id) + const promotedOnParents = parents.filter((parent) => + getProxyWidgets(parent).some( + ([id, name]) => id === nodeId && name === parentWidget.name + ) + ) + + if (!promotedOnParents.length) return + const childWidgets = getChildWidgets(node, parentWidget.name) - promoteWidgetsToProxy(node, childWidgets, parents) + promoteWidgetsToProxy(node, childWidgets, promotedOnParents) } /**