Libsass is throwing an error when processing GOV.UK Frontend, as Libsass doesn't support CSS min() and max() which is used in the width container component.
Specifically, Libsass doesn't like calc() being used in max() as Libsass tries to process it as a Sass function and can't because calc() is CSS function for use in the browser. I've investigated it in an issue for finder frontend if you'd like more information.
Dart Sass and (ironically) Ruby Sass does support CSS min/max - the things that's stopping these being used is that there doesn't seem to be any Rails friendly way of using Dart Sass (without refactoring vast swathes of code so Webpacker can be used) and Ruby Sass is deprecated.
Because there doesn't seem to be much movement in getting Libsass to have support for CSS min/max, would it be possible to place the notch support behind a flag so it can be turned off? I think the default should be to keep it on, but to have the ability to exclude it with a flag when needed.
Hey @injms,
Is there an error message you can share?
I don't think we want to turn off parts of GOV.UK Frontend based on the Sass compiler used without exploring other options.
If you could share a minimal reproduction of this issue there might be another way to solve this.
Thanks for raising :)
Something to note: we use node-sass to build GOV.UK Frontend which under the hood uses libsass.
Sorry, I should've added the full error message - here it is:
Failure/Error: <!--[if gt IE 8]><!--><%= stylesheet_link_tag "application" %><!--<![endif]-->
ActionView::Template::Error:
Error: "calc(0px)" is not a number for `max'
on line 1169 of stdin, in function `max`
from line 1169 of stdin
>> @supports (margin: max(calc(0px))) {
-------------------^
# stdin:1169
# ./app/views/layouts/finder_layout.html.erb:5:in `_app_views_layouts_finder_layout_html_erb___4327627882324680024_70165332331320'
# ./spec/controllers/finders_controller_spec.rb:62:in `block (4 levels) in <top (required)>'
# ------------------
# --- Caused by: ---
# SassC::SyntaxError:
# Error: "calc(0px)" is not a number for `max'
# on line 1169 of stdin, in function `max`
# from line 1169 of stdin
# >> @supports (margin: max(calc(0px))) {
#
# -------------------^
# stdin:1169
I'll sketch up a minimal reproduction too and give a shout when I'm done.
It's a really odd one because Libsass says it doesn't support CSS min/max, but it works with GOV.UK Frontend that does use min/max, yet still throws this error. 馃お
The only difference I can see between the two is that node-sass uses Libsass 3.5.4 and SassC Ruby / SassC Rails uses 3.5.2 - but that shouldn't affect this as there's no mention of min/max for these versions in the Libsass release notes.
As a workaround, I don't think we should turn it off by default. An option to be able to turn it off would be gratefully received, as then a deprecated dependency could be removed - but it's completely a nice to have as things are working at the moment and it's not a bug with GOV.UK Frontend.
Hi @injms,
It sounds like your Sass processor is being run twice, or it's being run on already-processed CSS rather than the original Sass source?
I wrote the min/max code you've linked to, but had to surround it in unquote() so it's treated as plain text, rather than the native Sass min/max.
margin: unquote("max(calc(0px))")
Yep, the removing of the unquote() now gives me the same error when using ruby-sass. Now to work out why it's being stripped out. I'll close this issue as it's not one that needs changes to GOV.UK Frontend.
Thanks @colinrotherham @nickcolley 馃槂
For future reference to anyone who stumbles across this, a solution was found in SassC Rails issue 93.
BUT this turns off CSS compression, so you'll need to run it through either another CSS compressor in the Rails assets pipeline, or compress it separately.
Relevant bit:
So by adding this to my config/environments/test.rb the problems went away:
# Set a css_compressor so sassc-rails does not overwrite the compressor when running the tests config.assets.css_compressor = nil
EDIT:
And here's how to add compression using the Sass gem. Add this to the config file as well:
config.sass.style = :compressed
Thanks for sharing Ian :)
This seems to come up a lot for Ruby users, I wonder if we can adjust the CSS with a workaround to prevent this issue.
I wonder if we changed margin: unquote("max(calc(0px))") to margin: unquote("max(0)") if this would still work?
@colinrotherham do you know why we used the calc method in this support statement?
libsass is planning to add CSS min() and max() support with their next release - it's in the milestone for 3.7.
@injms Ah that's great.
So it sounds like the new Sass min/max feature will cleverly use the native CSS methods, or fall back to the standard Sass ones: https://sass-lang.com/documentation/syntax/special-functions#min-and-max
So one day we can then use native CSS min()/max() in Sass without unquote() 馃帀
@nickcolley Sadly it would break the @supports test. To work with Safari on iOS 11+ the units (px) need to be specified otherwise I presume max() evaluates to nothing and doesn't run.
So this would work for Safari:
margin: unquote("max(0px)")
But, we added calc() into the @supports test because it's used in the "display cutout" media query itself, so it's the responsible thing to test for it before we attempt to use it.
Another method I've seen to avoid the clash between CSS and Sass - but not tested in this case - is that capitalising makes Sass ignore the function.
(Since CSS doesn't care about case you could capitalise any or all of the letters.)
So this contrived example
.selector {
margin-top: Max(20px, 400px);
margin-left: max(10px, 500px);
}
compiles to
.selector {
margin-top: Max(20px, 400px);
margin-left: 500px;
}
Most helpful comment
For future reference to anyone who stumbles across this, a solution was found in SassC Rails issue 93.
BUT this turns off CSS compression, so you'll need to run it through either another CSS compressor in the Rails assets pipeline, or compress it separately.
Relevant bit:
EDIT:
And here's how to add compression using the Sass gem. Add this to the config file as well: