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

Update imageFeed.js #224

Closed
wants to merge 9 commits into from
Closed

Conversation

tachyon-beep
Copy link

@tachyon-beep tachyon-beep commented Mar 31, 2024

This is a (somewhat over the top) rewrite of the Image Feed system based on my previous abandoned pull request.

The feed:

  • lets the user filter images based on source node.
  • groups images from the same run into batches with a visible marker.

Additionally, the slider for height as well as two new controls have been moved to the ComfyUI menu.

Locking the feed to the left or right has not yet been implemented but I can readd it if its a 'must go' feature.
Known 'image nodes' are defined in an array, with unrecognised nodes caught via a catch fall filter setting. I'm currently looking for a better way to identify 'image' nodes for inclusion in the filter.

Tested on Firefox and Chromium.

This is a aewrite of the Image Feed system based on my previous abandoned pull request.

The feed:
- lets the user filter images based on source node. 
- groups images from the same run into batches with a visible marker.

Additionally, the slider for height as well as two new controls have been moved to the ComfyUI menu.

Locking the feed to the left or right has not yet been implemented. Known 'image nodes' are defined in an array, with unrecognised nodes caught via a catch fall filter setting.
@tachyon-beep
Copy link
Author

The big weakness of this implementation is that I haven't found a way to dynamically identify 'image' nodes. There's a 'custom node' fallback, but this is deeply unsatisfying.

All feedback is welcome, but feedback on that issue specifically is extra welcome.

Fixed minor logic bug in execution_start.
Fix divider bar CSS.
Fixed bug I created in legacy getVal's
Fixed custom node button interactions with filter.
Updated getVal so it actually works the way I thought it did.
More bug squashing.
Dealing with other peoples problems.
@pythongosssss
Copy link
Owner

Hey, thanks, would you please be able to split the different parts into their own PRs (probably: batch markers, filtering nodes, ui refactor changes), as its difficult to review with everything all in one.

For some early feedback:

  • Since ComfyUI is shared across multiple extensions + core for CSS variables, could you either uniquely prefix the variables or move them from :root, you should also use the standard ComfyUI variables so it matches the users theme.
  • You can identify nodes that currently have images by checking if node.imgs is set, you can't know if a node might have an image
  • I frequently resize the feed/columns depending on which monitor + image resolution i'm generating, so I wouldn't be happy to have to go to settings to change this each time
  • Left & right would be required
  • I can't scroll the image feed anymore in Firefox

@tachyon-beep tachyon-beep marked this pull request as draft June 19, 2024 06:22
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

2 participants