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

[polaris.shopify.com] Load correct Inter variable font weights #10895

Merged
merged 7 commits into from
Oct 5, 2023

Conversation

tjonx
Copy link
Contributor

@tjonx tjonx commented Oct 5, 2023

WHY are these changes introduced?

Fixes #10894

WHAT is this pull request doing?

Load the correct font weights for regular 400 450 and semibold 600 650 tokens.

Updates migration guide and any other documentation (like the readme) where the Inter font load string is present to include the correct weights, and removes unnecessary weights (100, 200, 300 and 800, 900)

Before:
image

After:
image

🎩 checklist

@tjonx tjonx requested review from a team as code owners October 5, 2023 14:33
@tjonx tjonx changed the base branch from main to next October 5, 2023 14:33
@tjonx tjonx self-assigned this Oct 5, 2023
@tjonx tjonx changed the title Tjonx/load inter variable font [polaris.shopify.com] Load correct Inter variable font weights Oct 5, 2023
@tjonx tjonx added 🤖Skip Changelog Causes CI to ignore changelog update check. #gsd:36550 labels Oct 5, 2023
@tjonx tjonx linked an issue Oct 5, 2023 that may be closed by this pull request
@@ -6,6 +6,6 @@
/>
<link
id="inter-font-link"
href="https://fonts.googleapis.com/css2?family=Inter:wght@100;200;300;400;500;600;700;800;900&display=swap"
href="https://fonts.googleapis.com/css2?family=Inter:wght@100;200;300;400;450;500;600;650;700;800;900&display=swap"
Copy link
Contributor

Choose a reason for hiding this comment

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

I saw you removed 100, 200, 300, 800, 900 in other places, can they be removed here too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at our tokens it seems like we can 450, 550, 650, 700

https://github.com/Shopify/polaris/blob/next/polaris-tokens/src/themes/base/font.ts#L96-L107

@tjonx tjonx merged commit 7f1c924 into next Oct 5, 2023
12 checks passed
@tjonx tjonx deleted the tjonx/load-inter-variable-font branch October 5, 2023 16:08
mrcthms pushed a commit that referenced this pull request Oct 12, 2023
<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes #10894


<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

Load the correct font weights for regular ~`400`~ `450` and semibold
~`600`~ `650` tokens.

Updates migration guide and any other documentation (like the readme)
where the Inter font load string is present to include the correct
weights, and removes unnecessary weights (`100`, `200`, `300` and `800`,
`900`)

Before:

![image](https://github.com/Shopify/polaris/assets/67433661/9755c831-1a1b-4ca0-883b-c225ef4f1161)


After:

![image](https://github.com/Shopify/polaris/assets/67433661/045edf8b-cdbe-4c6c-97f7-f47b6182c705)


### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
mrcthms pushed a commit that referenced this pull request Oct 12, 2023
<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes #10894


<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

Load the correct font weights for regular ~`400`~ `450` and semibold
~`600`~ `650` tokens.

Updates migration guide and any other documentation (like the readme)
where the Inter font load string is present to include the correct
weights, and removes unnecessary weights (`100`, `200`, `300` and `800`,
`900`)

Before:

![image](https://github.com/Shopify/polaris/assets/67433661/9755c831-1a1b-4ca0-883b-c225ef4f1161)


After:

![image](https://github.com/Shopify/polaris/assets/67433661/045edf8b-cdbe-4c6c-97f7-f47b6182c705)


### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
…fy#10895)

<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes Shopify#10894


<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

Load the correct font weights for regular ~`400`~ `450` and semibold
~`600`~ `650` tokens.

Updates migration guide and any other documentation (like the readme)
where the Inter font load string is present to include the correct
weights, and removes unnecessary weights (`100`, `200`, `300` and `800`,
`900`)

Before:

![image](https://github.com/Shopify/polaris/assets/67433661/9755c831-1a1b-4ca0-883b-c225ef4f1161)


After:

![image](https://github.com/Shopify/polaris/assets/67433661/045edf8b-cdbe-4c6c-97f7-f47b6182c705)


### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖Skip Changelog Causes CI to ignore changelog update check.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[polaris.shopify.com] Inter needs to be loaded as a variable weight font
3 participants