mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-02 11:40:00 +00:00
Fix "Dragging from input slot connected to SubgraphInputNode creates new link instead of moving existing one" (#1184)
Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: bymyself <cbyrne@comfy.org>
This commit is contained in:
@@ -150,25 +150,59 @@ export class LinkConnector {
|
||||
const link = network.links.get(linkId)
|
||||
if (!link) return
|
||||
|
||||
try {
|
||||
const reroute = network.getReroute(link.parentId)
|
||||
const renderLink = new MovingInputLink(network, link, reroute)
|
||||
// Special handling for links from subgraph input nodes
|
||||
if (link.origin_id === SUBGRAPH_INPUT_ID) {
|
||||
// For subgraph input links, we need to handle them differently
|
||||
// since they don't have a regular output node
|
||||
const subgraphInput = network.inputNode?.slots[link.origin_slot]
|
||||
if (!subgraphInput) {
|
||||
console.warn(`Could not find subgraph input for slot [${link.origin_slot}]`)
|
||||
return
|
||||
}
|
||||
|
||||
const mayContinue = this.events.dispatch("before-move-input", renderLink)
|
||||
if (mayContinue === false) return
|
||||
try {
|
||||
const reroute = network.getReroute(link.parentId)
|
||||
const renderLink = new ToInputFromIoNodeLink(network, network.inputNode, subgraphInput, reroute, LinkDirection.CENTER, link)
|
||||
|
||||
renderLinks.push(renderLink)
|
||||
// Note: We don't dispatch the before-move-input event for subgraph input links
|
||||
// as the event type doesn't support ToInputFromIoNodeLink
|
||||
|
||||
this.listenUntilReset("input-moved", (e) => {
|
||||
e.detail.link.disconnect(network, "output")
|
||||
})
|
||||
} catch (error) {
|
||||
console.warn(`Could not create render link for link id: [${link.id}].`, link, error)
|
||||
return
|
||||
renderLinks.push(renderLink)
|
||||
|
||||
this.listenUntilReset("input-moved", () => {
|
||||
link.disconnect(network, "input")
|
||||
})
|
||||
} catch (error) {
|
||||
console.warn(`Could not create render link for subgraph input link id: [${link.id}].`, link, error)
|
||||
return
|
||||
}
|
||||
|
||||
link._dragging = true
|
||||
inputLinks.push(link)
|
||||
} else {
|
||||
// Regular node links
|
||||
try {
|
||||
const reroute = network.getReroute(link.parentId)
|
||||
const renderLink = new MovingInputLink(network, link, reroute)
|
||||
|
||||
const mayContinue = this.events.dispatch("before-move-input", renderLink)
|
||||
if (mayContinue === false) return
|
||||
|
||||
renderLinks.push(renderLink)
|
||||
|
||||
this.listenUntilReset("input-moved", (e) => {
|
||||
if ("link" in e.detail && e.detail.link) {
|
||||
e.detail.link.disconnect(network, "output")
|
||||
}
|
||||
})
|
||||
} catch (error) {
|
||||
console.warn(`Could not create render link for link id: [${link.id}].`, link, error)
|
||||
return
|
||||
}
|
||||
|
||||
link._dragging = true
|
||||
inputLinks.push(link)
|
||||
}
|
||||
|
||||
link._dragging = true
|
||||
inputLinks.push(link)
|
||||
}
|
||||
|
||||
state.connectingTo = "input"
|
||||
|
||||
@@ -18,6 +18,7 @@ export class ToInputFromIoNodeLink implements RenderLink {
|
||||
readonly fromSlotIndex: number
|
||||
readonly fromPos: Point
|
||||
fromDirection: LinkDirection = LinkDirection.RIGHT
|
||||
readonly existingLink?: LLink
|
||||
|
||||
constructor(
|
||||
readonly network: LinkNetwork,
|
||||
@@ -25,6 +26,7 @@ export class ToInputFromIoNodeLink implements RenderLink {
|
||||
readonly fromSlot: SubgraphInput,
|
||||
readonly fromReroute?: Reroute,
|
||||
public dragDirection: LinkDirection = LinkDirection.CENTER,
|
||||
existingLink?: LLink,
|
||||
) {
|
||||
const outputIndex = node.slots.indexOf(fromSlot)
|
||||
if (outputIndex === -1 && fromSlot !== node.emptySlot) {
|
||||
@@ -35,6 +37,7 @@ export class ToInputFromIoNodeLink implements RenderLink {
|
||||
this.fromPos = fromReroute
|
||||
? fromReroute.pos
|
||||
: fromSlot.pos
|
||||
this.existingLink = existingLink
|
||||
}
|
||||
|
||||
canConnectToInput(inputNode: NodeLike, input: INodeInputSlot): boolean {
|
||||
@@ -46,10 +49,17 @@ export class ToInputFromIoNodeLink implements RenderLink {
|
||||
}
|
||||
|
||||
connectToInput(node: LGraphNode, input: INodeInputSlot, events: CustomEventTarget<LinkConnectorEventMap>) {
|
||||
const { fromSlot, fromReroute } = this
|
||||
const { fromSlot, fromReroute, existingLink } = this
|
||||
|
||||
const newLink = fromSlot.connect(input, node, fromReroute?.id)
|
||||
events.dispatch("link-created", newLink)
|
||||
|
||||
if (existingLink) {
|
||||
// Moving an existing link
|
||||
events.dispatch("input-moved", this)
|
||||
} else {
|
||||
// Creating a new link
|
||||
events.dispatch("link-created", newLink)
|
||||
}
|
||||
}
|
||||
|
||||
connectToSubgraphOutput(): void {
|
||||
@@ -96,7 +106,14 @@ export class ToInputFromIoNodeLink implements RenderLink {
|
||||
}
|
||||
}
|
||||
}
|
||||
events.dispatch("link-created", newLink)
|
||||
|
||||
if (this.existingLink) {
|
||||
// Moving an existing link
|
||||
events.dispatch("input-moved", this)
|
||||
} else {
|
||||
// Creating a new link
|
||||
events.dispatch("link-created", newLink)
|
||||
}
|
||||
}
|
||||
|
||||
connectToOutput() {
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { FloatingRenderLink } from "@/canvas/FloatingRenderLink"
|
||||
import type { MovingInputLink } from "@/canvas/MovingInputLink"
|
||||
import type { MovingOutputLink } from "@/canvas/MovingOutputLink"
|
||||
import type { RenderLink } from "@/canvas/RenderLink"
|
||||
import type { ToInputFromIoNodeLink } from "@/canvas/ToInputFromIoNodeLink"
|
||||
import type { ToInputRenderLink } from "@/canvas/ToInputRenderLink"
|
||||
import type { LGraphNode } from "@/LGraphNode"
|
||||
import type { LLink } from "@/LLink"
|
||||
@@ -26,7 +27,7 @@ export interface LinkConnectorEventMap {
|
||||
"before-move-input": MovingInputLink | FloatingRenderLink
|
||||
"before-move-output": MovingOutputLink | FloatingRenderLink
|
||||
|
||||
"input-moved": MovingInputLink | FloatingRenderLink
|
||||
"input-moved": MovingInputLink | FloatingRenderLink | ToInputFromIoNodeLink
|
||||
"output-moved": MovingOutputLink | FloatingRenderLink
|
||||
|
||||
"link-created": LLink | null | undefined
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { describe, expect, it } from "vitest"
|
||||
import { describe, expect, it, vi } from "vitest"
|
||||
|
||||
import { LGraphNode } from "@/litegraph"
|
||||
import { LinkConnector } from "@/canvas/LinkConnector"
|
||||
import { ToInputFromIoNodeLink } from "@/canvas/ToInputFromIoNodeLink"
|
||||
import { SUBGRAPH_INPUT_ID } from "@/constants"
|
||||
import { LGraphNode, type LinkNetwork } from "@/litegraph"
|
||||
import { NodeInputSlot } from "@/node/NodeInputSlot"
|
||||
import { NodeOutputSlot } from "@/node/NodeOutputSlot"
|
||||
import { isSubgraphInput, isSubgraphOutput } from "@/subgraph/subgraphUtils"
|
||||
@@ -103,6 +106,55 @@ describe("Subgraph slot connections", () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe("LinkConnector dragging behavior", () => {
|
||||
it("should drag existing link when dragging from input slot connected to subgraph input node", () => {
|
||||
// Create a subgraph with one input
|
||||
const subgraph = createTestSubgraph({
|
||||
inputs: [{ name: "input1", type: "number" }],
|
||||
})
|
||||
|
||||
// Create a node inside the subgraph
|
||||
const internalNode = new LGraphNode("InternalNode")
|
||||
internalNode.id = 100
|
||||
internalNode.addInput("in", "number")
|
||||
subgraph.add(internalNode)
|
||||
|
||||
// Connect the subgraph input to the internal node's input
|
||||
const link = subgraph.inputNode.slots[0].connect(internalNode.inputs[0], internalNode)
|
||||
expect(link).toBeDefined()
|
||||
expect(link!.origin_id).toBe(SUBGRAPH_INPUT_ID)
|
||||
expect(link!.target_id).toBe(internalNode.id)
|
||||
|
||||
// Verify the input slot has the link
|
||||
expect(internalNode.inputs[0].link).toBe(link!.id)
|
||||
|
||||
// Create a LinkConnector
|
||||
const setConnectingLinks = vi.fn()
|
||||
const connector = new LinkConnector(setConnectingLinks)
|
||||
|
||||
// Now try to drag from the input slot
|
||||
connector.moveInputLink(subgraph as LinkNetwork, internalNode.inputs[0])
|
||||
|
||||
// Verify that we're dragging the existing link
|
||||
expect(connector.isConnecting).toBe(true)
|
||||
expect(connector.state.connectingTo).toBe("input")
|
||||
expect(connector.state.draggingExistingLinks).toBe(true)
|
||||
|
||||
// Check that we have exactly one render link
|
||||
expect(connector.renderLinks).toHaveLength(1)
|
||||
|
||||
// The render link should be a ToInputFromIoNodeLink, not MovingInputLink
|
||||
expect(connector.renderLinks[0]).toBeInstanceOf(ToInputFromIoNodeLink)
|
||||
|
||||
// The input links collection should contain our link
|
||||
expect(connector.inputLinks).toHaveLength(1)
|
||||
expect(connector.inputLinks[0]).toBe(link)
|
||||
|
||||
// Verify the link is marked as dragging
|
||||
expect(link!._dragging).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("Type compatibility", () => {
|
||||
it("should respect type compatibility for SubgraphInput connections", () => {
|
||||
const subgraph = createTestSubgraph({
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { ISlotType } from "@/interfaces"
|
||||
import type { TWidgetType } from "@/types/widgets"
|
||||
|
||||
import { describe, expect, it, vi } from "vitest"
|
||||
import { describe, expect, it } from "vitest"
|
||||
|
||||
import { LGraphNode, Subgraph } from "@/litegraph"
|
||||
import { BaseWidget } from "@/widgets/BaseWidget"
|
||||
@@ -339,7 +339,7 @@ describe("SubgraphWidgetPromotion", () => {
|
||||
|
||||
// The promoted widget should preserve the original tooltip
|
||||
expect(promotedWidget.tooltip).toBe(originalTooltip)
|
||||
|
||||
|
||||
// The promoted widget should still function normally
|
||||
expect(promotedWidget.name).toBe("value") // Uses subgraph input name
|
||||
expect(promotedWidget.type).toBe("number")
|
||||
|
||||
Reference in New Issue
Block a user