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

New Menu & Workflow Management #3112

Open
wants to merge 46 commits into
base: master
Choose a base branch
from

Conversation

pythongosssss
Copy link
Contributor

Quite a few changes in here for this update, been working on it for a while and any feedback would be greatly appreciated.
I've tried to make this as backwards compatible as possible and included the new menu as a setting for people to enable and try out, but like always its hard to know if this will conflict with any extensions so any testing people can do would also be great.

Updates:

  • New option to enable this menu ([Beta] Use new menu and workflow management)
  • Changes the floating menu to be a fixed top menu
  • Add workflow management
  • Multiple open workflows + undo/redo is tracked per workflow
  • New endpoints
    • GET /userdata?dir=the_userdata_dir_to_list&recurse=true/false&split=true/false - Lists files in a userdata folder (split is a bit of a weird one, but it will return the files split by the OS path separator - happy if someone has a better solution for this, needed for building the tree)
    • DELETE /userdata/{encoded_file} - deletes the file
    • POST /userdata/{encoded_file}/move/{encoded_dest}?overwrite=true/false - Moves a file, optionally overwriting an existing file
  • Adds materialdesignicons for the icons, it had a good selection but if there are better alternatives can reconsider. License.
  • Allows sending images to other workflows LoadImage nodes

Overview:
image
Save menu ("save" is now saving as user workflows, "export" is download file):
image
Light theme & queue:
image
Queue options:
image
Multiple open workflows & progress:
image
Responsive:
image
image
image
image
Send to workflow:
image
image
Workflow search with partial matching:
image

⚠️ Potential issues for extensions ⚠️

  • Canvas no longer fills the entire body, instead fills all remaining space (Allows for elements to sit around to the canvas instead of always needing to be floated over the top), so manual HTML widgets being floated over the canvas may need to take the top of the cavas rect into account when positioning
  • Adding buttons to the menu will now need adding to the new menu, you can check the setting Comfy.UseNewMenu to see if the top menu is enabled, example:
app.menu.settingsGroup.append(new(await import("/scripts/ui/components/button.js")).ComfyButton({
	icon: "cupcake",
	action: () => alert("hello"),
	tooltip: "Example button",
        content: "Example button"
}))

@asagi4
Copy link
Contributor

asagi4 commented Mar 21, 2024

I gave this a quick test. Seems like an improvement.
However, I have some design nitpicks.

  • I would like to be able to move the queue button. I don't like it in the top right corner. At least let me pick whether it's on the left or the right.
  • Is there any way to adjust the size of the buttons? They're a bit small for my liking. I'd prefer not to have to edit CSS manually. ComfyUI could use some customizability in general when it comes to sizes.
  • The way the selected workflow moves to the top of the open list in the workflow menu bothers me. This moves UI elements around and while I'm no UI designer, it is kind of annoying. Maybe just make the active workflow a bit more obvious instead of moving it? (eg. change its background colour in addition to the bolded font)

@pythongosssss
Copy link
Contributor Author

Thanks for the feedback

  1. I'll need to have a think about how to approach this, a user customizable toolbar isn't going to be in scope of what im aiming for here, and i'm not sure how common a request it would be to not have it in the top right (although so far 1/1 bits of feedback are to move it 😅)
  2. You can ctrl+scroll in your browser to zoom the page, that will increase the size of all elements on the page which im not sure if that is what you'd be after (see screenshot below)
    image
  3. I copied the re-ordering of recently open things from vscode, which may not be the best inspiration, and i'm happy to change it to not re-order and I've added a highlight to the open workflow
    image

@asagi4
Copy link
Contributor

asagi4 commented Mar 23, 2024

For point 1, I discovered ComfyUI's user.css and there's a pretty easy way to put the queue button first. I can just have

.comfyui-queue-button { 
  order: -1;
}

in there.

Though that seems to cause the dropdown to render a bit weirdly, I'm fine with doing it that way since I don't need to edit any CSS that would get overridden by updates.

EDIT: Weird rendering gets worked around with

.comfyui-split-button-popup.comfyui-popup.right.open {
  left: 10px !important;
}

I also like the non-reordering menu much better. I don't really have to pay attention to what I'm clicking when switching workflows since I know where it'll be in the menu, and that feels better since I'm not constantly clicking the wrong workflow.

@Amorano
Copy link

Amorano commented Mar 24, 2024

Given a few of us have injected extra buttons for things in our repos (my node colorizer e.g.) is there an easy way to register things INTO the menu? Or will I need to hack it like the original floating window?

@pythongosssss
Copy link
Contributor Author

The example at the bottom shows adding a simple button to the button group containing settings (the cupcake):

app.menu.settingsGroup.append(new(await import("/scripts/ui/components/button.js")).ComfyButton({
	icon: "cupcake",
	action: () => alert("hello"),
	tooltip: "Example button",
        content: "Example button"
}))

image
image
Adding new individual button:

app.menu.saveButton.element.after(new(await import("/scripts/ui/components/button.js")).ComfyButton({
	icon: "hamburger",
	action: () => alert("hello"),
	tooltip: "Example button",
}).element);

// This one will collapse on small screens
app.menu.saveButton.element.after(new(await import("/scripts/ui/components/button.js")).ComfyButton({
	icon: "pizza",
	action: () => alert("hello"),
	tooltip: "Example button 2",
    content: "Example button 2",
    classList: "comfyui-button comfyui-menu-mobile-collapse primary"
}).element);

image
image

And an example with a popup

const btn = new(await import("/scripts/ui/components/button.js")).ComfyButton({
	icon: "earth",
	tooltip: "Example button",
});
const hello = document.createElement("span");
hello.textContent = "Hello World";
const popup = new(await import("/scripts/ui/components/popup.js")).ComfyPopup({
	target: btn.element,
	classList: "comfyui-popup"
}, hello);
Object.assign(popup.element.style, {
	background: "var(--bg-color)",
	padding: "10px"
});
btn.withPopup(popup);
app.menu.actionsGroup.element.after(btn.element);

image

@ShisoFox
Copy link

If it's in scope, I'd also love to have those folder-based submenus in the builtin nodes.

@VelvetToroyashi
Copy link

On behalf of a few chrome users here (namely, @ronan36880), the menu bar seems to simply disappear when clicking onto the canvas. I'm unable to personally reproduce this issue on Firefox, however.

@ronan36880
Copy link

Hi yes, I've managed to replicate a bug where the menu bar disappears on my chrome install.
Chrome: 123.0.6312.86
OS: Windows 11 Version 23H2 (Build 22631.3374)

I believe I have narrowed it down to this call to canvas.focus() which appears to be recalculating layout, which ends up making the page scroll slightly:

chrome_2024-03-29_21-20-59.mp4

Also attached here is a performance trace (with paint instrumentation enabled) I have captured earlier today which replicates the issue.

I wasn't able to replicate the bug with the new menu setting disabled.

@pythongosssss
Copy link
Contributor Author

Thank you for that, looks like it was due to the bounding rect calculating using the specified width/height of the canvas element instead of using 100% to fill the area, so I now clear those values before getting the rect

@ShashwatDubey99
Copy link

Can UI be optimized for mobile devices?

@pythongosssss pythongosssss marked this pull request as ready for review May 13, 2024 16:19
@comfyanonymous
Copy link
Owner

what do you think about an option to put the bar at the bottom instead of the top?

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.

None yet

8 participants