mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-21 23:34:31 +00:00
fix: add beforeChange/afterChange to convertToSubgraph for proper undo (#7791)
## Summary - Adds missing `beforeChange()` and `afterChange()` lifecycle calls to `convertToSubgraph()` method ## Problem When converting nodes to a subgraph and then pressing Ctrl+Z to undo, the node positions were being changed from their original locations instead of being properly restored. ## Root Cause The `convertToSubgraph()` method in `LGraph.ts` was missing the `beforeChange()` and `afterChange()` lifecycle calls that are needed for proper undo/redo state tracking. These calls record the graph state before and after modifications. The inverse operation `unpackSubgraph()` already has these calls (see line 1742), so this is simply matching the existing pattern. ## Solution Add `beforeChange()` at the start of the method (after validation) and `afterChange()` before the return. ## Testing 1. Create a workflow with several nodes positioned in specific locations 2. Select 2-3 nodes 3. Right-click → "Convert Selection to Subgraph" 4. Press Ctrl+Z to undo 5. Verify nodes return to their exact original positions Fixes comfyanonymous/ComfyUI#11514 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7791-fix-add-beforeChange-afterChange-to-convertToSubgraph-for-proper-undo-2d86d73d36508125a2c4e4a412cced4a) by [Unito](https://www.unito.io) --------- Co-authored-by: RUiNtheExtinct <deepkarma001@gmail.com>
This commit is contained in:
@@ -1522,6 +1522,22 @@ export class LGraph
|
||||
} {
|
||||
if (items.size === 0)
|
||||
throw new Error('Cannot convert to subgraph: nothing to convert')
|
||||
|
||||
// Record state before conversion for proper undo support
|
||||
this.beforeChange()
|
||||
|
||||
try {
|
||||
return this._convertToSubgraphImpl(items)
|
||||
} finally {
|
||||
// Mark state change complete for proper undo support
|
||||
this.afterChange()
|
||||
}
|
||||
}
|
||||
|
||||
private _convertToSubgraphImpl(items: Set<Positionable>): {
|
||||
subgraph: Subgraph
|
||||
node: SubgraphNode
|
||||
} {
|
||||
const { state, revision, config } = this
|
||||
const firstChild = [...items][0]
|
||||
if (items.size === 1 && firstChild instanceof LGraphGroup) {
|
||||
@@ -1728,6 +1744,7 @@ export class LGraph
|
||||
|
||||
subgraphNode._setConcreteSlots()
|
||||
subgraphNode.arrange()
|
||||
|
||||
this.canvasAction((c) =>
|
||||
c.canvas.dispatchEvent(
|
||||
new CustomEvent('subgraph-converted', {
|
||||
@@ -1746,9 +1763,23 @@ export class LGraph
|
||||
if (!(subgraphNode instanceof SubgraphNode))
|
||||
throw new Error('Can only unpack Subgraph Nodes')
|
||||
|
||||
// Record state before unpacking for proper undo support
|
||||
this.beforeChange()
|
||||
|
||||
try {
|
||||
this._unpackSubgraphImpl(subgraphNode, options)
|
||||
} finally {
|
||||
// Mark state change complete for proper undo support
|
||||
this.afterChange()
|
||||
}
|
||||
}
|
||||
|
||||
private _unpackSubgraphImpl(
|
||||
subgraphNode: SubgraphNode,
|
||||
options?: { skipMissingNodes?: boolean }
|
||||
) {
|
||||
const skipMissingNodes = options?.skipMissingNodes ?? false
|
||||
|
||||
this.beforeChange()
|
||||
//NOTE: Create bounds can not be called on positionables directly as the subgraph is not being displayed and boundingRect is not initialized.
|
||||
//NOTE: NODE_TITLE_HEIGHT is explicitly excluded here
|
||||
const positionables = [
|
||||
@@ -2019,7 +2050,6 @@ export class LGraph
|
||||
}
|
||||
|
||||
this.canvasAction((c) => c.selectItems(toSelect))
|
||||
this.afterChange()
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user