[material-ui] Merge `CssVarsProvider` into `ThemeProvider` #43115

siriwatknp · 2024-07-30T08:52:53Z

closes #43033

Summary of the change

ThemeProvider becomes a wrapper that renders either a ThemeContext or a CssVarsProvider based on the theme.colorSchemes node.
- This will preserve the same behavior for apps that migrate from v5 to v6.
createTheme becomes a wrapper that calls either the previous createTheme or extendTheme based on the user input.
- The default createTheme() does not come with CSS variables by default.
  It has to be enabled with { cssVariables: true } flag.
  
  I tried to make it enables by default but it breaks a lot of tests that depends on computed style checking because CSS variables does not produce the result in JSDOM, this make me think that it'd break a lot of projects too so it should be enabled by user)
- The palette customization still works, see the different scenario in the demo below
Fix the hydration issue, mode from useColorScheme hook is undefined at hydration phase.
Support custom color schemes apart from light and dark.
Docs

Demo

The sandbox below demonstrates use cases for using the new ThemeProvider.

https://codesandbox.io/p/sandbox/material-ui-cra-ts-forked-6tjzq2?file=%2Fsrc%2Fdemos%2FDarkApp.tsx

Improvements

Some features in CssVarsProvider are independent of CSS variables.
For example, disableTransitionOnChange, useColorScheme hook, and system preference.
This is a demo for using these features without generating CSS variables.

I have followed (at least) the PR section of the contributing guide.

…-css-vars-provider

mui-bot · 2024-07-30T08:55:20Z

Netlify deploy preview

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

TabList: parsed: +20.17% , gzip: +15.83%
StepButton: parsed: +22.08% , gzip: +17.12%
SpeedDial: parsed: +20.48% , gzip: +15.89%
AccordionSummary: parsed: +24.57% , gzip: +18.60%
BottomNavigationAction: parsed: +24.82% , gzip: +18.66%
Masonry: parsed: +30.98% , gzip: +22.53%
Hidden: parsed: +31.42% , gzip: +22.76%
Fab: parsed: +24.41% , gzip: +18.42%
OutlinedInput: parsed: +24.33% , gzip: +18.49%
NativeSelect: parsed: +23.29% , gzip: +17.78%
CardActionArea: parsed: +24.98% , gzip: +18.74%
ButtonBase: parsed: +25.44% , gzip: +18.94%
TableSortLabel: parsed: +23.95% , gzip: +18.03%
TabScrollButton: parsed: +23.54% , gzip: +17.69%
InputBase: parsed: +26.25% , gzip: +19.28%
TimelineOppositeContent: parsed: +31.03% , gzip: +22.59%
TimelineContent: parsed: +31.09% , gzip: +22.60%
FilledInput: parsed: +24.31% , gzip: +18.39%
DialogContentText: parsed: +31.57% , gzip: +22.79%
StepContent: parsed: +27.92% , gzip: +20.59%
and 107 more changes

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 9c35d23

…-css-vars-provider

This reverts commit 2603b25.

mnajdova

The implementation looks good, left some initial feedback.

mnajdova · 2024-08-01T09:16:01Z

packages/mui-material/src/styles/ThemeProviderWithVars.tsx

@@ -2,20 +2,18 @@
 import * as React from 'react';
 import { unstable_createCssVarsProvider as createCssVarsProvider, SxProps } from '@mui/system';
 import styleFunctionSx from '@mui/system/styleFunctionSx';
-import extendTheme, { SupportedColorScheme, CssVarsTheme } from './extendTheme';
+import createThemeWithVars, { SupportedColorScheme, CssVarsTheme } from './createThemeWithVars';


We should use the public API as much as possible, e.g. createTheme({ cssVariables: true }), as this is how users would use it.

mnajdova · 2024-08-01T09:17:17Z

packages/mui-material/src/styles/ThemeProviderWithVars.tsx

 const {
  CssVarsProvider,
  useColorScheme,
  getInitColorSchemeScript: deprecatedGetInitColorSchemeScript,
 } = createCssVarsProvider<SupportedColorScheme, typeof THEME_ID>({
  themeId: THEME_ID,
-  theme: defaultTheme,
+  theme: createThemeWithVars,


What's the motivation from changing from defaultTheme to a create theme function usage? Do we plan to have two default themes, one with vars and one without?

This is performance improvement. The previous behavior will create a default theme when you import the component but with this change, it will be called when the component is rendered.

Also, the previous behavior will always create a default theme even if the user passes in a custom theme. With this change, the default theme won't be created if the user provides a custom theme.

packages/mui-material/src/styles/createTheme.test.js

mnajdova · 2024-08-01T09:20:01Z

packages/mui-material/src/styles/createTheme.test.js

+    const theme = createTheme({
+      cssVariables: false,
+      colorSchemes: { light: true },
+      palette: { mode: 'dark' },
+    });


What should this generate, I am a bit confused. Is this even a valid input?

This case test a default dark app with light mode.

It's similar to:

createTheme({ cssVariables: false, colorSchemes: { dark: true }, palette: { mode: 'light' }, });

What should this generate, I am a bit confused. Is this even a valid input?

Yes, you can think of having the provided palette as the default color scheme. The reason I go with this path is to support the existing palette customization so we don't have to change much docs.

What I understand:

The palette values will continue working the same way as before, but the values will also be copied into colorSchemes

Is that correct?

If it's correct, what's the use of copying the palette values into the color schemes? In other words, how is theme.colorSchemes going to be used in our codebase or in users codebases when cssVariables: false?

Is the idea to use colorSchemes from now on and eventually stop using palette?

The palette values will continue working the same way as before, but the values will also be copied into colorSchemes Is that correct?

Yes, that's my purpose. My intention is that the colorSchemes is a new feature to create a second and more color schemes. palette is considered a default color scheme and will continue to work at least in v6.

If it's correct, what's the use of copying the palette values into the color schemes? In other words, how is theme.colorSchemes going to be used in our codebase or in users codebases when cssVariables: false?

Got it. The ThemeProvider contains a state that store a user selected mode, a part of the theme is calculated from theme.colorSchemes[selectedMode]. For example, when user is in dark mode, the theme that passed through context will be:

{ …theme.colorSchemes.dark, typography: …, spacing: …, breakpoints: …, }

We have to do this so that it still work with apps that use values from theme.palette.* when switching between modes.

Is the idea to use colorSchemes from now on and eventually stop using palette?

"use colorSchemes from now on" Yes, but "eventually stop using palette" could be, it will be clearer in v6 when people start using it.

This makes sense to me 👌🏼

When we discuss the v7 theme structure, we can discuss the plan for theme.palette.

mnajdova · 2024-08-01T09:20:43Z

packages/mui-material/src/styles/createTheme.test.js

@@ -23,6 +56,60 @@ describe('createTheme', () => {
    expect(theme.palette.secondary.main).to.equal(green.A400);
  });

+  describe('CSS variables', () => {
+    it('should have a light as a default colorScheme if only `palette` is provided', () => {


Ah nice, this means we can use the old theme structure and still have vars generated. This is great!

Yes, that's one of the goal.

packages/mui-material/src/styles/createThemeWithVars.d.ts

mnajdova · 2024-08-01T09:35:57Z

https://deploy-preview-43130--material-ui.netlify.app/material-ui/customization/css-theme-variables/configuration/

By the end of the guide, can we have an example we can share that has everything set up. There are quite a few things that needs to be generated as expected, e.g. setting both light and dark color schemes etc. I feel like the guide is not complete and hard to follow.

mnajdova · 2024-08-01T09:37:48Z

https://deploy-preview-43130--material-ui.netlify.app/material-ui/customization/css-theme-variables/overview/#colors

We are referencing some old APIs/ways to do things. Should we update this page reflecting the latest changes?

mnajdova · 2024-08-01T09:40:43Z

https://deploy-preview-43130--material-ui.netlify.app/material-ui/migration/migration-css-theme-variables/

Still uses the old CssVarsProvider and extendTheme in the guide

siriwatknp · 2024-08-02T09:24:12Z

https://deploy-preview-43130--material-ui.netlify.app/material-ui/migration/migration-css-theme-variables/

Still uses the old CssVarsProvider and extendTheme in the guide

I am removing this page. It's considered a new feature, so I don't think migration is accurate.

siriwatknp · 2024-08-02T10:09:16Z

https://deploy-preview-43130--material-ui.netlify.app/material-ui/customization/css-theme-variables/configuration/

By the end of the guide, can we have an example we can share that has everything set up. There are quite a few things that needs to be generated as expected, e.g. setting both light and dark color schemes etc. I feel like the guide is not complete and hard to follow.

That page is about configuring the feature. To see light and dark color schemes, I think it should be in the usage guide, I will add some example to https://deploy-preview-43130--material-ui.netlify.app/material-ui/customization/css-theme-variables/usage/.

DiegoAndai

Hey Jun! Good work, it looks like a headache-inducing task to merge these 😅 but it's looking great overall 😊

My main question about the logic is here: #43115 (comment)

packages/mui-material/src/styles/createTheme.test.js

packages/mui-material/src/styles/ThemeProvider.test.tsx

packages/mui-material/src/styles/createThemeWithVars.js

…-css-vars-provider

siriwatknp · 2024-08-05T04:38:46Z

@DiegoAndai My idea is to merge this PR first to release this week to test that everything is working as expected and then let the DevEx team review the docs PR, is that okay with you?

DiegoAndai

@siriwatknp that makes sense to me, I have two more questions:

packages/mui-material/src/styles/createTheme.ts

packages/mui-material/src/themeCssVarsAugmentation/index.d.ts

siriwatknp added 14 commits July 29, 2024 14:42

wip

037f62b

Merge branch 'next' of https://github.com/mui/material-ui into remove…

e2fae64

…-css-vars-provider

rename type

2258db1

only call extendTheme when CssVarsProvider is used

e22d068

create new ThemeProvider and createTheme

5ba82d5

finish first step

3106aa9

move some props to ThemeProviderCssVariablesProps

ab0424a

remove old createTheme and extendTheme

e7bf7dc

fix import paths

36ec12e

fix all tests

2603b25

Merge branch 'next' of https://github.com/mui/material-ui into remove…

71d750a

…-css-vars-provider

switch to new ThemeProvider

4e0f400

fix export path

82aa32e

switch to enable method

584056e

siriwatknp added package: material-ui Specific to @mui/material customization: theme Centered around the theming features labels Jul 30, 2024

siriwatknp added 13 commits July 30, 2024 16:32

fix types

de9fb70

Merge branch 'next' of https://github.com/mui/material-ui into remove…

15440a5

…-css-vars-provider

fix rsc

d1b7783

fix test

bfac926

widen the type

20d1da5

Revert "fix all tests"

e076c5c

This reverts commit 2603b25.

fix disableTransitionOnChange test

28873d0

fix getInitialTheme

fd557f5

add test

7337a7d

fix types

7d8f690

flatten nested folder

0bc2274

fix types

d239b2f

fix lint

cc97041

mnajdova reviewed Aug 1, 2024

View reviewed changes

siriwatknp added 2 commits August 1, 2024 20:18

fix test

8c3f2e0

remove unused type

26d992c

DiegoAndai reviewed Aug 2, 2024

View reviewed changes

packages/mui-material/src/styles/createTheme.test.js Outdated Show resolved Hide resolved

packages/mui-material/src/styles/ThemeProvider.test.tsx Outdated Show resolved Hide resolved

packages/mui-material/src/styles/createThemeWithVars.js Outdated Show resolved Hide resolved

siriwatknp added 5 commits August 2, 2024 22:10

replace fireEvent with user.selectOptions

b98e8dc

fix as suggested

66331ae

fix test

023c756

use createTheme

7d2dde9

prevent theme augmentation tests

a56e5c8

siriwatknp requested review from mnajdova and DiegoAndai August 3, 2024 02:36

siriwatknp added 4 commits August 5, 2024 09:50

Merge branch 'next' of https://github.com/mui/material-ui into remove…

1814385

…-css-vars-provider

perf: remove unnecessary iteration

4306f26

fix: custom color schemes

47d98eb

add createColorScheme

9ca6e73

siriwatknp force-pushed the remove-css-vars-provider branch from c6a36ea to 9ca6e73 Compare August 5, 2024 04:29

fix types

9c35d23

DiegoAndai reviewed Aug 5, 2024

View reviewed changes

packages/mui-material/src/styles/createTheme.ts Show resolved Hide resolved

packages/mui-material/src/themeCssVarsAugmentation/index.d.ts Show resolved Hide resolved

siriwatknp requested a review from DiegoAndai August 5, 2024 15:04

DiegoAndai approved these changes Aug 5, 2024

View reviewed changes

siriwatknp merged commit 5d4b7b6 into mui:next Aug 6, 2024
22 checks passed

aarongarciah mentioned this pull request Aug 8, 2024

[material-ui][docs] Audit and copyedit the v6 migration doc #43073

Merged

oliviertassinari mentioned this pull request Aug 17, 2024

[docs] Fix link to createTheme source #43332

Merged

ZeeshanTamboli mentioned this pull request Sep 5, 2024

[material-ui][theme] Fix wrong cssVariables TypeScript type in Theme #43608

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[material-ui] Merge `CssVarsProvider` into `ThemeProvider` #43115

[material-ui] Merge `CssVarsProvider` into `ThemeProvider` #43115

siriwatknp commented Jul 30, 2024 •

edited

Loading

mui-bot commented Jul 30, 2024 •

edited

Loading

mnajdova left a comment

mnajdova Aug 1, 2024

mnajdova Aug 1, 2024

siriwatknp Aug 1, 2024

mnajdova Aug 1, 2024

siriwatknp Aug 1, 2024

siriwatknp Aug 1, 2024

siriwatknp Aug 1, 2024 •

edited

Loading

DiegoAndai Aug 2, 2024

siriwatknp Aug 2, 2024 •

edited

Loading

DiegoAndai Aug 5, 2024

mnajdova Aug 1, 2024

siriwatknp Aug 1, 2024

mnajdova commented Aug 1, 2024

mnajdova commented Aug 1, 2024

mnajdova commented Aug 1, 2024 •

edited

Loading

siriwatknp commented Aug 2, 2024

siriwatknp commented Aug 2, 2024 •

edited

Loading

DiegoAndai left a comment •

edited

Loading

siriwatknp commented Aug 5, 2024

DiegoAndai left a comment

[material-ui] Merge CssVarsProvider into ThemeProvider #43115

[material-ui] Merge CssVarsProvider into ThemeProvider #43115

Conversation

siriwatknp commented Jul 30, 2024 • edited Loading

Summary of the change

Demo

Improvements

mui-bot commented Jul 30, 2024 • edited Loading

Netlify deploy preview

Bundle size report

mnajdova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siriwatknp Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siriwatknp Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnajdova commented Aug 1, 2024

mnajdova commented Aug 1, 2024

mnajdova commented Aug 1, 2024 • edited Loading

siriwatknp commented Aug 2, 2024

siriwatknp commented Aug 2, 2024 • edited Loading

DiegoAndai left a comment • edited Loading

Choose a reason for hiding this comment

siriwatknp commented Aug 5, 2024

DiegoAndai left a comment

Choose a reason for hiding this comment

[material-ui] Merge `CssVarsProvider` into `ThemeProvider` #43115

[material-ui] Merge `CssVarsProvider` into `ThemeProvider` #43115

siriwatknp commented Jul 30, 2024 •

edited

Loading

mui-bot commented Jul 30, 2024 •

edited

Loading

siriwatknp Aug 1, 2024 •

edited

Loading

siriwatknp Aug 2, 2024 •

edited

Loading

mnajdova commented Aug 1, 2024 •

edited

Loading

siriwatknp commented Aug 2, 2024 •

edited

Loading

DiegoAndai left a comment •

edited

Loading