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

[material][CssVarsProvider] Fix HSL breaking button styles #39869

Merged
merged 4 commits into from
Dec 18, 2023

Conversation

gitstart
Copy link
Contributor

@gitstart gitstart commented Nov 13, 2023

Descriptions

Fixes color breaks when HSL colors is used.

closes #37400


This code was written and reviewed by GitStart Community. Growing great engineers, one PR at a time.

Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@gitstart gitstart marked this pull request as ready for review November 13, 2023 11:48
@gitstart
Copy link
Contributor Author

@mnajdova @brijeshb42 @siriwatknp this PR is ready for review

@mui-bot
Copy link

mui-bot commented Nov 13, 2023

Netlify deploy preview

https://deploy-preview-39869--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 95e163f

@zannager zannager added package: system Specific to @mui/system customization: theme Centered around the theming features labels Nov 13, 2023
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

The logic looks good to me 👍 However, I wonder if this won't break some apps that depend on the previous logic of the color utils. @siriwatknp what do you think? Maybe tobe safe we can create a new decompose function that we would use in the CssVarsProvider only.

@danilo-leal danilo-leal changed the title CssVarsProvider - using HSL colors break Button styles [system][CssVarsProvider] Fix HSL breaking button styles Nov 21, 2023
@siriwatknp
Copy link
Member

The logic looks good to me 👍 However, I wonder if this won't break some apps that depend on the previous logic of the color utils. @siriwatknp what do you think? Maybe tobe safe we can create a new decompose function that we would use in the CssVarsProvider only.

I think the proper fix is to convert hsl to rgb for all the channel colors.

We could improve this in the future by replacing all channel tokens with color-mix().

image

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Thanks for initiating the fix!

@mnajdova
Copy link
Member

@siriwatknp this is what I meant, I haven't expressed myself clearly :D

@mnajdova mnajdova merged commit 68c3b37 into mui:master Dec 18, 2023
19 checks passed
@mnajdova mnajdova changed the title [system][CssVarsProvider] Fix HSL breaking button styles [material][CssVarsProvider] Fix HSL breaking button styles Dec 18, 2023
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customization: theme Centered around the theming features package: system Specific to @mui/system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CssVarsProvider - using HSL colors break Button styles
5 participants