Repository navigation
fix: Keep the wide report header when resizing back to a wide window with expenses selected - #100923
Conversation
… selected Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
|
I created this PR from an upstream branch because I couldn't find a fork of 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. |
|
Both failing checks are pre-existing breakages on
Once #100919 merges and the EvidenceThis PR changes exactly one file: The branch is already up to date. The PR base is 1. typecheck — two This is a silent merge conflict between two separately-merged PRs: #99894 made 2. test (job 1) — the identical test fails on That is byte-for-byte the failure on this PR. #100919's own test plan notes this same test still fails independently of its typecheck fix. Local verification: I reverted |
|
Re-checked — nothing has changed since my analysis above. Still no fix pushed: both failures are
Current state of the two blockers:
So Nothing to do here. Once I did not push anything, because fixing an unrelated |
|
🤖 Web test steps verification results: ❌ fail Blocked before any test step could run: the pre-started web session went to a blank page with 0 accessible DOM nodes immediately on handoff, and stayed blank through repeated read-only checks over 20+ minutes. The dev server itself was healthy ( The steps below come from the linked issue #100871, since the
For the record, the PR branch Re-run the view run · no recording available |
|
@MelvinBot please close this PR. The proposal has not been approved by a CME yet. |
|
Closed. The proposal on #100871 hasn't been approved by a CME yet, so this PR shouldn't stay open. One thing to check: the view run · no recording available |
…derWideRHPSelectionHeader Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Fixed — the branch was 772 commits behind Both failures were in 34 checks pass. The only remaining failure is What was failing and why
The merge was clean — no conflicts. After merging, the only diff against AI Tests re-run after the mergeThese supersede the results in the PR description, which reported a pre-existing
view run · no recording available |
|
🤖 Web test steps verification results: Confirmed the RHP header shows the mobile "Select multiple" title at narrow width and the wide "N selected" dropdown at wide width for a selected expense in a 2-expense report. The one caveat is that the exact long-press gesture could not be driven because agent-device web has no long-press/hold support, so selection was entered via checkbox instead. On the failing checks: they were already fixed by the view run · no recording available |
|
@MelvinBot could you add regression tests for resizing with selected expenses in wide/super-wide RHPs, including the existing selection cleanup? |
Covers resizing a wide/super-wide RHP with expenses selected in both directions, and locks the existing 'one or no transactions remain' selection cleanup so it keeps running outside the narrow header branch. Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
|
Added, pushed as The file renders Resizing with expenses selected
Existing selection cleanup
Mutation check — the tests fail without the fixI verified both halves by mutating
AI tests run locally on the new file
One thing worth your judgment: these are unit tests over mocked layout hooks, so they lock the branch decision rather than a real browser resize. The |
Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
|
Fixed the one failure that belonged to this PR (Oxfmt). Every other failure is 1. Oxfmt — fixedThe new regression-test file wasn't formatted. Ran 2. typecheck, ESLint, and
|
…derWideRHPSelectionHeader Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
Fixed — merged the latest None of it was this PR's fault. 34 checks pass. The only remaining failure is Why one duplicate line broke five different checksA duplicate
I confirmed the causation locally rather than assuming it: with the duplicate present, AI Tests re-run after the merge
The merge kept the PR's scope intact: the diff against
view run · no recording available |
|
Resolved ✅ |
|
@MelvinBot please update the PR description with the content below and prefix the title with PR descriptionExplanation of ChangeUse the existing wide-RHP-aware layout value to restore the normal expense-report header when a selected report is resized back to a wide window. Preserve the selected expenses and existing selection cleanup. Fixed Issues$ #100871 TestsPrecondition: sign in with a test account that has a workspace expense report containing at least two expenses.
Check the JS console for errors introduced by this change. Offline tests
Additional check performed on iOS native: disable Force offline and reopen the report. Verify the offline banner disappears, the report remains accessible, and the total is unchanged. QA StepsSame as Tests and Offline tests. Also verify a narrow pane that is not a wide/super-wide RHP retains its narrow selection header. Verify selection cleanup still occurs when fewer than two visible expenses remain. Check the JS console for errors introduced by this change. PR Author Checklist
Screenshots/VideosAndroid: NativeAndroid-pr100923.mp4Android: mWeb ChromeAndroid-mWeb-pr100923-merged.mp4iOS: Nativeios-pr100923.mp4iOS: mWeb Safariios-mWeb-pr100923.mp4MacOS: Chrome / SafariChrome-pr100923.mp4 |
|
🤖 Done — PR description replaced with the content you provided, and the title is now Note: the 5 attached files in your comment couldn't be loaded, so the description uses the video links exactly as written in your message. |
|
@MelvinBot please mark this PR as ready for review. |
|
🤖 Done — this PR is now marked Ready for review. |
|
@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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid-pr100923.mp4Android: mWeb ChromeAndroid-mWeb-pr100923-merged.mp4iOS: HybridAppios-pr100923.mp4iOS: mWeb Safariios-mWeb-pr100923.mp4MacOS: Chrome / SafariChrome-pr100923.mp4 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 carlosmiceli 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/carlosmiceli in version: 9.4.90-0 🚀
|
|
🤖 No help site changes are required for this PR, so I did not create a draft docs PR. Why: This is a responsive-layout regression fix, not a product change. It only decides which header renders when a report with selected expenses is shown in a wide or super-wide RHP on a wide screen — the code now keeps the normal report header instead of the narrow Select multiple header in that one case, gated on Nothing the help site documents changes:
Because there is no docs change to make, there is no linked help site PR for you to review or mark @nyomanjyotisa, if you disagree, reply with the article and the behavior you want captured and I will open the draft PR. |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.4.90-2 🚀
Bundle Size Analysis (Sentry): |




Explanation of Change
Use the existing wide-RHP-aware layout value to restore the normal expense-report header when a selected report is resized back to a wide window. Preserve the selected expenses and existing selection cleanup.
Fixed Issues
$ #100871
PROPOSAL: #100871 (comment)
Tests
Precondition: sign in with a test account that has a workspace expense report containing at least two expenses.
Check the JS console for errors introduced by this change.
Offline tests
Additional check performed on iOS native: disable Force offline and reopen the report. Verify the offline banner disappears, the report remains accessible, and the total is unchanged.
QA Steps
Same as Tests and Offline tests.
Also verify a narrow pane that is not a wide/super-wide RHP retains its narrow selection header. Verify selection cleanup still occurs when fewer than two visible expenses remain.
Check the JS console for errors introduced by this change.
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-pr100923.mp4
Android: mWeb Chrome
Android-mWeb-pr100923-merged.mp4
iOS: Native
ios-pr100923.mp4
iOS: mWeb Safari
ios-mWeb-pr100923.mp4
MacOS: Chrome / Safari
Chrome-pr100923.mp4