Skip to content
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

Theme Variants #683

Closed
tomdye opened this issue Mar 10, 2020 · 2 comments
Closed

Theme Variants #683

tomdye opened this issue Mar 10, 2020 · 2 comments
Assignees

Comments

@tomdye
Copy link
Member

tomdye commented Mar 10, 2020

We would like to support variants of our themes.

Problem

Currently we use css-variables to share colours / sizes etc across the theme and when we build these we both inline and bullet proof these variables into the generated CSS. css-variables are currently defined at the :root level and imported into each of the css files that uses them. This means that we end up with multiple copies of the css-variables in the css bundle and that we are unable to override these variables easily to change the appearance of a theme as we cannot unload the previous set of variables or easily control the specificity of them.

With our current configuration, to build theme variants we would need to build multiple variants of our themes with different variables, this would create large css files which we would like to avoid.

We would like to support setting both the theme and the variant at both a global and a widget level. This would allow us to follow the same pattern we currently use for either injecting or prop-passing themes in order the pass variants.

We need to ensure discoverability of theme variants so that a user can import and use them easily.

Proposal

In order to enable theme variants we should separate out the css and the css-variables from our themes so that we can support theme variants by simply changing the set of variables we supply. This would mean that we would no longer support the generic variables.css file we currently do and would not include these variables in our theme builds.

In order to support theme variants via css-variables we could convert our variables files into css-modules that contain variables within a class. This class can then be applied to a domnode in order to cascade those variables to any domnodes beneath which are using them.

We can support this by making changes to the theme middleware (and mixin) to return a variant class which widgets can then apply to their root to support a variant. Furthermore we should change our theme index file signature to return a variants object which can be used to aid discovery and setting of a default variant.

Changes required

framework

We will need to make changes to the existing theme middleware / mixin to support setting a theme variant and retrieving a class that applies the current variant. We will need to deal with variant defaults also.

IE11

In order to support IE11 we will need to use a polyfill for run time css-variables. A suitable poly fill can be found here.

cli-build-theme

We will need to ensure that cli-build-theme no longer looks for and inlines a variables.css file. This has always been a special case that cli-build-theme had to cater for in order to support inlining of css variables. We will need to remove our existing bulletproofing fallbacks in order to ensure ie11 behaves as expected and to ensure that variant variables always override with expected specificity.

existing themes

Existing themes will need to have their variables.css converted to a variant .m.css file and export a default variant from their index file. We will need to remove all imports of variables.css from our themes. Some folder structure changes may be required

widgets

Our widgets will need to be updated to set a variant class on their root elements. This will ensure that they receive the appropriate css-variables for their theme variant.

@tomdye
Copy link
Member Author

tomdye commented Mar 11, 2020

PR's raised (linked above), without tests as yet, for all the above required changes besides the IE11 shim.
Happy for someone to pick these up and carry them over the line whilst I'm on holiday.

@agubler
Copy link
Member

agubler commented Apr 17, 2020

Landed.

@agubler agubler closed this as completed Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants