Fix: Regressions from #97005 - #98283
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@nyomanjyotisa 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] |
|
@ShridharGoel @JS00001 Could you please review this PR. Thanks |
|
@codex review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73961c8b6c
ℹ️ 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".
|
@thelullabyy Looks like #98193 is not fixed, it changes back to the previous number: Screen.Recording.2026-08-11.at.7.30.34.PM.mov |
|
#98200 - Fixed 
|
|
lmk when this is ready please |
|
#98192 - Fixed 
|
|
#98196 - Fixed Screen.Recording.2026-08-11.at.7.37.03.PM.mov |
|
Carousel disappears after merging expenses: Screen.Recording.2026-08-11.at.8.05.21.PM.mov |
|
@JS00001 @thelullabyy Maybe we can go ahead with this PR and then handle #98193 and this separately? These might not be blockers. What do you think? |
|
Lemme check if I can raise a quick fix, but yes, if we are in urgent, I think we can raise another PR for it. I also don't think it can be a DB @ShridharGoel @JS00001 |
|
@ShridharGoel are either of those two reproducible on prod? |
|
@ShridharGoel Kindly check again, I pushed the fix for this issue #98283 (comment) |
|
So which issues are fixed in this PR, and which are being suggested to be demoted, could you make a list please? |
|
@JS00001 I think this PR fixes all issues. But let's see if @ShridharGoel can reproduce any other bugs |
Thanks, it works for the first time but if you merge again just after merging once, then the carousel buttons disappear. |
|
@ShridharGoel could you make sure you have videos for all reported blockers please? Thanks both for the hard work on this! |
|
#98193 - Fixed Screen.Recording.2026-08-11.at.10.39.01.PM.mov |
Yes, all four are present now: #98283 (comment) |
|
Nice, reviewing, thanks! |
|
This one is present, but shouldn't be a blocker: Screen.Recording.2026-08-11.at.10.44.25.PM.mov |
|
Demoting that one, @thelullabyy lets do a follow up please |
|
@JS00001 That one isn't reported I think, I had found it while testing this PR |
|
@thelullabyy I just noticed the size, I dont really feel comfortable CPing a 600 line diff, would we be able to revert this PR and merge it back to main with these changes added on top? |
|
@JS00001 I think most of them are UTs. What do you think? Should we CP staging then? |
|
There are still ~150 lines of regular changes, which is a lot to bank on not having any other bugs, how much work would it be to revert the main PR and add these changes there? |
|
Hmm in that case, I think we can revert the PR and then re-raise another PR with proper fix then @JS00001 |
|
@thelullabyy could you please raise a PR for the revert? |
|
@ShridharGoel Could you please help me raise the straight revert PR...? It's ~1 AM for me now, I think I would be unavailable till tmr morning... |
|
@MelvinBot Create a straight revert of #97005 |
|
There were conflicts btw |
|
🤖 I couldn't create a straight revert of #97005 — it doesn't apply cleanly.
Conflicting files:
Because reverting #97005 would also delete the Options:
Reply with |
|
@JS00001 Since the revert will not be straightforward, should we go ahead with this fix (after getting it tested by QA) ? |
|
@MelvinBot do the reconciled revert |
|
@thelullabyy can you please try reverting locally and check if the conflicts are complex or not? |
|
@ShridharGoel I reverted here: #98346, and fixed conflicts manually |
|
🤖 Done — reconciled revert of #97005 opened as a draft: #98349 A straight Verification: The PR is assigned to you. Please fill in the Tests/QA sections and manually verify the expense/report header + carousel navigation, then mark it Ready for review. Since it was created from an upstream branch (you don't have push access to update it directly), I left branch-ownership instructions on the PR if you want to move it to your fork. |
|
@MelvinBot Why is your revert PR different from #98346 ? |
|
|
4 participants
Explanation of Change
Fixed Issues
$ #98200
$ #98196
$ #98193
$ #98192
PROPOSAL:
Tests
Verify that these bugs are no longer reproducible
$ #98200
$ #98196
$ #98193
$ #98192
Offline tests
QA Steps
Verify that these bugs are no longer reproducible
$ #98200
$ #98196
$ #98193
$ #98192
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
98193.mov
98200.mov
98196.mov
98192.mov