Skip to content

Use useDecisionModal for Search bulk actions and SelectionToolbar alerts - #103554

Merged
roryabraham merged 4 commits into
mainfrom
claude-migrateSearchBulkActionsDecisionModals
Oct 14, 2026
Merged

roryabraham merged 4 commits into
mainfrom
claude-migrateSearchBulkActionsDecisionModals

Conversation

@MelvinBot

@MelvinBot MelvinBot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

Batch 2 of the DecisionModal → useDecisionModal migration. Search bulk actions and the report SelectionToolbar each mounted their own one-button DecisionModal alerts and kept visibility state for them. These five alerts now open through the global useDecisionModal():

  • Search bulk actions (useSearchBulkActions / SearchBulkActionsButton):
    • "You appear to be offline": every trigger site now calls showOfflineModal(). The deferModalPresentationAfterPopoverDismiss wrapper in bulk pay is kept, so iOS still waits for the popover to close.
    • "Download failed": each trigger site now passes its prompt explicitly. Empty-report exports use downloadFailedEmptyReportDescription with the count. All other failures use downloadFailedDescription. Before, the prompt read a shared emptyReportsCount that was reset in only one place. A later, unrelated failure could show the stale "empty report" text.
    • "Expensify Card statement PDF: one feed at a time": shown from exportExpensifyCardStatementPDF.
  • SelectionToolbar:
    • "Download failed" from onExportFailed. This also fixes a bug. On wide layouts the toolbar unmounts after a CSV export starts, so the inline modal never appeared when the export failed. The global modal does not depend on the toolbar staying mounted.
    • "You appear to be offline": same text as the already-migrated MoneyReportHeaderModals.

Removed the old visibility state, close handlers, emptyReportsCount, and the related return fields. SearchBulkActionsButton.tsx no longer imports DecisionModal. The ProcessMoneyReportHoldMenu block in SelectionToolbar is left for Batch 3.

Behavior notes:

  • shouldHandleNavigationBack is now true, so browser Back closes these alerts.
  • Props are a snapshot taken when the modal opens, which is why each call site passes its own prompt.

Tests: the bulk-action hook tests now mock useDecisionModal (as useExportActionsTest does) and assert the title and prompt shown. A new test covers the empty-report prompt.

AI Tests (run by MelvinBot):

  • npm run typecheck: passed
  • ESLint on changed files: no new errors. The 11 remaining no-unsafe-type-assertion errors already exist on main.
  • oxfmt and npm run spell-changed: clean
  • React Compiler check: SearchBulkActionsButton and SelectionToolbar compile. useSearchBulkActions fails with the same errors on main (refs accessed during render), so this PR doesn't regress it.
  • Jest: useSearchBulkActionsPayTest, useSearchBulkActionsDownloadReceiptsTest, useSearchBulkActionsDownloadPDFTest, useSearchBulkActionsExportTest, and SearchBulkActionsButtonTest passed (114 tests). The full Jest suite and Storybook smoke test were not run locally.
  • Manual checks: web passed, except closing an alert with browser Back, which wasn't tested. On Android, the selection-toolbar Export offline alert got stuck and couldn't be closed. See Screenshots/Videos.

Fixed Issues

$ #102976
PROPOSAL:

Tests

  1. Sign in with an account that has a workspace, 2 or more unreported expenses, an expense report with 2 or more expenses, and an empty report (no expenses).
  2. Go to Account > Troubleshoot and turn on Force offline (or turn off the device network).
  3. Go to Search > Expenses, select one or more expenses, open the bulk-actions dropdown, and choose Export > any CSV template (for example, Basic export).
  4. Verify a "You appear to be offline." alert appears with the prompt "You can't take this action right now." and a "Got it" button.
  5. Press browser Back (web) or the hardware Back button (Android). Verify the alert closes and you stay on Search with the selection kept.
  6. Go to Search > Reports, select a report, and choose Download report from the bulk-actions dropdown.
  7. Verify the same offline alert appears, and clicking "Got it" closes it.
  8. Open the expense report with 2 or more expenses. Select the expenses (long-press on mobile), then choose More > Export > any CSV template from the selection toolbar.
  9. Verify the same offline alert appears and clicking "Got it" closes it. Verify the report still responds to taps and scrolling after the alert closes.
  10. Turn off Force offline (or turn the network back on).
  11. Go to Search > Reports, select only the empty report, and choose Export > Basic export.
  12. Verify a "Download failed" alert appears with the message "You can't export an empty report." and not the generic "Your download couldn't be completed. Please try again later." text. Click "Got it" and verify it closes.
  13. Verify that no errors appear in the JS console

Offline tests

  1. Turn on Force offline in Account > Troubleshoot (or turn off the device network).
  2. In Search > Expenses, select expenses and choose Export > any CSV template. Verify the "You appear to be offline." alert appears and closes with "Got it" and with Back.
  3. In Search > Reports, select a report and choose Download report. Verify the same offline alert appears and closes.
  4. In an expense report, select 2 or more expenses and choose More > Export > any CSV template from the selection toolbar. Verify the same offline alert appears and closes, and the report still responds afterward.

QA Steps

Same as Tests, on staging. For the offline steps, use Force offline in Account > Troubleshoot or turn off the device network.

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I verified any copy / text that was added to the app is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If the PR adds or modifies the UI:
    • I asked an AI agent to review the changes for accessibility issues and addressed its findings.
    • I tested with a screen reader (VoiceOver on macOS) and verified all new/changed elements are reachable with a logical focus order.
    • I verified all new/changed elements have meaningful accessible names and roles.
    • I verified state changes are announced (e.g. checked/unchecked, expanded/collapsed, selected).
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Tested by MelvinBot on an Android emulator (standalone NewDot) at commit 0a22005. Recording · Run

⚠️ Tests step 9 failed: the selection-toolbar Export offline alert appeared, but "Got it" and hardware Back stopped responding. It stayed stuck after reconnecting. The Search bulk-action alerts worked.

Offline alert after Export in Search Expenses Offline alert after Download report in Search Reports Selection toolbar offline alert stuck after reconnecting

Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

Tested by MelvinBot on web (Chrome, dev build) at commit 0a22005. Recording · Run

Every alert showed the right text and closed with "Got it". Tests step 5 (browser Back) was not tested because the test tool can't press Back.

Offline alert after Export in Search Expenses Offline alert after Download report in Search Reports Offline alert after Export from report selection toolbar Download failed alert with empty-report message

MelvinBot and others added 2 commits October 9, 2026 07:02
Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>
Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team October 9, 2026 07:08
@melvin-bot melvin-bot Bot added Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web labels Oct 9, 2026
@MelvinBot

Copy link
Copy Markdown
Contributor Author

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:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-migrateSearchBulkActionsDecisionModals
git checkout -b claude-migrateSearchBulkActionsDecisionModals upstream/claude-migrateSearchBulkActionsDecisionModals
git push -u origin claude-migrateSearchBulkActionsDecisionModals

Then you can close this PR and open a new one from your fork.

@codecov

codecov Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

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.

Files with missing lines Coverage Δ
.../MoneyRequestReportView/SelectionToolbar/index.tsx 71.59% <100.00%> (-3.66%) ⬇️
src/components/Search/SearchBulkActionsButton.tsx 91.25% <100.00%> (-1.15%) ⬇️
src/hooks/useSelectedTransactionsActions.ts 79.56% <ø> (ø)
src/hooks/useSearchBulkActions.ts 80.75% <68.75%> (+0.53%) ⬆️
... and 72 files with indirect coverage changes

@MelvinBot

MelvinBot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Android test steps verification results: ❌ fail

The offline alerts from Search bulk Export and Download report work, but the offline alert opened from the report SelectionToolbar Export became stuck and blocked all input, even after reconnecting. The PR's Tests section is still a TODO, so I derived these steps from the PR description.
Offline alert still stuck after reconnecting and waiting 90+ seconds

Step Status
1. Search > Expenses: select an item, go offline, choose Export > CSV template. The "You appear to be offline." alert appears with a confirm button.Selected 1 expense, turned on airplane mode, chose Export > Current view. The alert showed the title "You appear to be offline.", the prompt "You can't take this action right now.", and a "Got it" button.
Offline alert after Export in Search Expenses
✅
2. Android hardware Back closes the alert and the app stays on Search.Back closed the alert. The app returned to Search > Expenses with 1 item still selected. No crash.
✅
3. Search > Reports: select a report, choose Download report. The same offline alert appears and its confirm button closes it.The alert appeared right away. Tapping "Got it" closed it and returned to the report list.
Offline alert after Download report in Search Reports
✅
4. Inside a report with 2 expenses: select both expenses, choose Export > CSV template from the SelectionToolbar. The same offline alert appears and can be closed.The alert showed the correct title, prompt, and button. Tapping "Got it" did not close it. Hardware Back did nothing. It stayed stuck after turning airplane mode off and waiting more than 90 seconds.
Offline alert from report SelectionToolbar Export (became undismissable)
❌
5. Online, Search > Reports: select only an empty report, choose Export > CSV template. A "Download failed" alert shows the empty-report message.Not reached. The app stayed frozen after step 4, and the tester could not relaunch the session.
❌
6. No red-box error, crash, or stuck overlay appears in any flow.No red-box error appeared, but the step 4 alert froze the UI. A separate, short freeze on the first Search Export attempt cleared once the network state caught up, so it was likely caused by toggling airplane mode on the emulator.
Offline alert still stuck after reconnecting and waiting 90+ seconds
❌

view run · view recording

@MelvinBot

MelvinBot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Web test steps verification results: ⚠️ partial

The offline and empty-report "Download failed" alerts work on web in Search bulk actions and the report selection toolbar, including the selection-toolbar Export that froze on Android. I could not test closing an alert with the browser Back button because the web test tool can't press Back. The PR's Tests section is still a TODO, so I derived these steps from the PR description.
Download failed alert with empty-report message

Step Status
1. Search > Expenses: select an item, go offline, choose Export > CSV template. The "You appear to be offline." alert appears with a confirm button.With Troubleshoot > Force offline on, chose Export > Basic export. The alert showed the title "You appear to be offline.", the prompt "You can't take this action right now.", and a "Got it" button.
Offline alert after Export in Search Expenses
✅
2. Browser Back closes the alert and the app stays on Search.Not tested. The web test tool has no browser Back action (it returned UNSUPPORTED_OPERATION). The alert's confirm button closed it in steps 1, 3, 4, and 5.
❌
3. Search > Reports: select a report, choose Download report. The same offline alert appears and its confirm button closes it.The same alert appeared. Clicking "Got it" closed it, and the app stayed on the Reports list.
Offline alert after Download report in Search Reports
✅
4. Inside a report with 2 expenses: select both expenses, choose Export > CSV template from the selection toolbar. The same offline alert appears and its confirm button closes it.Chose More > Export > Basic export with both expenses selected. The same alert appeared. Clicking "Got it" closed it, and the app stayed on the report with both expenses still selected.
Offline alert after Export from report selection toolbar
✅
5. Online, Search > Reports: select only an empty report, choose Export > CSV template. A "Download failed" alert shows the empty-report message.With Force offline off, chose Export > Basic export on the $0.00 empty report. The alert showed the title "Download failed" and the message "You can't export an empty report." instead of the generic text. Clicking "Got it" closed it.
Download failed alert with empty-report message
✅
6. No new JS console errors, crash, or stuck overlay appears in any flow.No crash or stuck overlay appeared, and every alert closed. All network responses returned status 200. The web test tool can't read the JS console, so this result is based on app behavior and network traffic.
✅

view run · view recording

@ZhenjaHorbach

Copy link
Copy Markdown
Contributor

@MelvinBot
Complete author checklist and add test cases

@MelvinBot

Copy link
Copy Markdown
Contributor Author

I updated the PR description. It now has Tests, Offline tests, and QA steps, plus the web and Android screenshots and recordings from my earlier test runs on commit 0a22005.

Still open before this is ready for review:

  • Android bug: the offline alert from the report selection toolbar's Export (Tests step 9) can't be closed on Android. It stayed stuck after reconnecting. The same flow works on web. This may be a timing problem, because the alert opens while the dropdown menu is still closing. I haven't confirmed that cause.
  • Checklist items I left unchecked because nobody has done them yet:
    • High Traffic account test
    • Screenshots for all platforms (iOS and mWeb are missing)
    • Tests passing on all platforms
    • No console errors (my test tool can't read the console)
    • Testing other affected components
What I checked and why
  • Done: Fixed Issues link, test steps (Tests, Offline, QA, and failure cases), offline testing, and unit tests.
  • Copy and code pattern: checked because the PR adds no new copy and follows the agreed useDecisionModal migration.
  • Checked as not applicable: assets, message editing, generic components, Storybook, deeplinks, UI and form styling, the accessibility block (the alerts look the same as before), and the main-merge retest.
  • MacOS: Chrome: left unchecked because step 5 (closing an alert with browser Back) wasn't tested.

view run

@ZhenjaHorbach

ZhenjaHorbach commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: HybridApp
    • Android: mWeb Chrome
    • iOS: HybridApp
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified any copy / text that was added to the app is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If the PR adds or modifies the UI:
    • I asked an AI agent to review the changes for accessibility issues and addressed its findings.
    • I tested with a screen reader (VoiceOver on macOS) and verified all new/changed elements are reachable with a logical focus order.
    • I verified all new/changed elements have meaningful accessible names and roles.
    • I verified state changes are announced (e.g. checked/unchecked, expanded/collapsed, selected).
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp
Screen.Recording.2026-10-14.at.11.31.37.mov
Screen.Recording.2026-10-14.at.11.32.13.mov
Screen.Recording.2026-10-14.at.11.32.51.mov
Android: mWeb Chrome
Screen.Recording.2026-10-14.at.11.31.37.mov
Screen.Recording.2026-10-14.at.11.32.13.mov
Screen.Recording.2026-10-14.at.11.32.51.mov
iOS: HybridApp
Screen.Recording.2026-10-14.at.11.43.53.mov

Uploading Screen Recording 2026-10-14 at 11.44.44.mov…

iOS: mWeb Safari
Screen.Recording.2026-10-14.at.11.25.09.mov
Screen.Recording.2026-10-14.at.11.25.41.mov
Screen.Recording.2026-10-14.at.11.26.24.mov
MacOS: Chrome / Safari
Screen.Recording.2026-10-14.at.11.20.58.mov
Screen.Recording.2026-10-14.at.11.21.05.mov
Screen.Recording.2026-10-14.at.11.21.14.mov
Screen.Recording.2026-10-14.at.11.21.28.mov
Screen.Recording.2026-10-14.at.11.21.48.mov
Screen.Recording.2026-10-14.at.11.22.07.mov

@ZhenjaHorbach

Copy link
Copy Markdown
Contributor

@MelvinBot
Fix conflicts

Keep main's removal of the offline guard on bulk Remove hold, and update the new offline unhold test to assert via the useDecisionModal mock.

Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I fixed the conflicts by merging main into this branch. The PR is now mergeable (a77a7dc).

  • src/hooks/useSearchBulkActions.ts: I kept main's change from Show Remove hold in Search bulk actions while offline. It removes the offline guard on bulk Remove hold, so that handler no longer calls showOfflineModal().
  • tests/unit/hooks/useSearchBulkActionsTest.ts: main added a test that checks isOfflineModalVisible, which this PR removes. I mocked useDecisionModal and changed the test to check that showDecisionModal is not called.

I couldn't run Jest, lint, or typecheck locally. They fail at startup in my environment, including on unrelated test suites. CI will run them on the new commit.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

The failing check is unrelated to this PR, so I didn't push a fix.

Analysis: test (job 7) failed on one test: IOURequestStepConfirmationPageTest › Report selection guard tests › should use the transaction report when it is allowed, even if it differs from the route. This PR doesn't touch that test or the IOU confirmation page. The test is flaky.

Evidence:

Check independent approval needs a reviewer approval, not a code change. Re-run test (job 7) to clear it.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Nothing has changed since my earlier comment. The failing check is still unrelated to this PR, so I didn't push a fix.

Analysis: test (job 7) failed on one flaky test: IOURequestStepConfirmationPageTest › Report selection guard tests › should use the transaction report when it is allowed, even if it differs from the route. This PR doesn't touch that test or the IOU confirmation page.

Evidence:

Next step: Re-run the failed test (job 7) job. I tried, but my token can't re-run Actions jobs. Check independent approval clears once a reviewer approves.


view run

@ZhenjaHorbach

Copy link
Copy Markdown
Contributor

@MelvinBot
Open the PR

@MelvinBot
MelvinBot marked this pull request as ready for review October 14, 2026 09:51
@MelvinBot
MelvinBot requested a review from a team as a code owner October 14, 2026 09:51
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I marked this PR as ready for review.

@melvin-bot
melvin-bot Bot requested review from ZhenjaHorbach and flaviadefaria and removed request for a team October 14, 2026 09:51
@melvin-bot

melvin-bot Bot commented Oct 14, 2026

Copy link
Copy Markdown

@ZhenjaHorbach 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]

@melvin-bot
melvin-bot Bot removed the request for review from a team October 14, 2026 09:52
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-14T09:58:33.226503Z a77a7dc Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a77a7dca94

ℹ️ 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".

const {showDecisionModal} = useDecisionModal();

const showOfflineModal = () => {
showDecisionModal({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Delay the global alert until the export popover closes

On Android, when an offline user selects an export option from the report selection toolbar, this synchronously presents the global native modal before ButtonWithDropdownMenu finishes dismissing its popover. The overlapping modals leave the alert stuck: neither “Got it” nor hardware Back responds, and the report remains blocked. Present this alert from the popover's post-dismiss callback or otherwise defer it until the originating menu has fully closed.

Useful? React with 👍 / 👎.

@ZhenjaHorbach ZhenjaHorbach Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MelvinBot
Can't reproduce
But makes sense, and we can add these changes just in case

diff --git a/src/hooks/useSelectedTransactionsActions.ts b/src/hooks/useSelectedTransactionsActions.ts
index bd88e40df03f40..904ab27f5107ca 100644
--- a/src/hooks/useSelectedTransactionsActions.ts
+++ b/src/hooks/useSelectedTransactionsActions.ts
@@ -466,6 +466,7 @@ function useSelectedTransactionsActions({
                               clearSelectedTransactions(true);
                           }
                         : () => beginExportWithTemplate(template.templateName, template.type, selectedTransactionIDs, template.name, template.policyID),
+                    shouldCallAfterModalHide: true,
                     addSeparatorBefore,
                 };
             };

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Nothing has changed since my last check. The only real failure is still a flaky test that this PR doesn't touch, so there's no code to fix. Please re-run test (job 7). I tried, but my token can't re-run jobs (HTTP 403).

Evidence:

  • The PR head is still a77a7dc. The failure is the same job run as before, and it hasn't been re-run.
  • The failing test is IOURequestStepConfirmationPageTest › Report selection guard tests › should use the transaction report when it is allowed, even if it differs from the route. It passes locally on this branch.
  • The same test group also fails on main and on unrelated PRs. Investigate workflow job failing on main: test / test (job 7) tracks the failure on main.

Check independent approval needs a reviewer approval, not a code change.


view run

@ZhenjaHorbach

Copy link
Copy Markdown
Contributor

@MelvinBot
Add these changes

diff --git a/src/hooks/useSelectedTransactionsActions.ts b/src/hooks/useSelectedTransactionsActions.ts
index bd88e40df03f40..904ab27f5107ca 100644
--- a/src/hooks/useSelectedTransactionsActions.ts
+++ b/src/hooks/useSelectedTransactionsActions.ts
@@ -466,6 +466,7 @@ function useSelectedTransactionsActions({
                               clearSelectedTransactions(true);
                           }
                         : () => beginExportWithTemplate(template.templateName, template.type, selectedTransactionIDs, template.name, template.policyID),
+                    shouldCallAfterModalHide: true,
                     addSeparatorBefore,
                 };
             };

Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team as a code owner October 14, 2026 10:09
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I added shouldCallAfterModalHide: true to the export menu items in src/hooks/useSelectedTransactionsActions.ts:469 and pushed it in 931c484. The file now matches your diff exactly. It applies to both the basic CSV export and template exports.

@melvin-bot
melvin-bot Bot requested review from roryabraham and removed request for a team October 14, 2026 10:09
@melvin-bot

melvin-bot Bot commented Oct 14, 2026

Copy link
Copy Markdown

@roryabraham 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]

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Nothing has changed since my last comment. The only real failure is still the flaky IOURequestStepConfirmationPageTest test in test (job 7), so there's no code fix to push.

What you need to do: Re-run the failed test (job 7) job from the Jest run. I tried to re-run it, but my token can't re-run jobs (HTTP 403).

Why it's unrelated:

  • This PR doesn't touch that test or the IOU confirmation page.
  • The test passes locally on this branch (a77a7dc).
  • The same test failed today on unrelated PRs.
  • No fix for the test has landed on main since yesterday.

Check independent approval needs a reviewer's approval, not a code change.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

This change is already on the PR, so I didn't add a new commit.

Commit 931c484 ("Run export template actions after the popover menu hides") adds shouldCallAfterModalHide: true at src/hooks/useSelectedTransactionsActions.ts:469. It is the current head of the branch, and its diff matches the one you posted.

@ZhenjaHorbach

ZhenjaHorbach commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

TS issue is not from this PR

@roryabraham

Copy link
Copy Markdown
Contributor

ts issue is unrelated

@roryabraham
roryabraham merged commit c00d280 into main Oct 14, 2026
42 of 44 checks passed
@roryabraham
roryabraham deleted the claude-migrateSearchBulkActionsDecisionModals branch October 14, 2026 14:47
@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions

Copy link
Copy Markdown
Contributor

🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/roryabraham in version: 9.5.10-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No help site update needed. This PR moves existing offline and "Download failed" alerts to the shared decision modal, which only changes how in-app error messages are shown, so no help article becomes incorrect.


view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/roryabraham in version: 9.5.12-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No help site update needed. This PR moves existing offline and "Download failed" alerts to the shared decision modal, which only changes how in-app error messages are shown, so no help article becomes incorrect.


view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/cristipaval in version: 9.5.12-1 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants