-
Notifications
You must be signed in to change notification settings - Fork 320
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
Spike how we can use sass to support custom properties on older browsers #4925
Comments
Initial, untested thoughtsFirstly, I think this would need to be a mixin since we're returning 2 properties. This is a shame as I reckon the experience of a function ie: something like The params of this mixin would need to be something to point to the custom property and the value said property is referencing as well as the CSS attribute we want to apply these values to. It could therefore look something like this: @mixin govuk-custom-property($property-name, $applied-value, $custom-property) {
#{$property-name}: $applied-value;
#{$property-name}: var(--#{$custom-property});
}
// example use
@include govuk-custom-property('color', $govuk-text-colour, 'govuk-text-color'); This then opens the door for how clever we want to be. If we presume that there is always a one to one relationship between the name of a given custom property and the sass variable it references, we could do something like this: @mixin govuk-custom-property($property-name, $variable-name) {
#{$property-name}: $govuk-#{$variable-name};
#{$property-name}: var(--govuk-#{$variable-name});
}
// example use
@include govuk-custom-property('color', 'text-colour'); This is of course untested and means that a non-custom property has to also be a sass variable and means that we can't pass something other than a sass variable as the first instance of the attribute. We could potentially remedy this by including an optional override param. Finally, something worth considering is if we want to be conscious of dark mode and webviews. Credit to @romaricpascal for this next part of the writeup. If we only wanted custom properties related to dark mode to be available to webviews, we could reference a sass variable to identify a page as a webview in the mixin to assess this: @mixin govuk-custom-property($property-name, $applied-value, $custom-property) {
#{$property-name}: $applied-value;
@if $govuk-for-app-webview {
#{$property-name}: var(--#{$custom-property});
}
} Something to be aware of here is that there's a distinction between custom properties that are only for dark mode/webviews and custom properties that could benefit govuk-frontend more generally. Likely there's some overlap. Therefore it may be beneficial to instead use the webview variable outside the mixin, like this: @include govuk-custom-property('color', $govuk-text-colour, if($govuk-for-app-webview, 'govuk-text-colour', null)); |
An amendment to my previous comment re: the single param, string interpolation solution: It turns out sass doesn't allow the interpolation of variables, for good reason which is that it makes the variable ecosystem harder to understand. As that page suggests though, we can instead define a map of private vars and reference that: $_govuk-applied-colours: (
"brand-colour": $govuk-brand-colour,
"text-colour": $govuk-text-colour
/* etc */
);
/** elsewhere… */
@mixin govuk-custom-property($property-name, $variable-name) {
if not map-has-key($_govuk-applied-colours, $variable-name) {
@error "Not a vald $variable-name";
}
#{$property-name}: map-get($variable-name);
#{$property-name}: var(--govuk-#{$variable-name});
} Having a private map has the added benefit of letting us loop over it to automatically generate custom properties. Thanks to @36degrees and @romaricpascal for surfacing these points. |
What
Spike a sass mixin or function that could return both a CSS custom property and the sass variable said property is associated with.
Why
As part of our assessment on the impact of custom properties on older browsers that don't support them, something we can explore is adding a fallback option to our CSS so that things still "work" in older browsers. We started exploring this in #4907 but found that it wasn't completely operable and think that handling this at the prep-processing phase will allow users to benefit from it more.
Who needs to work on this
Developers
Who needs to review this
Developers
Done when
The text was updated successfully, but these errors were encountered: