-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: update MM Sans and MM Poly fonts to use PostScript names #23650
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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. |
app/components/UI/Bridge/components/TransactionDetails/BridgeStepDescription.test.tsx
Outdated
Show resolved
Hide resolved
| />, | ||
| ); | ||
|
|
||
| const textElement = getByText(/ETH/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to move this out into a separate fix that doesn't use this static font family as an assertion as it will make the main PR fail
…3659) ## **Description** This PR fixes a test that would fail after the expo-font changes in #23517. The `BridgeStepDescription.test.tsx` file had hardcoded font family assertions (e.g., `'Geist Medium'`, `'Geist Regular'`) that will break when font file names change to hyphenated format (e.g., `'Geist-Medium'`, `'Geist-Regular'`). Instead of updating to new hardcoded values, this PR references the centralized `fontStyles` definitions from `app/styles/common.ts`, making the test more maintainable and ensuring a single source of truth for font families. **Changes:** - Import `fontStyles` from `app/styles/common.ts` - Replace `'Geist Medium'` assertions with `fontStyles.medium.fontFamily` - Replace `'Geist Regular'` assertions with `fontStyles.normal.fontFamily` ## **Changelog** CHANGELOG entry: null ## **Related issues** Related to #23517 (main expo-font PR) Related to #23650 (brand font fixes PR) ## **Manual testing steps** ```gherkin Feature: BridgeStepDescription test suite Scenario: developer runs test suite Given the font family definitions have been updated to use hyphens When developer runs yarn jest BridgeStepDescription.test.tsx Then all 22 tests should pass successfully ``` ## **Screenshots/Recordings** N/A - Test-only change ### **Before** Test assertions used hardcoded font family strings that would fail after font renaming. ### **After** Test assertions reference `fontStyles` from `common.ts` and pass with new font names. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Update BridgeStepDescription tests to reference centralized fontStyles instead of hardcoded font family strings. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 4b8f8d2. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude <[email protected]>
See [slack thread](https://consensys.slack.com/archives/C02U025CVU4/p1765308999002049?thread_ts=1764805350.992209&cid=C02U025CVU4) for test builds of this PR ## **Description** This PR implements `expo-font` to improve font rendering and maintainability of fonts in MetaMask Mobile. The change provides several benefits: [(from docs)](https://docs.expo.dev/develop/user-interface/fonts/#with-expo-font-config-plugin) 1. Fonts are available immediately when the app starts on a device. 2. No additional code required to load fonts in a project asynchronously when the app starts. 3. Fonts are consistently available across all devices where the app is installed because they're bundled within the app. ### Key Changes - Added expo-font plugin configuration in app.config.js with all app fonts - Updated font family naming convention from space-separated (e.g., "Geist Regular") to hyphenated (e.g., "Geist-Regular") for better compatibility - Updated snapshots to reflect font family name 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: 1. **Separated Concerns** All fixes owned by teams outside of `@metamask-design-system-team` and `@mobile-platform` have been moved to a separate PR: #23650 2. **Combined Build for Testing** A build that includes changes from both PRs is available in [this Slack thread](https://consensys.slack.com/archives/C02U025CVU4/p1764805350992209) for end-to-end testing. 3. **Phased Review Process** * **Phase 1 (Current):** Seeking approvals from QA, Mobile Platform, and Design System teams on core logic changes *without* snapshot updates * **Phase 2 (After Approval):** Once approvals are received, snapshot updates will be pushed * **Phase 3 (Final Review):** Re-review focused only on snapshot changes **Benefits of This Approach:** * Limits review scope to Mobile Platform and Design System teams * Minimizes risk of merge conflicts and PR churn * PR #23650 remains low-risk, with only minor brand and marketing font changes **Remaining Work:** * [x] Obtain approvals from QA, Mobile Platform, and Design System teams * [x] Push snapshot updates (pending approvals) * [x] Final review and merge ## **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** ```gherkin Feature: Font rendering with expo-font Scenario: user views app with new font system Given the app is running with expo-font enabled When user navigates through various screens Then all fonts should render correctly without font family, font weight or text cut off issues And font weights and styles should be consistent across the app ``` ## **Screenshots/Recordings** ### **Before** No before recordings, I can't seem to reproduce the font loading issues but they exist see [slack thread](https://consensys.slack.com/archives/C02U025CVU4/p1764805763176469?thread_ts=1764805350.992209&cid=C02U025CVU4) ### **After** All fonts rendering as expected. No incorrect font weights, system fonts (incorrect font family), or cut-off text **iOS** https://github.com/user-attachments/assets/d73b1049-b5d4-43e9-9aea-2273e216e549 Key screens: - MM Sans & MM Poly working - Tabs built with `@metamask/design-system-react-native` working <img width="398" height="259" alt="Screenshot 2025-12-10 at 8 41 08 AM" src="https://github.com/user-attachments/assets/d5a11151-19b9-455c-86b7-c69531239054" /> <img width="427" height="291" alt="Screenshot 2025-12-10 at 8 41 14 AM" src="https://github.com/user-attachments/assets/54d64ffe-5410-4c86-b2a7-30ed19b898df" /> <img width="412" height="120" alt="Screenshot 2025-12-10 at 8 41 23 AM" src="https://github.com/user-attachments/assets/56734145-114a-443c-a4de-ed1274cfb2e5" /> **Android** https://github.com/user-attachments/assets/1d14744b-0d2c-4aaa-b18b-9eb5175a6af6 Key screens: - MM Sans & MM Poly working - Tabs built with `@metamask/design-system-react-native` working <img width="442" height="362" alt="Screenshot 2025-12-10 at 8 31 02 AM" src="https://github.com/user-attachments/assets/3ef52f3c-0ae9-46ed-aca8-cab75d3b1666" /> <img width="413" height="183" alt="Screenshot 2025-12-10 at 8 31 06 AM" src="https://github.com/user-attachments/assets/e1e69c06-e3cd-4e27-96f9-f9e0a607e798" /> <img width="318" height="127" alt="Screenshot 2025-12-10 at 8 30 33 AM" src="https://github.com/user-attachments/assets/8b683b25-f87c-4bab-b395-f6555cc05467" /> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Integrates Expo Font and standardizes font family names, updating snapshots across the app. > > - **Fonts**: > - Add `expo-font` configuration in `app.config.js` to bundle/load fonts at startup. > - Rename font families from space-separated to hyphenated (e.g., `Geist Regular` → `Geist-Regular`). > - **Tests**: > - Refresh widespread snapshots to reflect new `fontFamily` values; no functional UI changes. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fc68231. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude <[email protected]>
7639538 to
a6630d2
Compare
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsAll 9 changed files contain purely cosmetic font family name standardization changes. The pattern is consistent:
These changes:
The fonts themselves remain the same - only the string identifiers are being normalized. If the fonts are correctly bundled in the app (which they must be for current functionality), these changes will render identically. This is a low-risk refactoring/cleanup PR that doesn't warrant E2E test execution. |
|
caieu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM



Dependency #23517 (merged)
Description
This PR is a fast-follow to #23517 (expo-font implementation) that isolates brand font changes (MM Sans and MM Poly) requiring review from code owners outside the mobile-platform and design-system teams.
Strategy
By splitting these changes into a separate PR, we can:
Changes
Updates MM Sans and MM Poly font references to use PostScript names with hyphens instead of spaces:
'MM Sans Regular'→'MMSans-Regular''MM Poly'/'MM Poly Regular'→'MMPoly-Regular'Removes platform-specific
Platform.selectwrappers since PostScript names work consistently across iOS and Android.Why PostScript Names?
iOS requires the PostScript name from font file metadata (not the Full Name or filename). By using PostScript names and naming our font files to match, both platforms can reference fonts consistently.
Changelog
CHANGELOG entry: null
Related issues
Related to: #23517
Manual testing steps
Screenshots/Recordings
Before
Brand fonts(MM Poly and MM Sans) will be currently broken
After
Screenshots and recordings taken from original PR #23517 which these changes were broken out from.
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
Replaces MM Sans, MM Poly, and Geist font references with PostScript names and removes platform-specific font handling across onboarding, modals, rewards, and confirmations.
MMSans-Regular,MMPoly-Regular, andGeist-*(Geist-Regular,Geist-Medium,Geist-Bold).Platformfont branching where applicable.UI/Perps/PerpsGTMModal.styles.ts: unify title font toMMPoly-Regular; keep system font fallback logic.UI/Predict/PredictGTMModal.styles.ts: set title font toMMPoly-Regular; keep system font for description.UI/Earn/...EarnMusdConversionEducationView.styles.ts: heading usesMMSans-Regular.Views/Onboarding/styles.tsandViews/OnboardingSuccess/index.styles.ts: titles useMMSans-Regular.UI/Rewards/.../OnboardingIntroStep.tsx: title text usesMMPoly-Regular.UI/ReviewModal/styles.tsandUI/DeleteWalletModal/styles.ts: replace Geist families withGeist-*.Views/confirmations/.../amount.styles.ts: input text usesGeist-Medium.Written by Cursor Bugbot for commit a6630d2. This will update automatically on new commits. Configure here.