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

Load IBM fonts via Google Fonts #241

Merged
merged 3 commits into from
Apr 7, 2023

Conversation

Eric-Arellano
Copy link
Collaborator

Unless you already had IBM's fonts installed locally, they would not be used. For example, this is how the docs look for me:

Screenshot 2023-03-31 at 12 49 10 PM

After, with this PR:

Screenshot 2023-03-31 at 12 48 36 PM

We can solve this by using Google Fonts and a CSS import.

Why Google Fonts?

We could also try distributing the font directly. I don't have an informed opinion here; ChatGPT suggests benefits of Google Fonts include ease of use & leveraging their CDN.

Why these font-weights?

Font weight gets set by the CSS rule font-weight:. So, I searched all font-weights we set. We legitimately use 400, 500, 600, and 700.

While our CSS also has 300, that CSS looks dead to me based on grepping _build/html in the docs sample build.

When we don't include a font-weight, the browser falls back to the nearest weight. So, 300 will fall back to 400. For italics, the browser will try to italicize the font for you if there is no font—but this is actually a bad experience.

The downside of including more font variants is that it increases the file size to download our site, which slows down performance, especially with slow Internet.

Comment on lines +3 to +5
/* Import IBM Plex Sans and IBM Plex Mono, at regular 400 and bold 700, italics for both. For
IBM Plex Sans, also include font weights 500 and 600 (non-italics), which are used in table of
contents. */
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't totally sure how to reason about this. For example:

  • should Mono have 500 and 600?
  • should Sans have italics for 500 and 600? Which seem to be used for table of contents

Generally, is it better to be "safe", or should we try to focus on better performance? How much do we care about performance and low-bandwidth with qiskit.org?

Comment on lines -113 to +121
font-weight: bold;
font-weight: 700;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are synonyms. I made the change to be consistent.

qiskit_sphinx_theme/pytorch_base/static/css/theme.css Outdated Show resolved Hide resolved
Copy link
Collaborator

@javabster javabster left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@Eric-Arellano Eric-Arellano merged commit eaa076a into Qiskit:main Apr 7, 2023
@Eric-Arellano Eric-Arellano deleted the load-ibm-fonts branch April 7, 2023 15:35
Eric-Arellano added a commit to Eric-Arellano/qiskit_sphinx_theme that referenced this pull request Apr 7, 2023
Unless you already had IBM's fonts installed locally, they would not be
used. For example, this is how the docs look for me:

![Screenshot 2023-03-31 at 12 49 10
PM](https://user-images.githubusercontent.com/14852634/229204915-4314ef87-bd70-4923-8b1a-9a851e8fd9a2.png)

After, with this PR:

![Screenshot 2023-03-31 at 12 48 36
PM](https://user-images.githubusercontent.com/14852634/229204795-401aea5c-597b-44c6-8452-bbf295902f9d.png)

We can solve this by using Google Fonts and a CSS import. 

## Why Google Fonts?

We could also try distributing the font directly. I don't have an
informed opinion here; ChatGPT suggests benefits of Google Fonts include
ease of use & leveraging their CDN.
  
## Why these font-weights?

Font weight gets set by the CSS rule `font-weight:`. So, I searched all
font-weights we set. We legitimately use 400, 500, 600, and 700.

While our CSS also has 300, that CSS looks dead to me based on grepping
`_build/html` in the docs sample build.

When we don't include a font-weight, the browser falls back to the
nearest weight. So, 300 will fall back to 400. For italics, the browser
will try to italicize the font for you if there is no font—but this is
actually a bad experience.

The downside of including more font variants is that it increases the
file size to download our site, which slows down performance, especially
with slow Internet.

---------

Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
javabster added a commit that referenced this pull request Apr 7, 2023
Unless you already had IBM's fonts installed locally, they would not be
used. For example, this is how the docs look for me:

![Screenshot 2023-03-31 at 12 49 10

PM](https://user-images.githubusercontent.com/14852634/229204915-4314ef87-bd70-4923-8b1a-9a851e8fd9a2.png)

After, with this PR:

![Screenshot 2023-03-31 at 12 48 36

PM](https://user-images.githubusercontent.com/14852634/229204795-401aea5c-597b-44c6-8452-bbf295902f9d.png)

We can solve this by using Google Fonts and a CSS import. 

## Why Google Fonts?

We could also try distributing the font directly. I don't have an
informed opinion here; ChatGPT suggests benefits of Google Fonts include
ease of use & leveraging their CDN.
  
## Why these font-weights?

Font weight gets set by the CSS rule `font-weight:`. So, I searched all
font-weights we set. We legitimately use 400, 500, 600, and 700.

While our CSS also has 300, that CSS looks dead to me based on grepping
`_build/html` in the docs sample build.

When we don't include a font-weight, the browser falls back to the
nearest weight. So, 300 will fall back to 400. For italics, the browser
will try to italicize the font for you if there is no font—but this is
actually a bad experience.

The downside of including more font variants is that it increases the
file size to download our site, which slows down performance, especially
with slow Internet.

---------

Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
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.

2 participants