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

Change Lato font to webfont (version 2.015) #639

Merged
merged 5 commits into from May 24, 2018

Conversation

Projects
None yet
3 participants
@jessetan
Contributor

jessetan commented May 15, 2018

This PR:

  • Adds additional formats (woff2, woff, eot) to the existing ttf format
  • The woff(2) files are about 25%-50% of the size of the ttf files, which decreases time to download
  • Supports modern browsers through woff(2), IE < 9 through eot, older Safari/Android through ttf. Unused font files are not downloaded by the browser.
  • Updates Lato to the latest official release from latofonts.com
  • Does not check for local installs of Lato. This can be added at the slight risk of making the font look different. Users with the font installed will not need to download the webfont.
  • Does not use character subsetting. Much smaller files are possible if only the Latin set is required
  • Makes the font look sliiiiighlty more squished vertically at the default size of 16px. This is true for any version of Lato I have tested, except for the ttf that the theme previously used. This is also present on readthedocs.org after rtfd/readthedocs.org#3982

Related: rtfd/readthedocs.org#3982, rtfd/readthedocs.org#3564, #524 and #642

@jessetan jessetan requested review from davidfischer, agjohnson and Blendify May 15, 2018

@davidfischer

I'm excited for this but there's a couple changes needed here, I think.

@@ -12,27 +12,32 @@
@font-face
font-family: 'Lato'
font-style: normal
src: url('../fonts/Lato/lato-regular.eot')
src: url('../fonts/Lato/lato-regular.ttf') format('truetype'), url('../fonts/Lato/lato-regular.svg#latoregular') format('svg')

This comment has been minimized.

@davidfischer

davidfischer May 15, 2018

Contributor

This didn't include the .woff version. As a result, I downloaded the TTF instead.

screen shot 2018-05-15 at 9 34 33 am

This comment has been minimized.

@jessetan

jessetan May 16, 2018

Contributor

Fixed in new commit

@font-face
font-family: 'Lato'
src: url('../fonts/Lato/lato-bolditalic.eot')
src: url('../fonts/Lato/lato-bolditalic.eot?#iefix') format('embedded-opentype'), url('../fonts/Lato/lato-bolditalic.woff2') format('woff2'), url('../fonts/Lato/lato-bolditalic.woff') format('woff'), url('../fonts/Lato/lato-bolditalic.ttf') format('truetype'), url('../fonts/Lato/lato-bolditalic.svg#latobold_italic') format('svg')

This comment has been minimized.

@davidfischer

davidfischer May 15, 2018

Contributor

I don't believe we use bold and italic in the theme anywhere. Is that wrong? This will result in additional disk usage for no benefit although that isn't a huge deal. Regardless, I'd say we shouldn't include it unless there's a reason.

This comment has been minimized.

@jessetan

jessetan May 16, 2018

Contributor

It's not possible to express bolditalic in rst syntax directly, but there are situations where it can happen, like bold text in a caption (which is italic by default) or italic text in a table header (which is already bold by default):

.. list-table::
    :header-rows: 1

    * - header
      - **bold header**
      - *italic header*
    * - cell
      - **bold cell**
      - *italic cell*

is rendered as:
table with italic header

This comment has been minimized.

@davidfischer

davidfischer May 16, 2018

Contributor

It doesn't look like it gets downloaded unless it's needed so I think we're fine here.

@davidfischer

This comment has been minimized.

Contributor

davidfischer commented May 15, 2018

Also, I believe these are slightly smaller than the ones we are using on .org. I will make a small PR to update the .org.

@jessetan

This comment has been minimized.

Contributor

jessetan commented May 16, 2018

Also, I believe these are slightly smaller than the ones we are using on .org. I will make a small PR to update the .org.

You generated the woff(2) on .org from the original ttf right? The font files in this PR came from the original latofonts.com website.

jessetan added some commits May 16, 2018

@jessetan

This comment has been minimized.

Contributor

jessetan commented May 16, 2018

Do we want to prefer a locally installed version of the font if available? This can save the download, although the version the user has installed may differ from what we use.

@davidfischer

This comment has been minimized.

Contributor

davidfischer commented May 16, 2018

You generated the woff(2) on .org from the original ttf right? The font files in this PR came from the original latofonts.com website.

Ya. You did the right thing. I replaced the generated ones with the ones from latofonts.com in rtfd/readthedocs.org#4093.

Do we want to prefer a locally installed version of the font if available? This can save the download, although the version the user has installed may differ from what we use.

I don't think it is necessary. Only a few people will have it.

@jessetan

This comment has been minimized.

Contributor

jessetan commented May 22, 2018

We could reduce bandwidth further by splitting each font file into a latin and one or more non-latin subsets. Browsers that view documentation that only uses latin characters will download the small font file. Other subsets will be downloaded as needed.
Since this requires some font file wrangling and a lot of testing, I prefer to not do that in this PR.

@Blendify Blendify merged commit 0051b91 into master May 24, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@@ -33,5 +38,4 @@
font-family: 'Roboto Slab'
font-style: normal
font-weight: 700
src: url('../fonts/RobotoSlab/roboto-slab-v7-bold.eot')
src: url('../fonts/RobotoSlab/roboto-slab-v7-bold.eot?#iefix') format('embedded-opentype'), url('../fonts/RobotoSlab/roboto-slab-v7-bold.woff2') format('woff2'), url('../fonts/RobotoSlab/roboto-slab-v7-bold.woff') format('woff'), url('../fonts/RobotoSlab/roboto-slab-v7-bold.ttf') format('truetype')
src: local('Roboto Slab Bold'), local('RobotoSlab-Bold'), url(../fonts/RobotoSlab-Bold.ttf) format('truetype')

This comment has been minimized.

@davidfischer

davidfischer Jun 6, 2018

Contributor

Somehow this PR reverted this change. I'll fix it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment