Docs/css variables material UI #32
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
mnajdova
commented
Mar 29, 2022
mnajdova
commented
- I have followed (at least) the PR section of the contributing guide.
5 tasks
siriwatknp
reviewed
Mar 30, 2022
Comment on lines
+9
to
+15
| ## Motivation | ||
|
|
||
| There are three main problem that would be solved by using CSS variables: | ||
|
|
||
| 1. Dark theme flash on SSR - the light theme is being loaded fist by default, and only after that the dark mode is appearing, causing a flash when opening the page. | ||
| 2. Bad debugging experience - if you open the dev tools and try to inpect the styles of some element, all you see is the calculated value it receives, without any information of where that value came from. | ||
| 3. Performance - at this moment, the dark and light themes are considered as different input in the `ThemeProvider`, causing the whole tree to re-render when the theme is changed. |
There was a problem hiding this comment.
I think we don't need to explain it here, instead link to the issue, RFC would be more concise.
siriwatknp
reviewed
Mar 30, 2022
| 2. Bad debugging experience - if you open the dev tools and try to inpect the styles of some element, all you see is the calculated value it receives, without any information of where that value came from. | ||
| 3. Performance - at this moment, the dark and light themes are considered as different input in the `ThemeProvider`, causing the whole tree to re-render when the theme is changed. | ||
|
|
||
| ## Solution |
There was a problem hiding this comment.
Suggested change
| ## Solution | |
| ## Usage |
I think "Usage" is more direct because we are explaining how to use?
siriwatknp
reviewed
Mar 30, 2022
| At this moment, the API does not require any breaking changes for the Material UI components, however it depends on using a new experimental provider for the theme, called `Experimental_CssVarsProvider`. | ||
| Except for providing the theme in the inner React context, this new provider has as a responsibility to generate CSS variables out of all tokens in the theme that are not functions, and make them available in the context. | ||
| All these variables, are available under a key in the theme, named `vars`. | ||
| The structure of this object is identical to the theme structure, the only difference is that the values represent some css varaibles. |
There was a problem hiding this comment.
Suggested change
| The structure of this object is identical to the theme structure, the only difference is that the values represent some css varaibles. | |
| The structure of this object is identical to the theme structure, the only difference is that the values represent some CSS variables. |
siriwatknp
reviewed
Mar 30, 2022
Comment on lines
+24
to
+38
| The best way to see this is by example: | ||
|
|
||
| {{"demo": "CssVarsProviderBasic.js", "iframe": true }} | ||
|
|
||
| If you try to inspec the two `<code>` elements you will see that the style applied on them are different. The one using directly a theme token, looks something like this: | ||
|
|
||
| ``` | ||
| <code style="color: rgb(25, 118, 210);">...</code> | ||
| ``` | ||
|
|
||
| The one using the CSS variables however, allows developers to track the path of where the token is defined in theme: | ||
|
|
||
| ``` | ||
| <code style="color: var(--md-palette-primary-main);">...</code> | ||
| ``` |
There was a problem hiding this comment.
I think this is hard to understand (I don't know what to look for when I first saw it). Maybe the markdown code is better than using demo.
siriwatknp
reviewed
Mar 30, 2022
|
|
||
| ### Customization | ||
|
|
||
| At this moment, the `Button` component is the only component supporting CSS variables (the support is not requiring any breaking changes). |
There was a problem hiding this comment.
This makes me wonder, should we release the doc + a guide (if the community wants to help make other components to support CSS variables) together?
There was a problem hiding this comment.
How about we create an issue and link it here? We can add variables in one more component, that can serve as a template PR, similar as when we don the migration to emotion.
siriwatknp
reviewed
Mar 30, 2022
Comment on lines
+66
to
+73
| declare module '@mui/material/styles' { | ||
| interface Theme { | ||
| vars: Omit< | ||
| MuiTheme, | ||
| 'typography' | 'mixins' | 'breakpoints' | 'direction' | 'transitions' | ||
| >; | ||
| } | ||
| } |
There was a problem hiding this comment.
Should it be more specific? for example, how to extend the palette.
|
I am closing this, will reopen a new one to point to master |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.