Skip to content

Custom node thumbnail variant #3682

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lerignoux
Copy link
Contributor

@lerignoux lerignoux commented Apr 29, 2025

I wanted to use the thumbnail variant in a custom node I am doing but noticed it was not available.
So I wanted to propose a simple setup for this leveraging the workflow name extensions.
Cf #3684

If a custom node provides a thumbnail Variant at the end of the filename. For instance
workflow_1.compareSlider.json
Then the front can toggle it's display for the relevant variant.

The title and description are then cleaned of the variant suffix.

It is a bit heavy on string computation, Do you think it is relevant to add caching/optimisations ?
If it is ok for integration I'll do a MR to update the documentation for this.

┆Issue is synchronized with this Notion page by Unito

@lerignoux lerignoux requested a review from a team as a code owner April 29, 2025 15:35

return `${basePath}${indexSuffix}.${template.mediaSubtype}`
}

const thumbnailVariantRe = /.*\.(?<variant>[^\.]+)/
const thumbnailVariantList = ['compareSlider', 'hoverDissolve', 'zoomHover']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would just need to udpate this list for new variants

@lerignoux lerignoux force-pushed the custom_node_thumbnail_variant branch 5 times, most recently from 6681e4f to 6bf6a71 Compare April 29, 2025 15:45
@lerignoux lerignoux force-pushed the custom_node_thumbnail_variant branch from 6bf6a71 to f0206af Compare April 29, 2025 15:50
@lerignoux lerignoux marked this pull request as draft April 29, 2025 16:07
@lerignoux lerignoux force-pushed the custom_node_thumbnail_variant branch 2 times, most recently from eb4489a to 01aa0f5 Compare April 30, 2025 02:32
@lerignoux lerignoux marked this pull request as ready for review April 30, 2025 02:32
@lerignoux lerignoux requested a review from a team as a code owner April 30, 2025 02:32
@lerignoux lerignoux force-pushed the custom_node_thumbnail_variant branch 3 times, most recently from da4ac03 to 852c539 Compare April 30, 2025 05:59
}
}
const wrapper = shallowMount(TemplateWorkflowCard, { propsData: data })
// @ts-expect-error 'thumbnailVariant is computed'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand why this is needed, the computed does exists. and the test works.
Any idea how to properly fix this ?

@lerignoux lerignoux force-pushed the custom_node_thumbnail_variant branch from 852c539 to fffc1c1 Compare April 30, 2025 06:03
@huchenlei
Copy link
Contributor

@christian-byrne Can you take a look?

@christian-byrne
Copy link
Contributor

christian-byrne commented May 1, 2025

Yes, I can took a look today and tomorrow. I would really like this feature to be integrated as well, and appreciate the PR. I can also look at #3710 today.

After looking at it briefly for the first time just now, my intial thoughts is that we might even be able to allow custom node to specify their own index.json in this format.

But let me look at code more properly first.

Also unrelated, but I can also help push through the model whitelisting feature today/tomrrow @lerignoux. Sorry for delay!

@lerignoux
Copy link
Contributor Author

Yes, I can took a look today and tomorrow. I would really like this feature to be integrated as well, and appreciate the PR. I can also look at #3710 today.

After looking at it briefly for the first time just now, my intial thoughts is that we might even be able to allow custom node to specify their own index.json in this format.

But let me look at code more properly first.

Also unrelated, but I can also help push through the model whitelisting feature today/tomrrow @lerignoux. Sorry for delay!

Thanks a lot, no worries, I know there are 30 PR pending in parallel :D

I can try to do the switch using an index.json like for Comfy if we prefer indeed. I naturally went for the "quickest" but could make sense to unify comfy and plugins, Waiting for your feedback then.

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.

3 participants