Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: preload inter font#19455

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
BrunoQuaresma merged 2 commits intomainfrombq/preload-inter-font
Aug 25, 2025
Merged

chore: preload inter font#19455

BrunoQuaresma merged 2 commits intomainfrombq/preload-inter-font
Aug 25, 2025

Conversation

BrunoQuaresma
Copy link
Collaborator

This aims to solve font rendering issues in Storybook like the inconsistent snapshot below.

Inconsistent snapshot:
image

References:

@BrunoQuaresmaBrunoQuaresma requested a review froma teamAugust 20, 2025 19:18
@BrunoQuaresmaBrunoQuaresma self-assigned thisAug 20, 2025
@BrunoQuaresmaBrunoQuaresma requested review fromjaaydenh and removed request fora teamAugust 20, 2025 19:18
@blink-soblink.so
Copy link
Contributor

High-level: Preloading Inter via a bundler URL + Helmet looks good for the main app, and the ambient module declaration for*.woff2?url is the right way to satisfy TS. The Navbar test changes improve isolation and avoid mounting the entire app.

Main gap: Storybook doesn’t renderApp, so the<Helmet><link rel="preload" ... /></Helmet> inApp.tsx won’t run there. Since the PR’s stated goal is stabilizing Storybook/Chromatic snapshots, we should also preload the font in the Storybook environment.

Suggested fix (uses existing HelmetProvider in preview):

// site/.storybook/preview.tsximport{Helmet}from"react-helmet-async";importinterWoff2from"@fontsource-variable/inter/files/inter-latin-wght-normal.woff2?url";constwithHelmet:Decorator=(Story)=>{return(<HelmetProvider><Helmet><linkrel="preload"as="font"type="font/woff2"href={interWoff2}crossOrigin="anonymous"/></Helmet><Story/></HelmetProvider>);};

Alternatively, add apreview-head.html with a<link rel="preload">, but using?url import in code avoids asset-path issues with Vite.

Nice-to-have:

  • If we rely on italics anywhere, consider also preloading the italic variable file.
  • Keepdocument.fonts.ready loader (already present) to ensure Chromatic waits for fonts.

If you add the Storybook preload, I’m confident this will address the snapshot variability. Happy to re-review after that change.

@jaaydenh
Copy link
Contributor

@BrunoQuaresma Should the link tag be added to .storybook/preview.tsx instead of App.tsx?

@BrunoQuaresma
Copy link
CollaboratorAuthor

I think it would be useful to preload the Inter font in the app too. Wdyt?

@jaaydenh
Copy link
Contributor

or actually, maybe have both preloads in App and preview.tsx

@BrunoQuaresma
Copy link
CollaboratorAuthor

Good catch! I we don't use the App in Storybook 🤦

Copy link
Member

@aslilacaslilac left a comment

Choose a reason for hiding this comment

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

I'm not convinced this actually does anything. for react-helmet to add this<link> to the head...

  • all of the javascript needs to be loaded
  • javascript needs to be parsed and executed
  • the virtual dom needs to be constructed
  • the reconciler needs to apply that virtual dom to the document
  • by this point I think the browser has probably already started loading the styles, because other elements have been applied into the dom which require its presence.

these storybook docs seem more promising to me:https://storybook.js.org/docs/configure/integration/images-and-assets#referencing-fonts-in-stories

@aslilac
Copy link
Member

also, about the import types: vite provides those types, we just haven't ever enabled them for some reason. 🙃 pr to fix:#19477

@BrunoQuaresma
Copy link
CollaboratorAuthor

@aslilac that makes sense. About thedocs you mentioned, I don't see a clear way to makefontsource to work nicely with it. A workaround that came up to my mind was to just copy and paste the font file in the public folder and use it. Wdyt?

@BrunoQuaresma
Copy link
CollaboratorAuthor

Thinking more about that, I don't think it would work since the file location would be in different places.

@BrunoQuaresma
Copy link
CollaboratorAuthor

I think I got it working. I didn't know vite was smart enough to figure out how to bundle static assets from node_modules :)

@aslilac
Copy link
Member

I think you can even leave off the/node_modules/ bit and just "import it" like you would from typescript

vite is pretty clever 😄

@BrunoQuaresma
Copy link
CollaboratorAuthor

@aslilac I tried, but it does not 😞

Copy link
Member

@aslilacaslilac left a comment

Choose a reason for hiding this comment

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

thanks bruno!

one last thought before you merge this tho, would it be worth adding a preload for the monospace font we use in the terminal stories? I think flickering has been a big problem there in the past. but in terms of inter specifically, this looks good.

BrunoQuaresma reacted with thumbs up emoji
@BrunoQuaresmaBrunoQuaresma merged commite7591aa intomainAug 25, 2025
27 checks passed
@BrunoQuaresmaBrunoQuaresma deleted the bq/preload-inter-font branchAugust 25, 2025 13:41
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsAug 25, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@aslilacaslilacaslilac approved these changes

@jaaydenhjaaydenhAwaiting requested review from jaaydenh

@ParkreinerParkreinerAwaiting requested review from ParkreinerParkreiner is a code owner

Assignees

@BrunoQuaresmaBrunoQuaresma

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@BrunoQuaresma@jaaydenh@aslilac

[8]ページ先頭

©2009-2025 Movatter.jp