feat: update amounts/percentages split logic to match OD - #79273
Conversation
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.
|
|
@ShridharGoel 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8e695fe74
ℹ️ 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".
|
Bug 1: Unable to edit the digits of a negative split. It becomes editable after removing the minus sign though. Screen.Recording.2026-01-10.at.12.55.47.PM.mov |
|
Posted on the issue about negative splits case - awaiting a decision. |
✅ Addressed. Screen.Recording.2026-01-10.at.15.00.39.mov |
@ShridharGoel We got confirmation that this is expected and aligns with OD - I think this PR is ready for a final pass and approve if everything looks good 🙌 ♻️ Merged main!
|
…7-splitsLogicUpdate
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-01-24.at.12.30.24.PM.movAndroid: mWeb ChromeScreen.Recording.2026-01-24.at.12.19.31.PM.moviOS: HybridAppScreen.Recording.2026-01-24.at.12.33.53.PM.moviOS: mWeb SafariScreen.Recording.2026-01-24.at.12.37.48.PM.movMacOS: Chrome / SafariScreen.Recording.2026-01-22.at.1.25.02.AM.mov |
|
@ShridharGoel Addressed all comments ✅
It seems fine to me, percentages should't go negative - so unless there's something else you're seeing that doesn't seem right on percentages side (that I don't see) - we should be good to move forward here. Unless there's something else - I think Approval from your side should be next on the list 🙌 |
Yes, but my concern was about showing values greater than 100% when amounts are negative. But seems like we'll not have a proper way to prevent that, so if product is fine with it - then we can go ahead. CC: @garrettmknight I see that no reviewer from product team got added here, so @garrettmknight will you be taking it ? |
|
Assigning a product team member. |
|
A bug which happens on main as well: On iOS, the field which the user is updating does not change when "Make splits even" is clicked, unless the user goes to some other field. Checked on web and iOS native only, it was not happening on web. Screen.Recording.2026-01-24.at.12.49.34.PM.mov |
|
We can create an issue for the above bug if needed. |
|
✅ That's expected - this PR implemented that on purpose: when a split is edited manually - only the other non-edited splits will auto adjust when others are added / edited. |
|
But it gets updated after clicking on another field, is that expected? And it happens only on mobile? |
ShridharGoel
left a comment
There was a problem hiding this comment.
Just awaiting product review
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #70799 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
Edit from my previous comment: the Now when it comes to the issue of native where if input is focused - and IMO this is expected behaviour - the reason it's different on Web it's because when |
Correct, but we can make the input lose focus when "Make splits even" is clicked (not in this PR though since that would be out of scope). Else, it would be confusing for the user since nothing happens (it doesn't lose focus) even on tapping outside the fields in some empty area. |
|
That can be done - but I feel we should decide on whether the |
|
@ShridharGoel is there a reason you didn't check off "I included screenshots or videos for tests on all platforms" in the reviewer checklist? |
|
@blimpich No, it just got missed. Thanks, I've updated it now. |
|
FYI: Going to wait to review the code for this until Product has ✅ this |
JmillsExpensify
left a comment
There was a problem hiding this comment.
All within product scope. (Thanks for the thorough videos of the testing steps).
blimpich
left a comment
There was a problem hiding this comment.
Looks great, thank you for writing tests!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/blimpich in version: 9.3.11-19 🚀
|
|
Reverted here: #81341 |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.3.12-1 🚀
|



Explanation of Change
In Classic, when a user splits an expense, we try to update the amounts for all splits to equal the amount of the original transaction automatically. We opted to go a simpler route with NewDot splits to start, avoiding any automatic updates, and punted adding this logic to a follow-up. This is that follow-up:
Problem: When users are editing the separate portions of a split they are typically trying to ensure all the portions add up to the original transaction amount.
Solution: Automatically change amounts and percentages when creating a split to add up to the original transaction amount to remove clicks/steps for the submitter. To do so, we'll implement our Classic logic:
Core logic:
Behavior for Adding Splits
Simple adding splits Example:
Further editing example:
Error handling:
Fixed Issues
$ #70799
PROPOSAL:
Tests
Test 1: Adding Splits Auto-Redistributes
Test 2: Edited Splits Stay Locked
Test 3: Editing Existing Saved Splits (All Locked)
Offline tests
Offline Test 1: Create Splits While Offline
Offline Test 2: Add Split While Offline
Offline Test 3: No Data Loss on Reconnect
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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: HybridApp
android-hybrid.mp4
Android: mWeb Chrome
android-mweb.mp4
iOS: HybridApp
ios-hybrid.mov
iOS: mWeb Safari
ios-mweb.mov
MacOS: Chrome / Safari
web.mov