Refactor chart default typefaces behind shared provider by roryabraham · Pull Request #92534 · Expensify/App

roryabraham · 2026-06-03T14:50:16Z

Explanation of Change

Centralizes chart Skia font loading in a global chartFontsCache (single-flight decode, useSyncExternalStore) and ChartFontsProvider, with useChartTypefaces / useChartFontManager reading shared context. Bar/Line charts and Victory HTML charts use the same cached typeface map and fontMgr.

Font assets are decoded once via CHART_SKIA_TYPEFACE_ASSETS; fontMgr is built from those typefaces plus supplemental Noto symbol/month fonts (no duplicate Neue/Mono/Kansas loads). CHART_FONT_FAMILY_NAMES is shared with VictoryTheme (adds Expensify Mono and Expensify New Kansas for Paragraph API fallback).

getChartSkiaTypeface resolves Victory label/legend styles by fontFamily, fontStyle, and fontWeight (including string 'bold', numeric semibold, and Kansas on polar charts). normalizeChartFontWeight keeps parsers and the resolver aligned.

Nested ChartFontsProvider instances wrap Victory PolarChart children and cartesian renderOutside because those Skia subtrees do not inherit font context from the base renderer.

VictoryChartPie must not call useVictoryChartContext; it parses the pie labelcomponent once and passes a template into VictoryChartPieLabel (prod tnode + slice API). The provider accepts an injectable value prop for a future server-side chart renderer CLI (#91528).

Fixed Issues

$ #91528

Tests

Inject the Victory chart HTML samples from the PR review thread (cartesian bar, grouped/horizontal bar, and polar pie with Kansas center label) into a report comment.
Confirm cartesian and polar charts render; bold labels/legends use the bold typeface; pie center text uses Expensify New Kansas where specified.
Open a Bar/Line chart in the app and confirm axis labels still render with correct typography (including CJK/symbol fallback via Noto families).
Confirm no chart-related errors in the browser JS console.
Run unit tests: getChartSkiaTypefaceTest.ts, VictoryTheme.test.ts.

Verify that no errors appear in the JS console

Offline tests

N/A — chart rendering does not depend on network for typeface loading.

QA Steps

(if needed) Sign into any account on NewDot, and create a workspace.
(if needed) Create two expenses with different merchants in the previous month, and one expense in the current month.
Go to Spend -> Insights -> Spend over time and make sure the line graph renders correctly.
Go to Spend -> Insights -> Top Merchants and make sure the pie chart renders correctly.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Co-authored-by: Cursor <cursoragent@cursor.com>

codecov · 2026-06-03T15:02:12Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
.../useChartFontManager/useChartFontManager.native.ts	`0.00% <ø> (ø)`
...s/hooks/useChartFontManager/useChartFontManager.ts	`0.00% <ø> (ø)`
src/components/Charts/context/chartWebFont.ts	`0.00% <0.00%> (ø)`
.../VictoryChartRenderer/BaseVictoryChartRenderer.tsx	`20.00% <0.00%> (ø)`
...nts/Charts/context/ChartDefaultTypefaceContext.tsx	`20.00% <20.00%> (ø)`
...ts/context/ChartDefaultTypefaceProvider.native.tsx	`20.00% <20.00%> (ø)`
...ts/Charts/context/ChartDefaultTypefaceProvider.tsx	`0.00% <0.00%> (ø)`
... and 33 files with indirect coverage changes

Co-authored-by: Cursor <cursoragent@cursor.com>

melvin-bot · 2026-06-03T15:20:20Z

@pecanoro 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]

situchan · 2026-06-03T15:30:56Z

+function useChartDefaultTypeface(): ChartDefaultTypeface {
+    const context = useContext(ChartDefaultTypefaceContext);
+    if (!context) {
+        throw new Error('useChartDefaultTypeface must be used within ChartDefaultTypefaceProvider');


New victory charts not loading. Failing here

Screen.Recording.2026-06-03.at.4.29.11.PM.mov

main branch:

Screen.Recording.2026-06-03.at.4.31.37.PM.mov

Is this happening only for polar charts? I wasn't able to reproduce it for cartesian charts

Polar charts also use Expensify New Kansas

thanks! That repro worked. Working on a fix

ok, It seems like the problem was that renderOutside uses some separate skia renderer that's not tied to the same React tree as the rest of the app. I've got a local fix that we can push forward now, but I'll also start working on an upstream fix.

Co-authored-by: Cursor <cursoragent@cursor.com>

Polar chart HTML specifies fontFamily per label line; parse it and select the Kansas typeface from ChartDefaultTypefaceProvider. Co-authored-by: Cursor <cursoragent@cursor.com>

roryabraham · 2026-06-03T16:09:20Z

Fixed the bug @situchan reported
Added Expensify New Kansas

roryabraham · 2026-06-03T16:11:49Z

actually, I need to expand the context value to include all font family/weight/style combos

Expose every FontUtils family variant (Neue, Mono, Kansas, emoji) and resolve Victory label styles via getChartSkiaTypeface. Co-authored-by: Cursor <cursoragent@cursor.com>

pecanoro

@roryabraham Why [No QA] on the title? QA could check that it didn't cause regressions and is still showing well, right?

situchan · 2026-06-04T03:34:05Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified there are no new alerts related to the canBeMissing param for useOnyx
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

android.mov

Android: mWeb Chrome

mchrome.mov

iOS: HybridApp

ios1.mov

ios2.mov

iOS: mWeb Safari

msafari.mov

MacOS: Chrome / Safari

web.mov

situchan · 2026-06-04T03:58:32Z

Regression Analysis

🔴 R1 — One failed font asset now blanks all chart typography (fault coupling)

This is the most impactful regression. Before, label typefaces and the Paragraph fontMgr were independent failure domains:

useChartDefaultTypeface (HTML labels/legends) loaded only Neue regular/bold via Skia useTypeface.
useChartFontManager (Bar/Line fontMgr) loaded only ExpensifyNeue + NotoSansSymbols + NotoSansSCMonths via useFonts.

After this PR, chartFontsCache.ts collapses everything into a single chained Promise.all:

loadChartFonts = loadChartSkiaTypefaces()   // Promise.all: Neue×4, Mono×4, Kansas×2, CustomEmoji
                   .then(buildChartFontsValue) // Promise.all: NotoSymbols, NotoSCMonths

loadTypefaceFromAsset uses Skia.Data.fromURI(uri), which rejects on a failed fetch. Any single rejection propagates to loadChartFontsOnce().catch → EMPTY_CHART_FONTS (all typefaces null, null fontMgr) → every chart loses all fonts.

Concrete consequences:

A transient web fetch failure for Mono, Kansas, or the CustomEmoji font — none of which Bar/Line charts use — now nukes the Neue axis labels that rendered fine before.
Because buildChartFontsValue's Noto Promise.all runs after and in the same chain, a Noto fetch failure discards the already-decoded Neue label typefaces. In the old code, VictoryChartLabel's Neue typeface had zero dependency on Noto.
The PR states "Offline tests: N/A — chart rendering does not depend on network for typeface loading" — but on web these are network fromURI fetches, and the PR expanded that dependency surface from 3 assets to 8.

Fix: make per-asset decode resilient (Promise.allSettled / null-on-failure per typeface) so one missing asset degrades that glyph only, instead of all-or-nothing. At minimum, don't let supplemental Noto failures discard the core Neue/Mono/Kansas typefaces.

🟠 R2 — `VictoryTheme.fontFamilies` expanded 3→5 and reordered, changing Bar/Line rendering

old: ['ExpensifyNeue', 'NotoSansSymbols', 'NotoSansSCMonths']
new: ['ExpensifyNeue', 'ExpensifyMono', 'ExpensifyNewKansas', 'NotoSansSymbols', 'NotoSansSCMonths']

This array is consumed by existing in-app Bar/Line charts in utils.ts in two places:

buildChartParagraph — the glyph fallback order for actual rendering. Glyphs absent in Neue but present in Mono/Kansas now resolve to Mono/Kansas before reaching Noto, so certain characters can change appearance.
canFontRenderText — fontFamilies.map(family => fontMgr.matchFamilyStyle(...)) now checks 5 typefaces instead of 3, making the renderability test more permissive. Labels that were previously deemed unrenderable (and hidden/ellipsized) may now render.

This is a real visual/behavior change to a high-traffic surface. Needs visual QA on Bar/Line charts with non-ASCII / symbol / currency content, not just the new Victory HTML samples.

chatgpt-codex-connector · 2026-06-04T04:03:37Z

Codex Review: Didn't find any major issues. Can't wait for the next one!

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

roryabraham · 2026-06-04T04:10:57Z

One failed font asset now blanks all chart typography

@situchan that's a great point, but I don't think it necessarily needs to be a blocker, since failed font loads are an edge case. Maybe we can address it with a followup issue?

situchan · 2026-06-04T04:11:43Z

One failed font asset now blanks all chart typography

@situchan that's a great point, but I don't think it necessarily needs to be a blocker, since failed font loads are an edge case. Maybe we can address it with a followup issue?

yes agree

roryabraham · 2026-06-04T04:14:45Z

Regarding this one:

R2 — VictoryTheme.fontFamilies expanded 3→5 and reordered, changing Bar/Line rendering ... Glyphs absent in Neue but present in Mono/Kansas now resolve to Mono/Kansas before reaching Noto, so certain characters can change appearance

Are we sure there are any glyphs absent in Neue but present in Mono/Kansas?
If yes, do we care that it's falling back on Mono/Kansas before Noto?

It would be easy to change the array order, but I don't want to lose my internal approvals by pushing something new if it's not necessary. Maybe could be another follow-up.

MelvinBot · 2026-06-04T04:16:15Z

Code review

Solid, well-contained refactor — the global single-flight cache + useSyncExternalStore is a clean way to decode the chart typefaces once and share them, and the typeface resolver is internally consistent (medium 500 → normal, semibold ≥600 → bold, with the new getChartSkiaTypefaceTest.ts covering the tricky cases). I didn't find a blocking correctness bug, and the author's web + iOS screenshots confirm cartesian/polar/pie all render. A few robustness suggestions below — none are merge-blockers.

Suggestions

1. Font loading is all-or-nothing, and the failure is cached (medium)

chartFontsCache.ts:48 uses Promise.all, so if a single Skia.Data.fromURI rejects (one blocked/404'd web asset, a transient fetch failure), the whole batch rejects → the catch at chartFontsCache.ts:114 returns EMPTY_CHART_FONTS and none of the typefaces that did decode get registered.

There's also a recovery wrinkle: on failure cachedChartFonts stays null, so getChartFontsSnapshot returns the same EMPTY_CHART_FONTS reference it returned before. notifyChartFontLoadListeners() fires, but useSyncExternalStore bails out on the Object.is-equal snapshot, and the useEffect(…, []) in useChartFonts has already run — so currently-mounted charts stay on the loading skeleton until they remount or some other chart mounts and the retry happens to succeed. A deterministic failure (genuinely missing asset) leaves them stuck for the session.

Consider Promise.allSettled and registering the successes individually, so one bad asset degrades gracefully instead of disabling all chart typography.

2. Image.resolveAssetSource(...).uri can deref null (low)

chartFontsCache.ts:25 — on native, a misconfigured/missing asset makes Image.resolveAssetSource return null and .uri throws. Combined with #1 that throw wipes the entire set. Image.resolveAssetSource(source)?.uri plus a guard would make it a clean per-asset skip.

3. Pie label template shares nested objects across slices (low / latent)

VictoryChartPieLabel.tsx:33 — parsing the template once and spreading it per slice is a nice win, but the spread is shallow, so the per-line Record fields (color/fontSize/fontWeight/fontFamily/fontStyle/lineHeight) are shared by reference across every slice. Safe today because VictoryChartLabel only reads them, but the old per-slice parse was defensive against accidental shared-state mutation. Worth a one-line comment that the template must be treated as read-only.

4. Unrecognized fontFamily silently falls back to regular Neue (info)

getChartSkiaTypeface.ts:33 returns 'EXP_NEUE' for any fontFamily not in singleFontFamily, dropping the requested weight/style. Fine as long as the chart HTML producer only emits the spaced family names (Expensify Neue / Expensify Mono / Expensify New Kansas) the resolver matches on — just flagging that a mismatch (e.g. an un-spaced name) would fail silently rather than warn.

Things I checked that are correct

No missing-provider throw. Every useChartTypefaces() / useChartFontManager() consumer is wrapped in a ChartFontsProvider, including the Skia-reconciler boundaries the PR correctly identified — BarChartContent/LineChartContent bodies, BaseVictoryChartRenderer, cartesian renderOutside, and the polar subtree. Pie labels (VictoryChartCategories → VictoryChartPie → VictoryChartPieLabel → VictoryChartLabel) sit inside the polar inner provider, in the same subtree — so no throw, matching the working pie screenshot.
useSyncExternalStore snapshot stability is correct: EMPTY_CHART_FONTS is a stable module constant and cachedChartFonts is assigned exactly once, so no "getSnapshot should be cached" loop.
Fire-and-forget loadChartFontsOnce() in the effect is safe — it always returns a promise with an internal .catch, no unhandled rejection.
processVictoryChartTree(tnode, typefaces.EXP_NEUE, null) keeps the prior SkTypeface | null contract.
Semibold ≥600 → bold is an intentional behavior change (was === 700), and the new unit test asserts it.

situchan · 2026-06-04T04:27:20Z

R2 — VictoryTheme.fontFamilies expanded 3→5 and reordered, changing Bar/Line rendering ... Glyphs absent in Neue but present in Mono/Kansas now resolve to Mono/Kansas before reaching Noto, so certain characters can change appearance

1. Are we sure there are any glyphs absent in Neue but present in Mono/Kansas?

Mono: zero. ExpensifyMono's coverage is a strict subset of ExpensifyNeue (0 codepoints in Mono but not Neue). Mono in the fallback chain is provably inert — anything Mono can render, Neue renders first. Its position can never change any output.
Kansas: exactly 3 codepoints not in Neue:

cp	char	name	effect of the change
U+20BA	₺	Turkish Lira sign	was Noto Sans → now Kansas Medium (visible reorder)
U+20BF	₿	Bitcoin sign	was Noto Sans → now Kansas Medium (visible reorder)
U+25CA	◊	Lozenge	only in Kansas → `canFontRenderText` flips false→true (newly renders)

2. If yes, do we care that it's falling back on Mono/Kansas before Noto?

Only two characters can actually shift appearance — ₺ and ₿ — and only if charts ever render Turkish Lira or Bitcoin amounts in labels/legends. For those, the symbol moves from Noto Sans to Kansas Medium (a display face) sitting next to Neue digits. Minor cosmetic inconsistency, not a breakage. The ◊ case is an improvement (renders instead of tofu).

It would be easy to change the array order, but I don't want to lose my internal approvals by pushing something new if it's not necessary. Maybe could be another follow-up.

Sure, definitely not blocker.

Worth noting:

The cheapest zero-risk fix isn't reordering Neue-vs-the-rest — it's putting Mono/Kansas after the Noto fonts in VictoryTheme.fontFamilies. That makes the ₺/₿ reorder a complete no-op (Noto still wins them) while keeping the ◊ improvement and changing nothing else.
VictoryTheme.fontFamilies (the Paragraph fallback chain, used only by Bar/Line in utils.ts) is a separate concern from registering Mono/Kansas on the fontMgr (which the pie Kansas label needs via getChartSkiaTypeface/matchFamilyStyle). The pie feature needs them registered, not necessarily in the paragraph fallback list — so you could keep registration and leave the fallback as the original Neue+Noto. But given it's 2 currency glyphs, append-at-end or a follow-up is totally reasonable.

situchan

All bot reviews are non blockers so approving

melvin-bot · 2026-06-04T04:28:21Z

@lakchote 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]

roryabraham · 2026-06-04T04:32:51Z

@pecanoro to my knowledge, not all of these charts are readily available on staging yet. But I added some clear QA steps for what is definitely available.

roryabraham · 2026-06-04T04:33:10Z

Merging through unrelated failing jest test, as agreed above

melvin-bot · 2026-06-04T04:33:23Z

@roryabraham looks like this was merged without a test passing. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

github-actions · 2026-06-04T04:34:18Z

🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here.

roryabraham · 2026-06-04T04:35:52Z

Follow up issue for hardening font loading: #92610

github-actions · 2026-06-04T04:39:56Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #92534.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-4f314ce7a13-85cb213-12b97f2575ed460c7b76409be473ec13005f7a7f/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-4f314ce-85cb213-fe1f670e30b17431510d055283ed085f3b6e5e10/index.html

Web 🕸️
https://92534.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-06-04T07:58:46Z

🚀 Deployed to staging by https://github.com/roryabraham in version: 9.3.99-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-06-04T08:01:20Z

Help site review — no changes required

No updates to App/docs/articles are needed for this PR, so I did not create a docs draft PR.

This is an internal rendering refactor with no customer-facing behavior change: it centralizes how chart Skia typefaces are decoded/cached/registered (src/components/Charts/** and the Victory chart Skia renderer). There are no new features, no new or renamed UI elements, tabs, settings, or buttons, and charts render with the same typography as before — the PR is even tagged [No QA]. The help site documents product features and workflows, not font-loading internals, and a search of App/docs/articles for any chart/typeface/rendering references returned nothing relevant.

@roryabraham, no help site PR was created because no documentation changes are required. If you believe a customer-facing behavior actually changed here, let me know and I'll draft the docs update.

roryabraham and others added 4 commits June 3, 2026 07:47

Extract chartWebFont helper for chart font loading

4149974

Co-authored-by: Cursor <cursoragent@cursor.com>

Add ChartDefaultTypefaceContext and platform providers

b82f739

Co-authored-by: Cursor <cursoragent@cursor.com>

Wire Victory charts through ChartDefaultTypefaceProvider

a3b4687

Co-authored-by: Cursor <cursoragent@cursor.com>

Fix lint in chart typeface context files

5af76f4

Co-authored-by: Cursor <cursoragent@cursor.com>

melvin-bot Bot assigned roryabraham Jun 3, 2026

roryabraham requested a review from situchan June 3, 2026 15:07

Remove unused chart typeface exports for knip

a13a7a5

Co-authored-by: Cursor <cursoragent@cursor.com>

roryabraham marked this pull request as ready for review June 3, 2026 15:19

roryabraham requested review from a team as code owners June 3, 2026 15:19

melvin-bot Bot requested a review from joekaufmanexpensify June 3, 2026 15:19

roryabraham requested a review from luacmartins June 3, 2026 15:19

melvin-bot Bot removed the request for review from a team June 3, 2026 15:19

roryabraham requested review from a team and removed request for joekaufmanexpensify June 3, 2026 15:19

melvin-bot Bot requested review from JmillsExpensify and pecanoro and removed request for a team June 3, 2026 15:19

situchan reviewed Jun 3, 2026

View reviewed changes

Comment thread src/components/Charts/context/ChartDefaultTypefaceProvider.native.tsx Outdated

situchan reviewed Jun 3, 2026

View reviewed changes

roryabraham and others added 3 commits June 3, 2026 08:41

Remove unnecessary displayName

9096176

Wrap Skia chart overlays with ChartDefaultTypefaceProvider

4318681

Co-authored-by: Cursor <cursoragent@cursor.com>

Load Expensify New Kansas for Victory chart labels

1e5309f

Polar chart HTML specifies fontFamily per label line; parse it and select the Kansas typeface from ChartDefaultTypefaceProvider. Co-authored-by: Cursor <cursoragent@cursor.com>

roryabraham requested a review from situchan June 3, 2026 16:09

Load all app Skia typefaces in ChartDefaultTypefaceProvider

b78dfa1

Expose every FontUtils family variant (Neue, Mono, Kansas, emoji) and resolve Victory label styles via getChartSkiaTypeface. Co-authored-by: Cursor <cursoragent@cursor.com>

roryabraham mentioned this pull request Jun 4, 2026

[No QA] Victory chart renderer PR 4: typeface provider refactor #91674

Closed

59 tasks

pecanoro approved these changes Jun 4, 2026

View reviewed changes

situchan approved these changes Jun 4, 2026

View reviewed changes

melvin-bot Bot requested a review from lakchote June 4, 2026 04:28

roryabraham changed the title ~~[No QA] Refactor chart default typefaces behind shared provider~~ Refactor chart default typefaces behind shared provider Jun 4, 2026

roryabraham merged commit 4f314ce into main Jun 4, 2026
51 of 58 checks passed

roryabraham deleted the rory/chart-default-typeface-provider branch June 4, 2026 04:33

melvin-bot Bot added the Emergency label Jun 4, 2026

roryabraham removed the Emergency label Jun 4, 2026

roryabraham mentioned this pull request Jun 4, 2026

Harden font loading for charts #92610

Open

This was referenced Jun 4, 2026

[No QA] Reorder font fallbacks in charts #92611

Merged

feat(vcr): wire XML input to PNG via in-app Victory renderers #92619

Draft

OSBotify mentioned this pull request Jun 4, 2026

Deploy Checklist: New Expensify 2026-06-04 #92626

Open

84 tasks

Conversation

roryabraham commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov Bot commented Jun 3, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

melvin-bot Bot commented Jun 3, 2026

Uh oh!

Uh oh!

situchan Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roryabraham Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

situchan Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

luacmartins Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

roryabraham Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

roryabraham Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

roryabraham commented Jun 3, 2026

Uh oh!

roryabraham commented Jun 3, 2026

Uh oh!

pecanoro left a comment

Choose a reason for hiding this comment

Uh oh!

situchan commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

situchan commented Jun 4, 2026

Regression Analysis

🔴 R1 — One failed font asset now blanks all chart typography (fault coupling)

🟠 R2 — VictoryTheme.fontFamilies expanded 3→5 and reordered, changing Bar/Line rendering

Uh oh!

chatgpt-codex-connector Bot commented Jun 4, 2026

Uh oh!

roryabraham commented Jun 4, 2026

Uh oh!

situchan commented Jun 4, 2026

Uh oh!

roryabraham commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MelvinBot commented Jun 4, 2026

Code review

Suggestions

Uh oh!

situchan commented Jun 4, 2026

Uh oh!

situchan left a comment

Choose a reason for hiding this comment

Uh oh!

melvin-bot Bot commented Jun 4, 2026

Uh oh!

roryabraham commented Jun 4, 2026

Uh oh!

roryabraham commented Jun 4, 2026

Uh oh!

Uh oh!

melvin-bot Bot commented Jun 4, 2026

roryabraham commented Jun 3, 2026 •

edited

Loading

codecov Bot commented Jun 3, 2026 •

edited by github-actions Bot

Loading

situchan Jun 3, 2026 •

edited

Loading

situchan commented Jun 4, 2026 •

edited

Loading

🟠 R2 — `VictoryTheme.fontFamilies` expanded 3→5 and reordered, changing Bar/Line rendering

roryabraham commented Jun 4, 2026 •

edited

Loading

roryabraham commented Jun 4, 2026 •

edited

Loading