Skip to content

Conversation

@benceruleanlu
Copy link
Member

@benceruleanlu benceruleanlu commented May 18, 2025

This PR adds a help sidebar overlay on top of the node library sidebar tab.

image

image

There are two ways to open this help panel, one through the node leaf itself:

image

And the other way is through the selection toolbox:

image

This uses the Marked and DOMPurify libraries for Markdown and HTML sanitization.

If you are authoring an extension, links such as assets/image.png will be parsed to custom_nodes/<extension name>/<WEB_DIRECTORY>/assets/image.png.

Related Core PR: Comfy-Org/ComfyUI#8179
Related Docs PR: Comfy-Org/docs#120

┆Issue is synchronized with this Notion page by Unito

@socket-security
Copy link

socket-security bot commented May 18, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​types/​dompurify@​3.0.51001006977100
Addeddompurify@​3.2.51001001008990
Addedmarked@​15.0.1110010010095100

View full report

@benceruleanlu benceruleanlu marked this pull request as ready for review May 18, 2025 22:07
@benceruleanlu benceruleanlu marked this pull request as draft May 19, 2025 03:17
@benceruleanlu benceruleanlu marked this pull request as ready for review May 19, 2025 07:50
Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

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

An initial thought, before I've had a chance to test fully:

Should node help be separate from the frontend repo, similar to workflow templates?

(I'm not currently leaning one way or another)

@benceruleanlu benceruleanlu marked this pull request as draft May 20, 2025 03:01
@comfyui-wiki
Copy link
Member

I think keeping the node info in a separate repo can make it easier to maintain. The community can correct the incorrect info and contribute to the document as well.

But I don't know whether we should keep the image in the same repo. I may add some image examples when updating the node info so it may contain lots of images, or we just keep the image in another repo and just use the github image links like https://raw.githubusercontent.com/Comfy-Org/example_workflows/main/controlnet/pose_controlnet_2_pass.png

@comfyui-wiki
Copy link
Member

comfyui-wiki commented May 20, 2025

Btw, I think when updating the info in docs , the image will upload to the docs repo as well, so we may not need to upload it twice if we are using github image links.

Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

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

Part of the reason I suggested another repo was to move the image problem out of frontend repo. I think it should be fine to keep both text and images in one repo, provided the repo is explicitly for docs / node info.

@benceruleanlu If this has already been brought up internally and decided against, please let me know! I'll get some opinions async / grab some folks for a very brief chat if not.

@benceruleanlu
Copy link
Member Author

If this has already been brought up internally and decided against, please let me know! I'll get some opinions async / grab some folks for a very brief chat if not.

IIRC Yoland mentioned how we should do it the same way that extension authors should so that we could serve as an example.

But the popular nodepacks could also serve as an example, so I do agree we should probably move the docs out of the core repo.

@webfiltered
Copy link
Contributor

Will check on that! My leaning atm is to use.. well, an example as an example. It should be included in the extension example / skeleton. I agree we should definitely use the same system to load the files as custom nodes, but I generally prefer any potentially-large, raw data be stored separate to the main code repo. It's just that it's a decision that impacts that repo forever.

@benceruleanlu
Copy link
Member Author

I have a python package setup working right now, just need to get it uploaded to PyPi under Comfy-Org and we should be good to go

Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

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

Overall this PR looks really good.

I asked design if there are any final feedback here.

Great work!

@christian-byrne
Copy link
Contributor

In later PR:

  • Use utils/modules added in this PR for other markdown rendering (e.g., in the info panel of the custom node manager dialog)

Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

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

Looking great! Just looked at regular nodes without any help package; really great UX.

It seems like DataTable was causing some problems - could you please jot down a vague approximation of what they were? Knowledge sharing is the only goal.

@benceruleanlu
Copy link
Member Author

It seems like DataTable was causing some problems - could you please jot down a vague approximation of what they were? Knowledge sharing is the only goal.

The markdown content is rendered in plain html, unstyled apart from browser and global, and I wanted to keep the consistency of the same styling from both the markdown content and the fallback content.

I didn't think we need anything special from DataTable anyway, and it came with primevue styling that didn't match our design docs, so I wanted to make it align with the markdown content without blasting it with overrides.

@comfyui-wiki
Copy link
Member

FYI: I submitted a PR for the node docs today Comfy-Org/embedded-docs#4

Migrated all the node docs from my website.

@webfiltered webfiltered linked an issue Jun 1, 2025 that may be closed by this pull request
1 task
Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

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

Apologies, wanted to review & merge today, but there are conflicts. Can you rebase and force push, please?

Not really expecting it, but if it does turn into a huge job w/merge conflicts, please let us know.

Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

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

LGTM once rebased!

@benceruleanlu benceruleanlu enabled auto-merge (squash) June 1, 2025 12:11
@benceruleanlu benceruleanlu merged commit 40cfc43 into main Jun 1, 2025
10 checks passed
@benceruleanlu benceruleanlu deleted the bl-help-menu branch June 1, 2025 13:24
lordTyrion pushed a commit to playbook3d/ComfyUI_frontend that referenced this pull request Jul 2, 2025
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.

[Feature Request]: Annotate node for Infos instead of using textboxes

5 participants