Update Skyscanner Relative and Larken family#490
Update Skyscanner Relative and Larken family#490Xiaoguang Liu (xiaogliu) merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reverts the font fallback updates previously introduced for FONT_FAMILY_BASE and FONT_FAMILY_LARKEN in bpk-foundations-web, restoring the intended Noto Serif/Noto Sans fallback stacks while keeping the simplified Chinese ordering change mentioned in the PR description.
Changes:
- Restores
FONT_FAMILY_BASEto useNoto Serif/Noto Serif Devanagari/Noto Serif Thaifallbacks (instead of the Noto Sans variants). - Restores
FONT_FAMILY_LARKENto useNoto Sans HebrewandNoto SansCJK fallbacks (instead of the Noto Serif variants). - Updates the generated token outputs (
.scss,.js,.d.ts,.raw.json) to match the source typography definition.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/bpk-foundations-web/src/base/typography.json | Updates the source token definitions for FONT_FAMILY_BASE and FONT_FAMILY_LARKEN. |
| packages/bpk-foundations-web/tokens/base.raw.json | Regenerated raw token output to reflect restored font stacks. |
| packages/bpk-foundations-web/tokens/base.scss | Regenerated SCSS tokens to reflect restored font stacks. |
| packages/bpk-foundations-web/tokens/base.default.scss | Regenerated default SCSS tokens to reflect restored font stacks. |
| packages/bpk-foundations-web/tokens/base.common.js | Regenerated CommonJS token output to reflect restored font stacks. |
| packages/bpk-foundations-web/tokens/base.es6.js | Regenerated ES module token output to reflect restored font stacks. |
| packages/bpk-foundations-web/tokens/base.es6.d.ts | Regenerated TypeScript declarations to reflect restored font stacks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove all Noto font fallbacks from FONT_FAMILY_BASE and FONT_FAMILY_LARKEN. Non-Latin fonts will be loaded per-locale via [lang] CSS overrides in backpack instead of globally. This reverts changes from PRs #469 and #485 that added Noto fonts to the global font-family stack, which caused all pages to download large CJK/non-Latin fonts and spiked CloudFront costs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Xiaoguang Liu (xiaogliu)
force-pushed
the
xiaogliu/revert-pr-485
branch
from
87558de to
cb8d7ad
Compare
| "props": { | ||
| "FONT_FAMILY_BASE": { | ||
| "value": "'Skyscanner Relative', 'Noto Sans Arabic', 'Noto Sans Hebrew', 'Noto Sans', 'Noto Sans Devanagari', 'Noto Sans Thai', 'Noto Sans TC', 'Noto Sans JP', 'Noto Sans KR', 'Noto Sans SC', -apple-system, BlinkMacSystemFont, 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', 'Fira Sans', 'Droid Sans', 'Helvetica Neue', sans-serif", | ||
| "value": "'Skyscanner Relative', -apple-system, BlinkMacSystemFont, 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', 'Fira Sans', 'Droid Sans', 'Helvetica Neue', sans-serif", |
There was a problem hiding this comment.
I think this fonts update (backpack-foundation version bump) can be added to another pr updating font-stack in Backpack. What do you think?
There was a problem hiding this comment.
It's better we can do that in same PR 👍 After we merge this PR, I'll update updating font-stack in Backpack
| }, | ||
| "FONT_FAMILY_LARKEN": { | ||
| "value": "'Larken', 'Noto Sans Arabic', 'Noto Serif Hebrew', 'Noto Serif', 'Noto Serif Devanagari', 'Noto Serif Thai', 'Noto Serif TC', 'Noto Serif JP', 'Noto Serif KR', 'Noto Serif SC', sans-serif", | ||
| "value": "'Larken', sans-serif", |
There was a problem hiding this comment.
Should we add more system fonts here? like the base-fonts value above?
There was a problem hiding this comment.
No, we don't need to add system fonts here. FONT_FAMILY_LARKEN is only used for hero/display headings — the generic sans-serif keyword is sufficient as a fallback since it resolves to the platform's default sans-serif font anyway.
FONT_FAMILY_BASE keeps system fonts (-apple-system, Roboto, etc.) because it's the body font used site-wide — we need more precise control over the fallback chain when Skyscanner Relative fails to load.
Per-locale Noto font loading for both BASE and LARKEN will be handled via [lang] CSS overrides in backpack#4343 instead of being in the global font-family stack.
| lineHeightXl: "2rem", | ||
| fontSizeXl: "1.5rem", | ||
| fontFamilyLarken: "'Larken', 'Noto Sans Arabic', 'Noto Serif Hebrew', 'Noto Serif', 'Noto Serif Devanagari', 'Noto Serif Thai', 'Noto Serif TC', 'Noto Serif JP', 'Noto Serif KR', 'Noto Serif SC', sans-serif", | ||
| fontFamilyLarken: "'Larken', sans-serif", |
There was a problem hiding this comment.
Why do we change the Larken font? As base.css is built from index.scss, which only use tokens.$bpk-font-family-base.
There was a problem hiding this comment.
Good question! While base.css only uses FONT_FAMILY_BASE, the FONT_FAMILY_LARKEN token is consumed by backpack's larken.scss for hero/display headings. The Noto fonts in LARKEN were also causing unnecessary font downloads on every page, same as BASE — so we're removing them here and handling per-locale Noto loading via [lang] CSS overrides in backpack instead.
Xiaoguang Liu (xiaogliu)
changed the title
Xiaoguang Liu (xiaogliu)
changed the title
Xiaoguang Liu (xiaogliu)
changed the title
Summary
Noto Serif/Noto Serif Devanagari/Noto Serif Thaifallbacks forFONT_FAMILY_BASE(was changed to Noto Sans variants in [Clov-1075]Update Skyscanner Relative and Larken family #485)Noto Sans Hebrew/Noto SansCJK fallbacks forFONT_FAMILY_LARKEN(was changed to Noto Serif variants in [Clov-1075]Update Skyscanner Relative and Larken family #485)Why
Unable to use GitHub's built-in revert due to time elapsed since merge causing conflicts.
Test plan
🤖 Generated with Claude Code