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

[system] Add flags for nested CssVarsProvider #35277

Merged
merged 11 commits into from Dec 5, 2022

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Nov 28, 2022

Changes

  • CssVarsProvider won't generate a new style sheet if the upperTheme.cssVarPrefix and the input theme.cssVarPrefix is the same.
  • By default, the nested CssVarsProvider will not create a new react context.
  • Added a flag, disableStyleSheetGeneration to always skip the style sheet generation.
  • Added a flag, disableNestedContext to force the provider to create its own context.

Here are the use cases:

  • Single source of state, multiple themes:

    <CssVarsProvider> // default theme
      ...
      <CssVarsProvider theme={extendTheme({ cssVarPrefix: 'theme2' })}>
      ...
    </CssVarsProvider>
  • Single source of state, siblings have the same theme:

     <CssVarsProvider> // default theme
       ...
       <CssVarsProvider theme={extendTheme({ cssVarPrefix: 'theme2' })}>
       
       <CssVarsProvider theme={extendTheme({ cssVarPrefix: 'theme2' })} disableStyleSheetGeneration>
       
       <CssVarsProvider theme={extendTheme({ cssVarPrefix: 'theme2' })} disableStyleSheetGeneration>
       ...
     </CssVarsProvider>
  • Separate state, same theme:

     <CssVarsProvider> // default theme
       ...
       <CssVarsProvider
         disableNestedContext
         colorSchemeSelector="#demo"
         colorSchemeNode={typeof document !== 'undefined' ? document.getElementById('demo') : undefined}
       >
       
     </CssVarsProvider>
  • Separate state, different theme:

     <CssVarsProvider> // default theme
       ...
       <CssVarsProvider
         theme={extendTheme({ cssVarPrefix: 'demo' })}
         modeStorageKey="demo-mode"
         defaultMode="system"
         disableNestedContext
       >
       
     </CssVarsProvider>

@siriwatknp siriwatknp added package: system Specific to @mui/system package: material-ui Specific to @mui/material package: joy-ui Specific to @mui/joy new feature New feature or request labels Nov 28, 2022
@mui-bot
Copy link

mui-bot commented Nov 28, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-35277--material-ui.netlify.app/

@material-ui/core: parsed: +0.06% , gzip: +0.09%
@material-ui/system: parsed: +0.45% , gzip: +0.55%
@mui/joy: parsed: +0.09% , gzip: +0.19%

Details of bundle changes

Generated by 🚫 dangerJS against bcdc2ff

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.

Some nits added, but overall this direction is much better than having a new nested provider component 👍 I would recommend in the PR description to add examples of a real use-cases where each if the scenarios would be useful :)

@@ -11,11 +11,15 @@ export default function DarkModeByDefault() {
// the props below are specific to this demo,
// you might not need them in your app.
//
theme={extendTheme({ cssVarPrefix: 'demo' })}
Copy link
Member

Choose a reason for hiding this comment

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

Could we create the theme outside of the render function.

@@ -41,8 +41,11 @@ export default function IdentifySystemMode() {
// The props below are specific to this demo,
// you might not need them in your app.
//
theme={extendTheme({ cssVarPrefix: 'demo' })}
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@ZeeshanTamboli
Copy link
Member

@siriwatknp Just pointing out to issue #35236 if it helps.

@siriwatknp siriwatknp marked this pull request as ready for review December 5, 2022 02:56
@siriwatknp siriwatknp merged commit 2183b98 into mui:master Dec 5, 2022
feliperli pushed a commit to jesrodri/material-ui that referenced this pull request Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request package: joy-ui Specific to @mui/joy package: material-ui Specific to @mui/material package: system Specific to @mui/system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants