Update font-stack in design tokens #136

didoo · 2022-03-30T10:06:09Z

📌 Summary

This resolves #128 - see the issue for more details on what problem we're trying to solve with this PR

🛠️ Detailed description

In this PR I have:

updated the font-stack design tokens in “typography”
- removed SF Pro Display/Text in Display/Text
  - as noticed by @heatherlarsen it was useless, -apple-system, BlinkMacSystemFont already do the job
- replaced Segoe UI Display/Text with Segoe UI in Display/Text
- added Helvetica, Arial, sans-serif to the sans block in Display/Text
  - we initially added also Roboto but then decided to remove it (reference)
- removed Ubuntu for the linux platform
  - reason is that apparently can cause issues (reference)
- added explicit emoji support for Display/Text
- replaced SF Mono with ui-monospace in Code
  - notice: this is the only way to access “SF Mono” via CSS and it works only on Safari
  - the previous implementation was wrong and didn't work, it just fell back to the default monospace in the browser
- added Menlo to Code
re-generated design tokens in output

👀 How to review

👉 Review by files changed

Reviewer's checklist:

+1 Percy
Confirm that PR has a changelog update via Changesets if needed

💬 Please consider using conventional comments when reviewing this PR.

🚨 Percy visual regression tests failing

All the visual regression tests in Percy are failing. There is not much we can do. The reasons are multiple:

The tests for Chrome/Firefox/Edge are likely running on some Linux or Windows machine/emulator, so having changed the font stacks (eg. Segoe + Roboto/Helvetica/Arial) is likely to cause a different font to be used on these OS.
Safari was not activated as target in Percy, so we can't see if the tests are failing on Mac/Safari but I did a test in my local machine, and the Display and Body sans text is exactly the same, while the difference is only on the Code monospace (and it's for the reason mentioned above, that the previous implementation was not using the right font declaration).

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1202045371056992

- replaced `Segoe UI Display/Text` with `Segoe UI` - added `Roboto, Helvetica, Arial, sans-serif` to the `sans` block - added explicit emoji support for `Display/Text` - added `Menlo` to `Code`

vercel · 2022-03-30T10:06:14Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

hds-flight-website – ./

🔍 Inspect: https://vercel.com/hashicorp/hds-flight-website/B7MqaevS3u6NWERJdJeLq9CxUZpc
✅ Preview: https://hds-flight-website-git-128-update-design-token-74989e-hashicorp.vercel.app

hds-components – ./

🔍 Inspect: https://vercel.com/hashicorp/hds-components/EHTgbQR26yJiPioHn2fj7TWK4EQK
✅ Preview: https://hds-components-git-128-update-design-tokens-fo-e82ffd-hashicorp.vercel.app

changeset-bot · 2022-03-30T10:06:15Z

🦋 Changeset detected

Latest commit: e8a860c

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

This PR includes changesets to release 2 packages

Name	Type
@hashicorp/design-system-components	Minor
@hashicorp/design-system-tokens	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

- replaced `SF Mono` with `ui-monospace` (it’s the only way to access “SF Mono” on Mac via CSS, see here https://qwtel.com/posts/software/the-monospaced-system-ui-css-font-stack/#:~:text=worth%20expanding%20on%3A-,ui%2Dmonospace,-Future%20system%20monospace; notice it works only for Safari (https://caniuse.com/extended-system-fonts)

MelSumner · 2022-03-30T17:43:07Z

[question] does design need to sign off on this decision?

didoo · 2022-03-30T18:42:21Z

[question] does design need to sign off on this decision?

I prefer to have a second eye on this (expecially considered that @heatherlarsen has a good eye for details, and background as front-end developer too) :)

heatherlarsen · 2022-03-30T21:02:36Z

Was doing a bit of research on this as the list is getting pretty lengthy and wanted to share an article I found about GitHub's explorations with font stacks. This article is a bit dated, but it's still linked on the Primer website, so seems relevant. I specifically wanted to note that GitHub originally started by including Roboto, Ubuntu, Cantarell, "Fira Sans", "Droid Sans", but later removed them because of feedback they received from Linux users: "Linux system fonts seem to be rather large and clunky, and their system fonts so inconsistent across distros"

Thankfully we're not including that full list, but what do you think about removing Roboto, Ubuntu from the list in an effort to avoid the same issues? Or is this something we want to take away as a potential research topic for the future?

Also wondering if we need to specifically call out "SF Pro Display" and "SF Pro Text"? I may be misinterpreting this, but it seems like we may be able to just rely on -apple-system, BlinkMacSystemFont: Apple Typography Guidelines

- “SF Pro Display/Text” was useless, `-apple-system, BlinkMacSystemFont` already do the job - “Ubuntu” and “Roboto” can cause issues (see this https://markdotto.com/2018/02/07/github-system-fonts/)

didoo · 2022-03-31T09:36:23Z

@heatherlarsen I have updated the font stack according to your suggestions, they all make sense. As it is now, our font stack will be almost identical to the one in GitHub, so we should be confidently safe enough :)

ashleemboyer

This looks good to me! ✨ It'll be awesome to use these in DevDot. 🎉

didoo · 2022-03-31T15:54:15Z

@heatherlarsen if you can have a last look and give me thumbs up, I can merge and release these changes

heatherlarsen · 2022-03-31T15:58:39Z

packages/tokens/dist/devdot/css/tokens.css

-  --token-typography-font-stack-text: -apple-system, BlinkMacSystemFont, SF Pro Text, Segoe UI Text, Ubuntu, sans-serif;
-  --token-typography-font-stack-code: SF Mono, Consolas, Ubuntu Mono, monospace;
+  --token-typography-font-stack-display: -apple-system, BlinkMacSystemFont, Segoe UI, Helvetica, Arial, sans-serif, Apple Color Emoji, Segoe UI Emoji, Segoe UI Symbol;
+  --token-typography-font-stack-text: -apple-system, BlinkMacSystemFont, Segoe UI, Helvetica, Arial, sans-serif, Apple Color Emoji, Segoe UI Emoji, Segoe UI Symbol;


[question] --token-typography-font-stack-display and --token-typography-font-stack-text are now the same. Are we able to simplify them down to the same token or no?

I would prefer not to, if that's OK for you. Reason being that 1) would require some refactoring on our side (not big deal) but potentially from our consumers (not great) but more importantly that 2) if in the future we want to differentiate for some reasons the two, having two separate tokens will make it seamless (we just release a new version of the package and that's it) while if we have a single token will require to manually check all the use cases where it's used and decide if it's a text or a display (maybe it's easy, maybe it's a nightmare).

The cost of maintaining both is zero now (there's not even performance problems). The potential cost of unifying them is unknown (and in the worst case could be pretty high).

Makes sense, thanks for clarifying.

heatherlarsen

lgtm!

didoo added 3 commits March 30, 2022 10:58

updated font-stack design tokens in “typography”

814235b

- replaced `Segoe UI Display/Text` with `Segoe UI` - added `Roboto, Helvetica, Arial, sans-serif` to the `sans` block - added explicit emoji support for `Display/Text` - added `Menlo` to `Code`

re-generated design tokens in output

13bc954

changeset

c17f142

didoo requested review from MelSumner, heatherlarsen and ashleemboyer March 30, 2022 10:06

didoo mentioned this pull request Mar 30, 2022

Synchronize CSS font-stack between DevDot and the design system #128

Closed

didoo added 2 commits March 30, 2022 11:45

re-generated design tokens in output

e645068

vercel bot temporarily deployed to Preview – hds-flight-website March 30, 2022 10:45 Inactive

vercel bot temporarily deployed to Preview – hds-components March 30, 2022 10:45 Inactive

updated changeset

8684efc

vercel bot temporarily deployed to Preview – hds-components March 30, 2022 10:56 Inactive

vercel bot temporarily deployed to Preview – hds-flight-website March 30, 2022 10:56 Inactive

changeset to bump components as minor (not just patch)

bce712a

vercel bot temporarily deployed to Preview – hds-flight-website March 30, 2022 11:45 Inactive

vercel bot temporarily deployed to Preview – hds-components March 30, 2022 11:45 Inactive

Merge branch 'main' into 128-update-design-tokens-font-stack

a38ab1e

vercel bot temporarily deployed to Preview – hds-flight-website March 31, 2022 09:09 Inactive

vercel bot temporarily deployed to Preview – hds-components March 31, 2022 09:09 Inactive

didoo added 3 commits March 31, 2022 10:28

re-generated tokens in output

6bb6c27

updated changeset

87e070e

vercel bot temporarily deployed to Preview – hds-flight-website March 31, 2022 09:30 Inactive

vercel bot temporarily deployed to Preview – hds-components March 31, 2022 09:30 Inactive

ashleemboyer approved these changes Mar 31, 2022

View reviewed changes

heatherlarsen reviewed Mar 31, 2022

View reviewed changes

heatherlarsen approved these changes Mar 31, 2022

View reviewed changes

Merge branch 'main' into 128-update-design-tokens-font-stack

e8a860c

vercel bot temporarily deployed to Preview – hds-flight-website April 1, 2022 09:02 Inactive

vercel bot temporarily deployed to Preview – hds-components April 1, 2022 09:02 Inactive

didoo merged commit 2da440f into main Apr 1, 2022

didoo deleted the 128-update-design-tokens-font-stack branch April 1, 2022 09:22

github-actions bot mentioned this pull request Apr 1, 2022

Version Packages #157

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update font-stack in design tokens #136

Update font-stack in design tokens #136

didoo commented Mar 30, 2022 •

edited

Loading

vercel bot commented Mar 30, 2022 •

edited

Loading

changeset-bot bot commented Mar 30, 2022 •

edited

Loading

MelSumner commented Mar 30, 2022

didoo commented Mar 30, 2022

heatherlarsen commented Mar 30, 2022

didoo commented Mar 31, 2022

ashleemboyer left a comment

didoo commented Mar 31, 2022

heatherlarsen Mar 31, 2022 •

edited

Loading

didoo Mar 31, 2022 •

edited

Loading

heatherlarsen Mar 31, 2022

heatherlarsen left a comment

Update font-stack in design tokens #136

Update font-stack in design tokens #136

Conversation

didoo commented Mar 30, 2022 • edited Loading

📌 Summary

🛠️ Detailed description

👀 How to review

🚨 Percy visual regression tests failing

vercel bot commented Mar 30, 2022 • edited Loading

hds-flight-website – ./

hds-components – ./

changeset-bot bot commented Mar 30, 2022 • edited Loading

🦋 Changeset detected

MelSumner commented Mar 30, 2022

didoo commented Mar 30, 2022

heatherlarsen commented Mar 30, 2022

didoo commented Mar 31, 2022

ashleemboyer left a comment

Choose a reason for hiding this comment

didoo commented Mar 31, 2022

heatherlarsen Mar 31, 2022 • edited Loading

Choose a reason for hiding this comment

didoo Mar 31, 2022 • edited Loading

Choose a reason for hiding this comment

heatherlarsen Mar 31, 2022

Choose a reason for hiding this comment

heatherlarsen left a comment

Choose a reason for hiding this comment

didoo commented Mar 30, 2022 •

edited

Loading

vercel bot commented Mar 30, 2022 •

edited

Loading

changeset-bot bot commented Mar 30, 2022 •

edited

Loading

heatherlarsen Mar 31, 2022 •

edited

Loading

didoo Mar 31, 2022 •

edited

Loading