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

Rename ImageCrop to Image Crop #5424

Merged
merged 3 commits into from
Oct 31, 2024

Conversation

KoreTeknology
Copy link
Contributor

Feedback

Nodes menu has inconsistency in names, some with spaces between words, other not. i made some minor changes on our local installations (students project). This is something important for the next releases. The actual menu push developers to add their category into the top level, what is not the best option for the users to search for a specific category, if the category exists into various levels.

also, it must be considered that developers would have a guided structure to include their custom nodes packages into existing categories (until the node requires a new one) for consistency and easy search.

Mockup

By re-arranging the main node menu, we give to developers a way to inlude their custom nodes in a practical manner.

menu_comfy_review

Pull request & future development

this first pull is about the Image Crop node, to give an overview on my next requests. if accepted, i will work make a complete proposal for this menu. i want to contribute to this project as a developer, mainly because my students are giving me a lot of user feedbacks and i believe it can serve the purpose of the community.
if you want to check my profil: https://www.linkedin.com/in/urieldeveaud/
thank you

Nodes menu has inconsistency in names, some with spaces between words, other not.
@huchenlei huchenlei added the Frontend Issue relates to the frontend UI (litegraph). label Oct 30, 2024
@huchenlei huchenlei closed this Oct 30, 2024
@huchenlei huchenlei reopened this Oct 30, 2024
@huchenlei huchenlei removed the Frontend Issue relates to the frontend UI (litegraph). label Oct 30, 2024
Copy link
Collaborator

@huchenlei huchenlei left a comment

Choose a reason for hiding this comment

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

Image Crop you are changing is the node unique id, which is not supposed to be modified. You should do

NODE_DISPLAY_NAME_MAPPINGS = {
    "ImageCrop": "Image Crop"
}

instead to modify display name.

@KoreTeknology
Copy link
Contributor Author

KoreTeknology commented Oct 30, 2024

NODE_DISPLAY_NAME_MAPPINGS = {
    "ImageCrop": "Image Crop"

that´s right!
Should i add the line to nodes.py in a new pull? it not exists

Include the node mapping name line for Image Crop Node
@huchenlei huchenlei added the Good PR This PR looks good to go, it needs comfy's final review. label Oct 31, 2024
@huchenlei
Copy link
Collaborator

@comfyanonymous PTAL. This is a straightforward change.

@huchenlei huchenlei changed the title Node menu items, inconsistency in naming convention Rename ImageCrop to Image Crop Oct 31, 2024
@comfyanonymous comfyanonymous merged commit f2aaa0a into comfyanonymous:master Oct 31, 2024
7 checks passed
tiangles pushed a commit to diffus-me/ComfyUI that referenced this pull request Nov 29, 2024
* Update nodes_images.py

Nodes menu has inconsistency in names, some with spaces between words, other not.

* Update nodes.py

Include the node mapping name line for Image Crop Node

* Update nodes_images.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good PR This PR looks good to go, it needs comfy's final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants