Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add output multi-link move using shift-click #32

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

webfiltered
Copy link
Collaborator

Problem

When an output connects to multiple inputs, and you'd like to move all of those links to another node, you are currently required to drag each new link one by one.

Solution

Hold shift to pull all links on an output to a new node, or new output. Does not disconnect links until they land on a valid target.

It functions with reroute nodes, however it requires that they are dropped onto the new output directly, not just anywhere on the node. This is expected, really.

Limitations

It -does not- currently work with the drop to blank space + search for new node. It will j ust rewire the first. This can probably be fixed easily enough.

When an output connects to multiple inputs, and you'd like to move all of those links to another node, you are currently required to drag each new link one by one.

This commit adds the ability to move everything at once, using Shift + Click (and drag).

It -does not- currently work with the drop to blank space + search for new node.  It will j ust rewire the first.  This can probably be fixed easily enough.

It -does- function with reroute nodes, however it requires that they are dropped onto the new output directly, not just anywhere on the node.  This is expected, really.
@webfiltered
Copy link
Collaborator Author

Would be great to know if this kind of feature PR is welcome, or if there isn't much interest. I do have more ideas, but am dealing with a large amount of manual effort merging each PR, due to my current setup.

@huchenlei
Copy link
Member

@webfiltered
Sorry for the late reply. I think when I fork this repo under ComfyOrg, it does not automatically add it to my watch list, so I missed your PRs. Going to review them all and get them merged!

What is the major blocker on your setup that requires manual work?

@huchenlei
Copy link
Member

If you need something merged, you can ping me directly on discord / matrix

@huchenlei
Copy link
Member

Actually would it be easier to create a reroute node when shift click output? I know someone was requesting this feature build into core.

d527d88e22bce8d29ca9aa89e67e64a8.mp4

It's currently in https://github.com/coolzilj/ComfyUI-LJNodes

@webfiltered
Copy link
Collaborator Author

No worries at all! The blocker is all me - my messy Comfy directory. I generally start tinkering there and have to redo everything into a clean install. And one of these PRs was a whitespace-matching nightmare in vscode. I really just need to get the (lovely!) new TS building and deploying directly into my install.

But anyway! To the nodes: I think a reroute would be a good option for a lot of people, but I do like having the choice to drag/drop where I want them.

Especially when the target drop zone is usually very large - for a reroute you could just drop on the canvas, and it should be the first option in the menu. But if you wanted it on the Checkpoint in that video, you'd just shift-click and drag anywhere on that large node.

But why not do both! I could add a config switch to control behaviour?

@huchenlei huchenlei merged commit 3175828 into Comfy-Org:master Jul 29, 2024
2 checks passed
huchenlei added a commit that referenced this pull request Jul 30, 2024
huchenlei added a commit that referenced this pull request Jul 30, 2024
@huchenlei
Copy link
Member

@webfiltered There is a regression on link release behavior. PTAL #37

@webfiltered
Copy link
Collaborator Author

Gross! Please tell me this didn't hit end users. =(

Video on #37 is screaming "LiteGraph pos-JS-exception" - nothing in console?

@huchenlei
Copy link
Member

image
image

@webfiltered
Copy link
Collaborator Author

I am struggling to locate the code in that screenshot - hopefully indicating that it's new and not just something I missed.

But I'm updating my PR repos now.

@huchenlei
Copy link
Member

I am struggling to locate the code in that screenshot - hopefully indicating that it's new and not just something I missed.

But I'm updating my PR repos now.

The change was made in #10

@webfiltered
Copy link
Collaborator Author

webfiltered commented Jul 30, 2024

Found it. My mistake - rooted in my messy PR workflow. I was ignoring the incoming changes when manually merging - completely forgot to audit them afterwards.

I'll get the TS building before I submit more. Fix commit (NOT verified) coming shortly.

Edit: a second too slow..! Thank you.

huchenlei pushed a commit that referenced this pull request Jul 30, 2024
When an output connects to multiple inputs, and you'd like to move all of those links to another node, you are currently required to drag each new link one by one.

This commit adds the ability to move everything at once, using Shift + Click (and drag).

It -does not- currently work with the drop to blank space + search for new node.  It will j ust rewire the first.  This can probably be fixed easily enough.

It -does- function with reroute nodes, however it requires that they are dropped onto the new output directly, not just anywhere on the node.  This is expected, really.
huchenlei added a commit that referenced this pull request Jul 30, 2024
* Add node disconnect shortcuts (#31)

* Fix loop break missing

* Fix logic - cannot reconnect AND disconnect

* Add ctrl + alt + click to disconnect nodes

Adds disconnect feature and very minor bug fixes (in separate commits):
- Ctrl + Alt + Click: Disconnect an input or output
- Ctrl + Alt + Click & Drag: Rewire any input/output to another node with a single click
- Added LiteGraph setting, on by default.

6036: skip_action = true

Not sure why skip_action was set to true, here.  It prevents disconnect and drag to a new output on the same click, so I've included it in the main commit.  Ideally, this should be controlled by a consumer hook, e.g. onDisconnectInput.

* Add output multi-link move using shift-click (#32)

When an output connects to multiple inputs, and you'd like to move all of those links to another node, you are currently required to drag each new link one by one.

This commit adds the ability to move everything at once, using Shift + Click (and drag).

It -does not- currently work with the drop to blank space + search for new node.  It will j ust rewire the first.  This can probably be fixed easily enough.

It -does- function with reroute nodes, however it requires that they are dropped onto the new output directly, not just anywhere on the node.  This is expected, really.

* Update empty-release event protocol

---------

Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants