Update Inter font and Playwright version #2026
Conversation
|
Storybook and Tailwind configuration previews: Ready Storybook: https://wordpress.github.io/openverse-frontend/_preview/2026 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
openverse-bot
added
✨ goal: improvement
|
Size Change: -7.65 kB (-1%) Total Size: 859 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Love variable fonts so this is great. I am a bit sad that this requires literally every single snapshot to be recreated.
I'm just curious though, why did the Playwright upgrade need a font upgrade too?
Approving, provided visual regression tests eventually pass.
src/assets/fonts.css
Outdated
| @font-face { | ||
| font-family: 'JetBrains Mono'; | ||
| font-style: normal; | ||
| font-weight: 400; | ||
| font-display: swap; | ||
| src: local('JetBrainsMono-Regular'), local('JetBrains Mono Regular'), | ||
| url('~assets/fonts/JetBrainsMono-Regular.woff2') format('woff2'), | ||
| url('~assets/fonts/JetBrainsMono-Regular.woff') format('woff'), | ||
| url('~assets/fonts/JetBrainsMono-Regular.otf') format('opentype'); |
There was a problem hiding this comment.
Are we not using the monospaced font anymore? It was being used in the advanced search query page iirc. It's still listed in the Tailwind config as mono so I think this was accidental.
There was a problem hiding this comment.
We should bring back the monospaced font, so probably it was a mistake.
| mono: ['"JetBrains Mono"', 'monospace'], | ||
| sans: ['"Inter"', 'sans-serif'], | ||
| serif: ['Times New Roman'], |
There was a problem hiding this comment.
We could potentially let serif go, I don't know if we use it anywhere in the app.
| "actual-title": "\"{لقب}\"", | ||
| "actual-title": "\"{title}\"", |
It's actually the other way around: we needed to fix fonts to update Playwright. You can see what problems we had with RTL font rendering in the Docker container in the linked issue's snapshots. We could try extracting out the Playwright changes from this PR, but I'm afraid that changes in Linux image version will change the font rendering once again, and make us re-write all the snapshots one more time. |
You mean the version before this PR, right? I checked out Safari, and I can see that it's using If you check the fonts tab on Safari with this PR, you'll see that it's loading the Inter font correctly. |
Yes! Sorry for not being clear. The PR looks correct. |
|
Playwright Failure Test Results It looks like some of the Playwright tests failed. You can download the Playwright trace https://github.com/WordPress/openverse-frontend/actions/runs/3632516952 Read more about how to use this artifact here: https://github.com/WordPress/openverse-frontend/blob/main/test/playwright/README.md#debugging |
2 similar comments
|
Playwright Failure Test Results It looks like some of the Playwright tests failed. You can download the Playwright trace https://github.com/WordPress/openverse-frontend/actions/runs/3632516952 Read more about how to use this artifact here: https://github.com/WordPress/openverse-frontend/blob/main/test/playwright/README.md#debugging |
|
Playwright Failure Test Results It looks like some of the Playwright tests failed. You can download the Playwright trace https://github.com/WordPress/openverse-frontend/actions/runs/3632516952 Read more about how to use this artifact here: https://github.com/WordPress/openverse-frontend/blob/main/test/playwright/README.md#debugging |
Fixes
Fixes #1781 by @obulat
Fixes #1754 by @obulat
Description
These two changes seem to be unrelated at first, but updating Playwright version required updating the fonts to fix the snapshots.
This PR adds a variable Inter font for platforms that support variable fonts, and more modern
woffandwoff2font files instead of thettffont files. It also updates the Playwright version to the latest 1.28.1, and uses ajammyDocker image (replacing the olderfocalimage).@panchovm, could you check if the fonts look right by running the app locally? The snapshots' fonts look different than what I see on my Mac, or on a Linux machine. Here, the first snapshot is from the current implementation, and the second one is what I see with this PR's changes. The letters look less squished together and more rounded.
Several interesting things about this PR:400font, instead we only had the500weighted one. I don't know how it worked.Testing Instructions
Run the app and check that fonts match the Figma snapshots.
The CI should also pass, and specifically RTL snapshots should not have any
NULLcharacters or breakages.Checklist
Update index.md).main) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin