fix: chart glyph fallback for currency symbols missing from the display font - #98621
Conversation
Expensify New Kansas (headline font) doesn't include the Vietnamese dong glyph (U+20AB), so VND totals rendered as tofu boxes. Neue (body font) has full glyph coverage and is already used for the chart legend without issue.
The Concierge chart is rendered via VictoryChartRenderer's Skia canvas drawing (client HTML renderer + headless victory-chart-renderer CLI), not the client-side PieChartContent component. Skia's Font/SkText draw with a single concrete typeface and never fall back to another font for a missing glyph, unlike CSS. The chart XML's "Total" label uses Expensify New Kansas, which doesn't include the Vietnamese dong glyph (U+20AB), so it rendered as a tofu box. getChartSkiaTypeface now accepts the text being rendered and swaps to a typeface with broader glyph coverage (Expensify Neue Bold) when the resolved typeface can't render it, fixing this for any currency symbol across all VictoryChartRenderer text draws (labels, pie labels, legend), not just VND.
# Conflicts: # tests/unit/getChartSkiaTypefaceTest.ts
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Nice! |
garrettmknight
left a comment
There was a problem hiding this comment.
Looks good from a product perspective
…magic control-char threshold canTypefaceRenderText contained "cant" (from "canT..."), tripping the no-negated-variables ESLint rule. Renamed to typefaceCanRenderText. Also extracted the 0x20 control-character threshold into a named constant with an explanatory comment per review feedback.
Reviewer Checklist
Screenshots/Videos |
|
@Ollyws are you still testing this one? |
|
Yeah almost done. |
|
@NicolasBonet Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 NicolasBonet has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/NicolasBonet in version: 9.4.57-0 🚀
|
|
🤖 No help site changes required. This PR is an internal rendering bug fix: The only related article, No draft PR was created. |
|
This PR failing in offline mode because of the issue #99060 |
|
@jponikarchuk that's expected. That feature doesn't work offline. I updated the offline test section to reflect that. |
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.4.57-3 🚀
Bundle Size Analysis (Sentry): |





Explanation of Change
Concierge donut chart's "Total" label used the Expensify New Kansas font, which lacks the Vietnamese dong glyph (₫, U+20AB), rendering incorrectly. Skia draws chart text with a single typeface and no CSS-style font fallback, so a missing glyph never substitutes.
getChartSkiaTypefacenow checks if the resolved typeface can render the given text and swaps to Expensify Neue Bold if not. Wired into allVictoryChartRenderertext draw/measure sites (label, legend, pie label layout).Fixed Issues
$ #97341
Tests
npm run server:vcr:testpasses.Offline tests
N/A feature doesn't work offline
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari