Skip to content

[Due for payment 2026-10-30] [$175] [Batch 1] Refactor DecisionModal usage to useDecisionModal in simple alert modals (8 modals) #102975

Closed

Description

@MelvinBot

Part of #80285.

Problem

Each of these call sites mounts its own DecisionModal. They should use the global useDecisionModal instead. All eight are one-button alerts: no first option, no onModalHide, and fixed translation strings. Each one becomes a single showDecisionModal({title, prompt, secondOptionText}) call, and you can delete the local isVisible state.

Modals in this batch

Instance Notes
src/pages/workspace/WorkspaceMembersPage.tsx:829 "Download failed". It is triggered from the "Download CSV" menu item inside a useMemo, so add showDecisionModal to its deps. You can drop isSmallScreenWidth (L137-139).
src/pages/workspace/perDiem/WorkspacePerDiemPage.tsx:414 "Download failed". Same shape as above. useState becomes unused.
src/pages/workspace/categories/WorkspaceCategoriesPage.tsx:712 "Download failed". Keep isSmallScreenWidth, which is still used elsewhere, but update the stale comment at L67.
src/pages/workspace/tags/WorkspaceTagsPage.tsx:824 "Download failed". It has two failure callbacks (multi-level and single-level tags).
src/pages/workspace/tags/ImportTagsOptionsPage.tsx:298 "Download failed". It opens from a link inside a global ConfirmModal prompt. Keep the close(() => showDecisionModal(...)) wrapper. Without it, the ConfirmModal is pushed under the alert and reappears after "OK". Fold the 4 identical callbacks into one helper.
src/pages/workspace/expensifyCard/DynamicWorkspaceExpensifyCardDetailsPage.tsx:505 "Offline". It is triggered by "Deactivate" while offline. You can remove useState and useResponsiveLayout.
src/pages/Search/SearchTransactionsChangeReport.tsx:380 "Offline". It has three trigger sites (create, select, and remove report). You can also drop the fragment wrapper.
src/components/ImportOnyxState/BaseImportOnyxState.tsx:56 "Invalid file". Visibility comes from props owned by index.tsx and index.native.tsx. Call showDecisionModal in each parent's .catch, then remove the isErrorModalVisible props.

Shared helper

The "Offline" and "Download failed" bodies are already duplicated in useExportActions.ts:114-128 and MoneyReportHeaderModals.tsx:71-85. Consider adding a small hook that returns showOfflineModal and showDownloadFailedModal, and using it here and in those two files. Batches 2 and 4 can then reuse it.

Shared gotchas

  1. shouldHandleNavigationBack flips from undefined to true (useDecisionModal.ts:14), so browser Back now closes these alerts. This matches the modals that are already migrated.
  2. Props become a snapshot taken at show time.
  3. Only the top of the global stack renders (ModalContext.tsx:157).

Acceptance criteria

  • All eight instances use useDecisionModal(), and no DecisionModal import remains in these files.
  • Each alert still shows the same title, prompt, and button. On ImportTags, the ConfirmModal does not reappear after the alert closes.
  • Existing relevant tests pass. Add or update coverage where needed.
Upwork Automation - Do Not Edit
Issue OwnerCurrent Issue Owner: @ZhenjaHorbach

Activity

  1. melvin-bot commented on Oct 8, 2026

    @melvin-bot

    @roryabraham, @ZhenjaHorbach Whoops! This issue is 2 days overdue. Let's get this updated quick!

  2. ZhenjaHorbach commented on Oct 9, 2026

    @ZhenjaHorbach
    Contributor

    @MelvinBot
    Create the PR to migrate all modals from the issue description

  3. MelvinBot commented on Oct 9, 2026

    @MelvinBot
    ContributorAuthor

    🤖 Draft PR created: Migrate batch 1 one-button alerts from DecisionModal to useDecisionModal
    Please mark it as "Ready for review" or ask me to do that for you when it is ready for review.

    All eight modals in this issue now call useDecisionModal(), and none of these files import DecisionModal anymore. A new useAlertModals hook supplies the shared "Offline" and "Download failed" alerts, and useExportActions and MoneyReportHeaderModals use it too. ImportTagsOptionsPage keeps the close(...) wrapper inside one shared helper, so the ConfirmModal does not reappear after the alert closes.

    Lint, typecheck, React Compiler, spell check, and 10 related Jest suites (77 tests, including a new useAlertModals test) all pass. I couldn't run a browser check of the "Invalid file" alert. The test session was stuck on a blank page, and the web driver can't upload files. Please fill in the Tests and QA sections before marking the PR ready.


    view run · no recording available

  4. melvin-bot commented on Oct 12, 2026

    @melvin-bot

    @roryabraham, @ZhenjaHorbach Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

  5. ZhenjaHorbach commented on Oct 13, 2026

    @ZhenjaHorbach
    Contributor

    Not overdue
    Try to complete reviewing the PR today

  6. changed the title [-][Batch 1] Refactor DecisionModal usage to useDecisionModal in simple alert modals (8 modals)[/-] [+][$175] [Batch 1] Refactor DecisionModal usage to useDecisionModal in simple alert modals (8 modals)[/+] on Oct 15, 2026
  7. melvin-bot commented on Oct 15, 2026

    @melvin-bot
  8. changed the title [-][$175] [Batch 1] Refactor DecisionModal usage to useDecisionModal in simple alert modals (8 modals)[/-] [+][Due for payment 2026-10-30] [$175] [Batch 1] Refactor DecisionModal usage to useDecisionModal in simple alert modals (8 modals)[/+] on Oct 23, 2026
  9. melvin-bot commented on Oct 23, 2026

    @melvin-bot

    @ZhenjaHorbach

    The solution for this issue has been 🚀 deployed to production 🚀 in version 9.5.18-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

    If no regressions arise, payment will be issued on 2026-10-30. 🎊

    The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the Contributor+ Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]. If no checklist is needed for this issue, you can click: [no checklist button]

  10. melvin-bot commented on Oct 26, 2026

    @melvin-bot

    @ZhenjaHorbach Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

  11. roryabraham commented on Oct 27, 2026

    @roryabraham
    Contributor

    @MelvinBot what code changes remain to be completed in the scope of this issue?

  12. MelvinBot commented on Oct 27, 2026

    @MelvinBot
    ContributorAuthor

    No code changes remain. Migrate batch 1 one-button alerts from DecisionModal to useDecisionModal merged on 2026-10-21 and reached production in 9.5.18-1. It covers all eight modals and the shared helper.

    The remaining work is not code. Someone still needs to complete the Contributor+ checklist, and payment is due 2026-10-30.

    How I checked current main
    • None of the eight files import DecisionModal. Each one now calls useDecisionModal directly or through the new useAlertModals hook.
    • useExportActions and MoneyReportHeaderModals also use useAlertModals.
    • ImportTagsOptionsPage keeps the close(...) wrapper inside one helper (ImportTagsOptionsPage.tsx:59-62).
    • The isErrorModalVisible props are gone. ImportOnyxState/index.tsx and index.native.tsx call showDecisionModal themselves.
    • WorkspaceCategoriesPage and WorkspacePerDiemPage still have a local showOfflineModal (example). It uses showConfirmModal, not DecisionModal, so it is outside this issue's scope.
  13. ZhenjaHorbach commented on Oct 27, 2026

    @ZhenjaHorbach
    Contributor

    @roryabraham
    Let's close this

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

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions