Fix breakdown rows toggling together when changing reimbursable status#84792
Fix breakdown rows toggling together when changing reimbursable status#84792
Conversation
Both the "Reimbursable" and "Non-reimbursable" rows in the expense report breakdown used the same visibility condition, causing them to appear and disappear together. Each row now independently controls its own visibility based on whether its respective spend value is non-zero. Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
|
Looks good. @MelvinBot Mark the pr as |
|
PR has been marked as ready for review ✅ |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp84792-android-hybrid-001.mp4Android: mWeb Chrome84792-mweb-chrome-001.mp4iOS: HybridApp84792-ios-hybrid-001.mp4iOS: mWeb Safari84792-mweb-safari-001.mp4MacOS: Chrome / Safari84792-web-chrome-002.mp4 |
There was a problem hiding this comment.
@deetergp @JmillsExpensify Changes LGTM and works well too.
All yours. Thanks.
JmillsExpensify
left a comment
There was a problem hiding this comment.
Great product fix. Thanks!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @deetergp 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/deetergp in version: 9.3.36-0 🚀
|
|
Deploy Blocker #84968 was identified to be related to this PR. |
As mentioned in OP, this PR simplified the implementation as follows:
But I agree that this would make ND and OD go out of sync in the following areas:
@JmillsExpensify Not sure if the simplified logic implemented in this PR is fine or if we need to align with OD. Please advise. |
@jponikarchuk This looks like a separate issue and is unrelated to this PR. Most likely, this is due to currency conversion. Do you get this problem if you create an expense in CLP itself? |
|
@rojiphil We have no issue with the local currency ScreenRecording_03-12-2026.05-56-09_1.MP4 |
Thanks. So, that's a separate unrelated issue to this PR. |
|
Now, we just have this pending comment to be addressed. |
|
I think we need to make it work exactly like Classic, I just missed that. So if we need time to make that work, or if it's not going to be possible, then I think we should revert for now. |
…ers breakdown Reverts the outOfPocket row visibility condition from PR #84792 to match Classic (OD) behavior: the Reimbursable row only shows when there are non-reimbursable expenses, not whenever reimbursableSpend > 0. Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.3.36-10 🚀
|
Explanation of Change
Both the "Reimbursable" (outOfPocket) and "Non-reimbursable" (companySpend) rows in the expense report total breakdown used the same visibility condition, causing them to appear and disappear together when toggling an expense's reimbursable status.
This PR makes each row independently control its own visibility based on whether its respective spend value is non-zero:
outOfPocket): now shows only whenreimbursableSpend > 0companySpend): shows only whennonReimbursableSpend > 0(was already correct inMoneyReportView.tsx, now also fixed inMoneyRequestReportTransactionList.tsx)The fix is applied in both components that render the breakdown:
MoneyReportView.tsx— the legacy report viewMoneyRequestReportTransactionList.tsx— the newer transaction list viewFixed Issues
$ #81939
PROPOSAL: #81939 (comment)
Tests
Offline tests
No offline-specific behavior changes — this is a purely visual/conditional rendering fix that depends on local report data already available in Onyx.
QA Steps
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