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.

Sign up for GitHub

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

Jump to bottom

Add support for custom fonts #2981

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Add support for custom fonts #2981

wants to merge 13 commits into from
+836 −248

Conversation

valentin0h
Copy link
Contributor

@valentin0h valentin0h commented Mar 14, 2025 •
edited
Loading

Adds support for custom fonts in GitBook by dynamically generating @font-face rules from user-defined font configurations and preloading the key font sources. It integrates these custom fonts into the layout and OG image generation

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Mar 14, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 58996a4

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

This PR includes changesets to release 2 packages
Name Type
gitbook-v2 Patch
gitbook Patch

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 14, 2025 •
edited
Loading

Summary of the deployments:

Version 1 (production)

Version URL Status
Latest commit https://0a0de84c.gitbook-open.pages.dev ✅
PR https://pr2981.gitbook-open.pages.dev ✅

Version 2 (experimental)

Version URL Status
Vercel https://gitbook-v2-978e9t4pj-gitbook.vercel.app ✅
Cloudflare https://5704061a-gitbook-open-v2-preview.gitbook.workers.dev ✅

Test content

Site v1 v2
GitBook https://0a0de84c.gitbook-open.pages.dev/docs.gitbook.com https://gitbook-v2-978e9t4pj-gitbook.vercel.app/url/docs.gitbook.com
E2E https://0a0de84c.gitbook-open.pages.dev/gitbook.gitbook.io/test-gitbook-open https://gitbook-v2-978e9t4pj-gitbook.vercel.app/url/gitbook.gitbook.io/test-gitbook-open

@argos-ci Argos CI
Copy link

argos-ci bot commented Mar 14, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
customers-v1 (Inspect) 👍 Changes approved 12 changed Mar 18, 2025, 12:01 PM
customers-v2 (Inspect) 👍 Changes approved 14 changed Mar 18, 2025, 12:03 PM
default (Inspect) 👍 Changes approved 24 changed Mar 18, 2025, 12:04 PM
v2-vercel (Inspect) 👍 Changes approved 23 changed Mar 18, 2025, 12:06 PM

@valentin0h valentin0h force-pushed the custom-fonts branch 3 times, most recently from 736c95e to ea6f29d Compare March 17, 2025 09:37
emmerich
emmerich reviewed Mar 18, 2025
View reviewed changes
Copy link
Contributor

@emmerich emmerich left a comment

Choose a reason for hiding this comment

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

Overall looks good, I'm not sure about the use of ReactDOM.preload in SSR though

packages/gitbook/src/components/RootLayout/CustomizationRootLayout.tsx

// Preconnect and preload custom fonts if needed
if (fontData.type === 'custom') {
ReactDOM.preconnect(GITBOOK_FONTS_URL);
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this work on the server? Preloading in Nextjs seems to have different recommendations: https://nextjs.org/docs/app/building-your-application/data-fetching/fetching#preloading-data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. The next preload mechanism appears to be more geared toward preloading next data rather than external resources like fonts. I haven't seen any examples in the wild using this pattern to preload custom fonts.

packages/gitbook/src/fonts/custom.ts Outdated
})
.join(', ');

// We could use the font-family name here, but to avoid extra normalization we're using 'CustomFont'
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I wonder if this will have a wider impact as the underlying font data might have a different name. Should be fine for now though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I deliberately hardcoded the font family to “CustomFont” to avoid the complexity of normalizing the user input. I'm not sure it matters about the underlying font data names, as we always reference the custom font via our standardized CSS variable and class. Unless I'm missing something, lemme know.

@valentin0h valentin0h marked this pull request as ready for review March 18, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SamyPesse SamyPesse Awaiting requested review from SamyPesse

@emmerich emmerich Awaiting requested review from emmerich

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@valentin0h @emmerich @SamyPesse