Skip to content

Migrate PaymentCardCurrencyModal to a @react-navigation modal screen#91120

Open
trasnake87 wants to merge 1 commit into
Expensify:mainfrom
trasnake87:paymentcard-currency-rnav-90470
Open

Migrate PaymentCardCurrencyModal to a @react-navigation modal screen#91120
trasnake87 wants to merge 1 commit into
Expensify:mainfrom
trasnake87:paymentcard-currency-rnav-90470

Conversation

@trasnake87
Copy link
Copy Markdown

@trasnake87 trasnake87 commented May 19, 2026

Explanation of Change

PaymentCardCurrencyModal was a react-native-modal (RIGHT_DOCKED) rendered inside PaymentCardChangeCurrencyForm, driven by local isCurrencyModalVisible/currency state. Because react-native-modal mounts and animates independently of the native stack, it animated inconsistently with the surrounding @react-navigation screens (the problem described in the parent migration #53493).

This migrates the picker to a dynamic @react-navigation modal route, following the pattern established by PR #88915 (DynamicExpenseLimitTypeSelectorPage):

  • Adds DYNAMIC_ROUTES.PAYMENT_CARD_CURRENCY_SELECTOR (path payment-card-currency) with the change-billing-currency screen as its only entry, and registers SETTINGS.SUBSCRIPTION.DYNAMIC_PAYMENT_CARD_CURRENCY_SELECTOR in SCREENS, the SettingsNavigatorParamList, the SettingsModalStackNavigator, and the linkingConfig. The new navigator entry is wrapped in withAgentAccessDenied to match the rest of the change-billing-currency flow.
  • Adds pages/settings/Subscription/PaymentCard/DynamicPaymentCardCurrencySelectorPage, which reads the selected currency from the CHANGE_BILLING_CURRENCY_FORM draft (falling back to the billing card currency from FUND_LIST, then USD), writes the chosen value to the draft via setDraftValues, navigates back via useDynamicBackPath, and applies the EUR_BILLING beta filter that previously lived in the modal.
  • Lifts the selected currency in PaymentCardChangeCurrencyForm from local useState to the CHANGE_BILLING_CURRENCY_FORM draft, navigates to the dynamic route on press (Navigation.navigate(createDynamicRoute(...))), drops the inline modal, and clears the draft on unmount. The non-security-code branch (used by the change-payment-currency screen) is unchanged in behavior — it still commits via changeBillingCurrency on selection.
  • Deletes src/components/AddPaymentCard/PaymentCardCurrencyModal.tsx; it has no remaining importers.
  • Adds a regression test covering the EUR_BILLING beta filter, the draft-vs-FUND_LIST selection precedence, and the setDraftValues + goBack behavior on row select.

Fixed Issues

$ #90470
PROPOSAL: #90470 (comment)

Tests

  1. Sign in as a Control/Collect workspace admin with a saved billing card.
  2. Open Settings → Subscription → click the three-dot menu on the billing card → Change billing currency.
  3. On the change-billing-currency screen, tap the Currency menu row.
  4. Verify the currency picker opens as a screen that slides in like other native screens (not a modal that animates independently).
  5. Select a different currency (e.g. AUD) and verify you return to the change-billing-currency screen with the Currency row now showing the picked value.
  6. Re-open the picker; verify the previously selected currency is highlighted.
  7. Enter a valid CVV, submit, and verify the billing currency is updated successfully.
  8. With the eurBilling beta disabled, verify EUR is not listed in the picker. With the beta enabled, verify EUR is listed.
  • Verify that no errors appear in the JS console

Offline tests

The currency picker is client-side navigation and writes only to a local form draft; behavior is identical offline. Submitting the change-billing-currency form offline behaves the same as before this change.

  • Verify that no errors appear in the JS console

QA Steps

Same as tests.

  • 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 followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
web_91120_trim.mp4

@trasnake87 trasnake87 requested review from a team as code owners May 19, 2026 21:14
@melvin-bot melvin-bot Bot requested review from gijoe0295 and trjExpensify May 19, 2026 21:14
@melvin-bot
Copy link
Copy Markdown

melvin-bot Bot commented May 19, 2026

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

@mountiny
Copy link
Copy Markdown
Contributor

Conflicts

Comment on lines +74 to +75

DynamicPaymentCardCurrencySelectorPage.displayName = 'DynamicPaymentCardCurrencySelectorPage';
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.

No need for this

Suggested change
DynamicPaymentCardCurrencySelectorPage.displayName = 'DynamicPaymentCardCurrencySelectorPage';

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the billing-currency picker used in the subscription payment card flow from an inline react-native-modal to a dynamic @react-navigation modal screen, so its animation and back behavior match the native stack.

Changes:

  • Added a new dynamic settings modal route/screen for selecting the billing currency (payment-card-currency) and wired it into screens, navigators, and deep linking config.
  • Updated PaymentCardChangeCurrencyForm to navigate to the new selector screen and persist the selected currency in the CHANGE_BILLING_CURRENCY_FORM draft.
  • Added a unit test covering EUR beta filtering, currency resolution precedence, and draft-write + back navigation behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/unit/pages/settings/DynamicPaymentCardCurrencySelectorPageTest.tsx Adds unit coverage for beta filtering, selection precedence, and selection side-effects (draft write + back).
src/SCREENS.ts Registers the new dynamic settings screen constant.
src/ROUTES.ts Adds DYNAMIC_ROUTES.PAYMENT_CARD_CURRENCY_SELECTOR dynamic suffix (payment-card-currency).
src/pages/settings/Subscription/PaymentCard/DynamicPaymentCardCurrencySelectorPage.tsx New selector page reading/writing the billing currency draft and applying EUR beta gating.
src/libs/Navigation/types.ts Adds the new screen to SettingsNavigatorParamList.
src/libs/Navigation/linkingConfig/config.ts Adds deep link mapping for the new dynamic selector route.
src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx Registers the new screen in the settings modal stack (wrapped with agent access denied).
src/components/AddPaymentCard/PaymentCardCurrencyModal.tsx Removes the old inline modal implementation.
src/components/AddPaymentCard/PaymentCardChangeCurrencyForm.tsx Navigates to the new selector and lifts selected currency into the billing currency form draft.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +40 to +46
const currency = formDraft?.[INPUT_IDS.CURRENCY] ?? initialCurrency ?? CONST.PAYMENT_CARD_CURRENCY.USD;

useEffect(() => {
return () => {
clearDraftValues(ONYXKEYS.FORMS.CHANGE_BILLING_CURRENCY_FORM);
};
}, []);
@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

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/ROUTES.ts 17.62% <ø> (ø)
src/SCREENS.ts 100.00% <ø> (ø)
src/libs/Navigation/linkingConfig/config.ts 76.92% <ø> (ø)
...entCard/DynamicPaymentCardCurrencySelectorPage.tsx 100.00% <100.00%> (ø)
...gation/AppNavigator/ModalStackNavigators/index.tsx 6.69% <0.00%> (-0.01%) ⬇️
...s/AddPaymentCard/PaymentCardChangeCurrencyForm.tsx 0.00% <0.00%> (ø)
... and 16 files with indirect coverage changes

@trasnake87 trasnake87 force-pushed the paymentcard-currency-rnav-90470 branch from 658dd22 to 6c19102 Compare May 20, 2026 19:07
@trasnake87
Copy link
Copy Markdown
Author

Rebased on main (conflict was just both branches adding new entries under DYNAMIC_ROUTES).

Addressed @mountiny's note — dropped .displayName and inlined the literal string for testID, matching the convention used by the other Dynamic* settings pages.

Also scoped the CHANGE_BILLING_CURRENCY_FORM draft to the security-code branch only (the one that pushes to the new selector screen). The inline picker branch (isSecurityCodeRequired=false, the "Change payment currency" flow under AddPaymentCard) no longer reads or clears the shared draft, so a stale draft from a parallel subscription-billing flow can't override initialCurrency and a concurrent unmount can't wipe an in-progress draft in another tab.

Replace the inline react-native-modal currency picker on the change-billing-
currency RHP with a dedicated @react-navigation modal screen registered as a
dynamic route, following the pattern established for ExpenseLimitTypeSelector
in Expensify#88915.

- Add DYNAMIC_ROUTES.PAYMENT_CARD_CURRENCY_SELECTOR (path
  'payment-card-currency') with the change-billing-currency screen as its only
  entry, register SETTINGS.SUBSCRIPTION.DYNAMIC_PAYMENT_CARD_CURRENCY_SELECTOR
  in SCREENS, the SettingsNavigatorParamList, the SettingsModalStackNavigator
  (gated by withAgentAccessDenied, matching the rest of the billing flow), and
  the linkingConfig.
- Add pages/settings/Subscription/PaymentCard/DynamicPaymentCardCurrencySelectorPage,
  which reads the selected currency from the CHANGE_BILLING_CURRENCY_FORM draft
  (falling back to the billing card currency from FUND_LIST, then USD), writes
  the chosen value to the draft via setDraftValues, navigates back via
  useDynamicBackPath, and applies the EUR_BILLING beta filter that previously
  lived in the modal.
- Lift the selected currency in PaymentCardChangeCurrencyForm from local
  useState to the CHANGE_BILLING_CURRENCY_FORM draft, navigate to the dynamic
  route on press, drop the inline modal, and clear the draft on unmount.
  The non-security-code branch (used by the change-payment-currency screen)
  is unchanged in behavior - it still commits via changeBillingCurrency on
  selection.
- Delete src/components/AddPaymentCard/PaymentCardCurrencyModal.tsx; it has no
  remaining importers.
- Add a regression test covering the EUR_BILLING beta filter, draft-vs-fund-list
  selection precedence, and the setDraftValues + goBack behavior on row select.

Fixed Issues: Expensify#90470
@trasnake87 trasnake87 force-pushed the paymentcard-currency-rnav-90470 branch from 6c19102 to da7a07a Compare May 20, 2026 19:22
@trjExpensify trjExpensify removed their request for review May 21, 2026 01:13
@trjExpensify
Copy link
Copy Markdown
Contributor

PR doesn’t need product input as a refactor PR. Unassigning and unsubscribing myself.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants