Migrate PaymentCardCurrencyModal to a @react-navigation modal screen#91120
Migrate PaymentCardCurrencyModal to a @react-navigation modal screen#91120trasnake87 wants to merge 1 commit into
Conversation
|
@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] |
|
Conflicts |
|
|
||
| DynamicPaymentCardCurrencySelectorPage.displayName = 'DynamicPaymentCardCurrencySelectorPage'; |
There was a problem hiding this comment.
No need for this
| DynamicPaymentCardCurrencySelectorPage.displayName = 'DynamicPaymentCardCurrencySelectorPage'; |
There was a problem hiding this comment.
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
PaymentCardChangeCurrencyFormto navigate to the new selector screen and persist the selected currency in theCHANGE_BILLING_CURRENCY_FORMdraft. - 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.
| const currency = formDraft?.[INPUT_IDS.CURRENCY] ?? initialCurrency ?? CONST.PAYMENT_CARD_CURRENCY.USD; | ||
|
|
||
| useEffect(() => { | ||
| return () => { | ||
| clearDraftValues(ONYXKEYS.FORMS.CHANGE_BILLING_CURRENCY_FORM); | ||
| }; | ||
| }, []); |
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.
|
658dd22 to
6c19102
Compare
|
Rebased on Addressed @mountiny's note — dropped Also scoped the |
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
6c19102 to
da7a07a
Compare
|
PR doesn’t need product input as a refactor PR. Unassigning and unsubscribing myself. |
Explanation of Change
PaymentCardCurrencyModalwas areact-native-modal(RIGHT_DOCKED) rendered insidePaymentCardChangeCurrencyForm, driven by localisCurrencyModalVisible/currencystate. Becausereact-native-modalmounts and animates independently of the native stack, it animated inconsistently with the surrounding@react-navigationscreens (the problem described in the parent migration #53493).This migrates the picker to a dynamic
@react-navigationmodal route, following the pattern established by PR #88915 (DynamicExpenseLimitTypeSelectorPage):DYNAMIC_ROUTES.PAYMENT_CARD_CURRENCY_SELECTOR(pathpayment-card-currency) with the change-billing-currency screen as its only entry, and registersSETTINGS.SUBSCRIPTION.DYNAMIC_PAYMENT_CARD_CURRENCY_SELECTORinSCREENS, theSettingsNavigatorParamList, theSettingsModalStackNavigator, and thelinkingConfig. The new navigator entry is wrapped inwithAgentAccessDeniedto match the rest of the change-billing-currency flow.pages/settings/Subscription/PaymentCard/DynamicPaymentCardCurrencySelectorPage, which reads the selected currency from theCHANGE_BILLING_CURRENCY_FORMdraft (falling back to the billing card currency fromFUND_LIST, then USD), writes the chosen value to the draft viasetDraftValues, navigates back viauseDynamicBackPath, and applies theEUR_BILLINGbeta filter that previously lived in the modal.PaymentCardChangeCurrencyFormfrom localuseStateto theCHANGE_BILLING_CURRENCY_FORMdraft, 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 viachangeBillingCurrencyon selection.src/components/AddPaymentCard/PaymentCardCurrencyModal.tsx; it has no remaining importers.EUR_BILLINGbeta filter, the draft-vs-FUND_LISTselection precedence, and thesetDraftValues+goBackbehavior on row select.Fixed Issues
$ #90470
PROPOSAL: #90470 (comment)
Tests
eurBillingbeta disabled, verify EUR is not listed in the picker. With the beta enabled, verify EUR is listed.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.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedScreenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
web_91120_trim.mp4