8000 [Manager] Keep node previews inside info panel bounds by christian-byrne · Pull Request #3140 · Comfy-Org/ComfyUI_frontend · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Manager] Keep node previews inside info panel bounds #3140

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 20, 2025
Merged

[Manager] Keep node previews inside info panel bounds #3140

merged 3 commits into from
Mar 20, 2025

Conversation

christian-byrne
Copy link
Contributor
@christian-byrne christian-byrne commented Mar 19, 2025 •
edited by sync-by-unito bot
Loading

Adjusts stlye. Before:

Selection_1135

After:

Selection_1133

┆Issue is synchronized with this Notion page by Unito

@christian-byrne christian-byrne requested a review from a team as a code owner March 19, 2025 06:28
webfiltered
webfiltered reviewed Mar 19, 2025
View reviewed changes
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.

Are you running into many !important that need more !important to override?

huchenlei
huchenlei suggested changes Mar 19, 2025
View reviewed changes
Copy link
Contributor
@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.

Maybe we can adjust style in the NodePreview component instead?

@christian-byrne
Copy link
Contributor Author

Are you running into many !important that need more !important to override?

The important was not necessary, good catch. Don't remember why I put it there.

@christian-byrne
Copy link
Contributor Author

Maybe we can adjust style in the NodePreview component instead?

Can you provide basis/rationale when request change?

@huchenlei
Copy link
Contributor

Maybe we can adjust style in the NodePreview component instead?

Can you provide basis/rationale when request change?

Ideally we should not need to use :deep to override component internal style. If it is a style that should be exposed, we should find a way to make it possible via class.

@huchenlei huchenlei merged commit 4899a1d into main Mar 20, 2025
10 checks passed
@huchenlei huchenlei deleted the manager/ui/node-preview-size branch March 20, 2025 01:22
@webfiltered
Copy link
Contributor

Some more on the why: :deep is a convenient, but complex, compute-heavy, and kind-of-hacky workaround for scoped CSS not being inheritable. It adds quite a bit of CSS-specificity to everything inside, which can lead to child components needing to artificially increase specificity of their own styles, which can get unnecessarily complex pretty fast.

@huchenlei huchenlei mentioned this pull request Mar 21, 2025
Merged
christian-byrne added a commit that referenced this pull request May 20, 2025
@christian-byrne
[Manager] Keep node previews inside info panel bounds (#3140)
639c712
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webfiltered webfiltered webfiltered left review comments

@huchenlei huchenlei huchenlei approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@christian-byrne @huchenlei @webfiltered
0