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

Updates to Marketing Typography #583

Merged
merged 10 commits into from
Jan 23, 2019
Merged

Conversation

sophshep
Copy link

@sophshep sophshep commented Oct 17, 2018

This PR starts to work some of GitHub's recent site design changes back into Primer, starting with typography. It does the following:

  • Renames variables and classes from being prefixed with alt- to suffixed with -mktg. This is more descriptive for how they are to be used, the packages they are in, and matches other Primer naming conventions.
  • Removes Roboto as our marketing font, and replaces it with InterUI
  • Updates the marketing font sizing scale, specifically
    • To simplify it / make it more aligned with Primer Core scale
    • To add h000 size, extra biggo
  • Removes $alt-mono-font because we don't need two mono-fonts
  • Remove alt-text-small since f5 does the same thing
  • Updates docs for all of the above

To-do:

  • Figure out how to host InterUI in Primer (maybe?)

/cc @primer/ds-core & @trosage (who wrote most of this in github/github)

@sophshep
Copy link
Author

Did a little research on how other design systems serve fonts. Solid assumes you have two fonts available on your app and then gives instructions for how to get them. Others (Polaris, Salesforce LDS, Trello) do not mention specifically how to get them. Gov.uk seems to serve them from within the system but I can't figure out all the specifics of how it works.

Since InterUI is open source, I'm leaning towards the first option: writing documentation for how to install it on a project rather than including it within Primer.

Any suggestions @primer/ds-core?

@jhuashao
Copy link

jhuashao commented Oct 25, 2018

Just my two cents:

Inter UI doesn't support a monospace font I think, and it's a little weird how Rasmus's website is displaying and styling the typeface itself:

screen shot 2018-10-25 at 11 56 33 am

screen shot 2018-10-25 at 11 56 19 am

There isn't a differentiation between display and body text for Inter-UI, and I believe the designer is doing it manually through the styling

Apple's -apple-system auto corrects between SF Pro Display and SF Pro Text, I think:
https://developer.apple.com/design/human-interface-guidelines/macos/visual-design/typography/

I think Inter-UI might be a headache for teams to go in manually to declare styling... but there could be information I'm missing!

@sophshep
Copy link
Author

Hey @jhuashao - thanks for your comment!

Inter UI doesn't support a monospace font I think / There isn't a differentiation between display and body text for Inter-UI

Luckily, we're only using Inter for headings (and only on Marketing sites). Body copy on any marketing site will use the system font stack that is used on the rest of GitHub. Same thing goes for mono, so we aren't defining anything special for marketing sites and it can match with the product.

My biggest question at the moment is what is the best approach for installing the font files. I personally think it would be best to link directly to https://github.com/rsms/inter/releases/tag/v3.0 and providing documentation, but I agree that this manual approach could be a headache!

@shawnbot
Copy link
Contributor

shawnbot commented Oct 26, 2018

My biggest question at the moment is what is the best approach for installing the font files. I personally think it would be best to link directly to https://github.com/rsms/inter/releases/tag/v3.0 and providing documentation, but I agree that this manual approach could be a headache!

I agree, and I think that we should at least consider distributing all of the web-ready files (.eot, .woff, etc.) necessary to display it. Otherwise people are just going to hack around and end up posting TrueType and OTF versions to FontSquirrel or whatever. 😱

@gladwearefriends
Copy link
Contributor

just wanted to leave a note here that there are still some alt- classes used in github/github so we should double check that is updated before this is merged in

as well as probably a handful of subdomains that are using them too, e.g. Partners site

i have some spare time between projects and will open a PR shortly to update in github/github!

@@ -221,7 +221,7 @@

/* Set to monospace font */
.text-mono {
font-family: $mono-font;
font-family: $mono-font !important;
Copy link
Contributor

@gladwearefriends gladwearefriends Nov 26, 2018

Choose a reason for hiding this comment

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

adding a note for my commit:
!important is needed in order for the monospace font to have effect with other marketing headers, e.g. h2-mktg when you want a big monospace header like on github.com/ten https://github.com/github/github/pull/102693#discussion_r236315756

https://github.com/github/github/pull/102693/files#diff-20a6e07f77d51d5f093dded1d213d52dR70

@zeke
Copy link
Contributor

zeke commented Dec 3, 2018

Hi friends! Drive-by lurker here. 👋

I'm curious what prompted the decision to replace Roboto with InterUI.

@Markushurni

This comment has been minimized.

@gladwearefriends
Copy link
Contributor

gladwearefriends commented Jan 22, 2019

I just added the font files to this PR!

@shawnbot has written an awesome sync tool https://github.com/primer/sync#readme that will move all scss from node_modules over to _sass so we can surface and use the css/font files from primer in our jekyll sites cc/ @primer/github-site-design

I think that wraps up all thats needed for this PR! if we can get another review/sanity check please 👀

@gladwearefriends gladwearefriends changed the title [WIP] Updates to Marketing Typography Updates to Marketing Typography Jan 22, 2019
@shawnbot
Copy link
Contributor

Okay, so here's the catch with primer-sync: it only does the SCSS files (**/*.scss). However, it should be pretty straightforward making it support font files (**/*.woff, right?).

@shawnbot shawnbot changed the base branch from master to release-11.0.0 January 23, 2019 19:21
@shawnbot
Copy link
Contributor

Okay, I'm going to test this out with primer-sync once the change in 1a7850c gets published. 🤞

@gladwearefriends
Copy link
Contributor

gladwearefriends commented Jan 23, 2019

Hey @zeke

I'm curious what prompted the decision to replace Roboto with InterUI.

Sorry to not respond sooner! I think the main driver for us to switch was mostly based on the visual design of InterUI and how it went well with with our new marketing site design direction. We also introduced a new monospaced font in our marketing design as well (SF Mono) and needed a display/header type that would work well with it; Roboto didnt fit the tone/personality we were going for anymore. We landed on Inter UI as it paired well with everything and is also open source and free, which is important to our heavily trafficked sites. :)

Hopefully I didnt leave any other more important reason for our switch unaccounted for cc/ @primer/github-site-design if i missed anything!

@shawnbot
Copy link
Contributor

Quick follow-up: primer-sync does copy .woff files now, but it puts them in the same place as all of the .scss files. That needs more work, but it shouldn't block this PR. Let's merge this into #498. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants