[docs] Sync the mode from page to demos #45661
Conversation
Netlify deploy previewBundle size report |
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
| <Typography | ||
| endDecorator={<Link href="/sign-up">Sign up</Link>} | ||
| sx={{ fontSize: 'sm', alignSelf: 'center' }} | ||
| <CssVarsProvider {...props}> |
There was a problem hiding this comment.
This demo is an iframe with mode toggle, so need to add dedicated CssVarsProvider (ThemeProvider) to follow the new demo guide.
siriwatknp
changed the title
| function IsolatedDemo({ root, children }) { | ||
| return ( | ||
| <SystemThemeProvider | ||
| theme={(upperTheme) => ({ | ||
| direction: upperTheme.direction, | ||
| })} | ||
| > | ||
| {React.cloneElement(children, { | ||
| disableNestedContext: true, | ||
| storageManager: null, | ||
| colorSchemeNode: root, | ||
| })} | ||
| </SystemThemeProvider> | ||
| ); |
There was a problem hiding this comment.
I'm not sure tu undersand why wee need both:
- the
SystemThemeProviderthat overrides the theme with a default one. - Passing props toe the children theme.
This props propagation looks weird to me.
From an outsider point of view, it feels more natural to write a demo with a wrapper like that
<ThemeProvider
theme={(upperTheme) => ({ direction: upperTheme.direction })}
disableNestedContext
storageManager={null}
colorSchemeNode={ } // Here is the only bloking point because we woudl need the demoId
>Compared to having to understand that
{{"demo": "Xxx.js", "isolated": true}} pass some props that should be passed to the theme provider of the demo.
But not sure it's doable
There was a problem hiding this comment.
From an outsider point of view, it feels more natural to write a demo with a wrapper like that
Yes, that's the point. Those props must be passed to the demo to work within the docs context.
When users open the demo on CodeSandbox or Stackblitz, the props will be empty which fallbacks to the initial values of the ThemeProvider.
I think it's a better experience for both maintainer and user. They don't need to know about disableNestedContext, storageManager, or colorSchemeNode to build demos.
|
I'm a bit lost because your first commit does not appear in the "files changed"
|
My mistake. The change is in this file https://github.com/mui/material-ui/pull/45661/files#diff-7527a515d20eff239825c53744ce9602043624db93e0a7275943c9fa95256bfb And the test file: https://github.com/mui/material-ui/pull/45661/files#diff-572a6275b39a1e33d9b58cae7f394a5b349d988a6bc8cfb842a757d060c69626 |
| {isolated ? ( | ||
| // Place ThemeProvider from MUI System here to disconnect the theme inheritance for Material UI and Joy UI | ||
| // The demo will need to handle the ThemeProvider itself. | ||
| <SystemThemeProvider | ||
| theme={(upperTheme) => ({ | ||
| direction: upperTheme.direction, // required for internal ThemeProvider | ||
| vars: upperTheme.vars, // required for styling Iframe | ||
| })} | ||
| > | ||
| {iframe ? ( | ||
| <DemoIframe name={name} isJoy={isJoy} isolated={isolated} {...other}> | ||
| {/* `children` needs to be a child of `DemoIframe` since the iframe implementation rely on `cloneElement`. */} | ||
| {/* the `colorSchemeNode` will be provided by DemoIframe through `window` prop */} | ||
| <IsolatedDemo cssVarPrefix={name}>{children}</IsolatedDemo> | ||
| </DemoIframe> | ||
| ) : ( | ||
| <IsolatedDemo cssVarPrefix={name} colorSchemeNode={root}> | ||
| {children} | ||
| </IsolatedDemo> | ||
| )} | ||
| </SystemThemeProvider> |
There was a problem hiding this comment.
@alexfauquette The new isolated flag is to support these kind of demos:
- Toolpad demo on the drawer page
- Joy UI tutorial
These are considered mini apps within the docs, so they have to be isolated from the page's theme and color mode.
Screen.Recording.2568-03-26.at.15.27.38.mov
The <SystemThemeProvider> is used as an internal workaround to disconnect the theme from page and these mini apps.
Note: I don't think MUI X needs this kind of demo, that's why I added it as a new flag so that it does not impact MUI X.
|
Not sure why the code coverage fails even though I added more tests in this PR 🥲 |
| @@ -16,7 +16,7 @@ const StyledMarkdownElement = styled(MarkdownElement)(({ theme }) => [ | |||
| overflow: 'auto', | |||
| marginTop: -1, | |||
| backgroundColor: 'hsl(210, 25%, 9%)', // a special, one-off, color tailored for the code blocks using MUI's branding theme blue palette as the starting point. It has a less saturaded color but still maintaining a bit of the blue tint. | |||
| border: 0, | |||
| border: '1px solid transparent', | |||
There was a problem hiding this comment.
To fix the layout shift caused by border width (in light mode was no border but in dark mode the border was 1px). This is noticeable in pages that have a lot of code block.
Before (notice the text at the bottom of the clip):
Screen.Recording.2568-03-26.at.18.31.18.mov
After (no longer shift):
Screen.Recording.2568-03-26.at.18.31.45.mov
|
In regards to the codecov results, there is an indirect change detected: https://app.codecov.io/gh/mui/material-ui/pull/45661/indirect-changes |
The mode toggle explanation: https://deploy-preview-45661--material-ui.netlify.app/experiments/docs/demos/#isolated-demo
Changes
docs/src/theming.tsx(test added), see this commit.docs/src/theming.tsx.disableCssVarsProviderusesCssVarsThemeNew:
isolatedflag for mini-app demos. For example:Test on X
To test this PR on MUI X repo:
package.jsonto this commitf26f8aa4381493c58d296e5ce997343b064985a7docs/package.jsontoPlease delete the node_modules and run
pnpm installbeforepnpm docs:dev.