[Payment due @situchan] [No QA] Victory Chart: Support horizontal bars by s77rt · Pull Request #91659 · Expensify/App

s77rt · 2026-05-26T00:27:26Z

Explanation of Change

Added support for horizontal bars. This works by basically swapping x, y pretty much everywhere (points, axis, domains, etc).
I have also added a patch for victory-native to do the same in useBarGroupPaths hook so the bars are rendered correctly

Fixed Issues

https://github.com/Expensify/Expensify/issues/636601
PROPOSAL:

Tests

Open report
Add comment
Note reportID and reportActionID
Run code:

const html2 = `<victorychart horizontal domain="{y: [0, 18000]}" domainpadding="{x: 44, y: 16}" height="464" width="680" padding="{top: 92, bottom: 84, left: 150, right: 32}" style="{ parent: { backgroundColor: '#F7F2EF', borderRadius: 16, width: '100%', maxWidth: 680, }, }" categories="{x: ['Ethan Brooks', 'Sofia Ramirez', 'Michelina Della Donna', 'Priya Patel', 'Alex Chen']}"><victorylabel x="32" y="40" text='Top employees by spend' style="{ fill: '#002E22', fontSize: 17, fontWeight: 700, fontFamily: 'Expensify Neue', }"/><victorylabel x="32" y="62" text='As of: May 6, 12:49 PM PT' style="{ fill: '#73857E', fontSize: 11, fontWeight: 400, fontFamily: 'Expensify Neue', }"/><victoryaxis tickValues="[0,1,2,3,4]" tickformat="['Ethan Brooks', 'Sofia Ramirez', 'Michelina Della Donna', 'Priya Patel', 'Alex Chen']" style="{ axis: {stroke: 'transparent'}, ticks: {stroke: 'transparent'}, grid: {stroke: 'transparent'}, tickLabels: { fill: '#73857E', fontSize: 11, fontWeight: 500, fontFamily: 'Expensify Neue', padding: 24, }, }"/><victoryaxis dependentaxis tickvalues="[0, 3000, 6000, 9000, 12000, 15000, 18000]" tickformat="['$0', '$3,000', '$6,000', '$9,000', '$12,000', '$15,000', '$18,000']" style="{ axis: {stroke: 'transparent'}, ticks: {stroke: 'transparent'}, grid: {stroke: '#E6E1DA', strokeWidth: 1}, tickLabels: { fill: '#73857E', fontSize: 11, fontWeight: 500, fontFamily: 'Expensify Neue', padding: 8, }, }"/><victorygroup horizontal offset="18"><victorybar barwidth="16" cornerradius="{top: 8, bottom: 0}" style="{data: {fill: '#13C96B'}}" data="[ {x: 'Ethan Brooks', y: 9450}, {x: 'Sofia Ramirez', y: 10500}, {x: 'Michelina Della Donna', y: 11900}, {x: 'Priya Patel', y: 13400}, {x: 'Alex Chen', y: 14300}, ]"/><victorybar barwidth="16" cornerradius="{top: 8, bottom: 0}" style="{data: {fill: '#1E90F2'}}" data="[ {x: 'Ethan Brooks', y: 8800}, {x: 'Sofia Ramirez', y: 10250}, {x: 'Michelina Della Donna', y: 11250}, {x: 'Priya Patel', y: 13900}, {x: 'Alex Chen', y: 12500}, ]"/></victorygroup><victorylegend x="250" y="416" orientation='horizontal' gutter="42" symbolspacer="10" style="{ labels: { fill: '#002E22', fontSize: 13, fontWeight: 500, fontFamily: 'Expensify Neue', }, }" data="[ {name: 'This month', symbol: {type: 'circle', fill: '#13C96B', size: 6}}, {name: 'Last month', symbol: {type: 'circle', fill: '#1E90F2', size: 6}}, ]"/></victorychart>`;

Onyx.merge("reportActions_[report ID]", {
    "[report action ID]": { message: [{ html: html2, type: "COMMENT" }] },
});

Verify the chart looks same as follow

Verify that no errors appear in the JS console

Offline tests

n/a

QA Steps

n/a

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

codecov · 2026-05-26T00:36:20Z

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines	Coverage Δ
...ers/VictoryChartRenderer/parsers/parserRegistry.ts	`100.00% <ø> (ø)`
...eport/ReportActionCompose/ComposerActionButton.tsx	`100.00% <ø> (ø)`
...ChartRenderer/components/VictoryChartCartesian.tsx	`9.09% <0.00%> (+1.39%)`	⬆️
...ctoryChartRenderer/context/VictoryChartContext.tsx	`10.00% <0.00%> (ø)`
...oryChartRenderer/components/VictoryChartSeries.tsx	`16.66% <0.00%> (-1.52%)`	⬇️
...s/VictoryChartRenderer/utils/parseDomainPadding.ts	`0.00% <0.00%> (ø)`
...rers/VictoryChartRenderer/utils/parseCategories.ts	`0.00% <0.00%> (ø)`
...enderers/VictoryChartRenderer/utils/parseOffset.ts	`0.00% <0.00%> (ø)`
...VictoryChartRenderer/parsers/victoryChartParser.ts	`0.00% <0.00%> (ø)`
...ictoryChartRenderer/parsers/victorySeriesParser.ts	`0.00% <0.00%> (ø)`
... and 5 more
... and 268 files with indirect coverage changes

github-actions · 2026-05-26T05:44:58Z

⚠️ This PR is possibly changing native code and/or updating libraries, it may cause problems with HybridApp. Please check if any patch updates are required in the HybridApp repo and run an AdHoc build to verify that HybridApp will not break. Ask Contributor Plus for help if you are not sure how to handle this. ⚠️

melvin-bot · 2026-05-26T05:59:04Z

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

melvin-bot · 2026-05-27T13:08:07Z

🎯 @situchan, thanks for reviewing and testing this PR! 🎉

A payment issue will be created for your review once this PR is deployed to production.
E/E issue linked to the PR - no internal issue.

If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created.

MelvinBot · 2026-05-27T13:11:23Z

Regression Analysis

1. `betweenGroupPadding` changed for ALL bar groups (including existing vertical charts)

Previously, VictoryChartBarGroup used a hardcoded BAR_BETWEEN_GROUP_PADDING = 2/3. Now it dynamically computes the value via parseOffset(). For existing vertical grouped bar charts without an offset attribute, the formula becomes:

offset = 0  →  groupWidth = barWidth  →  betweenGroupPadding = 1 - (barWidth * pointsCount / boundSize)

This produces a different value than the old constant 0.667, meaning existing vertical grouped bar chart spacing will change. This is the highest-risk regression in this PR.

2. `displayName` still has the old typo

The component was renamed from VictoryCharBarGroup → VictoryChartBarGroup, but the displayName on line 55 still reads 'VictoryCharBarGroup' (missing the t in "Chart"). Minor — only affects React DevTools — but worth fixing for consistency.

3. Non-horizontal chart paths are safe

I verified that for isHorizontal = false/undefined, all conditional branches in the axis parser, series parser, domain parser, and domain-padding parser fall through to logic identical to the pre-PR behavior. No regression there.

4. `parseDomainPadding` signature change is safe

The added isHorizontal parameter only affects parseDomainPadding, which is only called from victoryChartParser.ts. No other callers exist.

5. `processVictoryChartTree` signature change is safe

The new rootProcessedResult parameter is only passed from the single external call site in VictoryChartContext.tsx (as null) and from the recursive self-call.

6. Unrelated change: `ComposerActionButton.tsx`

Adds import React from 'react' — benign, but unrelated to horizontal bars. Should probably be in a separate commit.

TL;DR: The main regression risk is item #1 — the betweenGroupPadding calculation change affects all grouped bar charts, not just horizontal ones. Existing vertical grouped bar charts will render with different bar spacing.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 14cab875af

ℹ️ 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".

situchan · 2026-05-27T13:17:37Z

Please check bot reviews and address valid ones

roryabraham · 2026-05-27T13:17:41Z

@s77rt can you look at the AI review comments?

s77rt · 2026-05-27T14:51:06Z

Bug: This month / Last month is overlapped with x labels on vertical chart

Yeah I noticed this. it seems that the Victory Chart padding is not exactly same as Victory Native padding, or I'm missing something, This is something that I didn't catch in the initial PR since in the first chart Monthly Spend the legend is also closer to the x-axis labels than in the expected result. Let's not block on this since the main target charts look good enough (the diff is not visible and no overlapping - let's fix this later though)

s77rt · 2026-05-27T14:55:50Z

@roryabraham @luacmartins Should be good to merge

luacmartins · 2026-05-27T14:57:32Z

Merging since @roryabraham had already approved

OSBotify · 2026-05-27T15:09:53Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-05-27T15:33:07Z

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

github-actions · 2026-05-27T15:42:07Z

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

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-8c3826c0e42-4205783-e3da9a99b9a526bb1b45607ea67788b5dffb07ee/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-8c3826c-4205783-f18fc8f5219db102fdc71b0636a56440bd86196a/index.html

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

👀 View the workflow run that generated this build 👀

OSBotify · 2026-05-27T17:57:46Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.3.84-0 🚀

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

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-05-27T18:03:34Z

I reviewed the changes in this PR against the help site files under App/docs/articles.

No help site changes are required.

This PR adds internal support for rendering horizontal bar charts in the Victory Chart HTML renderer. The changes are entirely in chart component code (parsers, context, victory-native patch) and do not introduce any new user-facing feature, setting, or workflow that would need documentation. The existing insights articles already reference bar charts at the appropriate level of abstraction — the horizontal orientation is a rendering detail controlled by chart HTML, not a user-facing option.

@s77rt, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review

OSBotify · 2026-05-28T10:50:51Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.3.86-0 🚀

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

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-05-28T10:53:31Z

I reviewed the changes in this PR and the existing help site articles under App/docs/articles.

No help site changes are required.

This PR adds horizontal bar chart support to the internal Victory Chart renderer (VictoryChartRenderer components and parsers). It's a rendering capability enhancement for charts embedded in HTML messages — it doesn't introduce any new user-facing features, settings, UI labels, or workflows that are covered by the help site documentation.

The existing Insights articles describe chart view modes (Bar, Line, Table, Pie) at a high level and don't specify bar orientation details, so no updates are needed there either.

@s77rt, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review

OSBotify · 2026-05-28T13:48:03Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.3.88-0 🚀

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

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-05-28T13:49:50Z

I reviewed all the changes in this PR and the existing help site articles under App/docs/articles.

No help site changes are required.

This PR adds internal support for rendering horizontal bar charts in the VictoryChart HTML renderer. All changes are in the rendering engine (parsers, components, context, victory-native patch) and don't introduce any new user-facing features, settings, or workflows that would need documentation.

The existing Insights documentation already covers "Bar View" as a chart type. Whether a bar chart renders horizontally or vertically is determined by the chart HTML from the backend — it's not a user-configurable setting.

@s77rt, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review

OSBotify · 2026-05-28T20:40:30Z

🚀 Deployed to production by https://github.com/mountiny in version: 9.3.88-2 🚀

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

melvin-bot · 2026-05-28T20:43:22Z

🤖 Payment issue created: #92015

s77rt added 7 commits May 25, 2026 21:58

add domain, domainPadding and padding to context

1305d5b

add parsePadding and parseDomain

837647c

add isHorizontal param

e843d84

pass rootProcessedResult to children to get access to isHorizontal

a2df0ef

update comment

7b3d23f

make domain and domainPadding aware of isHorizontal

3543be6

axix horizontal bar support

c743bf4

melvin-bot Bot assigned s77rt May 26, 2026

s77rt added 7 commits May 26, 2026 02:24

remove comments

adc0af3

add categories and handle data for horizontal bars

1807152

remove console log

df3367f

lint

ce14f7c

pass down isHorizontal

7ec4ce9

add victory-native patch to support horizontal bars

99e160e

parse group offset

24bc5e0

lint

cbf63d9

s77rt marked this pull request as ready for review May 26, 2026 05:58

s77rt requested review from a team as code owners May 26, 2026 05:58

melvin-bot Bot requested review from pecanoro and trjExpensify and removed request for a team May 26, 2026 05:58

s77rt requested review from luacmartins, roryabraham and situchan and removed request for pecanoro May 26, 2026 05:59

chatgpt-codex-connector Bot reviewed May 27, 2026

View reviewed changes

Comment thread patches/victory-native/victory-native+41.20.2+001+horizontal-bars.patch

Comment thread patches/victory-native/victory-native+41.20.2+001+horizontal-bars.patch

Comment thread ...ts/HTMLEngineProvider/HTMLRenderers/VictoryChartRenderer/components/VictoryChartBarGroup.tsx Outdated

roryabraham previously approved these changes May 27, 2026

View reviewed changes

s77rt added 2 commits May 27, 2026 15:06

fix displayName

e3398c2

handle barWidth and betweenGroupPadding fallback

ca27658

s77rt dismissed stale reviews from roryabraham and luacmartins via ca27658 May 27, 2026 14:39

luacmartins approved these changes May 27, 2026

View reviewed changes

luacmartins merged commit 8c3826c into Expensify:main May 27, 2026
62 of 77 checks passed

luacmartins mentioned this pull request May 27, 2026

[Due for payment 2026-07-13] [$500] Victory Chart: Legend overlaps x-axis labels on vertical charts #91860

Closed

OSBotify mentioned this pull request May 27, 2026

Deploy Checklist: New Expensify 2026-05-27 #91866

Closed

75 tasks

roryabraham mentioned this pull request May 27, 2026

Version Packages FormidableLabs/victory-native-xl#659

Merged

melvin-bot Bot mentioned this pull request May 28, 2026

[Due for payment 2026-06-04] Payment issue for E/App PR#91659 #92015

Closed

Uh oh!

Conversation

s77rt commented May 26, 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 May 26, 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!

github-actions Bot commented May 26, 2026

Uh oh!

melvin-bot Bot commented May 26, 2026

Uh oh!

melvin-bot Bot commented May 27, 2026

Uh oh!

MelvinBot commented May 27, 2026

Regression Analysis

1. betweenGroupPadding changed for ALL bar groups (including existing vertical charts)

2. displayName still has the old typo

3. Non-horizontal chart paths are safe

4. parseDomainPadding signature change is safe

5. processVictoryChartTree signature change is safe

6. Unrelated change: ComposerActionButton.tsx

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

situchan commented May 27, 2026

Uh oh!

roryabraham commented May 27, 2026

Uh oh!

s77rt commented May 27, 2026

Uh oh!

s77rt commented May 27, 2026

Uh oh!

luacmartins commented May 27, 2026

Uh oh!

Uh oh!

OSBotify commented May 27, 2026

Uh oh!

github-actions Bot commented May 27, 2026

Uh oh!

github-actions Bot commented May 27, 2026

Uh oh!

OSBotify commented May 27, 2026

Uh oh!

MelvinBot commented May 27, 2026

Uh oh!

OSBotify commented May 28, 2026

Uh oh!

MelvinBot commented May 28, 2026

Uh oh!

OSBotify commented May 28, 2026

Uh oh!

MelvinBot commented May 28, 2026

Uh oh!

OSBotify commented May 28, 2026

Uh oh!

melvin-bot Bot commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

s77rt commented May 26, 2026 •

edited

Loading

codecov Bot commented May 26, 2026 •

edited by github-actions Bot

Loading

1. `betweenGroupPadding` changed for ALL bar groups (including existing vertical charts)

2. `displayName` still has the old typo

4. `parseDomainPadding` signature change is safe

5. `processVictoryChartTree` signature change is safe

6. Unrelated change: `ComposerActionButton.tsx`