mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
fix: batch image import into single undo entry
Wrap handleFileList in emitBeforeChange()/emitAfterChange() so dropping multiple images creates one undo entry instead of one per node. Add JSDoc to the undocumented batching APIs and unit tests for ChangeTracker batching.
This commit is contained in:
@@ -3972,14 +3972,41 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
)
|
||||
}
|
||||
|
||||
/** @todo Refactor to where it belongs - e.g. Deleting / creating nodes is not actually canvas event. */
|
||||
/**
|
||||
* Signals the start of a compound graph operation. All graph mutations
|
||||
* between this call and the matching {@link emitAfterChange} are treated
|
||||
* as a single undoable action by the change tracking system.
|
||||
*
|
||||
* Emits a `litegraph:canvas` DOM event with `subType: 'before-change'`,
|
||||
* which `ChangeTracker` listens for to suppress intermediate state
|
||||
* snapshots. Calls are nestable — only the outermost pair triggers a
|
||||
* state check.
|
||||
*
|
||||
* Always pair with {@link emitAfterChange} in a `try/finally` block.
|
||||
*
|
||||
* @example
|
||||
* ```ts
|
||||
* canvas.emitBeforeChange()
|
||||
* try {
|
||||
* // multiple graph mutations...
|
||||
* } finally {
|
||||
* canvas.emitAfterChange()
|
||||
* }
|
||||
* ```
|
||||
*/
|
||||
emitBeforeChange(): void {
|
||||
this.emitEvent({
|
||||
subType: 'before-change'
|
||||
})
|
||||
}
|
||||
|
||||
/** @todo See {@link emitBeforeChange} */
|
||||
/**
|
||||
* Signals the end of a compound graph operation started by
|
||||
* {@link emitBeforeChange}. When the outermost pair completes, the
|
||||
* change tracking system takes a single state snapshot and records
|
||||
* one undo entry for all mutations since the matching
|
||||
* `emitBeforeChange`.
|
||||
*/
|
||||
emitAfterChange(): void {
|
||||
this.emitEvent({
|
||||
subType: 'after-change'
|
||||
|
||||
@@ -1720,21 +1720,26 @@ export class ComfyApp {
|
||||
if (fileList.length === 0) return
|
||||
if (!fileList[0].type.startsWith('image')) return
|
||||
|
||||
const imageNodes = await pasteImageNodes(this.canvas, fileList)
|
||||
if (imageNodes.length === 0) return
|
||||
this.canvas.emitBeforeChange()
|
||||
try {
|
||||
const imageNodes = await pasteImageNodes(this.canvas, fileList)
|
||||
if (imageNodes.length === 0) return
|
||||
|
||||
if (imageNodes.length > 1) {
|
||||
const batchImagesNode = await createNode(this.canvas, 'BatchImagesNode')
|
||||
if (!batchImagesNode) return
|
||||
if (imageNodes.length > 1) {
|
||||
const batchImagesNode = await createNode(
|
||||
this.canvas,
|
||||
'BatchImagesNode'
|
||||
)
|
||||
if (!batchImagesNode) return
|
||||
|
||||
this.positionBatchNodes(imageNodes, batchImagesNode)
|
||||
this.canvas.selectItems([...imageNodes, batchImagesNode])
|
||||
|
||||
imageNodes.forEach((imageNode, index) => {
|
||||
imageNode.connect(0, batchImagesNode, index)
|
||||
})
|
||||
} else {
|
||||
this.canvas.selectItems(imageNodes)
|
||||
imageNodes.forEach((imageNode, index) => {
|
||||
imageNode.connect(0, batchImagesNode, index)
|
||||
})
|
||||
} else {
|
||||
this.canvas.selectItems(imageNodes)
|
||||
}
|
||||
} finally {
|
||||
this.canvas.emitAfterChange()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
66
src/scripts/changeTracker.test.ts
Normal file
66
src/scripts/changeTracker.test.ts
Normal file
@@ -0,0 +1,66 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { ComfyWorkflow } from '@/platform/workflow/management/stores/workflowStore'
|
||||
import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
|
||||
import { ChangeTracker } from './changeTracker'
|
||||
|
||||
vi.mock('@/scripts/app', () => ({
|
||||
app: {
|
||||
graph: {},
|
||||
rootGraph: { serialize: () => ({ nodes: [], links: [] }) },
|
||||
canvas: { ds: { scale: 1, offset: [0, 0] } }
|
||||
}
|
||||
}))
|
||||
vi.mock('@/scripts/api', () => ({
|
||||
api: {
|
||||
dispatchCustomEvent: vi.fn(),
|
||||
apiURL: vi.fn((path: string) => path)
|
||||
}
|
||||
}))
|
||||
vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({
|
||||
useWorkflowStore: () => ({ getWorkflowByPath: () => null })
|
||||
}))
|
||||
vi.mock('@/stores/subgraphNavigationStore', () => ({
|
||||
useSubgraphNavigationStore: () => ({ exportState: () => [] })
|
||||
}))
|
||||
|
||||
describe('ChangeTracker', () => {
|
||||
let tracker: ChangeTracker
|
||||
|
||||
beforeEach(() => {
|
||||
const mockWorkflow = { path: 'test.json' } as unknown as ComfyWorkflow
|
||||
const initialState = {
|
||||
version: 1,
|
||||
nodes: [],
|
||||
links: [],
|
||||
last_node_id: 0,
|
||||
last_link_id: 0
|
||||
} as unknown as ComfyWorkflowJSON
|
||||
tracker = new ChangeTracker(mockWorkflow, initialState)
|
||||
})
|
||||
|
||||
describe('beforeChange / afterChange batching', () => {
|
||||
it('calls checkState only when outermost afterChange completes', () => {
|
||||
const checkStateSpy = vi.spyOn(tracker, 'checkState')
|
||||
|
||||
tracker.beforeChange()
|
||||
tracker.afterChange()
|
||||
|
||||
expect(checkStateSpy).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('suppresses checkState for nested calls until fully unwound', () => {
|
||||
const checkStateSpy = vi.spyOn(tracker, 'checkState')
|
||||
|
||||
tracker.beforeChange()
|
||||
tracker.beforeChange()
|
||||
|
||||
tracker.afterChange()
|
||||
expect(checkStateSpy).not.toHaveBeenCalled()
|
||||
|
||||
tracker.afterChange()
|
||||
expect(checkStateSpy).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -34,6 +34,13 @@ export class ChangeTracker {
|
||||
activeState: ComfyWorkflowJSON
|
||||
undoQueue: ComfyWorkflowJSON[] = []
|
||||
redoQueue: ComfyWorkflowJSON[] = []
|
||||
/**
|
||||
* Nesting counter for compound operations. While greater than zero,
|
||||
* {@link checkState} is suppressed. Incremented by {@link beforeChange},
|
||||
* decremented by {@link afterChange}. When it returns to zero,
|
||||
* `checkState()` runs and captures a single undo entry for all mutations
|
||||
* since the first `beforeChange`.
|
||||
*/
|
||||
changeCount: number = 0
|
||||
/**
|
||||
* Whether the redo/undo restoring is in progress.
|
||||
@@ -203,10 +210,24 @@ export class ChangeTracker {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Marks the start of a compound operation. Increments the nesting
|
||||
* counter to suppress {@link checkState} until the matching
|
||||
* {@link afterChange} call completes.
|
||||
*
|
||||
* Typically called via `LGraphCanvas.emitBeforeChange()` through the
|
||||
* `litegraph:canvas` DOM event, rather than directly.
|
||||
*/
|
||||
beforeChange() {
|
||||
this.changeCount++
|
||||
}
|
||||
|
||||
/**
|
||||
* Marks the end of a compound operation. Decrements the nesting
|
||||
* counter; when it reaches zero, calls {@link checkState} to capture
|
||||
* a single undo entry for all mutations since the first
|
||||
* {@link beforeChange}.
|
||||
*/
|
||||
afterChange() {
|
||||
if (!--this.changeCount) {
|
||||
this.checkState()
|
||||
|
||||
Reference in New Issue
Block a user