Skip to content
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

Use new font loading strategy #726

Merged
merged 4 commits into from
May 29, 2018
Merged

Use new font loading strategy #726

merged 4 commits into from
May 29, 2018

Conversation

36degrees
Copy link
Contributor

@36degrees 36degrees commented May 29, 2018

This is mirroring the work done by @Nooshu and @steventux in GOV.UK Template (alphagov/govuk_template#341):

This PR has been raised to solve one of the major frontend performance issues that is currently live on GOV.UK. Four WOFF compressed web-fonts are base64 encoded and are sitting on the browsers critical path. This 200KB CSS file must be downloaded and parsed before page rendering can start. This single download is approximately 82% of the render blocking assets which stop the browser showing any content to the user.

Base64 encoding into a CSS file is now considered an anti-pattern so the font loading method is being revisited.

This PR removes the Base64 encoded font and reverts to a standard HTTP request to load the font files. This gives us the added advantage of being able to offer multiple font sources. The browser can then choose the most appropriate supported font format.

The fonts being used are unchanged. We have created a new WOFF2 version of the font which offers approximately 25% better compression and no change in quality.

This PR is also taking advantage of a new CSS property that is available in modern browsers: font-display. This property defines how the browser behaves during the font download. Browsers that don't support this property will ignore it.

Further details including an overview of the performance impact can be found in the original PR.

Additionally, this PR also:

  • Renames the existing govuk-file-url helper to govuk-image-url, and adds a new govuk-font-url helper.
  • Renames the existing $govuk-global-images to $govuk-images-path and adds two new variables – $govuk-fonts-path and $govuk-assets-path, which is used as the base for both $govuk-images-path and $govuk-fonts-path.
  • Fixes a bug whereby print styles were being 'rasterized' into the screen styles when generating the IE8 stylesheet (this is a bug in sass-mq, and has also been raised upstream).

https://trello.com/c/X57aAcmP/960-2-adopt-the-same-font-loading-strategy-as-for-template

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-726 May 29, 2018 13:34 Inactive
@36degrees 36degrees force-pushed the new-font-loading-strategy branch from bbb0ff0 to 4ae8977 Compare May 29, 2018 13:37
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-726 May 29, 2018 13:37 Inactive
@36degrees 36degrees force-pushed the new-font-loading-strategy branch from 4ae8977 to 856ab18 Compare May 29, 2018 13:40
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-726 May 29, 2018 13:41 Inactive
Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! And thank you again @Nooshu @steventux.

I've tested the review application, and looks great.

Just need to add a CHANGELOG entry.

@36degrees
Copy link
Contributor Author

Will wait for #725 to merge and rebase to avoid conflicts before adding CHANGELOG entries.

36degrees added 4 commits May 29, 2018 15:05
We need to handle two types of assets now – images and fonts, so having one generic ‘file’ function for images is even more misleading.

Rename the file-url function to govuk-image-url, update the two components that use it, and rename the image path variable whilst we’re at it.
This is mirroring the work done by @Nooshu and @steventux in GOV.UK Template (alphagov/govuk_template#341):

> This PR has been raised to solve one of the major frontend performance issues that is currently live on GOV.UK. Four WOFF compressed web-fonts are base64 encoded and are sitting on the browsers critical path. This 200KB CSS file must be downloaded and parsed before page rendering can start. This single download is approximately 82% of the render blocking assets which stop the browser showing any content to the user.
>
> Base64 encoding into a CSS file is now considered an anti-pattern so the font loading method is being revisited.
>
> This PR removes the Base64 encoded font and reverts to a standard HTTP request to load the font files. This gives us the added advantage of being able to offer multiple font sources. The browser can then choose the most appropriate supported font format.
>
> The fonts being used are unchanged. We have created a new WOFF2 version of the font which offers approximately 25% better compression and no change in quality.
>
> This PR is also taking advantage of a new CSS property that is available in modern browsers: font-display. This property defines how the browser behaves during the font download. Browsers that don't support this property will ignore it.

Further details including an overview of the performance impact can be found in the original PR.
When rasterizing media queries (when `$mq-responsive` is false) sass-mq was incorrectly rasterizing the print styles as well.

This has been raised against the upstream sass-mq as well: sass-mq/sass-mq#111
@36degrees
Copy link
Contributor Author

@NickColley happy with 83b9e53?

@NickColley
Copy link
Contributor

@36degrees excellent, great notes 👍

@36degrees 36degrees merged commit 3aabcc7 into master May 29, 2018
@36degrees 36degrees deleted the new-font-loading-strategy branch May 29, 2018 14:29
NickColley added a commit that referenced this pull request May 29, 2018
Changes from [govuk_template](https://github.com/alphagov/govuk_template):

Breaking changes:
- Removed stylesheets relating to `govuk_template.css`, we now recommend people bundle stylesheets in their application.
- Removed stylesheets relating to `print`, we do this at a component level.
- Removed stylesheets relating to `font`, we now have a [new font loading strategy](#726) that does not require a different bundle.
- Removed `ie8` specific class being added to the `html` element, since we don't require functionality
- Remove `ie.js`, used to shim HTML5 files, add JSON2 support, we now recommend people bundle in their application.

- Removed functionality to remove `js-enabled` class if `window.GOVUK` is not defined, at the moment we can't guarantee that this global will be set with GOV.UK Frontend.
See alphagov/govuk_template#248 for details on this feature.

Features:
- Added a main block, now skip link works by default, this block can be overridden.

Patch:
- Moved Skip link into it's own block, calling the [Skip link component](https://github.com/alphagov/govuk-frontend/tree/master/src/components/skip-link) by default instead of being inlined.
- Moved footer into it's own block, calling the [Footer component](https://github.com/alphagov/govuk-frontend/tree/master/src/components/footer) by default instead of being inlined.
- Moved header into it's own block, calling the [Header component](https://github.com/alphagov/govuk-frontend/tree/master/src/components/header) by default instead of being inlined.
- Changed the default `main` `id` to be `#main-content` to avoid being [styled by GOV.UK Elements](https://github.com/alphagov/govuk_elements/blob/8216538c1d2efb8b33372a22f28e1ea81d879ecd/assets/sass/elements/_layout.scss#L9).
@Nooshu
Copy link

Nooshu commented May 29, 2018

Awesome work @36degrees! Very happy to see this in Frontend!

NickColley added a commit that referenced this pull request May 31, 2018
Changes from [govuk_template](https://github.com/alphagov/govuk_template):

Breaking changes:
- Removed stylesheets relating to `govuk_template.css`, we now recommend people bundle stylesheets in their application.
- Removed stylesheets relating to `print`, we do this at a component level.
- Removed stylesheets relating to `font`, we now have a [new font loading strategy](#726) that does not require a different bundle.
- Removed `ie8` specific class being added to the `html` element, since we don't require functionality
- Remove `ie.js`, used to shim HTML5 files, add JSON2 support, we now recommend people bundle in their application.

- Removed functionality to remove `js-enabled` class if `window.GOVUK` is not defined, at the moment we can't guarantee that this global will be set with GOV.UK Frontend.
See alphagov/govuk_template#248 for details on this feature.

Features:
- Added a main block, now skip link works by default, this block can be overridden.

Patch:
- Moved Skip link into it's own block, calling the [Skip link component](https://github.com/alphagov/govuk-frontend/tree/master/src/components/skip-link) by default instead of being inlined.
- Moved footer into it's own block, calling the [Footer component](https://github.com/alphagov/govuk-frontend/tree/master/src/components/footer) by default instead of being inlined.
- Moved header into it's own block, calling the [Header component](https://github.com/alphagov/govuk-frontend/tree/master/src/components/header) by default instead of being inlined.
- Changed the default `main` `id` to be `#main-content` to avoid being [styled by GOV.UK Elements](https://github.com/alphagov/govuk_elements/blob/8216538c1d2efb8b33372a22f28e1ea81d879ecd/assets/sass/elements/_layout.scss#L9).
@NickColley NickColley mentioned this pull request May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants