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

[Cleanup] Remove duplicated group header drawing logic #2453

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

huchenlei
Copy link
Member

@huchenlei huchenlei commented Feb 6, 2025

Group titlebar drawing has already been implemented in litegraph here: https://github.com/Comfy-Org/litegraph.js/blob/9d5e94859ed7b5668ae31f3d772848ded6e8f9de/src/LGraphGroup.ts#L158-L163

This PR removes the duplicated drawing logic.

┆Issue is synchronized with this Notion page by Unito

@huchenlei huchenlei requested a review from a team as a code owner February 6, 2025 18:41
@huchenlei huchenlei merged commit 2b18949 into main Feb 6, 2025
10 checks passed
@huchenlei huchenlei deleted the remove_draw_group branch February 6, 2025 18:52
@webfiltered
Copy link
Contributor

Oof, sorry. I meant to address this months ago. I believe it was unintentionally left in when I (or you?) added header drawing into litegraph, so this is technically fixing a long-standing regression.

Just flagging in case it is relevant: I paused on the change because it changed the alpha of the group headers slightly.

@huchenlei
Copy link
Member Author

Oof, sorry. I meant to address this months ago. I believe it was unintentionally left in when I (or you?) added header drawing into litegraph, so this is technically fixing a long-standing regression.

Just flagging in case it is relevant: I paused on the change because it changed the alpha of the group headers slightly.

The alpha change is probably so slight that playwright screenshot comparison is not even complaining haha.

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.

2 participants