Skip to content

Commit 452bc77

Browse files
fix: show promoted advanced widgets on ECS (#15423)
## Summary Forward-ports the promoted advanced-widget visibility invariant from #14647 to the store-backed ECS rendering path. Promoted host inputs are user-selected public controls and bypass only the advanced-widget filter. Explicitly hidden widgets remain hidden. ## Red-green verification - `4e2084d8a0` adds canvas and App Mode regressions; both fail on the ECS base because visibility only recognizes external links. - `796648dcae` derives promotion from the host input's canonical `widgetId`; all regressions pass. ## Test plan - [x] Confirm canvas regression fails before the fix - [x] Confirm App Mode regression fails before the fix - [x] Confirm targeted unit suite passes after the fix (39 tests) - [x] Run typecheck, lint, and format checks --------- Co-authored-by: Amp <amp@ampcode.com>
1 parent 1c4985f commit 452bc77

6 files changed

Lines changed: 130 additions & 5 deletions

File tree

browser_tests/tests/subgraph/subgraphPromotion.spec.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,63 @@ test.describe(
9494
'Promoted Widget Visibility in Vue Mode',
9595
{ tag: ['@vue-nodes'] },
9696
() => {
97+
test(
98+
'Promoted advanced widget remains visible when global advanced widgets are disabled',
99+
{ tag: ['@node'] },
100+
async ({ comfyPage }) => {
101+
const subgraphNodeId =
102+
await test.step('Convert a node with hidden advanced widgets to a subgraph', async () => {
103+
await comfyPage.settings.setSetting(
104+
'Comfy.Node.AlwaysShowAdvancedWidgets',
105+
false
106+
)
107+
const modelSamplingNode = await comfyPage.nodeOps.addNode(
108+
'ModelSamplingFlux',
109+
{},
110+
{ x: 500, y: 200 }
111+
)
112+
await comfyPage.nextFrame()
113+
await expect(
114+
comfyPage.vueNodes.getNodeLocator(
115+
String(modelSamplingNode.id)
116+
)
117+
).toBeVisible()
118+
119+
await modelSamplingNode.click('title')
120+
const subgraphNode = await modelSamplingNode.convertToSubgraph()
121+
return String(subgraphNode.id)
122+
})
123+
124+
await test.step('Promote an advanced interior widget', async () => {
125+
await comfyPage.vueNodes.enterSubgraph(subgraphNodeId)
126+
const interiorNode =
127+
comfyPage.vueNodes.getNodeByTitle('ModelSamplingFlux')
128+
await expect(interiorNode).toBeVisible()
129+
await interiorNode
130+
.getByText('Show advanced inputs', { exact: true })
131+
.click()
132+
await expect(
133+
interiorNode.getByLabel('max_shift', { exact: true })
134+
).toBeVisible()
135+
await comfyPage.subgraph.promoteWidget(interiorNode, 'max_shift')
136+
await comfyPage.subgraph.exitViaBreadcrumb()
137+
})
138+
139+
await test.step('Keep the promoted widget visible on the host', async () => {
140+
await expectPromotedWidgetNamesToContain(
141+
comfyPage,
142+
subgraphNodeId,
143+
'max_shift'
144+
)
145+
await expect(
146+
comfyPage.vueNodes
147+
.getNodeLocator(subgraphNodeId)
148+
.getByLabel('max_shift', { exact: true })
149+
).toBeVisible()
150+
})
151+
}
152+
)
153+
97154
test('Promoted text widget renders and enters the subgraph in Vue mode', async ({
98155
comfyPage
99156
}) => {

src/renderer/extensions/vueNodes/composables/useProcessedWidgets.test.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,12 +278,16 @@ describe('promoted subgraph widgets', () => {
278278
cleanupComplexPromotionFixtureNodeType()
279279
})
280280

281-
function processHostWidget(graph: LGraph, hostNode: SubgraphNode) {
281+
function processHostWidget(
282+
graph: LGraph,
283+
hostNode: SubgraphNode,
284+
explicitWidgetIds = true
285+
) {
282286
const id = hostNode.widgets[0]?.widgetId
283287
if (!id) throw new Error('Expected the promoted host widget to be keyed')
284288
return computeProcessedWidgets({
285289
nodeData: hostNode._state,
286-
widgetIds: [id],
290+
widgetIds: explicitWidgetIds ? [id] : undefined,
287291
graphId: graph.id,
288292
showAdvanced: false,
289293
isGraphReady: true,
@@ -292,6 +296,17 @@ describe('promoted subgraph widgets', () => {
292296
})[0]
293297
}
294298

299+
function setHostWidgetOptions(
300+
hostNode: SubgraphNode,
301+
options: IBaseWidget['options']
302+
) {
303+
const id = hostNode.widgets[0]?.widgetId
304+
if (!id) throw new Error('Expected the promoted host widget to be keyed')
305+
if (!useWidgetValueStore().updateOptions(id, options)) {
306+
throw new Error('Expected promoted host widget state')
307+
}
308+
}
309+
295310
function recordInteriorError() {
296311
useExecutionErrorStore().recordNodeErrors({
297312
[SOURCE_EXECUTION_ID]: {
@@ -324,6 +339,24 @@ describe('promoted subgraph widgets', () => {
324339
)
325340
})
326341

342+
it.for([
343+
['shows on the node canvas', { advanced: true }, false, true],
344+
['shows when selected in App Mode', { advanced: true }, true, true],
345+
[
346+
'stays hidden when explicitly hidden',
347+
{ advanced: true, hidden: true },
348+
true,
349+
false
350+
]
351+
] as const)('%s', ([, options, explicitWidgetIds, visible]) => {
352+
const { graph, hostNode } = setupComplexPromotionFixture()
353+
setHostWidgetOptions(hostNode, options)
354+
355+
expect(processHostWidget(graph, hostNode, explicitWidgetIds).visible).toBe(
356+
visible
357+
)
358+
})
359+
327360
it('clears errors on both the interior source and the host', () => {
328361
const { graph, hostNode } = setupComplexPromotionFixture()
329362
recordInteriorError()

src/renderer/extensions/vueNodes/composables/useProcessedWidgets.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ function buildSlotMetadata(
146146
originOutputName: link
147147
? originNode?.outputs?.[link.originSlot]?.name
148148
: undefined,
149+
promoted: input.widgetId !== undefined,
149150
type: String(input.type)
150151
}
151152
if (input.name) metadata.set(input.name, slotInfo)
@@ -166,11 +167,11 @@ function getHostNode(
166167
function isWidgetVisible(
167168
options: IWidgetOptions,
168169
showAdvanced: boolean,
169-
linked = false
170+
ignoreAdvanced = false
170171
): boolean {
171172
const hidden = options.hidden ?? false
172173
const advanced = options.advanced ?? false
173-
return !hidden && (!advanced || showAdvanced || linked)
174+
return !hidden && (!advanced || showAdvanced || ignoreAdvanced)
174175
}
175176

176177
function hasWidgetError(
@@ -375,7 +376,11 @@ function processWidget(
375376
)
376377

377378
const slotInfo = ctx.slotMetadata.get(widgetState.name)
378-
const visible = isWidgetVisible(options, ctx.showAdvanced, slotInfo?.linked)
379+
const visible = isWidgetVisible(
380+
options,
381+
ctx.showAdvanced,
382+
slotInfo?.linked || slotInfo?.promoted
383+
)
379384
const isDisabled = slotInfo?.linked || widgetState.disabled
380385
const widgetOptions = isDisabled ? { ...options, disabled: true } : options
381386
const value = widgetState.value as WidgetValue

src/renderer/extensions/vueNodes/types/widgetGrid.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export interface WidgetSlotMetadata {
1010
linked: boolean
1111
originNodeId?: NodeId
1212
originOutputName?: string
13+
promoted: boolean
1314
type: string
1415
}
1516

src/stores/widgetValueStore.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,24 @@ describe('useWidgetValueStore', () => {
230230
).toBe(false)
231231
})
232232

233+
it('updateOptions preserves existing options and reports missing widgets', () => {
234+
const store = useWidgetValueStore()
235+
store.registerWidget(
236+
seedA,
237+
state('number', 100, { options: { min: 0, max: 10 } })
238+
)
239+
240+
expect(store.updateOptions(seedA, { advanced: true })).toBe(true)
241+
expect(store.getWidget(seedA)?.options).toEqual({
242+
min: 0,
243+
max: 10,
244+
advanced: true
245+
})
246+
expect(
247+
store.updateOptions(widgetId(graphA, toNodeId('missing'), 'seed'), {})
248+
).toBe(false)
249+
})
250+
233251
it('deleteWidget removes registered widgets from node order', () => {
234252
const store = useWidgetValueStore()
235253
const steps = widgetId(graphA, toNodeId('node-1'), 'steps')

src/stores/widgetValueStore.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,16 @@ export const useWidgetValueStore = defineStore('widgetValue', () => {
157157
return true
158158
}
159159

160+
function updateOptions(
161+
widgetId: WidgetId,
162+
options: Partial<WidgetState['options']>
163+
): boolean {
164+
const state = getWidget(widgetId)
165+
if (!state) return false
166+
state.options = { ...state.options, ...options }
167+
return true
168+
}
169+
160170
function deleteWidget(widgetId: WidgetId): boolean {
161171
if (!isWidgetId(widgetId)) return false
162172

@@ -241,6 +251,7 @@ export const useWidgetValueStore = defineStore('widgetValue', () => {
241251
getWidget,
242252
getWidgetRenderState,
243253
setValue,
254+
updateOptions,
244255
deleteWidget,
245256
getNodeWidgets,
246257
getNodeWidgetIds,

0 commit comments

Comments
 (0)