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.

Sign up for GitHub

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

Jump to bottom

feat: IBM Plex Sans Variable #329

Merged
merged 20 commits into from
Aug 21, 2019
Merged

feat: IBM Plex Sans Variable #329

merged 20 commits into from
Aug 21, 2019

Conversation

vpicone
Copy link
Contributor

@vpicone vpicone commented Aug 12, 2019 •
edited
Loading

closes #330

This switches our Plex loading strategy to use local variable fonts. When subsetting the variable fonts I also included those pesky arrows.

We cut the requests from 7→2 and the payload from 126kb to 56kb. This also opens the door for experimenting with more expressive type tokens and better interpolation between those tokens.

tenor

@vercel
Copy link

vercel bot commented Aug 12, 2019 •
edited
Loading

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://gatsby-theme-carbon-git-fork-vpicone-add-variable-fonts.carbon-design-system.now.sh

@vpicone vpicone changed the title feat: use variable fonts feat: IBM Plex Sans Variable Aug 12, 2019
@vpicone vpicone requested review from alisonjoseph and abbeyhrt and removed request for alisonjoseph August 12, 2019 03:42
abbeyhrt
abbeyhrt approved these changes Aug 12, 2019
View reviewed changes
Copy link
Contributor

@abbeyhrt abbeyhrt left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@shixiedesign
Copy link

@vpicone to review this, do we make sure visually the fonts are not different from current deployment? Coz I noticed a few small things between this deployment and https://gatsby-theme-carbon.netlify.com/ and https://www.ibm.com/design/language/elements. Lemme know if I'm reviewing for the correct thing and I will post detailed notes.

@vpicone
Copy link
Contributor Author

vpicone commented Aug 12, 2019 •
edited
Loading

@shixiedesign the url for the master branch/most up to date version of the theme is https://gatsby-theme-carbon.now.sh (not netlify). That netlify link is out of date.

This doesn't change anything about the font styles, just the Plex file we're using.

@shixiedesign
Copy link

shixiedesign commented Aug 12, 2019 •
edited
Loading

@vpicone I noticed these things:

  1. Biggest thing is almost all types are squashed vertically.
  2. The single quote is no longer square.
  3. — and type after the — might have gotten overall a bit smaller...not sure if this is actually a bug.

Aug-12-2019 11-09-24

@vpicone
Copy link
Contributor Author

vpicone commented Aug 12, 2019

@shixiedesign woah! will have to investigate more closely... this PR uses the plex files directly (as opposed to google fonts). I wonder if google fonts has been doing something unusual with our font files. Thanks so much for giving this a careful look!

@vpicone
Copy link
Contributor Author

vpicone commented Aug 15, 2019

@shixiedesign it looks like I was too restrictive with the characters I took out. Could you do that comparison again now?

shixiedesign
shixiedesign reviewed Aug 15, 2019
View reviewed changes
Copy link

@shixiedesign shixiedesign left a comment

Choose a reason for hiding this comment

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

Something happened to links!

image

Aaand that's the only thing I noticed. 🎉

@vpicone vpicone requested a review from shixiedesign August 17, 2019 04:15
shixiedesign
shixiedesign approved these changes Aug 19, 2019
View reviewed changes
Copy link

@shixiedesign shixiedesign left a comment

Choose a reason for hiding this comment

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

Looking great! 🎉

alisonjoseph
alisonjoseph approved these changes Aug 20, 2019
View reviewed changes
Copy link
Member

@alisonjoseph alisonjoseph left a comment

Choose a reason for hiding this comment

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

🚀 nice!

@vpicone vpicone merged commit e5d53b0 into carbon-design-system:master Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abbeyhrt abbeyhrt abbeyhrt approved these changes

@shixiedesign shixiedesign shixiedesign approved these changes

@alisonjoseph alisonjoseph alisonjoseph approved these changes

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Explore using IBM Plex Sans Variable
5 participants
@vpicone @shixiedesign @alisonjoseph @abbeyhrt @jeanservaas