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

Change Lato font to webfont (version 2.015) #639

Merged
merged 5 commits into from May 24, 2018
Merged

Change Lato font to webfont (version 2.015) #639

merged 5 commits into from May 24, 2018

Conversation

jessetan
Copy link
Contributor

@jessetan 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 Remove typekit fonts readthedocs.org#3982

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

Copy link
Contributor

@davidfischer davidfischer left a comment

Choose a reason for hiding this comment

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

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')
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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')
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

@davidfischer
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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 readthedocs/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
Copy link
Contributor Author

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
@@ -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')
Copy link
Contributor

Choose a reason for hiding this comment

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

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants