Add selection mode Submit/Approve/Pay actions for mobile layout.#85796
Add selection mode Submit/Approve/Pay actions for mobile layout.#85796puneetlath merged 40 commits intoExpensify:mainfrom
Conversation
Signed-off-by: krishna2323 <[email protected]>
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.
|
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
…2502-3 Made-with: Cursor # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx
Signed-off-by: krishna2323 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b74539f3f8
ℹ️ 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".
src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: krishna2323 <[email protected]>
Code ReviewCI Status
Failing TestThe test const hasActualPaymentOptions = paymentButtonOptions.some((opt) =>
Object.values(CONST.IOU.PAYMENT_TYPE).some((type) => type === opt.value)
);
const hasPayInSelectionMode = allExpensesSelected && hasPayAction && hasActualPaymentOptions;Since the mock returns an empty array, Code Observations1. Logic duplication between The new hook ( Ideally, 2. Heavy Onyx subscription load The hook creates ~20 3. The same check 4. Minor: The change from SummaryThe overall approach of extracting selection mode logic into a reusable hook for mobile narrow layout is sound. The main concerns are:
|
|
Please address AI reviews and still add production gate (both web and mobile) to avoid revert. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ede93d634
ℹ️ 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".
|
Kicked off an adhoc. I'll have to come back to this one. |
|
🚧 @JmillsExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
I think it's better. Removed the beta gate. |
Signed-off-by: krishna2323 <[email protected]>
Sorry, what am I being bumped on? Is this ready for me to review?
I think no beta is ideal. Any particular reason why we need it here? |
…02-3 Made-with: Cursor
Signed-off-by: krishna2323 <[email protected]>
|
@MelvinBot check the last commit and confirm that the conflict was resolved correctly—it was a bit complex. |
yes |
|
The merge conflict resolution in the last commit (58d51b6) looks correct. Here's a summary: Files with conflicts: What was merged:
Verified:
No issues found — the resolution correctly preserves both the PR's selection mode payment actions and main's bulk duplicate handling. |
…onvention Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
|
🚧 @puneetlath 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! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
| if (!isUserValidated) { | ||
| handleUnvalidatedAccount(report); | ||
| return true; |
There was a problem hiding this comment.
@MelvinBot This should return true only when payment type is NOT Elsewhere.
The behavior recently changed in #86714 which was merged 3 hrs ago.
As our PR was merged later, can you create followup PR?
There was a problem hiding this comment.
Created followup PR: #87536
The fix adds an optional paymentMethodType parameter to shouldBlockAction and skips the isUserValidated check when the payment type is ELSEWHERE, matching the behavior from #86714. Also added unit tests for both cases.
Please mark it as "Ready for review" when it is ready.
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.3.58-0 🚀
Bundle Size Analysis (Sentry): |
|
I reviewed the changes in this PR against the help site articles under Finding: The article Help site PR created: #87570 The update adds a note explaining that when all expenses are selected on mobile, the bulk action menu also includes report-level actions (Submit, Approve, Pay) depending on report status and role. |
|
Deploy Blocker #87644 was identified to be related to this PR. |
|
This PR failing because of the issue #87644 |
Explanation of Change
Fixed Issues
$ #72502
PROPOSAL:
Tests
Same as QA Steps
Verify that no errors appear in the JS console
Offline tests
QA Steps
1. Submit action via selection mode (Mobile)
MoneyRequestReportActionsList)2. Approve action via selection mode (Mobile)
3. Pay action via selection mode (Mobile)
4. Approve with delegate restriction (Mobile)
5. Approve with held expenses (Mobile)
6. Pay as business (Mobile)
7. Partial selection — no report-level actions (Mobile)
ButtonWithDropdownMenu(not the KYC dropdown)8. KYC dropdown vs plain dropdown rendering (Mobile-specific)
MoneyReportHeaderKYCDropdown(KYC-integrated)ButtonWithDropdownMenu9. Offline behavior (Mobile)
10. Submit blocked by policy rules (Mobile)
preventSelfApprovalis enabled and the submitter is the next approverPR 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: Native
android_native.mp4
Android: mWeb Chrome
test_1-2-3.mp4
test_4.1.mp4
test_4.mp4
test_5.mp4
test_6.mp4
test_8.mp4
test_9.mp4
test_10.mp4
iOS: Native
ios_native.mp4
iOS: mWeb Safari
ios_safari.mp4
MacOS: Chrome / Safari
web_chrome.mp4