Fix: Add payment dynamic #7566
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,39 +1,16 @@ | ||
| import React from 'react'; | ||
| import {withOnyx} from 'react-native-onyx'; | ||
| import {ActivityIndicator} from 'react-native'; | ||
| import themeColors from '../../styles/themes/default'; | ||
| import CONST from '../../CONST'; | ||
| import Navigation from '../../libs/Navigation/Navigation'; | ||
| import AddPaymentMethodMenu from '../AddPaymentMethodMenu'; | ||
| import getClickedElementLocation from '../../libs/getClickedElementLocation'; | ||
| import * as PaymentUtils from '../../libs/PaymentUtils'; | ||
| import * as PaymentMethods from '../../libs/actions/PaymentMethods'; | ||
| import ONYXKEYS from '../../ONYXKEYS'; | ||
| import Log from '../../libs/Log'; | ||
| import {propTypes, defaultProps} from './kycWallPropTypes'; | ||
|
|
||
| // This component allows us to block various actions by forcing the user to first add a default payment method and successfully make it through our Know Your Customer flow | ||
| // before continuing to take whatever action they originally intended to take. It requires a button as a child and a native event so we can get the coordinates and use it | ||
|
|
@@ -57,6 +34,9 @@ class KYCWall extends React.Component { | |
| } | ||
|
|
||
| componentWillUnmount() { | ||
| if (this.props.listenResize) { | ||
| window.removeEventListener('resize', null); | ||
| } | ||
| PaymentMethods.kycWallRef.current = null; | ||
| } | ||
|
|
||
|
|
@@ -78,6 +58,19 @@ class KYCWall extends React.Component { | |
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Set position of the transfer payment menu | ||
| * | ||
| * @param {Object} position | ||
| */ | ||
|
|
||
| setPositionAddPaymentMenu(position) { | ||
| this.setState({ | ||
| anchorPositionTop: position.anchorPositionTop, | ||
| anchorPositionLeft: position.anchorPositionLeft, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Take the position of the button that calls this method and show the Add Payment method menu when the user has no valid payment method. | ||
| * If they do have a valid payment method they are navigated to the "enable payments" route to complete KYC checks. | ||
|
|
@@ -89,13 +82,19 @@ class KYCWall extends React.Component { | |
| // Check to see if user has a valid payment method on file and display the add payment popover if they don't | ||
| if (!PaymentUtils.hasExpensifyPaymentMethod(this.props.cardList, this.props.bankAccountList)) { | ||
| Log.info('[KYC Wallet] User does not have valid payment method'); | ||
| let clickedElementLocation = getClickedElementLocation(event.nativeEvent); | ||
| let position = this.getAnchorPosition(clickedElementLocation); | ||
| if (this.props.listenResize) { | ||
| window.addEventListener('resize', () => { | ||
|
There was a problem hiding this comment. Oh, hmm, this is gonna bind a new event listener each time. That can have unintended consequences. The best practice would be to set an event listener once on component init and then clean it up on unmount... Here's an example of what that looks like using a class component https://reactjs.org/docs/hooks-effect.html#example-using-classes-1 There was a problem hiding this comment. hmm, we already added a remove listener when the modal close. The scenario is add listener when the modal appears and remove listener when the modal closes. There was a problem hiding this comment. Please fix this. Here are the instructions...
Generally speaking even if we are focusing on the web side for consistency we should use the https://reactnative.dev/docs/0.66/dimensions#addeventlistener There was a problem hiding this comment. @phivh right now, we're adding a event listener each time the window is resized. There was a problem hiding this comment. ok, let me take into this one There was a problem hiding this comment. @marcaaron @deetergp @rushatgabhane created follow-up PR for this one. Please take a look when you guys have a chance. |
||
| clickedElementLocation = getClickedElementLocation(event.nativeEvent); | ||
| position = this.getAnchorPosition(clickedElementLocation); | ||
| this.setPositionAddPaymentMenu(position); | ||
| }); | ||
| } | ||
| this.setState({ | ||
| shouldShowAddPaymentMenu: true, | ||
| }); | ||
| this.setPositionAddPaymentMenu(position); | ||
| return; | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import React from 'react'; | ||
| import {propTypes, defaultProps} from './kycWallPropTypes'; | ||
| import BaseKYCWall from './BaseKYCWall'; | ||
|
|
||
| const KYCWall = props => ( | ||
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| <BaseKYCWall {...props} listenResize /> | ||
| ); | ||
|
|
||
| KYCWall.propTypes = propTypes; | ||
| KYCWall.defaultProps = defaultProps; | ||
| KYCWall.displayName = 'KYCWall'; | ||
|
|
||
| export default KYCWall; | ||
|
There was a problem hiding this comment. New line above |
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import BaseKYCWall from './BaseKYCWall'; | ||
|
|
||
| export default BaseKYCWall; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import PropTypes from 'prop-types'; | ||
| import userWalletPropTypes from '../../pages/EnablePayments/userWalletPropTypes'; | ||
|
|
||
| const propTypes = { | ||
| /** Route for the Add Bank Account screen for a given navigation stack */ | ||
| addBankAccountRoute: PropTypes.string.isRequired, | ||
|
|
||
| /** Route for the Add Debit Card screen for a given navigation stack */ | ||
| addDebitCardRoute: PropTypes.string.isRequired, | ||
|
|
||
| /** Route for the KYC enable payments screen for a given navigation stack */ | ||
| enablePaymentsRoute: PropTypes.string.isRequired, | ||
|
|
||
| /** Where to place the popover */ | ||
| popoverPlacement: PropTypes.string, | ||
|
|
||
| /** Listen for window resize event on web and desktop */ | ||
| listenResize: PropTypes.bool, | ||
|
|
||
| ...userWalletPropTypes, | ||
| }; | ||
|
|
||
| const defaultProps = { | ||
| // eslint-disable-next-line react/default-props-match-prop-types | ||
| userWallet: {}, | ||
| popoverPlacement: 'top', | ||
| listenResize: false, | ||
| }; | ||
|
|
||
| export {propTypes, defaultProps}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import React from 'react'; | ||
| import BasePaymentsPage from './BasePaymentsPage'; | ||
|
|
||
| const PaymentsPage = () => ( | ||
| <BasePaymentsPage listenResize /> | ||
|
There was a problem hiding this comment. Not going to leave a full review here, but please see this draft PR which explains proper usage of boolean props to enable or disable features... @rushatgabhane please 👀 and thank you 🙇 There was a problem hiding this comment. And applied this would be changed to something like |
||
| ); | ||
|
|
||
| PaymentsPage.displayName = 'PaymentsPage'; | ||
|
|
||
| export default PaymentsPage; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import BasePaymentsPage from './BasePaymentsPage'; | ||
|
|
||
| export default BasePaymentsPage; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import PropTypes from 'prop-types'; | ||
| import walletTransferPropTypes from '../walletTransferPropTypes'; | ||
| import {withLocalizePropTypes} from '../../../../components/withLocalize'; | ||
| import {windowDimensionsPropTypes} from '../../../../components/withWindowDimensions'; | ||
|
|
||
| const propTypes = { | ||
| /** Wallet balance transfer props */ | ||
| walletTransfer: walletTransferPropTypes, | ||
|
|
||
| /** List of betas available to current user */ | ||
| betas: PropTypes.arrayOf(PropTypes.string), | ||
|
|
||
| /** Are we loading payment methods? */ | ||
| isLoadingPaymentMethods: PropTypes.bool, | ||
|
|
||
| /** Listen for window resize event on web and desktop. */ | ||
| listenResize: PropTypes.bool, | ||
|
|
||
| ...withLocalizePropTypes, | ||
|
|
||
| ...windowDimensionsPropTypes, | ||
| }; | ||
|
|
||
| const defaultProps = { | ||
| walletTransfer: { | ||
| shouldShowConfirmModal: false, | ||
| }, | ||
| betas: [], | ||
| isLoadingPaymentMethods: true, | ||
| listenResize: false, | ||
| }; | ||
|
|
||
| export {propTypes, defaultProps}; |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove line break