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] Introduce mode to CssVarsProvider #29418

Merged
merged 40 commits into from
Nov 2, 2021

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Oct 31, 2021

Summary

This PR separates mode from colorScheme, so that the application can achieve the upcoming toggle on the Joy page (design link).

Screen Shot 2564-11-01 at 10 45 39

The mode will always be day | night | system. The application cannot replace or add more modes. However, they can have unlimited color schemes (light, dark is the default color scheme from the design system).

Primer design system also apply this approach

Changes

Demo: https://deploy-preview-29418--material-ui.netlify.app/joy/

  • useColorScheme provide mode and colorScheme.
  • developer can control the color scheme in day and night mode separately.
  • system mode is implemented
  • application can adjust the default mode, default color schemes
    <CssVarsProvider
      theme={{
        colorSchemes: { // light & dark is the default color schemes
          extraLight: {...},
          trueDark: {...},
        }
      }}
      defaultMode="system" // use system mode preference
      defaultColorScheme={{
        day: 'extraLight', // by default `extraLight` will be used in day mode
        night: 'trueDark', // by default `trueDark` will be used in night mode
      }}
    />

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 31, 2021

Details of bundle changes

@material-ui/system: parsed: +7.36% , gzip: +4.66%

Generated by 🚫 dangerJS against 826cfc3

@siriwatknp siriwatknp marked this pull request as ready for review November 1, 2021 04:14
} as unknown as MediaQueryList);
});
afterEach(() => {
window.matchMedia = originalMatchmedia;
Copy link
Member Author

Choose a reason for hiding this comment

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

need to reset the matchMedia, otherwise, it will affect some tests in the project.

@@ -0,0 +1,207 @@
import * as React from 'react';
Copy link
Member Author

@siriwatknp siriwatknp Nov 1, 2021

Choose a reason for hiding this comment

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

This file is for the preview demo. (will be deleted before merging).

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.

I really think we should use dark and light instead of day and night.

const { mode, setMode, colorScheme, setColorScheme } = useColorScheme();

// When mounted on client, now we can show the UI
React.useEffect(() => setMounted(true), []);
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

To prevent SSR className mismatch because on the server there is no mode | colorScheme. Here is the explanation https://github.com/pacocoursey/next-themes#usetheme

This page is for demo only, will be removed before merging.

packages/mui-system/src/cssVars/createCssVarsProvider.d.ts Outdated Show resolved Hide resolved
packages/mui-system/src/cssVars/useCurrentColorScheme.ts Outdated Show resolved Hide resolved
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.

LGTM 🤘

@siriwatknp siriwatknp merged commit 2a158ff into mui:master Nov 2, 2021
@zannager zannager added the package: system Specific to @mui/system label Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: system Specific to @mui/system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants