[pickers] Support `theme.vars` #6778

alexfauquette · 2022-11-08T11:54:46Z

mui-bot · 2022-11-08T11:59:32Z

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

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	669.4	1,030.1	698.4	814	139.556
Sort 100k rows	ms	598.7	1,049.4	598.7	880.12	158.528
Select 100k rows	ms	203.3	487.5	258.4	291.92	101.734
Deselect 100k rows	ms	143.4	280.4	202	217.28	50.213

Generated by 🚫 dangerJS against aa08209

alexfauquette · 2022-11-08T14:16:45Z

@siriwatknp I'm not sure about how to extend the theme types. For now, I put the

import type {} from '@mui/material/themeCssVarsAugmentation';

Into the root index.ts file. Would you have some recommendation about it?

siriwatknp · 2022-11-08T14:20:36Z

packages/x-date-pickers-pro/src/DateRangePickerDay/DateRangePickerDay.tsx

-      backgroundColor: alpha(theme.palette.primary.light, 0.6),
+      color: (theme.vars || theme).palette.primary.contrastText,
+      backgroundColor: theme.vars
+        ? `rgba(${theme.vars.palette.primary.lightChannel} / ${0.6})`


Suggested change

? `rgba(${theme.vars.palette.primary.lightChannel} / ${0.6})`

? `rgba(${theme.vars.palette.primary.lightChannel} / 0.6)`

The curry bracket is not needed (x the others).

siriwatknp · 2022-11-08T14:23:53Z

packages/x-date-pickers-pro/src/index.ts

@@ -1,3 +1,5 @@
+import type {} from '@mui/material/themeCssVarsAugmentation';


Suggested change

import type {} from '@mui/material/themeCssVarsAugmentation';

This should not be enabled by the library, it should be done by the consumer instead.

Can you try importing it into the MUI X global project instead?

what do you mean by

importing it into the MUI X global project

Without it, TypeScript complains about theme.vars as it's not available on the standard Theme interface.
Can we solve it somehow without importing themeCssVarsAugmentation?

Even if we fix it on our side, make sure to test a bundled version of our package with skipLIbCheck: false because I suspect it will fail without this import.

Doesn't it make sense for @mui/x-date-pickers to import the module augmentation that it uses internally ?

Doesn't it make sense for @mui/x-date-pickers to import the module augmentation that it uses internally

I am open to any option but the developers have to opt-in to get the type. Meaning when they install @mui/x-date-pickers and @mui/material, they should not see theme.vars.

If they want to have theme.vars, they have to opt-in (either import from @mui/material or @mui/x-date-pickers).

I struggle to understand how we could use theme.vars in our files, and hide the theme.vars type from users

siriwatknp · 2022-11-08T14:29:03Z

packages/x-date-pickers-pro/src/DateRangePickerDay/DateRangePickerDay.tsx

+      color: theme.palette.getContrastText(
+        theme.vars
+          ? `rgba(${theme.vars.palette.primary.lightChannel} / ${0.6})`
+          : alpha(theme.palette.primary.light, 0.6),
+      ),


Suggested change

color: theme.palette.getContrastText(

theme.vars

? `rgba(${theme.vars.palette.primary.lightChannel} / ${0.6})`

: alpha(theme.palette.primary.light, 0.6),

),

color: theme.vars ? theme.vars.palette.text.primary : theme.palette.getContrastText(alpha(theme.palette.primary.light, 0.6)),

I think this is the closest result because alpha(theme.palette.primary.light, 0.6) mostly gives very light color.

flaviendelangle · 2022-11-15T09:52:58Z

packages/x-date-pickers/src/internals/utils/themeVar.ts

+
+export type ExtendedTheme = Theme | CssVarsTheme;
+
+export const getThemePalette = (theme: ExtendedTheme): Theme['palette'] | ThemeVars['palette'] => {


I do like this solution a lot

@siriwatknp If you're good with this solution I will do the same in the data-grid

Honestly, I don't want to go that way because the issue is just about typings. I found a way to make TypeScript happy by adding the themeCssVarsAugmentation to tsconfig.json.

{ "extends": "../../../tsconfig.json", "compilerOptions": { "types": ["react", "mocha", "node"], "noImplicitAny": true }, "include": [ "src/**/*", "../../../node_modules/@mui/monorepo/test/utils/initMatchers.ts", + "../../../node_modules/@mui/material/themeCssVarsAugmentation" ] }

Note: you will need to add it to all tsconfig.json and tsconfig.build.json in all packages/*.

@alexfauquette @flaviendelangle What do you think?

Seems to work.

Confirmed, you need to add it to tsconfig.build.json too, otherwise, yarn build fails.

clearly less hacky, and I confirm yarn release:build succeed

Seems like apps using @mui/x-date-pickers with this approach also works and don't have access to theme.vars

github-actions · 2022-11-15T10:06:39Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

packages/x-date-pickers-pro/src/DateRangePickerDay/DateRangePickerDay.tsx

siriwatknp

Left one last comment, the rest looks good.

…ckerDay.tsx Co-authored-by: Siriwat K <siriwatkunaporn@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

siriwatknp

🤩 Can't wait for this!

alexfauquette added 3 commits November 8, 2022 12:33

add TS theme.vars

3eb226e

straight forward modifications

b9f37f9

Add alpha colors

aafedab

alexfauquette added the customization: css Design CSS customizability label Nov 8, 2022

alexfauquette added 2 commits November 8, 2022 15:07

move TS imports

2fe6fa5

prettier and proptypes

bbccaf5

alexfauquette requested review from cherniavskii and siriwatknp November 8, 2022 14:14

siriwatknp reviewed Nov 8, 2022

View reviewed changes

alexfauquette mentioned this pull request Nov 8, 2022

[DataGrid] Support theme.vars #6784

Merged

feedbacks

6444dda

flaviendelangle reviewed Nov 15, 2022

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 15, 2022

alexfauquette added 2 commits November 16, 2022 10:53

Merge remote-tracking branch 'upstream/next' into css-vars

9e6cfeb

new solution for TS

42e515f

alexfauquette force-pushed the css-vars branch from c75d68d to 42e515f Compare November 16, 2022 10:01

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 16, 2022

alexfauquette added 2 commits November 16, 2022 11:26

add doc

6ebb63a

scripts

236bd40

flaviendelangle approved these changes Nov 16, 2022

View reviewed changes

siriwatknp reviewed Nov 16, 2022

View reviewed changes

packages/x-date-pickers-pro/src/DateRangePickerDay/DateRangePickerDay.tsx Outdated Show resolved Hide resolved

siriwatknp requested changes Nov 16, 2022

View reviewed changes

Update packages/x-date-pickers-pro/src/DateRangePickerDay/DateRangePi…

aa08209

…ckerDay.tsx Co-authored-by: Siriwat K <siriwatkunaporn@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

alexfauquette requested a review from siriwatknp November 22, 2022 10:36

siriwatknp approved these changes Nov 23, 2022

View reviewed changes

alexfauquette merged commit 87d3b48 into mui:next Nov 23, 2022

alexfauquette deleted the css-vars branch November 23, 2022 08:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pickers] Support `theme.vars` #6778

[pickers] Support `theme.vars` #6778

alexfauquette commented Nov 8, 2022

mui-bot commented Nov 8, 2022 •

edited

alexfauquette commented Nov 8, 2022

siriwatknp Nov 8, 2022 •

edited

siriwatknp Nov 8, 2022

alexfauquette Nov 8, 2022

cherniavskii Nov 8, 2022

flaviendelangle Nov 9, 2022

siriwatknp Nov 9, 2022

alexfauquette Nov 9, 2022

siriwatknp Nov 8, 2022

flaviendelangle Nov 15, 2022

alexfauquette Nov 15, 2022

siriwatknp Nov 16, 2022 •

edited

siriwatknp Nov 16, 2022

siriwatknp Nov 16, 2022

alexfauquette Nov 16, 2022

flaviendelangle Nov 16, 2022

github-actions bot commented Nov 15, 2022

siriwatknp left a comment

siriwatknp left a comment

	? `rgba(${theme.vars.palette.primary.lightChannel} / ${0.6})`
	? `rgba(${theme.vars.palette.primary.lightChannel} / 0.6)`

		@@ -1,3 +1,5 @@
		import type {} from '@mui/material/themeCssVarsAugmentation';


		export type ExtendedTheme = Theme \| CssVarsTheme;

		export const getThemePalette = (theme: ExtendedTheme): Theme['palette'] \| ThemeVars['palette'] => {

[pickers] Support theme.vars #6778

[pickers] Support theme.vars #6778

Conversation

alexfauquette commented Nov 8, 2022

mui-bot commented Nov 8, 2022 • edited

alexfauquette commented Nov 8, 2022

siriwatknp Nov 8, 2022 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siriwatknp Nov 16, 2022 • edited

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

github-actions bot commented Nov 15, 2022

siriwatknp left a comment

Choose a reason for hiding this comment

siriwatknp left a comment

Choose a reason for hiding this comment

[pickers] Support `theme.vars` #6778

[pickers] Support `theme.vars` #6778

mui-bot commented Nov 8, 2022 •

edited

siriwatknp Nov 8, 2022 •

edited

siriwatknp Nov 16, 2022 •

edited