-
Notifications
You must be signed in to change notification settings - Fork 5k
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
MDL v2 Theming Strategy #4456
Comments
ThemingYeah, so, theming is a large and complicated discussion :). I'll address what I can below but maybe worth moving into a separate larger issue? Maintainability
I think it depends on what aspects of the component are deemed "them-able". For example, in the case of a button, we'd apply all stylistic treatments that distinguish it as a "raised button" within the base css. Then, in the theme css, we would perform color applications. // _variables.scss
$mdl-button-raised-bg-color: white !default;
$mdl-button-raised-text-color: $mdl-typography-text-color-dark !default; // imported from mdl-typography
// mdl-button.scss
.mdl-button {
// omitting typography stuff...
border-radius: 2px;
cursor: pointer;
}
.mdl-button--raised {
box-shadow: /* .... */;
}
// mdl-button-theme.scss
.mdl-button--raised {
background-color: $mdl-button-raised-bg-color;
color: $mdl-button-raised-text-color;
} Then if you wanted to override this in sass: $mdl-button-raised-bg-color: $google-blue-500;
$mdl-button-raised-text-color: white;
@import 'mdl-button/mdl-button-theme'; Or in plain css: /* Button CSS is loaded, but no theme css. */
.mdl-button--raised {
background-color: #2196f3;
color: white;
} I imagine this would be manual, and I imagine it would primarily deal with colors. A lot of preceding implementations have taken the route of actually exposing too much as theme-able. While end users are free to alter and modify and override as they see fit, there are only certain things - mostly having to do with color - that the spec facilitates theming by default. This includes element dimensions. A button, for example, has a certain size that according to the spec should not change. However, if someone wants to make a gigantic CTA button for their site, they can simply attach an additional class and make the necessary overrides. Customizing Primary + Accent ColorsI think you're right in that this serves as the basis for a theme. Here's what I've been thinking for this, which we can discuss further:
Example: // mdl-theme/_mixins.scss
@mixin mdl-theme($primary-color, $accent-color) {
// override all component-specific theme vars.
$mdl-button-raised-bg-color: correct-bg-color($primary-color) !global;
$mdl-button-raised-text-color: $primary-color !global;
$mdl-checkbox-bg-color: $primary-color !global;
// ...
}
// file-using-mdl-theme.scss
@import 'mdl-colors/variables';
@import 'mdl-theme/mixins';
// Sets up all of the correct theme variables.
@include mdl-theme($mdl-color-indigo-500, $mdl-color-pink-a200);
@include 'mdl-button/mdl-button-theme';
@include 'mdl-checkbox/mdl-checkbox-theme';
// .... This would then override the default variables within the theme file and output the proper CSS. This Gist demonstrates a very simple example of how this might work.
I actually have to take off for the day but I'll address the last part of this first thing monday. In short:
@mixin css-var-prop($property-name, $css-var, $fallback-value) {
@supports (--css-variables: 1) {
& {
#{$property-name}: $css-var;
}
}
#{$property-name}: $fallback-value;
} |
One of the things I was hoping we'd do is reduce a bunch of these "variable variables" and simple variables within the codebase. For example
The raised background color is just "white", there isn't any maintenance benefit provided by having this. It isn't used in any calculations in the CSS, so it isn't worth having. The raised text color, is just a reference to the typography variable. This also isn't used in calculations for the component, therefore there isn't a benefit. I think we should heavily look at restricting our variable usage only to places where they provide a tangible benefit. They also shouldn't necessarily be |
Here's a potential thought on how to handle theming across all systems we want to support. This strategy works with both PostCSS and SASS. The idea would be for our "template" output to be valid CSS that uses CSS custom variables for theming, with a fixed colour fallback: :root {
--primary-color: #EF5350;
}
.md-button--raised {
background: #EF5350;
background: var(--primary-color);
} This is a good thing to do anyway, as it allows us to provide theming to systems that make use of it dynamically in the browser (e.g. systems that use native CSS property support or include a polyfill). This could easily be achieved in both SASS: $primary-color: #EF5350;
:root {
--primary-color: $primary-color;
}
.md-button--raised {
background: $primary-color;
background: var(--primary-color);
} and in PostCSS (by cleverly using the custom property plugin's options): :root {
--primary-color: #EF5350;
}
.md-button--raised {
background: var(--primary-color);
} Once we have the output CSS, and if we wanted to use a different color (say, :root {
--primary-color: #EF5350;
}
.md-button--raised {
background: #EF5350;
background: #42A5F5;
} The second definition would override the first, and we'd have successfully customized the colors. The only thing to ensure is that every usage of The textual substitution could happen on the customizer tool, or perhaps more interestingly, as part of an npm install script. And systems that want to use the CSS custom properties would simply not do the textual substitution. Thoughts? |
I thought is what we'd be doing anyways @sgomes... So SGTM 😉. That is the great thing about cascading and CSS Custom Properties. |
@sgomes That does seem like a good approach that doesn't directly tie us to what sass is doing. It also means we don't need a separate theme CSS file. The double definitions seem a bit weird, but hopefully that goes away as custom props become supported. |
@traviskaufman We could also have a quick pass with CSSO (which does work directly in the browser) or similar tools to remove the duplicates. |
Since we're going with PostCSS (via #4514), can we go with saying we're going to be using custom properties / custom property sets for theming? |
Maintainability
One aspect is maintainability. How do we determine which properties should live in the base file or the theme file? Should we e.g. move all colour properties to the theme file? How about size properties? Also, is this something we do manually, or through some tool?
Customising the primary and the accent
I suspect that the simplest level of customisation that users will want to do is simply to change the primary and accent colours for their site. This actually translates into a few more colours, but we can select those automatically:
Our current customizer allows for downloading a custom CSS file with the entire set of MDL styles, using a particular set of colours. The theme file approach would allow us to instead provide add-on theme files, which would override the default colours.
However, how would this play with the modularisation effort? Would we have:
Deep integration with frameworks and end-user code
The final aspect we need to consider is how we can have deep theming integration both with npm and with the frameworks we expect to use our code as a base:
--primary-color
,--accent-color
, etc.)?None of these questions need to be answered on this PR, but they're interesting topics to have on the back of our minds; it may help to have some sense of the direction we're going to take early on, since it could translate into constraints on tool and architecture decisions.
The text was updated successfully, but these errors were encountered: