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

[docs-infra] Use CSS vars for API pages #41911

Closed
wants to merge 4 commits into from

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Apr 15, 2024

Now you can reload an API page in dark mode without having the light mode blinking
🎉

I modified all the API pages at once because they are generated by a script

Those modification leads to an error message in the API pages but I've no idea about why

Warning: Prop `style` did not match. Server: "--Paper-shadow:var(--muidocs-shadows-4)" Client: "--Paper-shadow:var(--muidocs-shadows-4);--Paper-overlay:linear-gradient(rgba(255 255 255 / 0.092), rgba(255 255 255 / 0.092))"

Can be fixed by this diff, but not sure it's a misusage on our side or an error in the vars support

diff --git a/packages/mui-material/src/Paper/Paper.js b/packages/mui-material/src/Paper/Paper.js
index fbb7d90343..53c63403f7 100644
--- a/packages/mui-material/src/Paper/Paper.js
+++ b/packages/mui-material/src/Paper/Paper.js
@@ -117,7 +117,7 @@ const Paper = React.forwardRef(function Paper(inProps, ref) {
         ...(variant === 'elevation' && {
           '--Paper-shadow': (theme.vars || theme).shadows[elevation],
           ...(theme.vars && {
-            '--Paper-overlay': theme.overlays?.[elevation],
+            '--Paper-overlay': theme.vars.overlays?.[elevation],
           }),
           ...(!theme.vars &&
             theme.palette.mode === 'dark' && {

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 17, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 16, 2024
@mui-bot
Copy link

mui-bot commented May 16, 2024

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 0f6ec60

}

function ToggleVarTheme(props) {
const { setMode } = useColorScheme();
Copy link
Member Author

Choose a reason for hiding this comment

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

The hook requires to be in a context of a theme with vars otherwise it throws error.
That's the reason why ToggleTheme is split into 2 components:

  • On for the CSS var if feasible
  • ANother one without useColorScheme(); if theme.vars is not detected

Comment on lines +244 to +246
backgroundColor: theme.vars
? `rgba(${theme.vars.palette.primaryDark[700]} / 0.1)`
: alpha(theme.palette.primaryDark[700], 0.1),
Copy link
Member Author

Choose a reason for hiding this comment

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

@danilo-leal I did my best to only do technical modifications, but if you can double check the colors

@alexfauquette alexfauquette requested review from siriwatknp and a team May 31, 2024 09:29
@alexfauquette alexfauquette changed the title [WIP] Use CSS vars for demos/API [docs-infra] Use CSS vars for API pages May 31, 2024
@alexfauquette alexfauquette marked this pull request as ready for review May 31, 2024 09:32
@zannager zannager added the scope: docs-infra Specific to the docs-infra product label May 31, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 31, 2024
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.

@alexfauquette Have you considered dynamic routes or using getLayout() instead of importing the provider directly to each page?

I don't know much about the differences, that's why I ask.

@aarongarciah
Copy link
Member

@alexfauquette @siriwatknp is this PR still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: out-of-date The pull request has merge conflicts and can't be merged scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants