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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update font to use v2 of GOV.UK Transport font. #1434

Merged
merged 6 commits into from Jun 7, 2019

Conversation

@aliuk2012
Copy link
Contributor

commented Jun 6, 2019

Based off @dashouse's PR #1356

This PR uses v2 of the font.

  • Adds new font files, reducing the amount of files from 12 to 4

  • Significantly better alignment with the baseline of Helvetica / Arial*

  • Tabular numbers are now rolled up in to the Light or Bold weights and set with font-feature-settings (CSS applied to existing mixin so $tabular: true still works)

  • resolves issue in Firefox where underlines appear too low

  • All browsers apart from IE8 and older support WOFF or WOFF2. IE8 and older will not download any font but will fallback to Arial

This PR does not remove any of the adjustments that were needed for v1.

Old font:

馃憠 Home page: https://govuk-frontend-review-pr-1434.herokuapp.com/?legacy=1
馃憠 Table component: https://govuk-frontend-review-pr-1434.herokuapp.com/components/table?legacy=1
馃憠 Tabular number example: https://govuk-frontend-review-pr-1434.herokuapp.com/components/table/table-with-head-and-caption/preview?legacy=1

New Font:

馃憠 Home page: https://govuk-frontend-review-pr-1434.herokuapp.com/
馃憠 Table Component: https://govuk-frontend-review-pr-1434.herokuapp.com/components/table
馃憠 Tabular number example: https://govuk-frontend-review-pr-1434.herokuapp.com/components/table/table-with-head-and-caption/preview

Thanks to @Nooshu for creating the original issue (alphagov/govuk_template#357 & #1012) and the recommendation to drop EOT files for older IE #1356 (comment).

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@aliuk2012 aliuk2012 marked this pull request as ready for review Jun 6, 2019

@aliuk2012 aliuk2012 referenced this pull request Jun 6, 2019

@aliuk2012 aliuk2012 force-pushed the al/add-v2-fonts branch from 17ea6c3 to fcf2414 Jun 6, 2019

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@nickcolley
Copy link
Contributor

left a comment

@aliuk2012 could we reference #1012 and #1356 (comment) please in the original PR notes?

What's the best way to test tabular numbers before and after?

src/helpers/_font-faces.scss Show resolved Hide resolved
@@ -16,14 +16,20 @@
/// @type List
/// @access public

$govuk-font-family: $govuk-font-family-nta !default;
$govuk-font-family: if($govuk-use-legacy-font,

This comment has been minimized.

Copy link
@nickcolley

nickcolley Jun 6, 2019

Contributor

Really like how this refactor has panned out nice work.

@aliuk2012 aliuk2012 added this to Needs review in Design System Sprint Board via automation Jun 6, 2019

@aliuk2012 aliuk2012 added this to the v3.0.0 milestone Jun 6, 2019

src/settings/_typography-font.scss Outdated Show resolved Hide resolved
src/settings/_typography-font-families.scss Outdated Show resolved Hide resolved
src/helpers/_font-faces.scss Outdated Show resolved Hide resolved
src/helpers/_font-faces.scss Outdated Show resolved Hide resolved
src/helpers/_typography.scss Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@36degrees

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2019

P.S. super commit structure and messages 馃憤

@aliuk2012

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2019

@36degrees

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2019

Oh, I meant the earlier commits with the detailed commit messages 馃憤

@aliuk2012 aliuk2012 force-pushed the al/add-v2-fonts branch from b58dc96 to 0212e42 Jun 6, 2019

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@36degrees
Copy link
Contributor

left a comment

Looking good, just one minor comment.

src/settings/_typography-font-families.scss Outdated Show resolved Hide resolved

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 6, 2019 Inactive

@36degrees
Copy link
Contributor

left a comment

馃憦

src/helpers/_font-faces.scss Outdated Show resolved Hide resolved
src/helpers/_font-faces.scss Outdated Show resolved Hide resolved

Design System Sprint Board automation moved this from Needs review to In progress Jun 6, 2019

@Nooshu

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

Excellent work @aliuk2012! Very excited to see this in Frontend. I'd say leave font-display with fallback for the moment. There's no reason for it to become blocker. We can create a few test pages and see which setting works best. Should be an easy change for a point release post v3 release.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1434 Jun 7, 2019 Inactive

dashouse and others added some commits May 16, 2019

Introduces v2 font as GOV.UK Transport
By default v2 font will been used unless the service is running in compatibility
mode.

Some services still need to use govuk template/elements. In the current
state, the user would end up downloading two versions of the font.

To avoid this issue we have created another flag to check if the the service
is running in compatiblity mode by having any of these flags set:
- $govuk-compatibility-govukfrontendtoolkit
- $govuk-compatibility-govuktemplate
- $govuk-compatibility-govukelements

The new flag '$govuk-use-legacy-font' can also be overriden by setting its
value to "true" just before importing govuk-frontend.
Removes support for EOT and browsers older than IE8
Embedded Open Type (.eot) is a proprietary format, which no other browsers use apart
from IE8 and older. As v2 font is so similiar to Arial, we have decided
to remove EOT format. This means that IE8 and older will not be downloading
any custom fonts like v1 and v2 of Transport but will instead use Arial by
default.

This commit also fixes an existing issue with IE 9-11 using EOT format
by default instead of WOFF. This is because the browser
would only look for and use the first format it finds that it supports.

Thanks goes to Matthew Hobbs (@Nooshu)[https://github.com/Nooshu] for this
recommendation.

An extract of Matt's recommendation can be found below and at this link:
#1356 (comment)

I'd recommend removing the EOT versions of the fonts from the `@font-face` declaration.
Only IE support this font format (including IE11). All but IE8 also support other font
formats that are available. According to the last 30 days GOV.UK analytics, IE8 has had
9,054 "users" out of 50,566,007 (0.018%).

EOT files can be compressed with Gzip (or Zopfli for 5% better compression),
but even with this compression we are asking old browsers on legacy devices
to download a fairly large amount of data (60KB) for unhinted versions of the
font ([only Vista and XP require hinted versions](https://docs.google.com/spreadsheets/d/1l2SKP9D3Fm5EVk10j9K1CUSg_jCkW7Pef60TzK0ZKlI/edit#gid=0)).
These browsers and devices are the least equipped to manage these assets.

Font | Size (bytes) | Saving (bytes) | Saving (%)
------------ | ------------- | ------------- | -------------
Bold EOT uncompressed | 58932 | 0 | 0
Bold EOT gzipped | 30366 | 28566 | 48.4
Bold EOT zopfli | 28115 | 30817 | 52.3
Light EOT uncompressed | 58708 | 0 | 0
Light EOT gzipped | 29885 | 28823 | 49.1
Light EOT zopfli | 27589 | 31119 | 53

**Totals**: Uncompressed download - 118KB, Compressed (gzip) - 60KB, Compressed (Zopfli) - 56KB.

I'd say it's a better strategy for this small number of devices is to use
the fallback font of Arial, and not have them require to download the
additional EOT assets. Since the new font now corrects the glyph placement
in the bounding box, the fallback will render correctly without the need for
additional adjustments to the vertical placement.

If it is we decide to keep the EOT files in the `@font-face` declaration
it's worth noting the container order will prevent IE9-11 from receiving
the WOFF version of the font. Browsers pick the first font format it supports
from the list. It will ignore any declarations after, (even if they are
also supported). In the current setup IE9, 10 and 11 will download the
EOT format even though they all support WOFF.

Since WOFF files are already compressed using Gzip compression, developers
don't need to worry about enabling compression on the server. If developers
forget to enable Gzip compression on the server for EOT files (since we
can't provide compressed versions of the EOT files) these users will be
downloading 118KB vs 83KB (WOFF). Due to [all the hacks required](https://www.paulirish.com/2009/bulletproof-font-face-implementation-syntax/)
to make fonts work properly in older versions of IE there looks to be no
way around this. This seems like another good reason to remove the EOT
sources from the declaration.

Recommended declaration
```css
@font-face {
    font-family: "nta";
    src: govuk-font-url("light-68f0c84f0e-v2.woff2") format("woff2"),
         govuk-font-url("light-222368e53d-v2.woff") format("woff");
    font-weight: normal;
    font-style: normal;
    font-display: swap;
}

@font-face {
    font-family: "nta";
    src: govuk-font-url("bold-9561e2d007-v2.woff2") format("woff2"),
         govuk-font-url("bold-5d3836b441-v2.woff") format("woff");
    font-weight: bold;
    font-style: normal;
    font-display: swap;
}
```
Add CSS for Tabular numbers inside existing mixin
Thanks goes to Matthew Hobbs (@Nooshu)[https://github.com/Nooshu] for this
recommendation.

An extract of Matt's recommendation can be found below and at this link:
#1012

Version 1 of the font was missing the details required to enable embedded
tabular numbers. Version 2 now allows us to control this tablular number
setting using CSS, thus removing the need for two other fonts
(tabular-light, tabular-bold). This tabular setting can be enabled using
the following example CSS:

```
.tabular {
    font-feature-settings: "tnum" 1;
}

@supports(font-variant-numeric: tabular-nums) {
    .tabular {
        font-feature-settings: normal;
        font-variant-numeric: tabular-nums;
    }
}```

The difference this setting makes can be seen in the screenshot below.
Older browser will fallback to font-feature-settings, where as newer browser
will see the @supports rule and use font-variant-numeric.
Adds CHANGELOG entry
Co-authored-by: Oliver Byford <oliver.byford@digital.cabinet-office.gov.uk>
revert back to using `font-display:fallback`
We need to have a better understanding and test what it means for the
font-display to use `swap` instead of fallback.

A new issue has been created:
#1437

@aliuk2012 aliuk2012 force-pushed the al/add-v2-fonts branch from d41d055 to 1a70708 Jun 7, 2019

@aliuk2012 aliuk2012 merged commit 8ecb22b into v3 Jun 7, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

Design System Sprint Board automation moved this from In progress to Done Jun 7, 2019

@aliuk2012 aliuk2012 referenced this pull request Jun 10, 2019
33 of 33 tasks complete

@36degrees 36degrees deleted the al/add-v2-fonts branch Jul 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
You can鈥檛 perform that action at this time.