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

Pinning / Unpinning multiple nodes at the same time doesn't work #4863

Closed
MNeMoNiCuZ opened this issue Sep 9, 2024 · 6 comments · Fixed by Comfy-Org/ComfyUI_frontend#791
Closed
Labels
Frontend Issue relates to the frontend UI (litegraph).

Comments

@MNeMoNiCuZ
Copy link

Expected Behavior

When having multiple nodes selected, I expect to be able to pin and unpin several nodes.

Additionally, pinning is not visible in the editor. It should, similar to LOCKING have a PIN-icon to make this state clear to the user.

It can be very frustrating to get a workflow where you can't move stuff around, but you don't understand why.

Actual Behavior

Selecting multiple nodes and pinning/unpinning only pins/unpins the node that you right-clicked on.

Steps to Reproduce

Open default workflow
CTRL + A to select all nodes
Right-click on a node
Select Pin from the list

Only the node you right-clicked on is pinned.

Debug Logs

Starting server

To see the GUI go to: http://127.0.0.1:8188

Other

I understand this may not be intended behavior, since the "Pin" is an option on the context menu of the node you selected.
It's possible that not all nodes or things in the UI support pinning.

But I believe this is not a good behavior.
Pinning, like Grouping, is a general behavior, and should not be node-specific.

Or rather:
When you have multiple things selected, the list should only show things that are available to be done on everything that you have selected.

@MNeMoNiCuZ MNeMoNiCuZ added the Potential Bug User is reporting a bug. This should be tested. label Sep 9, 2024
@PGCRT
Copy link

PGCRT commented Sep 10, 2024

Multiple unpin doesn't work, we should also be able to collapse if pinned, just like before.

@PGCRT
Copy link

PGCRT commented Sep 10, 2024

So I came here first to post a request about missing node color and BG options since the new frontend update, and I just found the solution, this is also fixing the Pin issue ,) How many things this will fix I don't know yet but hey, I just removed this argument from the launcher, "--front-end-version Comfy-Org/ComfyUI_frontend@latest" thats it!

I guess we read too well the documentations ,) I hope this will help

@MNeMoNiCuZ
Copy link
Author

So I came here first to post a request about missing node color and BG options since the new frontend update, and I just found the solution, this is also fixing the Pin issue ,) How many things this will fix I don't know yet but hey, I just removed this argument from the launcher, "--front-end-version Comfy-Org/ComfyUI_frontend@latest" thats it!

I guess we read too well the documentations ,) I hope this will help

That could be a workaround if it's something you need urgently.
But looking for future stability, a proper solution is better :)

@darkflare
Copy link

Also now that the "lock" function was replaced by the "pinned" one.

Replaced locking with pinning in core Comfy-Org/ComfyUI_frontend#734)

A visual indicator of a "pinned" node would be nice for fast visual confirmation.

@huchenlei
Copy link
Collaborator

Press "p" should pin/unpin all selected nodes.

@huchenlei huchenlei reopened this Sep 11, 2024
@huchenlei huchenlei added Frontend Issue relates to the frontend UI (litegraph). and removed Potential Bug User is reporting a bug. This should be tested. labels Sep 11, 2024
@MNeMoNiCuZ
Copy link
Author

Press "p" should pin/unpin all selected nodes.

@huchenlei Thanks for the advice.
That's good, but it should also be contextually changed to "Pin All" or "Unpin All" if you have multiple nodes selected.

I think it doesn't make sense to show the context menu for one item, when you have multiple selected.
Having multiple items selected means you're intending to do something with the selected things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend Issue relates to the frontend UI (litegraph).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants