From 735afc4a2e3b10df1e7ed70e5b2481f01734aac8 Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Mon, 5 Jun 2023 14:45:58 -0400 Subject: [PATCH 1/5] Refactor `TransferBalancePage` into a functional component --- .../settings/Payments/TransferBalancePage.js | 289 +++++++++--------- 1 file changed, 144 insertions(+), 145 deletions(-) diff --git a/src/pages/settings/Payments/TransferBalancePage.js b/src/pages/settings/Payments/TransferBalancePage.js index eafa2de82b317..6096293f7c7de 100644 --- a/src/pages/settings/Payments/TransferBalancePage.js +++ b/src/pages/settings/Payments/TransferBalancePage.js @@ -1,5 +1,5 @@ import _ from 'underscore'; -import React from 'react'; +import React, {useEffect, useCallback} from 'react'; import {View, ScrollView} from 'react-native'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; @@ -65,65 +65,65 @@ const defaultProps = { walletTransfer: {}, }; -class TransferBalancePage extends React.Component { - constructor(props) { - super(props); +const TransferBalancePage = (props) => { - this.paymentTypes = [ - { - key: CONST.WALLET.TRANSFER_METHOD_TYPE.INSTANT, - title: this.props.translate('transferAmountPage.instant'), - description: this.props.translate('transferAmountPage.instantSummary', { - rate: this.props.numberFormat(CONST.WALLET.TRANSFER_METHOD_TYPE_FEE.INSTANT.RATE), - minAmount: CurrencyUtils.convertToDisplayString(CONST.WALLET.TRANSFER_METHOD_TYPE_FEE.INSTANT.MINIMUM_FEE), - }), - icon: Expensicons.Bolt, - type: CONST.PAYMENT_METHODS.DEBIT_CARD, - }, - { - key: CONST.WALLET.TRANSFER_METHOD_TYPE.ACH, - title: this.props.translate('transferAmountPage.ach'), - description: this.props.translate('transferAmountPage.achSummary'), - icon: Expensicons.Bank, - type: CONST.PAYMENT_METHODS.BANK_ACCOUNT, - }, - ]; - - PaymentMethods.resetWalletTransferData(); - const selectedAccount = this.getSelectedPaymentMethodAccount(); - - // Select the default payment account when page is opened, - // so that user can see that preselected on choose transfer account page - if (!selectedAccount || !selectedAccount.isDefault) { - return; - } - - PaymentMethods.saveWalletTransferAccountTypeAndID(selectedAccount.accountType, selectedAccount.methodID); - } + const paymentTypes = [ + { + key: CONST.WALLET.TRANSFER_METHOD_TYPE.INSTANT, + title: props.translate('transferAmountPage.instant'), + description: props.translate('transferAmountPage.instantSummary', { + rate: props.numberFormat(CONST.WALLET.TRANSFER_METHOD_TYPE_FEE.INSTANT.RATE), + minAmount: CurrencyUtils.convertToDisplayString(CONST.WALLET.TRANSFER_METHOD_TYPE_FEE.INSTANT.MINIMUM_FEE), + }), + icon: Expensicons.Bolt, + type: CONST.PAYMENT_METHODS.DEBIT_CARD, + }, + { + key: CONST.WALLET.TRANSFER_METHOD_TYPE.ACH, + title: props.translate('transferAmountPage.ach'), + description: props.translate('transferAmountPage.achSummary'), + icon: Expensicons.Bank, + type: CONST.PAYMENT_METHODS.BANK_ACCOUNT, + }, + ]; /** * Get the selected/default payment method account for wallet transfer * @returns {Object|undefined} */ - getSelectedPaymentMethodAccount() { - const paymentMethods = PaymentUtils.formatPaymentMethods(this.props.bankAccountList, this.props.cardList); + const getSelectedPaymentMethodAccount = useCallback(() => { + const paymentMethods = PaymentUtils.formatPaymentMethods(props.bankAccountList, props.cardList); const defaultAccount = _.find(paymentMethods, (method) => method.isDefault); const selectedAccount = _.find( paymentMethods, - (method) => method.accountType === this.props.walletTransfer.selectedAccountType && method.methodID === this.props.walletTransfer.selectedAccountID, + (method) => method.accountType === props.walletTransfer.selectedAccountType && method.methodID === props.walletTransfer.selectedAccountID, ); return selectedAccount || defaultAccount; - } + }, [props.bankAccountList, props.cardList, props.walletTransfer.selectedAccountType, props.walletTransfer.selectedAccountID]); + + useEffect(() => { + // Reset to the default account when the page is opened + PaymentMethods.resetWalletTransferData(); + }, []); + + useEffect(() => { + const selectedAccount = getSelectedPaymentMethodAccount(); + if (!selectedAccount || !selectedAccount.isDefault) { + return; + } + + PaymentMethods.saveWalletTransferAccountTypeAndID(selectedAccount.accountType, selectedAccount.methodID); + }, [getSelectedPaymentMethodAccount]); /** * @param {String} filterPaymentMethodType */ - navigateToChooseTransferAccount(filterPaymentMethodType) { + const navigateToChooseTransferAccount = (filterPaymentMethodType) => { PaymentMethods.saveWalletTransferMethodType(filterPaymentMethodType); // If we only have a single option for the given paymentMethodType do not force the user to make a selection - const combinedPaymentMethods = PaymentUtils.formatPaymentMethods(this.props.bankAccountList, this.props.cardList); + const combinedPaymentMethods = PaymentUtils.formatPaymentMethods(props.bankAccountList, props.cardList); const filteredMethods = _.filter(combinedPaymentMethods, (paymentMethod) => paymentMethod.accountType === filterPaymentMethodType); if (filteredMethods.length === 1) { @@ -133,120 +133,119 @@ class TransferBalancePage extends React.Component { } Navigation.navigate(ROUTES.SETTINGS_PAYMENTS_CHOOSE_TRANSFER_ACCOUNT); + }; + + if (props.walletTransfer.shouldShowSuccess && !props.walletTransfer.loading) { + return ( + + + + + ); } - render() { - if (this.props.walletTransfer.shouldShowSuccess && !this.props.walletTransfer.loading) { - return ( - - - - - ); - } - const selectedAccount = this.getSelectedPaymentMethodAccount(); - const selectedPaymentType = - selectedAccount && selectedAccount.accountType === CONST.PAYMENT_METHODS.BANK_ACCOUNT ? CONST.WALLET.TRANSFER_METHOD_TYPE.ACH : CONST.WALLET.TRANSFER_METHOD_TYPE.INSTANT; + const selectedAccount = getSelectedPaymentMethodAccount(); + const selectedPaymentType = + selectedAccount && selectedAccount.accountType === CONST.PAYMENT_METHODS.BANK_ACCOUNT ? CONST.WALLET.TRANSFER_METHOD_TYPE.ACH : CONST.WALLET.TRANSFER_METHOD_TYPE.INSTANT; - const calculatedFee = PaymentUtils.calculateWalletTransferBalanceFee(this.props.userWallet.currentBalance, selectedPaymentType); - const transferAmount = this.props.userWallet.currentBalance - calculatedFee; - const isTransferable = transferAmount > 0; - const isButtonDisabled = !isTransferable || !selectedAccount; - const errorMessage = !_.isEmpty(this.props.walletTransfer.errors) ? _.chain(this.props.walletTransfer.errors).values().first().value() : ''; + const calculatedFee = PaymentUtils.calculateWalletTransferBalanceFee(props.userWallet.currentBalance, selectedPaymentType); + const transferAmount = props.userWallet.currentBalance - calculatedFee; + const isTransferable = transferAmount > 0; + const isButtonDisabled = !isTransferable || !selectedAccount; + const errorMessage = !_.isEmpty(props.walletTransfer.errors) ? _.chain(props.walletTransfer.errors).values().first().value() : ''; - const shouldShowTransferView = - PaymentUtils.hasExpensifyPaymentMethod(this.props.cardList, this.props.bankAccountList) && this.props.userWallet.tierName === CONST.WALLET.TIER_NAME.GOLD; + const shouldShowTransferView = + PaymentUtils.hasExpensifyPaymentMethod(props.cardList, props.bankAccountList) && props.userWallet.tierName === CONST.WALLET.TIER_NAME.GOLD; - return ( - - Navigation.navigate(ROUTES.SETTINGS_PAYMENTS)} + return ( + + Navigation.navigate(ROUTES.SETTINGS_PAYMENTS)} + > + Navigation.goBack()} + onCloseButtonPress={() => Navigation.dismissModal(true)} + /> + + + + - Navigation.goBack()} - onCloseButtonPress={() => Navigation.dismissModal(true)} - /> - - - - - - {_.map(this.paymentTypes, (paymentType) => ( - this.navigateToChooseTransferAccount(paymentType.type)} - /> - ))} - - {this.props.translate('transferAmountPage.whichAccount')} - {Boolean(selectedAccount) && ( + + {_.map(paymentTypes, (paymentType) => ( this.navigateToChooseTransferAccount(selectedAccount.accountType)} + key={paymentType.key} + title={paymentType.title} + description={paymentType.description} + iconWidth={variables.iconSizeXLarge} + iconHeight={variables.iconSizeXLarge} + icon={paymentType.icon} + success={selectedPaymentType === paymentType.key} + wrapperStyle={{ + ...styles.mt3, + ...styles.pv4, + ...styles.transferBalancePayment, + ...(selectedPaymentType === paymentType.key && styles.transferBalanceSelectedPayment), + }} + onPress={() => navigateToChooseTransferAccount(paymentType.type)} /> - )} - - {this.props.translate('transferAmountPage.fee')} - {CurrencyUtils.convertToDisplayString(calculatedFee)} - - - - PaymentMethods.transferWalletBalance(selectedAccount)} - isDisabled={isButtonDisabled || this.props.network.isOffline} - message={errorMessage} - isAlertVisible={!_.isEmpty(errorMessage)} + ))} + + {props.translate('transferAmountPage.whichAccount')} + {Boolean(selectedAccount) && ( + navigateToChooseTransferAccount(selectedAccount.accountType)} /> + )} + + {props.translate('transferAmountPage.fee')} + {CurrencyUtils.convertToDisplayString(calculatedFee)} - - - ); - } -} + + + PaymentMethods.transferWalletBalance(selectedAccount)} + isDisabled={isButtonDisabled || props.network.isOffline} + message={errorMessage} + isAlertVisible={!_.isEmpty(errorMessage)} + /> + + + + ); +}; TransferBalancePage.propTypes = propTypes; TransferBalancePage.defaultProps = defaultProps; From a4326ce6708073319d5fe6b0ff726fa3bb0b9ee8 Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Mon, 5 Jun 2023 14:58:47 -0400 Subject: [PATCH 2/5] Add missing `displayName` property --- src/pages/settings/Payments/TransferBalancePage.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pages/settings/Payments/TransferBalancePage.js b/src/pages/settings/Payments/TransferBalancePage.js index 6096293f7c7de..eb55dc963bea8 100644 --- a/src/pages/settings/Payments/TransferBalancePage.js +++ b/src/pages/settings/Payments/TransferBalancePage.js @@ -249,6 +249,7 @@ const TransferBalancePage = (props) => { TransferBalancePage.propTypes = propTypes; TransferBalancePage.defaultProps = defaultProps; +TransferBalancePage.displayName = 'TransferBalancePage'; export default compose( withLocalize, From 77ae604938921452f9c1df02ef92bb3b1e5b3c4c Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Mon, 5 Jun 2023 15:08:58 -0400 Subject: [PATCH 3/5] Run prettier --- src/pages/settings/Payments/TransferBalancePage.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/pages/settings/Payments/TransferBalancePage.js b/src/pages/settings/Payments/TransferBalancePage.js index eb55dc963bea8..40fac733d6d35 100644 --- a/src/pages/settings/Payments/TransferBalancePage.js +++ b/src/pages/settings/Payments/TransferBalancePage.js @@ -66,7 +66,6 @@ const defaultProps = { }; const TransferBalancePage = (props) => { - const paymentTypes = [ { key: CONST.WALLET.TRANSFER_METHOD_TYPE.INSTANT, @@ -135,7 +134,7 @@ const TransferBalancePage = (props) => { Navigation.navigate(ROUTES.SETTINGS_PAYMENTS_CHOOSE_TRANSFER_ACCOUNT); }; - if (props.walletTransfer.shouldShowSuccess && !props.walletTransfer.loading) { + if (props.walletTransfer.shouldShowSuccess && !props.walletTransfer.loading) { return ( { const isButtonDisabled = !isTransferable || !selectedAccount; const errorMessage = !_.isEmpty(props.walletTransfer.errors) ? _.chain(props.walletTransfer.errors).values().first().value() : ''; - const shouldShowTransferView = - PaymentUtils.hasExpensifyPaymentMethod(props.cardList, props.bankAccountList) && props.userWallet.tierName === CONST.WALLET.TIER_NAME.GOLD; + const shouldShowTransferView = PaymentUtils.hasExpensifyPaymentMethod(props.cardList, props.bankAccountList) && props.userWallet.tierName === CONST.WALLET.TIER_NAME.GOLD; return ( From 5b62ce3d467315d67c28b3b78f38eecf045bfa0b Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Mon, 5 Jun 2023 16:05:51 -0400 Subject: [PATCH 4/5] Declare component with the `function` keyword instead of using an arrow function --- src/pages/settings/Payments/TransferBalancePage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/settings/Payments/TransferBalancePage.js b/src/pages/settings/Payments/TransferBalancePage.js index 40fac733d6d35..b294b1c208052 100644 --- a/src/pages/settings/Payments/TransferBalancePage.js +++ b/src/pages/settings/Payments/TransferBalancePage.js @@ -65,7 +65,7 @@ const defaultProps = { walletTransfer: {}, }; -const TransferBalancePage = (props) => { +function TransferBalancePage(props) { const paymentTypes = [ { key: CONST.WALLET.TRANSFER_METHOD_TYPE.INSTANT, @@ -243,7 +243,7 @@ const TransferBalancePage = (props) => { ); -}; +} TransferBalancePage.propTypes = propTypes; TransferBalancePage.defaultProps = defaultProps; From 657ec67928cbe473f8d9ddeefa00c5914790b33d Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Wed, 7 Jun 2023 14:22:28 -0400 Subject: [PATCH 5/5] PR comments - Consolidate into a single `useEffect` with empty array dependency, to only run once on initial render - Change `useCallback` function into a regular function --- .../settings/Payments/TransferBalancePage.js | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/pages/settings/Payments/TransferBalancePage.js b/src/pages/settings/Payments/TransferBalancePage.js index a7b4bcf44aa39..15b9346756d96 100644 --- a/src/pages/settings/Payments/TransferBalancePage.js +++ b/src/pages/settings/Payments/TransferBalancePage.js @@ -1,5 +1,5 @@ import _ from 'underscore'; -import React, {useCallback, useEffect} from 'react'; +import React, {useEffect} from 'react'; import {View, ScrollView} from 'react-native'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; @@ -90,7 +90,7 @@ function TransferBalancePage(props) { * Get the selected/default payment method account for wallet transfer * @returns {Object|undefined} */ - const getSelectedPaymentMethodAccount = useCallback(() => { + function getSelectedPaymentMethodAccount() { const paymentMethods = PaymentUtils.formatPaymentMethods(props.bankAccountList, props.cardList); const defaultAccount = _.find(paymentMethods, (method) => method.isDefault); @@ -99,26 +99,12 @@ function TransferBalancePage(props) { (method) => method.accountType === props.walletTransfer.selectedAccountType && method.methodID === props.walletTransfer.selectedAccountID, ); return selectedAccount || defaultAccount; - }, [props.bankAccountList, props.cardList, props.walletTransfer.selectedAccountType, props.walletTransfer.selectedAccountID]); - - useEffect(() => { - // Reset to the default account when the page is opened - PaymentMethods.resetWalletTransferData(); - }, []); - - useEffect(() => { - const selectedAccount = getSelectedPaymentMethodAccount(); - if (!selectedAccount || !selectedAccount.isDefault) { - return; - } - - PaymentMethods.saveWalletTransferAccountTypeAndID(selectedAccount.accountType, selectedAccount.methodID); - }, [getSelectedPaymentMethodAccount]); + } /** * @param {String} filterPaymentMethodType */ - const navigateToChooseTransferAccount = (filterPaymentMethodType) => { + function navigateToChooseTransferAccount(filterPaymentMethodType) { PaymentMethods.saveWalletTransferMethodType(filterPaymentMethodType); // If we only have a single option for the given paymentMethodType do not force the user to make a selection @@ -132,7 +118,20 @@ function TransferBalancePage(props) { } Navigation.navigate(ROUTES.SETTINGS_PAYMENTS_CHOOSE_TRANSFER_ACCOUNT); - }; + } + + useEffect(() => { + // Reset to the default account when the page is opened + PaymentMethods.resetWalletTransferData(); + + const selectedAccount = getSelectedPaymentMethodAccount(); + if (!selectedAccount) { + return; + } + + PaymentMethods.saveWalletTransferAccountTypeAndID(selectedAccount.accountType, selectedAccount.methodID); + // eslint-disable-next-line react-hooks/exhaustive-deps -- we only want this effect to run on initial render + }, []); if (props.walletTransfer.shouldShowSuccess && !props.walletTransfer.loading) { return (