ComfyUI-Custom-Scripts icon indicating copy to clipboard operation
ComfyUI-Custom-Scripts copied to clipboard

Image Feed: UX Overhauling

Open birdddev opened this issue 1 year ago • 6 comments

  • Makes the panel resizable using a bar on the open side
  • Slight styling changes
  • Removed column amount/image feed size options
  • Added image size option
  • Grid should auto-adjust based on the panel size and desired image size
  • Holding Ctrl while resizing will snap the panel to the current image size * step
  • Changed the "Resize Feed" setting text to a Cog emoji
  • Use the .div for image items instead of .img to adjust brightness (The image could be smaller than parts of the button due to differing aspect ratios, but the button itself is always the desired image size. 1:1)
  • Moved css variables to the new image-feed-root
  • Always show the inner feed box

https://github.com/pythongosssss/ComfyUI-Custom-Scripts/assets/47731506/e7030bfc-3330-4622-9229-ab3695478084


I ended up having free time unexpectedly, so I went ahead and tried to work on some of those suggestions I had.

Doing a draft for now, since I would like feedback on the current changes from another user's perspective, and it's slightly broken in a few ways at the moment. Also pretty messy. The "right" location settings menu is broken because it's attached to the panel, so width changes from adjusting image size makes it do bad things.


Definitely want

  • [x] Hold shift while changing panel's width to also adjust image size. (This plus the Ctrl + Drag should make it have feature parity with the previous setup, but with a more intuitive way of adjusting.)
  • [x] Rework the settings box to be a "pop-up" that isn't parented to the feed. (Is there a proper way to do this already with something from comfy/litegraph? I know it has pop-ups, but I don't know how adjustable they are, or how I would go about using them.)
  • [x] Fix flickering cursor when resizing too fast.
  • [x] Make empty feed not take up more space than header buttons.
  • [x] Update Readme with new features.
  • [x] Fix vertical locations not expanding feed height fully.

Nice to have

  • [x] ~Adjust the panel's width after changing locations and on load so it doesn't snap outward. (Or change it so the images shrink like I wanted to do and allow for the panel to be dragged smaller than image size.)~
  • [ ] An option to show live previews in the image feed where the next image would be instead of on the prompt thing.
  • [x] ~Have a little area near the settings/clear/close buttons, that when dragging, shows a preview to snap the panel to a different location. On release, changes the location to it.~ Just moved to the setting popup instead.
  • [x] A switch for descending/ascending images in the "header bar" or in the panel's settings. (These last 2 should allow you to remove those options from the ComfyUI settings to reduce clutter.)
  • [ ] Separator bar per prompt for each collection of images (if a prompt puts multiple images in the feed, then separate from next queue prompt? Make optional probably, opt-in even, as it may be ugly?)

birdddev avatar Sep 03 '23 23:09 birdddev

Looks really nice so far!

(Is there a proper way to do this already with something from comfy/litegraph)

Not really anything nice, there is the litegraph ContextMenu but it isnt overly useful for this scenario My only comment with the current state is that when the feed is empty, it shouldnt take up more space than the header buttons

pythongosssss avatar Sep 05 '23 16:09 pythongosssss

Oh, yeah just having min width be the size of the header buttons is super good. I assume vertically I would let the empty feed be able to get cut off, but the header needs to be always visible somehow.

Not really anything nice...

Shame, I can make my own implementation for now (I don't really like how the popups in comfy/litegraph work anyways, as clicking outside of them don't cancel them). I Would've liked to use something that exists and improve it upstream, but I can use this to try stuff and then get it upstreamed later.

Where would you want me to put the widget implementation? Not the image feed's settings menu, but the re-usable popup widget itself, or should I do a separate pull request for that?

birdddev avatar Sep 07 '23 04:09 birdddev

You can either just put it in the image feed file for now and I can move it out if it gets reused, or into the common folder

pythongosssss avatar Sep 08 '23 18:09 pythongosssss

Here is how the popup looks. image image image It will stay open until you click somewhere outside the popup, or if you hover over the popup "window" and then mouseleave. There's also an option for triggering the window on hover of the attached element, or on click. It should try to always stay inside the window.

birdddev avatar Sep 09 '23 00:09 birdddev

Looking really nice! a couple of comments from testing:

  • You can't resize the panel so images are overflowed with the panel on the top/bottom, i.e. i can't make the height smaller than this:
    image

  • When the panel is in top/bottom mode, you can add .pysssss-image-feed-list:empty { display: none } to get rid of this area:
    image

  • You can make the width/height > 100% of the window and you can then loose the resize/settings and get stuck! 😅

Otherwise its really great, and the new settings popup is really nice having it all in one place and works well

pythongosssss avatar Sep 22 '23 19:09 pythongosssss

Yeah, I saw the problems with the top/bottom modes. I just don't have the time at the moment to deal with it anymore. If you think you can take care of it before this wednesday (I don't know if I'll have time by even then tbh). I am totally down for you pulling this and then pushing fixes for these things quickly right after if you like, just so people can have these improvements sooner than later. Especially if you know how to fix them rn. I don't think I'll add any more features to this with this PR, so any fixes or patches after what's here is viable. I just haven't been able to look in to it yet :pensive:

In my experience I haven't been able to drag the panel larger than the window since that's just not possible lmao, but that might be because Wayland just doesn't allow that? windows/x11/mac probably allow events outside the window, so that's probably something I just can't test for. Using min(max()) in the panel size equations should fix that probably, like how I was using it before.

birdddev avatar Sep 25 '23 11:09 birdddev