-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[website] Fix design kits showcase throwing an error #35093
[website] Fix design kits showcase throwing an error #35093
Conversation
|
@@ -259,10 +259,11 @@ export default function DesignKits() { | |||
...theme.applyDarkStyles({ | |||
background: `linear-gradient(to bottom, ${ | |||
(theme.vars || theme).palette.primaryDark[900] | |||
} 0%, ${alpha((theme.vars || theme).palette.primaryDark[900], 0)} 30%, ${alpha( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alpha channel was set to 0 anyway, so I just used transparent color instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
} 0%, ${alpha((theme.vars || theme).palette.primaryDark[900], 0)} 30%, ${alpha( | ||
(theme.vars || theme).palette.primaryDark[900], | ||
0, | ||
)} 70%, ${(theme.vars || theme).palette.primaryDark[900]} 100%)`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder what is the alternative for alpha
/lighten
/darken
utility functions when using CSS variables?
For alpha
, something like this should work, right?
'rgba(${theme.vars.palette.primary.mainChannel} / 0.5'
But then, not every color in the palette has a xxxChannel
representation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we observed a similar problem in mui/mui-x#6784
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @siriwatknp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'rgba(${theme.vars.palette.primary.mainChannel} / 0.5'
this is correct, and yes, we are generating channels for some colors not all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this case?
'rgba(${theme.vars.palette.primaryDark[900]} / 0.5)'
The above won't work, and there's no channel for primaryDark[900]
color.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rgba(${theme.vars.palette.primaryDark[900]} / 0.5)
Where is it? The gradient looks fine to me.
0, | ||
)} 70%, ${(theme.vars || theme).palette.primaryDark[900]} 100%)`, | ||
} 0%, ${ | ||
'rgba(255,255,255,0)' // transparent does not work in Safari & Mobile device |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intended to be"Safari on iOS", or "Safari and mobile device(s)", in which case, which?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially, I wanted to use 'transparent'
, but then I noticed this comment and copied it:
'rgba(255,255,255,0)' // transparent does not work in Safari & Mobile device |
In https://css-tricks.com/thing-know-gradients-transparent-black/ it says it's Safari - both desktop and mobile.
But it's an article from 2017 and using 'transparent'
doesn't seem to make any difference in Safari 16:
Screen.Recording.2022-11-10.at.19.47.41.mov
I'll update the comments to only mention Safari and also link to the article above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, using rgba(255,255,255,0)
everywhere didn't look good in Safari. I'll push a fix
Signed-off-by: Olivier Tassinari <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks! @cherniavskii
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this pull request indicates that the rgba settings do with for safari. If that's right then I think this is good to go. Does rgba still not work for mobile devices?
I like the choice to define the transparent setting above and reuse it. Helped clean things up.
It's not about rgba not working, it's about Safari treating transparent black and transparent white colors differently - see https://css-tricks.com/thing-know-gradients-transparent-black/ |
Signed-off-by: Olivier Tassinari <[email protected]> Co-authored-by: Olivier Tassinari <[email protected]> Co-authored-by: Michał Dudak <[email protected]>
Signed-off-by: Olivier Tassinari <[email protected]> Co-authored-by: Olivier Tassinari <[email protected]> Co-authored-by: Michał Dudak <[email protected]>
See https://mui.zendesk.com/agent/tickets/5696 and https://mui.zendesk.com/agent/tickets/5695