-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[joy-ui] Introduce color inversion utilities #38916
Conversation
Netlify deploy preview@mui/joy: parsed: +0.60% , gzip: -0.20% 😍 Bundle size reportDetails of bundle changes (Toolpad) |
@mnajdova This PR is the changes I mentioned to you last week. It's the draft to prove that the new implementation works better and is easier to customize. Please have a look and if it looks good to you, I will apply the changes to all components with more tests. preview: https://deploy-preview-38916--material-ui.netlify.app/experiments/joy/color-inversion/ |
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.
This PR tackles much more than what the title indicates. It's not just about removing the context, it's changing the API too.
What do you think about doing the following:
- keep the API (the
invertedColors
prop) for the components that we believe it should be supported in - export a
applyColorInversion(variant, color)
util for custom components (or one-off Joy UI components that we don't support it by default - remove the context usage.
* @param color a supported theme color palette | ||
* @returns (theme: ThemeFragment) => Record<DefaultColorPalette, CSSObject> | ||
*/ | ||
export const applySolidInversion = (color: ColorPaletteProp) => (theme: ThemeFragment) => { |
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.
Do we plan to expose 4 different methods? Can't we have one where we would accept the variant & the 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.
Do we plan to expose 4 different methods? Can't we have one where we would accept the variant & the color?
We need 2 functions so far since only soft
and solid
come with a background. I think a separate function sounds better in the long term.
Sounds good to me. Changed the PR title to "Introduce color inversion utilities" and preserved the current API ( |
…color-inversion-2
…color-inversion-2
…color-inversion-2
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.
So much cleaner 🤩 I love that we staged the work into removing context only. I wonder if we even need to drop the feature at all, I think we can experiment with this for a while.
sx={[ | ||
(theme) => ({}), | ||
// @ts-expect-error no `unknown` color from theme palette | ||
applySoftInversion('unknown'), |
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.
Does it work with custom colors too if developers add them in the theme?
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.
Yes, they can add more color by augmenting the ColorPalettePropOverrides
from the theme.
// explicit color. Color inversion has no effect. | ||
<Chip variant="soft" color="primary">…</Chip> | ||
``` | ||
:::info |
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.
Not a deal breaker for this PR, but we have too many info callouts on this page. Every second paragraph feels like a callout. I would rather convert them to text, as otherwise they don't serve the purpose to "call out".
With the new approach, how do I configure the settings for a particular variant (soft, solid) and color? Previously this could be done via the |
The easiest and fastest way is to use <Card
variant="solid"
color="primary"
invertedColors
sx={theme => ({
"& :not([data-skip-inverted-colors], [data-skip-inverted-colors] *)" {
[theme.getColorSchemeSelector('light' | 'dark')]: {
'--variant-plainColor': …
…
}
}
})}
> You just need to use the same selectors as it is internally used. |
Sandbox: https://codesandbox.io/s/joy-ui-cra-ts-forked-h8prkz?file=/src/App.tsx
closes #38912
For detailed motivation, read #38912.
The utilities let developers apply the color inversion on any component.
Breaking Changes
Color inversion context has been removed. To apply color inversion on any parent component, use an independent function named
applySolidInversion
orapplySoftInversion
to thesx
prop orstyled
.Child components that have explicit
color
prop will use the color inversion variables by default. To opt out for a specific component, setdata-skip-inverted-colors
attribute to the component.Todo