Style System: Improve automated variable fallbacks#94
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/itsjonq/g2/dg8ofn0yv |
sarayourfriend
left a comment
There was a problem hiding this comment.
LGTM and seems to work great. Left some nits and tips for types and some questions about the approach but otherwise 👍
packages/create-styles/src/createCompiler/plugins/cssVariables.js
Outdated
Show resolved
Hide resolved
packages/create-styles/src/cssCustomProperties/createRootStore.js
Outdated
Show resolved
Hide resolved
packages/create-styles/src/cssCustomProperties/createRootStore.js
Outdated
Show resolved
Hide resolved
| /** | ||
| * Stores CSS config variables that are expected to be added to :root. | ||
| * This is used for CSS variable fallback handling for IE 11. | ||
| */ |
There was a problem hiding this comment.
Type correction: state is probably better typed as Record<string, string> rather than a plain object type. That should be corrected throughout and you can alias it using @typedef {Record<string, string>} State if you'd like (probably for the best).
There was a problem hiding this comment.
@saramarcondes I apologize... I don't know how to do this 🙈.
| * Interprets and retrieves the CSS fallback value of a declaration rule. | ||
| * | ||
| * @param {string} declaration A CSS declaration rule to parse. | ||
| * @param {object} rootStore A store for CSS root variables. |
There was a problem hiding this comment.
This can be typed as RootStore by importing it @param {import('./createRootStore').RootStore}
| * @param {object} rootStore A store for CSS root variables. | |
| * @param {import('./createRootStore').RootStore} rootStore A store for CSS root variables. |
| * variables. | ||
| * | ||
| * @param {string} content Stylis content to parse. | ||
| * @param {object} rootStore A store for CSS root variables. |
packages/create-styles/src/cssCustomProperties/createRootStore.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Sara Marcondes <saram@fastmail.com>
…to fix/css-variable-fallbacks
Co-authored-by: Sara Marcondes <saram@fastmail.com>
|
@saramarcondes I pushed my fixes for the items you suggested (thank you!). |
This update fixes the CSS variable fallback handling for IE11.
This solution has been tested and confirmed working in IE11 (via VirtualBox):
🙈 The Problem
The previous plugin solution relied on an unsupported CSSOM function to retrieve the CSS variable value from
:root(more details here).For example, let's pretend this style exists:
The previous solution attempted to retrieve the
--colorvalue with this:In IE11, this would result in an empty string (
'') because IE11 doesn't understand what--coloreven is.🏪 Alternative Root "Store"
Instead of relying on the CSS
:root {}object, I replicated the same mechanic by establishing a "virtual" store that exists in JS. When the Style System is created, the initial CSS variables will be used to create and establish this "root store".When the Stylis plugin goes to work, the parser will be able to retrieve the fallback values from this virtual root store (instead of using CSS
:root {}).🔌 Plugin
The majority of the plugin code was copied over from stylis-plugin-css-variables. Changes were mainly around adding the virtual store for variable fallback resolution.
🤝 Trade offs
This automated solution only supports variables that are:
:root {}levelcreateStyleSystemThat means locally scoped variables will not be resolved. Also, it (currently) won't resolve variables being defined/modified after the fact.
For the purposes of G2's Component system, I think it's okay if we only support values defined in our variable system. It may encourage users to work with those values, rather than define custom values of their own.
This does affect theming though. The virtual root store won't know how to re-resolve and re-render new theme values that are being pass into
<ThemeProvider />.Perhaps this is okay? At minimum, the UI can render for IE11 (which is great). However, it won't be fully featured compared to other browsers (e.g. theming, night mode, etc...)
🕹 Testing
(The easiest way to test)
Fire up local Storybook and inspect element. If you can see the variable fallbacks, then the system is working 👍 .
As mentioned above, it's using the virtual "root store" strategy rather than
getComputedStyle().getPropertyValue(), which means it will work in IE11.Note: The CSS variable fallback handling are enabled in
developmode. In production, the plugin only kicks in for IE11.Resolves #89