[No QA] Victory chart renderer PR 4: typeface provider refactor#91674
Draft
roryabraham wants to merge 5 commits into
Draft
[No QA] Victory chart renderer PR 4: typeface provider refactor#91674roryabraham wants to merge 5 commits into
roryabraham wants to merge 5 commits into
Conversation
Mirrors the upstream PR (FormidableLabs/victory-native-xl#657) into the installed package so the Bun renderer can drive CartesianChart headlessly. Adds pixelmatch / pngjs for the upcoming reference-PNG test and marks @shopify/react-native-skia as a trusted dependency for Bun. Co-authored-by: Cursor <cursoragent@cursor.com>
Replaces the PR 2 hello-world entry with a real headless `CartesianChart` that draws two bars to a Skia offscreen surface and writes the result as a PNG. To get `victory-native` and `@shopify/react-native-skia` to load under Bun, the React Native peers are stubbed via tsconfig path mappings dropped into both installed packages by patch-package; Metro ignores those tsconfigs so the in-app build is untouched. Co-authored-by: Cursor <cursoragent@cursor.com>
- Switch the Bun CLI to a `.tsx` JSX entry so the chart tree types cleanly through `createElement` without `react/no-children-prop` workarounds. - Replace the PR 2 hello-world smoke test with a real reference-PNG check that decodes the rendered PNG with `pngjs`, compares it to a checked-in `__tests__/__golden__/smoke.png` via `pixelmatch`, and supports `UPDATE_GOLDEN=1 bun test` to regenerate the golden intentionally. - Extend the server ESLint overrides to cover `.tsx` so the subproject's Bun-aware tsconfig wins for the new entry point. Co-authored-by: Cursor <cursoragent@cursor.com>
Extract chart regular/bold typefaces behind shared context so App and future CLI paths can share useChartDefaultTypeface without duplicating Skia useTypeface asset loading. Co-authored-by: Cursor <cursoragent@cursor.com>
Wrap BaseVictoryChartRenderer so VictoryChartProvider, labels, and legend resolve default typefaces through context at runtime. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
0d6b665 to
390a69e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
PR 4 of 6 in the Victory chart renderer rollout (stacked on #91672).
Refactors chart default typefaces behind a shared
ChartDefaultTypefaceContextso App and the future Bun CLI can share the sameuseChartDefaultTypeface()contract. App-sideChartDefaultTypefaceProviderkeeps the existing SkiauseTypeface(require(...))asset loading for Expensify Neue regular/bold. Behavior-preserving: same fonts, same in-product Victory chart output. No CLI changes in this PR.Mount point for PR 5:
ChartDefaultTypefaceProviderwrapsVictoryChartProviderinBaseVictoryChartRenderer(src/components/HTMLEngineProvider/HTMLRenderers/VictoryChartRenderer/BaseVictoryChartRenderer.tsx).Fixed Issues
$ #91528
PROPOSAL:
Tests
<victorychart>content).Offline tests
N/A — chart typeface loading is unchanged; offline chart rendering behavior should match prior PR 3 stack.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
Made with Cursor