Decouple the file download modal from report and Expensify Card logic by MelvinBot · Pull Request #104534 · Expensify/App

MelvinBot · 2026-10-21T07:40:46Z

Explanation of Change

src/components/Modal contained report-specific and Expensify Card statement-specific wrappers. Each one repeated the same visibility and hide-animation handling. This PR keeps the modal layer free of business logic: one generic wrapper handles the modal lifecycle, and two domain components outside src/components/Modal own the data. It follows the promise-based modal pattern from Move hold/reject educational modals to the global modal stack. The user-visible behavior should not change, except for the timing noted below.

Modal/FileDownloadModal.tsx (moved from PDFDownloadModal.tsx) takes presentation props only: title, status (CONST.FILE_DOWNLOAD_STATUS: generating / ready / failed), message, onDownload, onClose, onModalHide, and the button flags. It has no Onyx, report, or card-statement imports.
Modal/Global/FileDownloadModalWrapper.tsx is the single generic wrapper. It owns isVisible and calls closeModal once from onModalHide, so the show promise resolves only after the modal finishes hiding. It resolves with CLOSE if the user dismissed the modal while the file was still generating, and CONFIRM otherwise. A dismissed modal keeps its last props while it hides. It replaces ReportPDFDownloadModalWrapper and ExpensifyCardStatementPDFDownloadModalWrapper.
ReportPDFDownloadModal.tsx (outside Modal/) is the component the global stack renders for a report. It owns the Onyx subscriptions and useStallLogger, derives the title, status, message, and onDownload, and renders FileDownloadModalWrapper. Because the global stack renders it, its subscriptions live as long as the modal, as they did on main. The report header or Search page can unmount while the PDF is generating, and the modal still picks up the filename.
ExpensifyCardStatementPDFDownloadModal.tsx does the same with statementParams, the EXPENSIFY_CARD_STATEMENT subscription, and downloadExpensifyCardStatementPDF.
useReportPDFDownloadModal and useExpensifyCardStatementPDFDownloadModal only call showModal with those components. The card statement hook keeps its fixed id, so the second show call with the statementKey updates the open modal in place, and exposes close by id. The requestID guard and the clear-selection-after-promise logic in useSearchBulkActions are unchanged.
MoneyReportHeaderModals.openPDFDownload now returns the show promise. SubmitPrimaryAction chains the Submit via PDF retract off it when the result is CLOSE, so there is no onCancel callback.

Behavior note: the Submit via PDF retract and stopAnimation() now run after the modal finishes hiding, not at the moment the user dismisses it.

Tests: added FileDownloadModalWrapperTest, useReportPDFDownloadModalTest, and useExpensifyCardStatementPDFDownloadModalTest. They cover the hide lifecycle, CLOSE vs CONFIRM results, frozen props while hiding, in-place updates without remounting, no reopen after dismissal, and updates that still reach the modal after the caller unmounts. Ported and removed ReportPDFDownloadModalTest and ExpensifyCardStatementPDFDownloadModalWrapperTest. Updated the mocks in useSearchBulkActionsDownloadPDFTest and SearchBulkActionsButtonTest.

Out of scope: Modal/Global/HoldMenuModalWrapper.tsx (existing, not touched) still reads report data from Onyx.

AI Tests (run locally by MelvinBot):

npm run typecheck: passed
npm run lint on the changed files: passed
npm run fmt (oxfmt): no changes outside this PR's files
npm run spell-changed on the changed files: no issues
react-compiler-compliance-check check <changed files>: passed
Jest (previous commit): 215 suites, 2257 tests passed. These cover all of tests/unit/hooks, FileDownloadModalWrapperTest, ModalContextCloseByIDTest, OnyxDrivenGlobalModalsTest, SearchBulkActionsButtonTest, the MoneyReportHeader* UI tests, and MoneyRequestReportViewTest. A mutation check confirmed that the new caller-unmount tests fail on the previous hook-owned subscriptions.
Jest (latest commit): ModalContextCloseByIDTest, FileDownloadModalWrapperTest, useReportPDFDownloadModalTest, useExpensifyCardStatementPDFDownloadModalTest, SearchBulkActionsButtonTest, and useSearchBulkActionsDownloadPDFTest passed (6 suites, 60 tests).
Not run: the full Jest suite and the Storybook smoke test. The modal was not exercised in a browser.

Fixed Issues

$ #104440
PROPOSAL: #104440 (comment)

Tests

Search > Reports, select one report, click "Download report". Verify the PDF modal opens with a spinner, then auto-downloads and shows "Download" when ready.
Repeat step 1 and click "Cancel" while generating. Verify the modal closes and does not reappear.
Open a submitted report, click More > "Download report". Verify the same flow as step 1.
On a workspace where you submit to yourself, choose "Submit via PDF" on a draft report and cancel while generating. Verify the report goes back to Draft. Repeat, wait until ready, then close. Verify the report stays submitted.
Search > Expenses, select an Expensify Card settlement, click "Export as PDF". Click "Download" when ready. Verify the file downloads and the modal closes.

Verify that no errors appear in the JS console

Offline tests

Go offline and try steps 1, 3 and 5. Verify only the offline modal appears.

QA Steps

Same as tests.

PR Author Checklist

Note

The C+ reviewer, daledah, did the manual, offline, and cross-platform testing and recorded the videos below (reviewer checklist). MelvinBot added the unit tests and ran the AI tests listed above. Items that don't apply to this PR are checked as N/A.

Screenshots/Videos

Android: Native

Recorded by daledah:

android.mov

Android: mWeb Chrome

Recorded by daledah:

android-mweb.mov

iOS: Native

Recorded by daledah:

ios.mov

iOS: mWeb Safari

Recorded by daledah:

ios-mweb.mov

MacOS: Chrome / Safari

Recorded by daledah:

web.mov

Move PDFDownloadModal into the modal layer as a presentation-only FileDownloadModal, replace the two domain-specific global wrappers with a single FileDownloadModalWrapper, and move the report and card statement Onyx/download logic into their hooks, which update the open modal in place and never reopen a dismissed one. Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot · 2026-10-21T07:40:58Z

I created this PR from an upstream branch because I don't have push access to your fork.

To take ownership of this branch and push updates, run:

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

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

Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot · 2026-10-21T07:44:17Z

Fixed the failing Compare knip issues against main check in 152d7d6.

Knip flagged FileDownloadStatus as an unused exported type in src/components/Modal/FileDownloadModal.tsx:171. No file imports it, so I removed it from the export. The type is still used inside the file.

Check independent approval is a reviewer-approval check, not a code failure. The test jobs were still pending when I checked.

view run

MelvinBot · 2026-10-21T07:51:44Z

The remaining failing check, test (job 8), is unrelated to this PR, so I didn't push any code. Re-running the failed job should clear it.

Analysis: The suite tests/unit/resolveSplitItemReportIDTest.ts never ran. Its Jest worker was killed by the runner (signal=SIGKILL), which usually means the runner ran out of memory. All 3,485 tests that ran passed.

Evidence: This PR doesn't change resolveSplitItemReportIDTest.ts or the code it tests.

The knip check now passes after the fix in 152d7d6. Check independent approval needs a reviewer's approval, not a code change.

view run

MelvinBot · 2026-10-21T07:56:52Z

🤖 Android test steps verification results: ❌ fail

I could not run the "Download as PDF" flow. After I opened the header "More" menu on a one-expense Draft report, the app stopped responding to all taps, back presses, and gestures. This happened before any code from this PR ran, so this run doesn't show a bug in the PR. The PR's Tests section lists only "Verify that no errors appear in the JS console", so I also tested the report PDF download modal flow that this PR changes.

Step	Status
Setup: an expense report with at least one expense exists Created a workspace and a $25.00 manual expense (Test Merchant, Advertising). This made the Draft report "Expense Report 2026-10-21" with one expense.	✅
Open the report header "More" menu and tap "Download as PDF". A modal opens in a generating state The "More" menu opened but showed only expense actions (Add expense, Hold, Split, Merge, Duplicate expense, Move to report). A "Download report" item was in the accessibility tree but had zero height, which suggests it isn't offered on a Draft report. The menu then froze and ignored all input, so the download modal never opened.	❌
The modal updates in place to a ready or error state without reopening Not reached. The app froze before the download modal opened.	❌
Download or Close dismisses the modal, and it doesn't reopen Not reached.	❌
Dismissing while the PDF is generating closes the modal, and it doesn't reopen when generation finishes Not reached.	❌
No errors appear in the JS console (documented PR step) Not reached. The tester also had no access to console or logcat output. Metro showed repeated `[Reanimated] Writing to value during component render` warnings, but those were logged before any PR code ran.	❌

To get a real result, rerun this on a fresh session with a submitted (not Draft) report, so the "Download as PDF" action is available.

view run · view recording

MelvinBot · 2026-10-21T08:02:43Z

🤖 Web test steps verification results: ⚠️ partial

The "Download as PDF" modal opened correctly in its generating state. When the PDF was ready, the app started the automatic download by sending the browser tab to www.expensify.com.dev/secure?secureType=pdfreport…. That host doesn't resolve in the test environment, so the browser left the app and the remaining checks couldn't run. This is a limit of the test environment, not a confirmed PR bug.

Step	Status
On a submitted expense report, open "More" and choose "Download as PDF". A modal opens in a generating state "More" showed "Download report". Clicking it opened the "Generate PDF" modal with "Please wait while we generate the PDF." and Cancel/Close buttons. No error appeared.	✅
The modal updates in place to a ready or error state without reopening Generation finished, and the app started the automatic download right away by sending the whole tab to the secure download URL. The test environment can't resolve that host (DNS_PROBE_FINISHED_NXDOMAIN), so the tab left the app before any ready or error state could be seen.	❌
Download or Close dismisses the modal, and it doesn't reopen Not reached. The session ended when the browser left the app.	❌
Dismissing while the PDF is generating closes the modal, and it doesn't reopen when generation finishes Not reached. The session ended when the browser left the app.	❌
No errors appear in the JS console (documented PR step) Not checked. The web test tool can't read the browser console.	❌

The PR's Tests section lists only the console-error check, so I also ran the report PDF download flow this PR changes. To check the ready and dismiss states, run this in a local dev environment where the secure download URL works.

view run · view recording

…Modal hook - Add useFileDownloadModal, which owns the fixed-id show, in-place updates, and no-reopen-after-dismiss lifecycle. The report and Expensify Card statement hooks now only derive the title, status, message, and download action. - Drop onCancel and the wrapper's onClose passthrough. The show promise resolves with CLOSE when the modal is dismissed while generating and CONFIRM otherwise, and SubmitPrimaryAction chains the retract off openPDFDownload(). - Pass the header text into FileDownloadModal instead of hardcoding it. Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot · 2026-10-21T09:48:15Z

I aligned the PR with the pattern from Move hold/reject educational modals to the global modal stack in ee24686. I also updated the PR description.

Generic hook: useFileDownloadModal now owns the fixed-id show, the in-place updates, and the no-reopen gate. The two domain hooks only derive title, status, message, and onDownload. They no longer call useModal() or import the wrapper.
No more onCancel: I removed onCancel and the wrapper's onClose passthrough. The wrapper resolves with CLOSE if dismissed while generating, CONFIRM otherwise. SubmitPrimaryAction chains the retract off openPDFDownload().
Header text: FileDownloadModal takes a title prop. Both hooks pass reportDetailsPage.generatingPDF.

Two behavior notes:

The Submit via PDF retract now runs after the modal finishes hiding, not at the moment of dismissal.
Without the onClose passthrough, the hook keeps updating the modal while it hides. The wrapper keeps rendering the props from the moment of dismissal, so a dismissed modal doesn't change as it fades out.

Checks

npm run typecheck, lint on the changed files, oxfmt, cspell, and the React Compiler check all passed.
14 related Jest suites passed (117 tests). New tests cover CLOSE vs CONFIRM, frozen props while hiding, and the passed-in header text.
A mutation check confirmed the no-reopen tests fail when the open-state gate in useFileDownloadModal is removed.
Not tested in a browser: the web test page went blank during onboarding setup, so the modal was not exercised.

view run · no recording available

codecov · 2026-10-21T09:51:50Z

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 Δ
src/CONST/index.ts	`95.90% <ø> (ø)`
...ponents/ExpensifyCardStatementPDFDownloadModal.tsx	`95.00% <100.00%> (+89.73%)`	⬆️
src/components/Modal/FileDownloadModal.tsx	`97.77% <100.00%> (ø)`
src/components/Modal/Global/ModalContext.tsx	`87.50% <ø> (+8.33%)`	⬆️
src/components/ReportPDFDownloadModal.tsx	`96.55% <100.00%> (+3.95%)`	⬆️
...hooks/useExpensifyCardStatementPDFDownloadModal.ts	`100.00% <100.00%> (+14.28%)`	⬆️
src/hooks/useReportPDFDownloadModal.ts	`100.00% <100.00%> (ø)`
src/libs/actions/IOU/ReportWorkflow.ts	`75.17% <ø> (-2.76%)`	⬇️
...mponents/Modal/Global/FileDownloadModalWrapper.tsx	`93.33% <93.33%> (ø)`
...eaderActions/MoneyReportHeaderSecondaryActions.tsx	`0.64% <0.00%> (+0.01%)`	⬆️
... and 3 more
... and 330 files with indirect coverage changes

daledah · 2026-10-22T09:03:54Z

@MelvinBot please resolve conflict

…wnloadModal Co-authored-by: daledah <daledah@users.noreply.github.com> # Conflicts: # src/components/MoneyReportHeaderModals.tsx # src/components/MoneyReportHeaderModalsContext.tsx # tests/unit/Search/SearchBulkActionsButtonTest.tsx

MelvinBot · 2026-10-22T09:16:38Z

Resolved the conflicts by merging main into this branch (ca13e0c). The PR is mergeable again.

main moved the hold/reject educational modals into the global modal stack. That overlapped with this PR's move of the PDF download modal into a hook. I kept both changes:

MoneyReportHeaderModals.tsx: kept main's split into a wrapper and a MoneyReportHeaderModalsContent component. The educational modals use main's useMoneyReportHeaderEducationalModals hook. The PDF modal still opens through this PR's useReportPDFDownloadModal. I removed the old useState and onCancel ref plus the inline <ReportPDFDownloadModal>.
MoneyReportHeaderModalsContext.tsx: RejectModalAction now comes from main's @hooks/useHoldRejectEducationalModal. I kept the PR's ModalStateChangePayload import.
SearchBulkActionsButtonTest.tsx: kept only the @components/Modal/FileDownloadModal mock. main already removed the two educational-modal mocks.

Checks: npm run typecheck passes. The 5 related Jest suites pass (55 tests).

npm run lint-changed reports one no-misused-promises error at MoneyReportHeaderSecondaryActions.tsx:397 (onPDFModalOpen: openPDFDownload). This PR already had that error before the merge, and the PR's ESLint check passes, so I left it alone.

Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot · 2026-10-22T09:20:05Z

Fixed the failing Oxfmt check. src/components/MoneyReportHeaderModals.tsx had a multi-line return (...) that Oxfmt collapses to one line. I ran npm run fmt and pushed the result in 522afc5. It changes formatting only.

Check independent approval will pass once the PR gets an approval. The test and branch-perf-tests jobs were still running when I checked.

view run

MelvinBot · 2026-10-22T09:20:38Z

⚠️ MelvinBot CI Fix Loop Detected

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.

…and guard the report PDF Onyx keys Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot · 2026-10-22T18:56:00Z

I pushed 69947a0 and added the behavior note to the description. Items 1 and 2 are done. I skipped item 3 because it fails lint. Item 1 also needed two extra changes.

Promise reset: Done, at useFileDownloadModal.ts:84. A new test showed this alone did not open a new modal, so I added:
- ModalContext.tsx:139: closeModalByID now releases the promise right away. Before, it did this inside the state updater, so a same-tick showModal with the same id got the old promise back.
- Guards so the closed modal's late .then can't clear the new modal's state: useFileDownloadModal.ts:71 and useExpensifyCardStatementPDFDownloadModal.ts:58.
Onyx keys: Done, at useReportPDFDownloadModal.ts:25-26. When idle, the key is still report_undefined, but Onyx already skips undefined member IDs. The helper only changes the '' case.
Modal import: Not changed. @dword-design/import-alias/prefer-alias reports @components/Modal as an error inside src/components/Modal and asks for './'.

Tests

I added same-tick close-then-show tests to ModalContextCloseByIDTest and useExpensifyCardStatementPDFDownloadModalTest. Both fail when the ModalContext change is reverted.
Jest: 222 suites and 2303 tests passed, including all of tests/unit/hooks, the modal suites, and the MoneyReportHeader* UI tests.
Typecheck, lint, and the React Compiler check passed.
spell-changed could not resolve the base ref here, so I did not run it.

daledah · 2026-10-23T02:36:36Z

@MelvinBot please open this PR

MelvinBot · 2026-10-23T02:38:50Z

I marked this PR as ready for review.

view run

melvin-bot · 2026-10-23T02:38:52Z

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

chatgpt-codex-connector · 2026-10-23T02:38:56Z

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-24T11:26:49.297715Z	`4895720`	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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69947a0ba7

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

Co-authored-by: daledah <daledah@users.noreply.github.com>

melvin-bot · 2026-10-23T02:56:05Z

@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 · 2026-10-23T02:56:25Z

I checked both comments and replied in each thread.

github-actions (CONSISTENCY-15): Fixed in a66607c. I split the comment into two sentences.
Codex (subscription lifetime): This is valid, and I haven't changed it yet. If the report header or Search page unmounts while the PDF is generating, the modal stays stuck on "generating." On main, the global modal owned the subscriptions, so this couldn't happen. Both fixes change the hook design, so I listed them in the thread. Should I move the subscriptions into a component the global stack renders?

view run

dylanexpensify

refactor, input not required

…modal stack renders Co-authored-by: daledah <daledah@users.noreply.github.com>

…lveModal out of FileDownloadModal Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot · 2026-10-23T16:59:07Z

I pushed 4895720 with all three follow-ups:

onPDFModalOpen: It now wraps the call in a function body, () => { openPDFDownload(); }. See MoneyReportHeaderSecondaryActions.tsx:397-399.
closeModalByID: I reverted it to match main. The promise resolves inside the state updater again. I also removed the two same-tick tests, one from ModalContextCloseByIDTest and one from useExpensifyCardStatementPDFDownloadModalTest. I kept the ModalStateChangePayload type export because MoneyReportHeaderModalsContext uses it.
FileDownloadModalWrapper: It now destructures resolveModal, and I removed the leftover comment and blank line. See FileDownloadModalWrapper.tsx:16.

I removed the closeModalByID bullet and the same-tick test mention from the PR description.

These checks passed: typecheck, lint and the React Compiler check on the changed files, and 6 affected Jest suites (60 tests).

roryabraham · 2026-10-23T20:31:57Z

Making this a draft, @daledah please reopen once you give it your C+ approval. Also please resolve review threads or other resolved back-and-forths with Melvin

MelvinBot · 2026-10-24T06:06:27Z

I updated the PR description with your steps:

Tests: your 5 steps replace the TODO. The JS console checkbox is still there.
Offline tests: added "Go offline and try steps 1, 3 and 5. Verify only the offline modal appears."
QA Steps: "Same as tests." replaces the TODO and the console checkbox, as in your text.

Nothing else in the description changed.

view run

daledah · 2026-10-24T06:14:55Z

Reviewer Checklist

Screenshots/Videos

Android: HybridApp

android.mov

Android: mWeb Chrome

android-mweb.mov

iOS: HybridApp

ios.mov

iOS: mWeb Safari

ios-mweb.mov

MacOS: Chrome / Safari

web.mov

daledah · 2026-10-24T07:05:15Z

@MelvinBot i've completed my checklist, please complete the author checklist

MelvinBot · 2026-10-24T07:08:49Z

I completed the PR Author Checklist. All 36 boxes are now checked.

Note added: A note above the checklist says you, as C+, did the manual, offline, and cross-platform testing, and links your reviewer checklist. MelvinBot added the unit tests and ran the AI tests. Items that don't apply are checked as N/A.
Screenshots/Videos: I added your five recordings to the matching platform sections and credited them to you.

The PR is still a draft. Mark it ready for review when you want it reviewed.

view run

daledah · 2026-10-24T11:21:39Z

@MelvinBot please open this PR

MelvinBot · 2026-10-24T11:23:10Z

I marked this PR as ready for review.

view run

daledah

LGTM

MelvinBot assigned daledah Oct 21, 2026

MelvinBot requested a review from a team October 21, 2026 07:40

melvin-bot Bot added Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web labels Oct 21, 2026

MelvinBot mentioned this pull request Oct 21, 2026

[$175] Decouple file download modal UI from report and Expensify Card business logic #104440

Open

Fix: remove unused FileDownloadStatus type export flagged by knip

152d7d6

Co-authored-by: daledah <daledah@users.noreply.github.com>

This comment has been minimized.

Sign in to view

Fix: Apply oxfmt formatting to MoneyReportHeaderModals

522afc5

Co-authored-by: daledah <daledah@users.noreply.github.com>

This comment has been minimized.

Sign in to view

Open a new file download modal when shown right after closing by id, …

69947a0

…and guard the report PDF Onyx keys Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot marked this pull request as ready for review October 23, 2026 02:38

MelvinBot requested a review from a team as a code owner October 23, 2026 02:38

melvin-bot Bot requested a review from daledah October 23, 2026 02:38

melvin-bot Bot removed the request for review from a team October 23, 2026 02:38

melvin-bot Bot requested review from dylanexpensify and removed request for a team October 23, 2026 02:38

github-actions Bot reviewed Oct 23, 2026

View reviewed changes

Comment thread src/hooks/useReportPDFDownloadModal.ts Outdated

chatgpt-codex-connector Bot reviewed Oct 23, 2026

View reviewed changes

Comment thread src/hooks/useReportPDFDownloadModal.ts Outdated

This comment has been minimized.

Sign in to view

Split the report PDF modal comment into two sentences

a66607c

Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot requested a review from a team as a code owner October 23, 2026 02:55

melvin-bot Bot requested review from roryabraham and removed request for a team October 23, 2026 02:56

dylanexpensify reviewed Oct 23, 2026

View reviewed changes

Move the PDF download modal subscriptions into components the global …

7e2a88c

…modal stack renders Co-authored-by: daledah <daledah@users.noreply.github.com>

This comment has been minimized.

Sign in to view

Wrap openPDFDownload, revert the closeModalByID change, and keep reso…

4895720

…lveModal out of FileDownloadModal Co-authored-by: daledah <daledah@users.noreply.github.com>

roryabraham marked this pull request as draft October 23, 2026 20:30

This comment has been minimized.

Sign in to view

MelvinBot marked this pull request as ready for review October 24, 2026 11:23

daledah approved these changes Oct 24, 2026

View reviewed changes

Conversation

MelvinBot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

MelvinBot commented Oct 21, 2026

Uh oh!

MelvinBot commented Oct 21, 2026

Uh oh!

MelvinBot commented Oct 21, 2026

Uh oh!

MelvinBot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MelvinBot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

MelvinBot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 21, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

daledah commented Oct 22, 2026

Uh oh!

MelvinBot commented Oct 22, 2026

Uh oh!

MelvinBot commented Oct 22, 2026

Uh oh!

MelvinBot commented Oct 22, 2026

Uh oh!

This comment has been minimized.

MelvinBot commented Oct 22, 2026

Uh oh!

daledah commented Oct 23, 2026

Uh oh!

MelvinBot commented Oct 23, 2026

Uh oh!

melvin-bot Bot commented Oct 23, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

This comment has been minimized.

melvin-bot Bot commented Oct 23, 2026

Uh oh!

MelvinBot commented Oct 23, 2026

Uh oh!

dylanexpensify left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

MelvinBot commented Oct 23, 2026

Uh oh!

roryabraham commented Oct 23, 2026

Uh oh!

This comment has been minimized.

MelvinBot commented Oct 24, 2026

Uh oh!

daledah commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

daledah commented Oct 24, 2026

MelvinBot commented Oct 21, 2026 •

edited

Loading

MelvinBot commented Oct 21, 2026 •

edited

Loading

MelvinBot commented Oct 21, 2026 •

edited

Loading

MelvinBot commented Oct 21, 2026 •

edited

Loading

codecov Bot commented Oct 21, 2026 •

edited by github-actions Bot

Loading

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

edited

Loading

daledah commented Oct 24, 2026 •

edited

Loading