From a568c0651f7f07d729098411566843b8dacb98f8 Mon Sep 17 00:00:00 2001 From: Benjamin Lu Date: Sat, 2 Aug 2025 00:56:41 -0400 Subject: [PATCH] Fix "Dragging from input slot connected to SubgraphInputNode creates new link instead of moving existing one" (#1184) Co-authored-by: Claude Co-authored-by: bymyself --- src/canvas/LinkConnector.ts | 64 ++++++++++++++----- src/canvas/ToInputFromIoNodeLink.ts | 23 ++++++- src/infrastructure/LinkConnectorEventMap.ts | 3 +- test/subgraph/SubgraphSlotConnections.test.ts | 56 +++++++++++++++- test/subgraph/SubgraphWidgetPromotion.test.ts | 4 +- 5 files changed, 127 insertions(+), 23 deletions(-) diff --git a/src/canvas/LinkConnector.ts b/src/canvas/LinkConnector.ts index c2bd9cf76f..349a903091 100644 --- a/src/canvas/LinkConnector.ts +++ b/src/canvas/LinkConnector.ts @@ -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" diff --git a/src/canvas/ToInputFromIoNodeLink.ts b/src/canvas/ToInputFromIoNodeLink.ts index a24203858b..0ed8e1b3a6 100644 --- a/src/canvas/ToInputFromIoNodeLink.ts +++ b/src/canvas/ToInputFromIoNodeLink.ts @@ -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) { - 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() { diff --git a/src/infrastructure/LinkConnectorEventMap.ts b/src/infrastructure/LinkConnectorEventMap.ts index 985d24dd4e..10e4a6fc70 100644 --- a/src/infrastructure/LinkConnectorEventMap.ts +++ b/src/infrastructure/LinkConnectorEventMap.ts @@ -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 diff --git a/test/subgraph/SubgraphSlotConnections.test.ts b/test/subgraph/SubgraphSlotConnections.test.ts index 1caa4bb60b..da9672daa8 100644 --- a/test/subgraph/SubgraphSlotConnections.test.ts +++ b/test/subgraph/SubgraphSlotConnections.test.ts @@ -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({ diff --git a/test/subgraph/SubgraphWidgetPromotion.test.ts b/test/subgraph/SubgraphWidgetPromotion.test.ts index 15faa938dd..5af4e36be3 100644 --- a/test/subgraph/SubgraphWidgetPromotion.test.ts +++ b/test/subgraph/SubgraphWidgetPromotion.test.ts @@ -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")