Ensure font is not loaded twice on slow networks #1242

NickColley · 2019-03-07T18:26:19Z

Prior to this change if you were loading GOV.UK Frontend alongside alphagov/govuk_template you would have two @font-face declarations that tell the browser how to load the 'nta' font.

This is normally not an issue since as soon as the browser downloads a version it won't download a second copy of the font.

However when a user is on a slow connection, the browser can try to download the font from both the alphagov/govuk_template declaration and GOV.UK Frontend declaration.
Which means that the end user will download the font twice (and it's variants) resulting in a much slower experience.

This is even more of a problem for users on a slow network since their bandwidth may already be constrained.

To avoid this we check if the user has indicated they are using GOV.UK Template by checking the relevant compatibility flag.

If they have set this flag, we avoid outputting the declarations with the assumption that they will already have them, avoiding the chance of the font being downloaded twice.

We should take care to consider how this could impact bringing the new version of the NTA font @dashouse has been working on.

Fixes #1001

How to test this in the GOV.UK Frontend review application.

Add the following HTML to the _generic.njk template to load the GOV.UK Template font

<link rel="stylesheet" media="all" href="/govuk_template/assets/stylesheets/fonts.css?0.26.0">

Set the compatibility flag for GOV.UK Template on.
Throttle network connection in Chrome Developer tools so and check if two fonts load.

You can do the same but changing the flag set in 2. to test the different variations.

NickColley · 2019-03-07T18:39:33Z

@injms @alex-ju if I did a test release branch of this would you be interested in testing it to confirm it resolves things your end?

36degrees

Have we tested this with GOV.UK Template to make sure this behaves as expected?

src/helpers/typography.test.js

36degrees · 2019-03-11T14:22:47Z

src/helpers/_typography.scss

+  // If the user is using the default NTA font we need to include the font-face declarations.
+  //
+  // We do not need to include the NTA font-face declarations if alphagov/govuk_template is being used since they will already be included.
+  @if ($govuk-font-family == $govuk-font-family-nta and not($govuk-compatibility-govuktemplate)) {


Do you think it's worth introducing a setting specifically for this, tied to compatibility mode by default, like we have $govuk-typography-use-rem?

It'd allow users to override it, if they need to.

(I can't actually think of a reason, other than perhaps if they don't have compatibility mode on but they want to declare the font face separately because e.g. they've got a departmental CDN or something?)

Yeah could be useful, I guess I was just being a bit lazy and didn't want to add and document a feature I wasn't sure people needed.

Happy to do this though.

Added this, not sure about the naming though...

36degrees · 2019-03-11T14:26:30Z

src/helpers/typography.test.js

+    expect(results.css.toString()).toContain(`font-family: "ntatabularnumbers"`)
+  })
+  describe('should not output NTA @font-face declarations', () => {
+    it('when a custom font-family is set', async () => {


This feels a bit backwards, because the expectation is being described in the describe block, and the context is being described in the it block.

I'd probably just remove the describe block and move the context into the it block?

For example:

it('does not include the font-face when a different font-family is used' ...)

36degrees · 2019-03-11T14:55:21Z

src/helpers/typography.test.js

+
+    const results = await sassRender({ data: sass, ...sassConfig })
+
+    expect(results.css.toString()).toContain(`@font-face`)


Can we cast to string once rather than three times?

NickColley · 2019-03-11T15:10:27Z

@36degrees I tested this by setting the compatibility flag to true for GOV.UK Template, and including the font.css file from GOV.UK Template in the review application.

I'm hoping to get some help testing this in a real world scenario from GOV.UK Publishing.

alex-ju · 2019-03-12T09:09:58Z

@NickColley sure, I'll test it on gov.uk - this was sitting in the backlog for quite a while - awesome to see it tackled 🏉

NickColley · 2019-03-12T10:52:43Z

@alex-ju if you want to test this, you can run the following

npm install --save alphagov/govuk-frontend#bdd839a7

alex-ju · 2019-03-13T10:34:39Z

@NickColley tested this and works as expected 👏

36degrees

👍

@font-face

Prior to this change if you were loading GOV.UK Frontend alongisde alphagov/govuk_template you would have two @font-face declarations that tell the browser how to load the 'nta' font. This is normally not an issue since as soon as the browser downloads a version it won't download a second copy of the font. However when a user is on a slow connection, the browser can try to download the font from the alphagov/govuk_template declaration and GOV.UK Frontend declaration. Which means that the end user will download the font twice (and it's variants) resulting in a much slower experience. This is even more of a problem for users on a slow network since their bandwidth may already be constrained. To avoid this we check if the user has indicated they are using GOV.UK Template by checking the relevant compatibility flag. If they have set this flag, we avoid outputting the declarations with the assumption that they will already have them, avoiding the chance of the font being downloaded twice. This feature can also be disabled with a top level $govuk-include-default-font-face flag to ensure that our users have full control.

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1242 March 7, 2019 18:26 Inactive

NickColley force-pushed the ensure-font-is-not-loaded-twice-with-compatibility-mode branch from 608a768 to 82d1b83 Compare March 7, 2019 18:36

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1242 March 7, 2019 18:36 Inactive

NickColley changed the title ~~Test when NTA @font-face declaration should output~~ Ensure font is not loaded twice on slow networks Mar 7, 2019

NickColley marked this pull request as ready for review March 7, 2019 18:39

NickColley added this to Needs review in Design System Sprint Board via automation Mar 8, 2019

kellylee-gds requested a review from 36degrees March 11, 2019 10:02

NickColley added this to the [NEXT] milestone Mar 11, 2019

36degrees reviewed Mar 11, 2019

View reviewed changes

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1242 March 11, 2019 17:10 Inactive

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1242 March 11, 2019 17:51 Inactive

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1242 March 11, 2019 17:58 Inactive

NickColley force-pushed the ensure-font-is-not-loaded-twice-with-compatibility-mode branch from a95ddc2 to 8c21245 Compare March 12, 2019 11:48

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1242 March 12, 2019 11:48 Inactive

NickColley force-pushed the ensure-font-is-not-loaded-twice-with-compatibility-mode branch from 8c21245 to bd68e7b Compare March 12, 2019 11:51

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1242 March 12, 2019 11:52 Inactive

36degrees approved these changes Mar 18, 2019

View reviewed changes

NickColley added 3 commits March 18, 2019 15:08

Test when NTA @font-face declaration should output

2da5d0e

Add CHANGELOG entry

632f98c

NickColley force-pushed the ensure-font-is-not-loaded-twice-with-compatibility-mode branch from bd68e7b to 632f98c Compare March 18, 2019 15:09

govuk-design-system-ci deployed to govuk-frontend-review-pr-1242 March 18, 2019 15:09 View deployment

NickColley merged commit e7f1315 into master Mar 18, 2019

Design System Sprint Board automation moved this from Needs review to Done Mar 18, 2019

NickColley deleted the ensure-font-is-not-loaded-twice-with-compatibility-mode branch March 18, 2019 15:27

NickColley mentioned this pull request Mar 18, 2019

Release v2.9.0 #1249

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure font is not loaded twice on slow networks #1242

Ensure font is not loaded twice on slow networks #1242

NickColley commented Mar 7, 2019 •

edited

NickColley commented Mar 7, 2019

36degrees left a comment

36degrees Mar 11, 2019

NickColley Mar 11, 2019

NickColley Mar 11, 2019

36degrees Mar 11, 2019

36degrees Mar 11, 2019

NickColley commented Mar 11, 2019 •

edited

alex-ju commented Mar 12, 2019

NickColley commented Mar 12, 2019

alex-ju commented Mar 13, 2019

36degrees left a comment


		const results = await sassRender({ data: sass, ...sassConfig })

		expect(results.css.toString()).toContain(`@font-face`)

Ensure font is not loaded twice on slow networks #1242

Ensure font is not loaded twice on slow networks #1242

Conversation

NickColley commented Mar 7, 2019 • edited

How to test this in the GOV.UK Frontend review application.

NickColley commented Mar 7, 2019

36degrees left a comment

Choose a reason for hiding this comment

36degrees Mar 11, 2019

Choose a reason for hiding this comment

NickColley Mar 11, 2019

Choose a reason for hiding this comment

NickColley Mar 11, 2019

Choose a reason for hiding this comment

36degrees Mar 11, 2019

Choose a reason for hiding this comment

36degrees Mar 11, 2019

Choose a reason for hiding this comment

NickColley commented Mar 11, 2019 • edited

alex-ju commented Mar 12, 2019

NickColley commented Mar 12, 2019

alex-ju commented Mar 13, 2019

36degrees left a comment

Choose a reason for hiding this comment

NickColley commented Mar 7, 2019 •

edited

NickColley commented Mar 11, 2019 •

edited