mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-31 13:29:55 +00:00
* [bugfix] Fix widget disconnection issue in subgraphs When disconnecting a node from a SubgraphInput, the target input's link reference was not being cleared in LLink.disconnect(). This caused widgets to remain greyed out because they still thought they were connected (slot.link was not null). The fix ensures that when a link is disconnected, the target node's input slot is properly cleaned up by setting input.link = null. Fixes #4922 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * [test] Add tests for LLink disconnect fix for widget issue Add comprehensive tests for the LLink.disconnect() method to verify that target input link references are properly cleared when disconnecting. This prevents widgets from remaining greyed out after disconnection. Tests cover: - Basic disconnect functionality with link reference cleanup - Edge cases with invalid target nodes - Preventing interference between different connections Related to #4922 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -414,6 +414,18 @@ export class LLink implements LinkSegment, Serialisable<SerialisableLLink> {
|
||||
* If `input` or `output`, reroutes will not be automatically removed, and retain a connection to the input or output, respectively.
|
||||
*/
|
||||
disconnect(network: LinkNetwork, keepReroutes?: 'input' | 'output'): void {
|
||||
// Clean up the target node's input slot
|
||||
if (this.target_id !== -1) {
|
||||
const targetNode = network.getNodeById(this.target_id)
|
||||
if (targetNode) {
|
||||
const targetInput = targetNode.inputs?.[this.target_slot]
|
||||
if (targetInput && targetInput.link === this.id) {
|
||||
targetInput.link = null
|
||||
targetNode.setDirtyCanvas?.(true, false)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const reroutes = LLink.getReroutes(network, this)
|
||||
|
||||
const lastReroute = reroutes.at(-1)
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect } from 'vitest'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { LLink } from '@/lib/litegraph/src/litegraph'
|
||||
import { LGraph, LGraphNode, LLink } from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
import { test } from './testExtensions'
|
||||
|
||||
@@ -14,4 +14,84 @@ describe('LLink', () => {
|
||||
const link = new LLink(1, 'float', 4, 2, 5, 3)
|
||||
expect(link.serialize()).toMatchSnapshot('Basic')
|
||||
})
|
||||
|
||||
describe('disconnect', () => {
|
||||
it('should clear the target input link reference when disconnecting', () => {
|
||||
// Create a graph and nodes
|
||||
const graph = new LGraph()
|
||||
const sourceNode = new LGraphNode('Source')
|
||||
const targetNode = new LGraphNode('Target')
|
||||
|
||||
// Add nodes to graph
|
||||
graph.add(sourceNode)
|
||||
graph.add(targetNode)
|
||||
|
||||
// Add slots
|
||||
sourceNode.addOutput('out', 'number')
|
||||
targetNode.addInput('in', 'number')
|
||||
|
||||
// Connect the nodes
|
||||
const link = sourceNode.connect(0, targetNode, 0)
|
||||
expect(link).toBeDefined()
|
||||
expect(targetNode.inputs[0].link).toBe(link?.id)
|
||||
|
||||
// Mock setDirtyCanvas
|
||||
const setDirtyCanvasSpy = vi.spyOn(targetNode, 'setDirtyCanvas')
|
||||
|
||||
// Disconnect the link
|
||||
link?.disconnect(graph)
|
||||
|
||||
// Verify the target input's link reference is cleared
|
||||
expect(targetNode.inputs[0].link).toBeNull()
|
||||
|
||||
// Verify setDirtyCanvas was called
|
||||
expect(setDirtyCanvasSpy).toHaveBeenCalledWith(true, false)
|
||||
})
|
||||
|
||||
it('should handle disconnecting when target node is not found', () => {
|
||||
// Create a link with invalid target
|
||||
const graph = new LGraph()
|
||||
const link = new LLink(1, 'number', 1, 0, 999, 0) // Invalid target id
|
||||
|
||||
// Should not throw when disconnecting
|
||||
expect(() => link.disconnect(graph)).not.toThrow()
|
||||
})
|
||||
|
||||
it('should only clear link reference if it matches the current link id', () => {
|
||||
// Create a graph and nodes
|
||||
const graph = new LGraph()
|
||||
const sourceNode1 = new LGraphNode('Source1')
|
||||
const sourceNode2 = new LGraphNode('Source2')
|
||||
const targetNode = new LGraphNode('Target')
|
||||
|
||||
// Add nodes to graph
|
||||
graph.add(sourceNode1)
|
||||
graph.add(sourceNode2)
|
||||
graph.add(targetNode)
|
||||
|
||||
// Add slots
|
||||
sourceNode1.addOutput('out', 'number')
|
||||
sourceNode2.addOutput('out', 'number')
|
||||
targetNode.addInput('in', 'number')
|
||||
|
||||
// Create first connection
|
||||
const link1 = sourceNode1.connect(0, targetNode, 0)
|
||||
expect(link1).toBeDefined()
|
||||
|
||||
// Disconnect first connection
|
||||
targetNode.disconnectInput(0)
|
||||
|
||||
// Create second connection
|
||||
const link2 = sourceNode2.connect(0, targetNode, 0)
|
||||
expect(link2).toBeDefined()
|
||||
expect(targetNode.inputs[0].link).toBe(link2?.id)
|
||||
|
||||
// Try to disconnect the first link (which is already disconnected)
|
||||
// It should not affect the current connection
|
||||
link1?.disconnect(graph)
|
||||
|
||||
// The input should still have the second link
|
||||
expect(targetNode.inputs[0].link).toBe(link2?.id)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user