-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
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
[docs][material-ui] Add default tokens viewer page #42916
Conversation
…-dark-mode-api
…-dark-mode-api
…-dark-mode-api
…-dark-mode-api
…-dark-mode-api
…-dark-mode-api
Netlify deploy previewBundle size reportDetails of bundle changes (Toolpad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments.
{{"component": "modules/material-ui/FontTokensViewer.js"}} | ||
|
||
:::success | ||
Using font variable is an alternative way to `theme.typography` when you cannot access the theme in Javascript. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using font variable is an alternative way to
theme.typography
when you cannot access the theme in Javascript.
This applies to every generated CSS variable, not only typography variables, right? Should we move this info to the top of the file, before the "Colors" section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This applies to every generated CSS variable, not only typography variables, right
Yes, you are right.
Should we move this info to the top of the file, before the "Colors" section?
I think I will just remove it since this page is about listing the default variables. It might be better to add link to a different page for learning how to use them.
|
||
<p class="description">A preview of the default CSS theme variables for built-in light and dark color schemes</p> | ||
|
||
The following tokens are generated when using `CssVarsProvider` in your application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following tokens are generated when using `CssVarsProvider` in your application. | |
The following tokens are generated when using [`CssVarsProvider`](/material-ui/customization/css-theme-variables/overview/). |
@@ -0,0 +1,63 @@ | |||
# Default tokens viewer | |||
|
|||
<p class="description">A preview of the default CSS theme variables for built-in light and dark color schemes</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Observation: it's always weird to read about light/dark color schemes and then see variables unrelated to color.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
|
||
</codeblock> | ||
|
||
## Shadows and overlays |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: are me missing a section in the docs for this in the Customization > Tokens section?
I've been having some trouble customizing the shadows and couldn't find much about it in the docs. The only page I found was under MUI System, which isn't ideal. I agree that we should have a page for it—especially since we have 25 shadow tokens, the least we could do is show them off 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear, it's out-of-scope of this PR right?
|
||
The demo below is a [Paper](/material-ui/react-paper/) component with different level of elevations in light and dark color schemes: | ||
|
||
{{"component": "modules/material-ui/ShadowsOverlaysViewer.js"}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section feels a bit weird: in all of the other sections I can see a list of the variables and their default values. I think we should have the list of variables in this section as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it feel inconsistent to me as well. I will switch back to a table-like format.
|
||
::: | ||
|
||
## Spacing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if there's only one variable, maybe this section would benefit from having a codeblock like the other sections so users can quickly scan the variables across all sections consistently.
<TokensTable isCopied={isCopied}> | ||
<thead> | ||
<tr> | ||
<th width="60%">Token</th> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other tables in the page use "CSS variable", which I think makes more sense.
<th width="60%">Token</th> | |
<th width="60%">CSS variable</th> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a short paragraph to explain how to convert JS dot notation to CSS variables? e.g. theme.palette.common.black
→ --mui-palette-common-black
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
```js JS | ||
theme.spacing(1, 2); // var(--mui-spacing) calc(2 * var(--mui-spacing)) | ||
``` | ||
|
||
```css CSS | ||
.some-class { | ||
margin: calc(var(--mui-spacing) * 2); | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this example is clear. Maybe users expect both snippets to be equivalent, and the first one using the multiple arity version of the function might cause confusion. I'd keep the example to the bare minimum.
@@ -0,0 +1,63 @@ | |||
# Default tokens viewer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rename the page from "Default tokens viewer" to "CSS vars viewer" or "Default CSS vars viewer" (or something similar). The former can be more easily confused with the existing "Default theme viewer" page.
Also, a token is not the same as a CSS var. "token" is the concept, something we give a value to and then consume via CSS vars, sx prop, styled callbacks that access the theme, and whatever other consuming method we could have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I like the idea of having these in the docs, and I believe it's a must. I don't know if this is the best approach, though.
I wonder if this information shouldn't be split into more pages, and each token having its own page under CSS Variables menu item, or live under the existing tokens' pages, in a CSS Variables section instead. As a user, I would expect to find the CSS Variables tokens in the Typography page. The way that's in here, the information feels a bit hidden from the users. cc @samuelsycamore @alelthomas
From a UX standpoint, nested scrolling is never a good thing. I'd go with a different design for these, something like Medusa UI does for colors, and be more 'visual' for the other tokens as well.
Leaving some examples:
Should we discuss this structure on Figma and come up with some explorations before a PR?
Using font variable is an alternative way to `theme.typography` when you cannot access the theme in Javascript. | ||
The font variable should be used with the CSS `font` property like this: | ||
|
||
```css |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having thought about this a bit more, I think we should give it some more thought and document the CSS vars closer to where we document the theme/tokens. Having two providers ( We already have the "Tokens" section (with different subsections: palette, typography, etc.) under "Customization". We can explore documenting the CSS vars in those subsections if we don't want to change much the structure now. I'd also love to see how "Default theme viewer" could somehow display the corresponding CSS vars for the corresponding tokens. It gets tricky because the default theme shape is different depending on the provider. TL;DR I think we should think about this a bit more instead of adding a new page. PS: I think it's important that we don't call CSS vars tokens, so we all have the same mental model:
|
@aarongarciah @zanivan Thanks for the comment, I totally agree about revisiting the place to move this info to. |
Blocked by #42839Similar to Joy UI, I think this is a very useful page for exploring the default CSS theme variables.