chore: implement expo font #23517
chore: implement expo font #23517
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
metamaskbot
added
the
team-design-system
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring alerts on: |
georgewrmarshall
force-pushed
the
fix-expo-font
branch
from
d13581f to
31fea79
Compare
There was a problem hiding this comment.
Ideally expo-font handles copying the font files to the android/ folder as apart of the pre-build process but because we aren't using expo to build the ios/ and android/ folders yet we have to manually add these as per the docs
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
| export const fontStyles: Record<string, TextStyle> = { | ||
| normal: { | ||
| fontFamily: 'Geist Regular', | ||
| fontFamily: 'Geist-Regular', |
There was a problem hiding this comment.
Updating fontStyles to match Post Script font family name
| }, | ||
| optionsContainer: { flexDirection: 'row', marginTop: 14 }, | ||
| option: { alignItems: 'center', paddingHorizontal: 14 }, | ||
| optionIcon: { fontSize: 24 }, |
There was a problem hiding this comment.
Will create other PRs to reduce all static font-family definitions
| import NavigationProvider from '../../Nav/NavigationProvider'; | ||
| import ControllersGate from '../../Nav/ControllersGate'; | ||
| import { isTest } from '../../../util/test/utils'; | ||
| import FontLoadingGate from './FontLoadingGate'; |
There was a problem hiding this comment.
Removing custom font preloader from Root. Will need to completely remove in a separate PR
| />, | ||
| ); | ||
|
|
||
| const textElement = getByText(/ETH/); |
There was a problem hiding this comment.
Should be refactored to not use static font family name
There was a problem hiding this comment.
As per the expo-font docs renaming font files that match the PostScript name
There was a problem hiding this comment.
PostScript name in Font Book
georgewrmarshall
force-pushed
the
fix-expo-font
branch
from
892d1e2 to
e1a8922
Compare
| const italicSuffix = resolvedStyle === 'italic' ? ' Italic' : ''; | ||
| const italicSuffix = resolvedStyle === 'italic' ? 'Italic' : ''; | ||
|
|
||
| return `Geist ${fontSuffix}${italicSuffix}`; | ||
| return `Geist-${fontSuffix}${italicSuffix}`; |
There was a problem hiding this comment.
Removing spaces and adding dash - to match font file renaming and postscript names
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
There was a problem hiding this comment.
PostScript name in Font Book
georgewrmarshall
dismissed stale reviews from brianacnguyen and Cal-L
via
afe4c04
georgewrmarshall
force-pushed
the
fix-expo-font
branch
2 times, most recently
from
afe4c04 to
252008b
Compare
…stscript names and library updates
CI environment generates Podfile.lock checksums that match main branch. Using main's Podfile.lock to avoid CI 'working tree dirty' errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
georgewrmarshall
dismissed stale reviews from Cal-L and brianacnguyen
via
fc68231
georgewrmarshall
force-pushed
the
fix-expo-font
branch
from
252008b to
fc68231
Compare
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsThis PR implements a font naming convention change from space-separated ("Geist Regular") to hyphen-separated ("Geist-Regular") format across the entire application. Key changes:
Risk assessment:
Selected tags rationale:
Not all tags needed because font issues would manifest similarly across all areas - if core functionality renders correctly with proper fonts, other areas will too. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #23517 +/- ##
==========================================
+ Coverage 78.73% 78.81% +0.08%
==========================================
Files 4042 4051 +9
Lines 105606 106197 +591
Branches 21267 21454 +187
==========================================
+ Hits 83144 83699 +555
+ Misses 16637 16615 -22
- Partials 5825 5883 +58 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
metamaskbot
added
the
release-7.62.0
See slack thread for test builds of this PR
Description
This PR implements
expo-fontto improve font rendering and maintainability of fonts in MetaMask Mobile. The change provides several benefits:(from docs)
Key Changes
Review and Merge Strategy
Snapshot Management Approach:
This PR updates font file names and font family definitions, which will trigger a large number of snapshot updates. To reduce the risk of the PR becoming outdated, I've adopted the following strategy:
Separated Concerns
All fixes owned by teams outside of
@metamask-design-system-teamand@mobile-platformhave been moved to a separate PR: fix: update MM Sans and MM Poly fonts to use PostScript names #23650Combined Build for Testing
A build that includes changes from both PRs is available in this Slack thread for end-to-end testing.
Phased Review Process
Benefits of This Approach:
Remaining Work:
Changelog
CHANGELOG entry: Fixes intermittent font rendering issues on first load of the app or import of SRP
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/MDP-603
Manual testing steps
Screenshots/Recordings
Before
No before recordings, I can't seem to reproduce the font loading issues but they exist see slack thread
After
All fonts rendering as expected. No incorrect font weights, system fonts (incorrect font family), or cut-off text. Below screen recordings and screenshots of this PR and fast follow PR combined in build
iOS
ios.after.mov
Key screens:
@metamask/design-system-react-nativeworkingAndroid
expo.font.android.build.mov
Key screens:
@metamask/design-system-react-nativeworkingPre-merge author checklist
Pre-merge reviewer checklist
Note
Integrates Expo Font and standardizes font family names, updating snapshots across the app.
expo-fontconfiguration inapp.config.jsto bundle/load fonts at startup.Geist Regular→Geist-Regular).fontFamilyvalues; no functional UI changes.Written by Cursor Bugbot for commit fc68231. This will update automatically on new commits. Configure here.