mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
fix: preserve CustomCombo options through clone and paste (#10853)
## Summary - Fix `CustomCombo` copy/paste so the combo keeps its option list and selected value - Scope the fix to `src/extensions/core/customWidgets.ts` instead of changing LiteGraph core deserialization - Replace the previous round-trip test with a regression test that exercises the actual clone/paste lifecycle - Fixes #9927 ## Root Cause `CustomCombo` option widgets override `value` to read from `widgetValueStore`. During `node.clone()` and clipboard paste, `configure()` restores widget values before the new node is added to the graph and before those widgets are registered in the store. That meant the option widgets read back as empty while `updateCombo()` was rebuilding the combo state, so `comboWidget.options.values` became blank on the pasted node. ## Fix Keep a local fallback value for each generated `option*` widget in `customWidgets.ts`. The getter now returns the store-backed value when available and falls back to the locally restored value before store registration. This preserves the option list during `clone().serialize()` and paste without hard-coding `CustomCombo` behavior into `LGraphNode.configure()`. ## Why No E2E Test This regression happens in the internal LiteGraph clipboard lifecycle: `clone() -> serialize() -> createNode() -> configure() -> graph.add()`. The failing state is the transient pre-add relationship between `CustomCombo`'s store-backed option widgets and `comboWidget.options.values`, which is not directly exposed through a stable DOM assertion in the current Playwright suite. A focused unit regression test is the most direct way to cover that lifecycle without depending on brittle canvas interaction timing. ## Test Plan - [x] Regression test covers `clone().serialize() -> createNode() -> configure() -> graph.add()` for `CustomCombo` - [ ] CI on the latest two commits (`81ac6d2ce`, `94147caf1`) - [ ] Manual: create `CustomCombo` -> add `alpha`, `beta`, `gamma` -> select `beta` -> copy/paste -> verify the pasted combo still shows all three options and keeps `beta` selected
This commit is contained in:
103
src/extensions/core/customWidgets.clone.test.ts
Normal file
103
src/extensions/core/customWidgets.clone.test.ts
Normal file
@@ -0,0 +1,103 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
|
||||
|
||||
import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import type { ComfyNodeDef } from '@/schemas/nodeDefSchema'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useExtensionStore } from '@/stores/extensionStore'
|
||||
import type { ComfyExtension } from '@/types/comfy'
|
||||
|
||||
const TEST_CUSTOM_COMBO_TYPE = 'test/CustomComboCopyPaste'
|
||||
|
||||
class TestCustomComboNode extends LGraphNode {
|
||||
static override title = 'CustomCombo'
|
||||
|
||||
constructor() {
|
||||
super('CustomCombo')
|
||||
this.serialize_widgets = true
|
||||
this.addOutput('value', '*')
|
||||
this.addWidget('combo', 'value', '', () => {}, {
|
||||
values: [] as string[]
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
function findWidget(node: LGraphNode, name: string) {
|
||||
return node.widgets?.find((widget) => widget.name === name)
|
||||
}
|
||||
|
||||
function getCustomWidgetsExtension(): ComfyExtension {
|
||||
const extension = useExtensionStore().extensions.find(
|
||||
(candidate) => candidate.name === 'Comfy.CustomWidgets'
|
||||
)
|
||||
|
||||
if (!extension) {
|
||||
throw new Error('Comfy.CustomWidgets extension was not registered')
|
||||
}
|
||||
|
||||
return extension
|
||||
}
|
||||
|
||||
describe('CustomCombo copy/paste', () => {
|
||||
beforeAll(async () => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
await import('./customWidgets')
|
||||
|
||||
const extension = getCustomWidgetsExtension()
|
||||
await extension.beforeRegisterNodeDef?.(
|
||||
TestCustomComboNode,
|
||||
{ name: 'CustomCombo' } as ComfyNodeDef,
|
||||
app
|
||||
)
|
||||
|
||||
if (LiteGraph.registered_node_types[TEST_CUSTOM_COMBO_TYPE]) {
|
||||
LiteGraph.unregisterNodeType(TEST_CUSTOM_COMBO_TYPE)
|
||||
}
|
||||
LiteGraph.registerNodeType(TEST_CUSTOM_COMBO_TYPE, TestCustomComboNode)
|
||||
})
|
||||
|
||||
afterAll(() => {
|
||||
if (LiteGraph.registered_node_types[TEST_CUSTOM_COMBO_TYPE]) {
|
||||
LiteGraph.unregisterNodeType(TEST_CUSTOM_COMBO_TYPE)
|
||||
}
|
||||
})
|
||||
|
||||
it('preserves combo options and selected value through clone and paste', () => {
|
||||
const graph = new LGraph()
|
||||
type AppWithRootGraph = { rootGraphInternal?: LGraph }
|
||||
const appWithRootGraph = app as unknown as AppWithRootGraph
|
||||
const previousRootGraph = appWithRootGraph.rootGraphInternal
|
||||
appWithRootGraph.rootGraphInternal = graph
|
||||
|
||||
try {
|
||||
const original = LiteGraph.createNode(TEST_CUSTOM_COMBO_TYPE)!
|
||||
graph.add(original)
|
||||
|
||||
findWidget(original, 'option1')!.value = 'alpha'
|
||||
findWidget(original, 'option2')!.value = 'beta'
|
||||
findWidget(original, 'option3')!.value = 'gamma'
|
||||
findWidget(original, 'value')!.value = 'beta'
|
||||
|
||||
const clonedSerialised = original.clone()?.serialize()
|
||||
|
||||
expect(clonedSerialised).toBeDefined()
|
||||
|
||||
const pasted = LiteGraph.createNode(TEST_CUSTOM_COMBO_TYPE)!
|
||||
pasted.configure(clonedSerialised!)
|
||||
graph.add(pasted)
|
||||
|
||||
expect(findWidget(pasted, 'value')!.value).toBe('beta')
|
||||
expect(findWidget(pasted, 'option1')!.value).toBe('alpha')
|
||||
expect(findWidget(pasted, 'option2')!.value).toBe('beta')
|
||||
expect(findWidget(pasted, 'option3')!.value).toBe('gamma')
|
||||
expect(findWidget(pasted, 'value')!.options.values).toEqual([
|
||||
'alpha',
|
||||
'beta',
|
||||
'gamma'
|
||||
])
|
||||
} finally {
|
||||
appWithRootGraph.rootGraphInternal = previousRootGraph
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -63,7 +63,7 @@ function onCustomComboCreated(this: LGraphNode) {
|
||||
(w) => w.name.startsWith('option') && w.value
|
||||
).map((w) => `${w.value}`)
|
||||
)
|
||||
if (app.configuringGraph) return
|
||||
if (app.configuringGraph || !this.graph) return
|
||||
if (values.includes(`${comboWidget.value}`)) return
|
||||
comboWidget.value = values[0] ?? ''
|
||||
comboWidget.callback?.(comboWidget.value)
|
||||
@@ -71,6 +71,9 @@ function onCustomComboCreated(this: LGraphNode) {
|
||||
comboWidget.callback = useChainCallback(comboWidget.callback, () =>
|
||||
this.applyToGraph!()
|
||||
)
|
||||
this.onAdded = useChainCallback(this.onAdded, function () {
|
||||
updateCombo()
|
||||
})
|
||||
|
||||
function addOption(node: LGraphNode) {
|
||||
if (!node.widgets) return
|
||||
@@ -78,16 +81,17 @@ function onCustomComboCreated(this: LGraphNode) {
|
||||
const widgetName = `option${newCount}`
|
||||
const widget = node.addWidget('string', widgetName, '', () => {})
|
||||
if (!widget) return
|
||||
let localValue = `${widget.value ?? ''}`
|
||||
|
||||
Object.defineProperty(widget, 'value', {
|
||||
get() {
|
||||
return useWidgetValueStore().getWidget(
|
||||
app.rootGraph.id,
|
||||
node.id,
|
||||
widgetName
|
||||
)?.value
|
||||
return (
|
||||
useWidgetValueStore().getWidget(app.rootGraph.id, node.id, widgetName)
|
||||
?.value ?? localValue
|
||||
)
|
||||
},
|
||||
set(v: string) {
|
||||
localValue = v
|
||||
const state = useWidgetValueStore().getWidget(
|
||||
app.rootGraph.id,
|
||||
node.id,
|
||||
|
||||
Reference in New Issue
Block a user