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

Add Noto Sans to the body font stack #2300

Merged
merged 2 commits into from
Nov 7, 2022
Merged

Add Noto Sans to the body font stack #2300

merged 2 commits into from
Nov 7, 2022

Conversation

simurai
Copy link
Contributor

@simurai simurai commented Nov 1, 2022

What are you trying to accomplish?

This adds Noto Sans to the $body-font stack. I think the break down will be something like:

  • -apple-system for Safari + Firefox on macOS
  • BlinkMacSystemFont for Chrome on macOS
  • Segoe UI for Windows
  • Noto Sans for Linux ✨ new ✨
  • Helvetica as legacy fallback
  • Arial as legacy fallback
  • sans-serif as a system fallback
  • Apple Color Emoji + Segoe UI Emoji for emojis

What approach did you choose and why?

Adding Noto Sans should improve alignment issues on Linux. See #1209.

What should reviewers focus on?

If anyone is on Linux and wants to test the new font stack, you could use the following in DevTools:

font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', 'Noto Sans', Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji';

Can these changes ship as is?

To better support Linux.
@simurai simurai requested a review from a team as a code owner November 1, 2022 06:36
@simurai simurai requested a review from langermank November 1, 2022 06:36
@changeset-bot
Copy link

changeset-bot bot commented Nov 1, 2022

🦋 Changeset detected

Latest commit: d451229

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/css Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@simurai simurai requested a review from vdepizzol November 1, 2022 06:38
@simurai simurai temporarily deployed to github-pages November 1, 2022 06:43 Inactive
@github-actions github-actions bot temporarily deployed to Storybook Preview November 1, 2022 06:44 Inactive
@vdepizzol
Copy link
Contributor

vdepizzol commented Nov 1, 2022

Awesome! 🎉 🎉

Side note: we may also have to update this on Primer React and Primer Primitives (which should be used soon as a reference for both implementations)

Copy link
Contributor

@langermank langermank left a comment

Choose a reason for hiding this comment

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

+1 adding this to Primitives!

@simurai simurai merged commit 5a0b9b2 into main Nov 7, 2022
@simurai simurai deleted the noto-sans branch November 7, 2022 08:17
@primer-css primer-css mentioned this pull request Nov 7, 2022
pketh added a commit to kinopio-club/kinopio-client that referenced this pull request Apr 26, 2024
> Hmm this appears to be a bug-by-design in the way Linux handles certain fonts like helvetica: chakra-ui/chakra-ui#2314 . Shipped a fix that specifies a Linux built-in font first: primer/css#2300 . Let me know if this works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants