Conversation
…nches Co-authored-by: Joe Ph <gijoe0295@users.noreply.github.com>
Co-authored-by: Joe Ph <gijoe0295@users.noreply.github.com>
Co-authored-by: Joe Ph <gijoe0295@users.noreply.github.com>
|
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: Then you can close this PR and open a new one from your fork. |
|
Web test result: partial — the core regression did not reproduce. The export status modal survived crossing the 800px breakpoint in both directions, and each resize was confirmed to be a genuine narrow/wide layout swap, not just a window-size change. Two items stayed unconfirmed for environment reasons: actual PDF file delivery and direct JS console inspection. Tested on dev NewDot at 1280x900 (wide) and 500x900 (narrow). The account had zero reports, so the tester created test data first — note that
What this proves. Before this change the modal lived under the Evidence (5)Spend > Reports wide layout showing 3 created draft reports
3 reports selected with bulk-action header
Export status modal Preparing download shown in wide 1280px layout
Same export status modal still visible after resizing to 500px narrow layout
Export status modal still visible after resizing back to 1280px wide layout
|
|
@MelvinBot Please resolve conflicts |
|
🤖 I did not resolve the conflicts — this PR looks obsolete, and resolving them would mean resurrecting code that What happened: #98521 ("Show export status in one app-level modal instead of per screen") landed on It already fixes this bug, more thoroughly than this PR does. The single modal is now I verified the reported repro specifically: the issue uses bulk select all reports → Download as PDF, and that path on Recommendation: close this PR. Every conflict resolves to "take Conflict detail and one residual gap worth a lookFive conflicting files, all of the same kind:
Residual gap — likely a separate issue, not this repro. The single-report PDF path still has the identical structural weakness. At Also worth noting: I left the branch untouched ( If you'd rather I push the merge anyway with |
|
@MelvinBot Please close this PR |
|
🤖 Closed. #98521 already fixes this on I left the branch Two loose ends on #97483, which is still open:
|









Explanation of Change
Bulk-exporting from Search (for example Spend > Reports > x selected > Download as PDF) stopped delivering the file if the browser crossed the narrow/wide layout breakpoint while the export was still preparing.
The backend export was never actually cancelled.
trackExportis auseStatesetter insideuseExportDownloadStatusModal, and that hook was called fromuseSearchBulkActions— i.e. from insideSearchBulkActionsButton, which lives below theshouldUseNarrowLayout ? <SearchPageNarrow/> : <SearchPageWide/>ternary inSearchPage. Crossing 800px swaps those two branches, unmounting the button and destroyingactiveExportID. TheExportDownloadStatusModalthat owns the "download when ready" effect went with it, so when the backend flippedEXPORT_DOWNLOAD_<exportID>toREADYnothing was left watching, and the file was never downloaded.This is the "lift the state" solution approved on the issue. The fix reuses the pattern the codebase already had for exactly this failure —
ExportDownloadStatusProvider, which exists so the money report header's export modal "survives orientation / layout changes that remount the header actions subtree":SearchPagenow wraps the narrow/wide ternary inExportDownloadStatusProvider, so the tracking state and the status modal live above both branches and are unaffected by the layout swap.ExportDownloadStatusProvidertakes an optionalonCleanupprop so Search can keep its own post-export cleanup (selectAllMatchingItems(false)+clearSelectedTransactions(undefined, true)); the default behaviour for the money report header is unchanged.useSearchBulkActionsnow readstrackExportfrom that shared context instead of owning its ownuseExportDownloadStatusModalinstance, andSearchBulkActionsButtonno longer renders the modal itself.shouldUseNarrowLayoutisisSmallScreenWidth || isInNarrowPaneModal, so this also covers tablet rotation, browser zoom, and window snapping — not just deliberate resizing.Note:
MoneyRequestReportView/SelectionToolbarstill owns its own instance and is fragile the same way. It was called out as out of scope on the issue and is left unchanged here.Fixed Issues
$ #97483
PROPOSAL: #97483 (comment)
AI Tests
Run locally by MelvinBot on the changed files:
npm run lint-changed— passednpm run typecheck— passednpm test -- --silent tests/unit/Search tests/unit/hooks/useSearchBulkActions tests/unit/hooks/useExport tests/unit/hooks/useSelectedTransactionsActions— 60 suites / 1588 tests passednpm test -- --silent tests/unit/MoneyReport tests/ui— 235 suites / 1909 tests passednpm run spell-changed -- <changed files>— 0 issuesnpm run react-compiler-compliance-check check <changed src files>— no regression.src/hooks/useSearchBulkActions.tsfails on both compilers, but it fails identically onorigin/main(9 babel / 8 oxc errors, same causes), so this PR introduces no new failure and no new divergence. The other three changed source files pass.New unit test
tests/unit/Search/ExportDownloadStatusProviderTest.tsxcovers the regression directly: it swaps between two distinct branch component types (asSearchPageNarrow/SearchPageWidedo) and asserts the status modal survives when the provider owns the state, and is lost when a branch owns it.Not run: browser verification. The web test session came up with a blank page (0 DOM nodes, no network activity) even though the dev server responded correctly to a direct HTTP check, so the resize flow could not be exercised end to end in this run.
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Suggested starting point (from the issue's reproduction steps):
Offline tests
The bulk export actions already show the offline modal and do not queue an export while offline; this PR does not change that path.
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas do they need to test for regressions.
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