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 workaround for font-optical-sizing issue in Safari 16 #11503

Merged
merged 3 commits into from
Jan 26, 2024

Conversation

aaronccasanova
Copy link
Member

@aaronccasanova aaronccasanova commented Jan 24, 2024

Fixes https://github.com/Shopify/polaris-internal/issues/1395

This PR adds a workaround for the font-optical-sizing issue in Safari 16

It's important to note this solution is NOT the best merchant experience we can provide, but resolves the immediate issue of thin font-weights rendering across the Admin. I suggest we follow up this PR with an exploration to matchopsz values to each Text variant font-size as described by the Google Fonts Glossary:

the numeric value for this axis should match the rendered font size in typographic points (1/72nd of an inch) in print, although browsers instead match it to the CSS px unit, since they have no concept of physical size

Before After
Screenshot 2024-01-25 at 3 56 16 PM Screenshot 2024-01-25 at 3 55 27 PM

Screenshot of the Polaris-Safari-16-Remove-Font-Optical-Sizing class applying in the iOS 16.2 simulator:

Screenshot 2024-01-25 at 4 17 35 PM

@aaronccasanova
Copy link
Member Author

/snapit

Copy link
Contributor

🫰✨ Thanks @aaronccasanova! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

!navigator.userAgent.includes('Chrome') &&
(navigator.userAgent.includes('Version/16.1') ||
navigator.userAgent.includes('Version/16.2') ||
navigator.userAgent.includes('Version/16.3'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Assume this is fixed in Safari 16.4 and above? If so, this is fine with me for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, to my understanding 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Reference links:

Blog post

Update: as of Safari 16.4, this bug has been fixed.

WebKit PR

also means we are going
fast enough that we can enable optical sizing on every font - because doing so is just an
attribute in a dictionary, rather than creating a whole new CTFont. That is the fix for this
bug: enabling optical sizing on all fonts.

@lgriffee lgriffee added the #gsd:38846 Admin Quality Improvements (Q1 2024) label Jan 26, 2024
@aaronccasanova aaronccasanova merged commit fb7d968 into main Jan 26, 2024
12 checks passed
@aaronccasanova aaronccasanova deleted the fix-font-optical-sizing branch January 26, 2024 19:38
const isSafari16 =
navigator.userAgent.includes('Safari') &&
!navigator.userAgent.includes('Chrome') &&
(navigator.userAgent.includes('Version/16.1') ||
Copy link
Member

Choose a reason for hiding this comment

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

Is there not a v16.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue started in v16.1

aaronccasanova pushed a commit that referenced this pull request Jan 26, 2024
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/[email protected]

### Patch Changes

- [#11499](#11499)
[`a6ac9928a`](a6ac992)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed layout and focus
styling for `Button` inside `ButtonGroup` with `fullWidth`


- [#11503](#11503)
[`fb7d96821`](fb7d968)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Added
workaround for `font-optical-sizing` issue in Safari 16

## [email protected]

### Patch Changes

- Updated dependencies
\[[`a6ac9928a`](a6ac992),
[`fb7d96821`](fb7d968)]:
    -   @shopify/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@alex-page
Copy link
Member

Can we document an issue somewhere to remove this temporary fix? My gut feeling is we could probably remove this fix when 16.16 releases and there are two stable versions with the fix.

@lgriffee
Copy link
Member

@alex-page Added an issue for it: https://github.com/Shopify/polaris-internal/issues/1394

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
#gsd:38846 Admin Quality Improvements (Q1 2024)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants