Significant line height differences between node-sass and dart-sass

[andymantell]

Rendering the govuk-frontend scss with dart-sass results in surprisingly significant line height differences when compared to node-sass. This is caused by the default precision of dart-sass being 10, whereas it is 5 in node-sass. The problematic code lies here, in the calculation of line height.

  @if not unitless($line-height) and unit($line-height) == unit($font-size) {
    $line-height: $line-height / $font-size;
  }

  @return $line-height;
}

This is a screenshot of the differences registered by backstop (In this case, from nhsuk-frontend, but it's the same line height code).

Over on nhsuk-frontend, we are now forcibly rounding to 5 decimal places so that we get consistent rendering across node-sass and dart-sass (Sounds minor, but people are likely to be hopping between different bits of the NHS regularly so as much consistency as possible is nice). I will open a pull request shortly with our solution, for discussion...

[gunjam]

Do you know if this issue affects both the dart-sass binary and the javascript version (npm sass), or just one or the other?

[andymantell]

I've only tested this with the npm sass variant of dart-sass but I suspect it would be an issue in all variants of it. I see no reason to suspect it would only be the JavaScript compiled version?

[36degrees]

From testing, it looks like 6 (or more) decimal places is required for the 'correct' computed line height in Chrome:

Font size	Desired line height	6+	5	4	3	2	1
80	80px	80px	80px	80px	80px	80px	80px
48	50px	50px	50.0002px	50.0016px	50.016px	49.92px	48px
36	40px	40px	40px	39.9996px	39.996px	39.96px	38.72px
27	30px	30px	30px	29.9997px	30.0059px	30.0144px	29.92px
24	30px	30px	30px	30px	30px	30px	28.8px
19	25px	25px	25px	25.0002px	25.0145px	25.1328px	24.96px
16	20px	20px	20px	20px	20px	20px	19.2px
14	20px	20px	20px	20.0004px	20.006px	20.1344px	20.16px

Testing with a precision of 7-10 decimal places resulted in identical results to 6 decimal places.

Testing methodology

With a page with this HTML:

<p class="govuk-body govuk-!-margin-0 govuk-!-font-size-80">govuk-!-font-size-80</p>
<p class="govuk-body govuk-!-margin-0 govuk-!-font-size-48">govuk-!-font-size-48</p>
<p class="govuk-body govuk-!-margin-0 govuk-!-font-size-36">govuk-!-font-size-36</p>
<p class="govuk-body govuk-!-margin-0 govuk-!-font-size-27">govuk-!-font-size-27</p>
<p class="govuk-body govuk-!-margin-0 govuk-!-font-size-24">govuk-!-font-size-24</p>
<p class="govuk-body govuk-!-margin-0 govuk-!-font-size-19">govuk-!-font-size-19</p>
<p class="govuk-body govuk-!-margin-0 govuk-!-font-size-16">govuk-!-font-size-16</p>
<p class="govuk-body govuk-!-margin-0 govuk-!-font-size-14">govuk-!-font-size-14</p>

Run this in the console:

$$('[class*="govuk-!-font-size"]').map(e => [ e.className, window.getComputedStyle(e).getPropertyValue('line-height')])

Worth noting that Bootstrap also use (and recommend) 6 decimal places because of issues with browser rounding.

In theory, dart sass is 'correct' here as it results in a line height that matches the one defined as part of the typography scale – node sass is wrong because of rounding issues.

However, the only difference between node-sass (with a precision of 5) and dart sass (with a precision of 10) should be at 48px. I'm therefore unsure why you're seeing such big differences on the table in the screenshot above – what browser was that screenshot taken in, and are you using different line-height / font-size combinations?

I'm wondering if instead of rounding to 5 decimal places we should instead use a precision of 6 in our build pipeline for the dist CSS and document a recommendation for teams using node sass to set their precision to 6 as well similar to what Bootstrap are doing?

[andymantell]

that would have been in headless Chrome, and using nhsuk-frontend which probably has different enough code by now that our results may be slightly different to govuk-frontend.

Have you tried doing a rendering in both and visually comparing them?

We've gone with rounding to 5dp so that regardless of what people do, things are at least consistent. I wouldn't have much confidence that people using node-sass would read the docs saying to set their precision to 6? There's quite a lot of hopping between NHS websites within a single journey so consistency felt like the thing to do for now for us.

Your suggestion is definitely tidier though. We're going to be left with this rounding oddness for the forseeable future.

[36degrees]

As I suspected, it looks as though rounding errors do affect the line height for a lot more of the font size / line-height combinations in NHS Frontend than in GOV.UK Frontend.

From testing with NHS Frontend (without changing anything) using the same technique:

Font size	Desired line height	Calculated line height at 5 dp
64	72px	72px
48	56px	56.0002px
32	40px	40px
22	32px	32.0001px
19	28px	27.9999px
16	24px	24px
14	24px	24.0001px

Testing methodology

With a page with this HTML:

<p class="nhsuk-u-margin-0 nhsuk-u-font-size-64">nhsuk-u-font-size-64</p>
<p class="nhsuk-u-margin-0 nhsuk-u-font-size-48">nhsuk-u-font-size-48</p>
<p class="nhsuk-u-margin-0 nhsuk-u-font-size-32">nhsuk-u-font-size-32</p>
<p class="nhsuk-u-margin-0 nhsuk-u-font-size-22">nhsuk-u-font-size-22</p>
<p class="nhsuk-u-margin-0 nhsuk-u-font-size-19">nhsuk-u-font-size-19</p>
<p class="nhsuk-u-margin-0 nhsuk-u-font-size-16">nhsuk-u-font-size-16</p>
<p class="nhsuk-u-margin-0 nhsuk-u-font-size-14">nhsuk-u-font-size-14</p>

Run this in the console:

$$('[class*="nhsuk-u-font-size"]').map(e => [ e.className, window.getComputedStyle(e).getPropertyValue('line-height')])

The fact that it affects 19px 'body' size explains, I think, why the screenshot you posted shows such a difference.

If we do change anything, I think we'll likely look to change the precision in our own build pipeline to 6dp (or just migrate our own build pipeline to dart-sass which'll force us to 10dp anyway) and document the precision somewhere. I accept it's more likely to be missed by teams, but because GOV.UK's only affected at 48px, which I think means only really heading-xl will be affected, I'm not sure it's really worth us worrying about.

I'm going to leave this open until I get the chance to play this back to the rest of the team and see what they want to do, and whether we want to do any more investigation like testing with other browsers or the visual diff you suggested.

Thanks ever so much for taking the time to flag this with us anyway – really appreciate it.

[andymantell]

Ah, that's interesting. Sorry for leading you up the garden path a bit! In which case, I completely agree with you then - that for govuk-frontend's purposes the rounding errors are much less significant and the 6dp recommendation is more than sufficient.

I'll close the attached PR...

Closes #2200

[36degrees]

Not at all, it was definitely worth investigating (and interesting 🤓 )! Really do appreciate you taking the time to share your findings with us.

[36degrees]

I think the things we need to do to resolve this are:

• change the precision in our own build pipeline to 6dp (unless we've already done Migrate GOV.UK Frontend build pipeline to use Dart Sass #2239 which'll force us to 10dp anyway)
• document the precision somewhere, for anyone using node-sass or Ruby Sass (but accept that teams are likely to miss it)

I've also noticed a separate but related issue with the grid which I've written up in #2361, and which we should do at the same time.

[colinrotherham]

@36degrees Does this need another look?

We talked about documenting 6dp in Node Sass or LibSass above

Appreciate that only our font size 50 (Chrome 50.0002px, Safari 56.00016px) had 5dp rounding issues versus much more obvious ones in the NHS typography scale, yet our next release ./dist compiled CSS will use 10dp via:

• Migrate GOV.UK Frontend build pipeline to use Dart Sass #3164