Repository navigation
[$175] Decouple file download modal UI from report and Expensify Card business logic #104440
Description
Context
Follow-up to #103708, which was merged with this cleanup considered non-blocking because ExpensifyCardStatementPDFDownloadModalWrapper followed the existing ReportPDFDownloadModalWrapper pattern. Related report migration: #102987.
Problem
The global modal layer currently contains domain-specific wrappers for reports and Expensify Card statements. Both duplicate visibility/hide-animation lifecycle handling and render feature-specific components, even though they ultimately share PDFDownloadModal.
src/components/Modal should contain business-logic-agnostic UI building blocks, with no knowledge of reports or Expensify Card statements. This is an architectural cleanup, not another migration to the global modal stack.
Proposed cleanup
- Provide a shared, generic
FileDownloadModalin the modal layer, reusing the existing PDF download UI where appropriate. - Keep presentation, generic download states, visibility, and hide-animation lifecycle in the shared UI. Pass status, display text, and callbacks rather than domain identifiers or parameters.
- Move/keep domain-specific behavior behind
useExpensifyCardStatementPDFDownloadModalanduseReportPDFDownloadModal: generation requests, Onyx subscriptions, filename resolution, download actions, error handling, and domain-specific cancellation. - Remove the duplicated domain-specific global wrappers. The modal layer should not import report/card-statement components or access their domain-specific Onyx keys.
Acceptance criteria
- Both hooks consume the same business-logic-agnostic modal UI;
src/components/Modalhas no report/card-statement business logic. - Open modals remain reactive as generation state and filenames arrive, without remounting or reopening a dismissed modal.
- Preserve automatic/manual downloads, loading/success/error states, existing close-on-download behavior, offline behavior, and report stall logging.
- Preserve report cancellation semantics, including submit-via-PDF, and clear Search selection only after the modal finishes hiding.
- Preserve stale-response protection and existing web/native presentation and dismissal behavior.
- Update tests to cover the shared UI lifecycle and both domain-specific hook flows. No intended user-visible behavior change.
Issue Owner
Current Issue Owner: @daledahUpwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~022113954745613435178
- Upwork Job ID: 2113954745613435178
- Last Price Increase: 2026-10-24
Activity
daledah commentedon Oct 21, 2026
@MelvinBot please implement this issue in one PR, following these steps:
Move
src/components/PDFDownloadModal.tsxinto the modal layer assrc/components/Modal/FileDownloadModal.tsx. It only takes presentation props (status,message,onDownload,onClose,onModalHide, button flags). No Onyx, no report or card-statement imports.Add a single
src/components/Modal/Global/FileDownloadModalWrapper.tsxthat ownsisVisibleand callscloseModalonce fromonModalHide. DeleteReportPDFDownloadModalWrapper.tsxandExpensifyCardStatementPDFDownloadModalWrapper.tsx.Move the logic of
ReportPDFDownloadModal.tsxintouseReportPDFDownloadModal: activereportIDin state, Onyx subscriptions,useStallLogger,downloadReportPDF, and the cancel rule (onCancelonly while the filename is still null). Show the wrapper with a fixedidand re-callshowModalwith the sameidwhile open so props update in place; track open state so a late update never reopens a dismissed modal.Same for
ExpensifyCardStatementPDFDownloadModal.tsxintouseExpensifyCardStatementPDFDownloadModal:statementParamsin state,EXPENSIFY_CARD_STATEMENTsubscription,downloadExpensifyCardStatementPDF, keep the fixed id andcloseModalByID. Leave therequestIDguard and "clear selection after the promise" logic inuseSearchBulkActionsuntouched.Switch
MoneyReportHeaderModals.openPDFDownloadtouseReportPDFDownloadModal, then deleteReportPDFDownloadModal.tsxandExpensifyCardStatementPDFDownloadModal.tsx. Nothing undersrc/components/Modal/**should import report or card-statement code or Onyx keys.Tests: add
FileDownloadModalWrapperTest,useReportPDFDownloadModalTestanduseExpensifyCardStatementPDFDownloadModalTestcovering the hide lifecycle, in-place updates, cancel semantics and no-reopen after dismiss. Port and removeReportPDFDownloadModalTestandExpensifyCardStatementPDFDownloadModalWrapperTest, update the mocks inuseSearchBulkActionsDownloadPDFTest.MelvinBot commentedon Oct 21, 2026
🤖 Draft PR created: Decouple the file download modal from report and Expensify Card logic
Please mark it as "Ready for review" or ask me to do that for you when it is ready for review.
All 6 steps are implemented in one PR:
Modal/FileDownloadModal.tsxtakes presentation props only.statusis a newCONST.FILE_DOWNLOAD_STATUS(generating / ready / failed).Modal/Global/FileDownloadModalWrapper.tsxownsisVisibleand callscloseModalonce fromonModalHide. Both domain wrappers are deleted.useReportPDFDownloadModalanduseExpensifyCardStatementPDFDownloadModalown the Onyx and download logic. They re-show the same fixed id to update the open modal in place. An open-state ref is cleared on dismiss, so a late update never reopens the modal.MoneyReportHeaderModals.openPDFDownloadnow uses the report hook.useSearchBulkActionslogic is unchanged.Checks: typecheck, lint, spell check, and the React Compiler check (Babel and OXC) passed. 298 tests across 20 related Jest suites passed, including the 3 new test files.
Not verified in a browser: I tried a web UI check, but the test browser session was blank because its onboarding setup failed. The modal has not been exercised manually.
Out of scope: the existing
Modal/Global/HoldMenuModalWrapper.tsxstill reads report data from Onyx. I left it unchanged.Your fork doesn't give me push access, so the branch is on upstream. The PR has a comment with steps to move it to your fork.
view run · no recording available
[-]Decouple file download modal UI from report and Expensify Card business logic[/-][+][$175] Decouple file download modal UI from report and Expensify Card business logic[/+]melvin-bot commentedon Oct 24, 2026
Job added to Upwork: https://www.upwork.com/jobs/~022113954745613435178