Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(28902): [FlashList migration] PaymentMethodList #29095

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,8 @@ PODS:
- Firebase/Performance (= 8.8.0)
- React-Core
- RNFBApp
- RNFlashList (1.6.1):
- React-Core
- RNFS (2.20.0):
- React-Core
- RNGestureHandler (2.12.0):
Expand Down Expand Up @@ -925,6 +927,7 @@ DEPENDENCIES:
- "RNFBApp (from `../node_modules/@react-native-firebase/app`)"
- "RNFBCrashlytics (from `../node_modules/@react-native-firebase/crashlytics`)"
- "RNFBPerf (from `../node_modules/@react-native-firebase/perf`)"
- "RNFlashList (from `../node_modules/@shopify/flash-list`)"
- RNFS (from `../node_modules/react-native-fs`)
- RNGestureHandler (from `../node_modules/react-native-gesture-handler`)
- "RNGoogleSignin (from `../node_modules/@react-native-google-signin/google-signin`)"
Expand Down Expand Up @@ -1134,6 +1137,8 @@ EXTERNAL SOURCES:
:path: "../node_modules/@react-native-firebase/crashlytics"
RNFBPerf:
:path: "../node_modules/@react-native-firebase/perf"
RNFlashList:
:path: "../node_modules/@shopify/flash-list"
RNFS:
:path: "../node_modules/react-native-fs"
RNGestureHandler:
Expand Down Expand Up @@ -1273,6 +1278,7 @@ SPEC CHECKSUMS:
RNFBApp: 729c0666395b1953198dc4a1ec6deb8fbe1c302e
RNFBCrashlytics: 2061ca863e8e2fa1aae9b12477d7dfa8e88ca0f9
RNFBPerf: 389914cda4000fe0d996a752532a591132cbf3f9
RNFlashList: 236646d48f224a034f35baa0242e1b77db063b1e
RNFS: 4ac0f0ea233904cb798630b3c077808c06931688
RNGestureHandler: dec4645026e7401a0899f2846d864403478ff6a5
RNGoogleSignin: ccaa4a81582cf713eea562c5dd9dc1961a715fd0
Expand Down
1 change: 1 addition & 0 deletions jest/setup.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'setimmediate';
import 'react-native-gesture-handler/jestSetup';
import '@shopify/flash-list/jestSetup';
import * as reanimatedJestUtils from 'react-native-reanimated/src/reanimated2/jestUtils';
import mockClipboard from '@react-native-clipboard/clipboard/jest/clipboard-mock';
import setupMockImages from './setupMockImages';
Expand Down
60 changes: 60 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
"@react-navigation/stack": "6.3.16",
"@react-ng/bounds-observer": "^0.2.1",
"@rnmapbox/maps": "^10.0.11",
"@shopify/flash-list": "^1.6.1",
"@types/node": "^18.14.0",
"@ua/react-native-airship": "^15.2.6",
"awesome-phonenumber": "^5.4.0",
Expand Down
129 changes: 71 additions & 58 deletions src/pages/settings/Wallet/PaymentMethodList.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import _ from 'underscore';
import React, {useCallback, useMemo} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import {FlatList} from 'react-native';
import {FlashList} from '@shopify/flash-list';
import lodashGet from 'lodash/get';
import {withOnyx} from 'react-native-onyx';
import {withNetwork} from '../../../components/OnyxProvider';
import styles from '../../../styles/styles';
import variables from '../../../styles/variables';
import * as StyleUtils from '../../../styles/StyleUtils';
import MenuItem from '../../../components/MenuItem';
import Button from '../../../components/Button';
import Text from '../../../components/Text';
import compose from '../../../libs/compose';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import ONYXKEYS from '../../../ONYXKEYS';
import CONST from '../../../CONST';
import * as Expensicons from '../../../components/Icon/Expensicons';
Expand All @@ -21,6 +20,8 @@ import * as PaymentUtils from '../../../libs/PaymentUtils';
import FormAlertWrapper from '../../../components/FormAlertWrapper';
import OfflineWithFeedback from '../../../components/OfflineWithFeedback';
import * as PaymentMethods from '../../../libs/actions/PaymentMethods';
import useLocalize from '../../../hooks/useLocalize';
import useNetwork from '../../../hooks/useNetwork';
import Log from '../../../libs/Log';
import stylePropTypes from '../../../styles/stylePropTypes';
import Navigation from '../../../libs/Navigation/Navigation';
Expand Down Expand Up @@ -84,6 +85,9 @@ const propTypes = {
/** Callback for whenever FlatList component size changes */
onListContentSizeChange: PropTypes.func,

/** Should menu items be selectable with a checkbox */
shouldShowSelectedState: PropTypes.bool,

/** React ref being forwarded to the PaymentMethodList Button */
buttonRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),

Expand All @@ -92,8 +96,6 @@ const propTypes = {

/** List container style */
style: stylePropTypes,

...withLocalizePropTypes,
};

const defaultProps = {
Expand All @@ -118,6 +120,7 @@ const defaultProps = {
onListContentSizeChange: () => {},
shouldEnableScroll: true,
style: {},
shouldShowSelectedState: false,
};

/**
Expand Down Expand Up @@ -173,6 +176,15 @@ function shouldShowDefaultBadge(filteredPaymentMethods, isDefault = false) {
function isPaymentMethodActive(actionPaymentMethodType, activePaymentMethodID, paymentMethod) {
return paymentMethod.accountType === actionPaymentMethodType && paymentMethod.methodID === activePaymentMethodID;
}

/**
* @param {Object} item
* @returns {String}
*/
function keyExtractor(item) {
return item.key;
}

function PaymentMethodList({
actionPaymentMethodType,
activePaymentMethodID,
Expand All @@ -182,23 +194,22 @@ function PaymentMethodList({
fundList,
filterType,
isLoadingPaymentMethods,
listHeaderComponent,
network,
onListContentSizeChange,
onPress,
shouldEnableScroll,
shouldShowSelectedState,
shouldShowAddPaymentMethodButton,
shouldShowAddBankAccount,
shouldShowEmptyListMessage,
shouldShowAssignedCards,
selectedMethodID,
listHeaderComponent,
onListContentSizeChange,
shouldEnableScroll,
style,
translate,
}) {
const filteredPaymentMethods = useMemo(() => {
const paymentCardList = fundList || {};
const {translate} = useLocalize();
const {isOffline} = useNetwork();

const filteredPaymentMethods = useMemo(() => {
if (shouldShowAssignedCards) {
const assignedCards = _.chain(cardList)
// Filter by physical, active cards associated with a domain
Expand Down Expand Up @@ -229,6 +240,8 @@ function PaymentMethodList({
});
}

const paymentCardList = fundList || {};

// Hide any billing cards that are not P2P debit cards for now because you cannot make them your default method, or delete them
const filteredCardList = _.filter(paymentCardList, (card) => card.accountData.additionalData.isP2PDebitCard);
let combinedPaymentMethods = PaymentUtils.formatPaymentMethods(bankAccountList, filteredCardList);
Expand All @@ -237,25 +250,27 @@ function PaymentMethodList({
combinedPaymentMethods = _.filter(combinedPaymentMethods, (paymentMethod) => paymentMethod.accountType === filterType);
}

if (!network.isOffline) {
if (!isOffline) {
combinedPaymentMethods = _.filter(
combinedPaymentMethods,
(paymentMethod) => paymentMethod.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || !_.isEmpty(paymentMethod.errors),
);
}

combinedPaymentMethods = _.map(combinedPaymentMethods, (paymentMethod) => ({
...paymentMethod,
onPress: (e) => onPress(e, paymentMethod.accountType, paymentMethod.accountData, paymentMethod.isDefault, paymentMethod.methodID),
iconFill: isPaymentMethodActive(actionPaymentMethodType, activePaymentMethodID, paymentMethod) ? StyleUtils.getIconFillColor(CONST.BUTTON_STATES.PRESSED) : null,
wrapperStyle: isPaymentMethodActive(actionPaymentMethodType, activePaymentMethodID, paymentMethod)
? [StyleUtils.getButtonBackgroundColorStyle(CONST.BUTTON_STATES.PRESSED)]
: null,
disabled: paymentMethod.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
}));
combinedPaymentMethods = _.map(combinedPaymentMethods, (paymentMethod) => {
const isMethodActive = isPaymentMethodActive(actionPaymentMethodType, activePaymentMethodID, paymentMethod);

return {
...paymentMethod,
onPress: (e) => onPress(e, paymentMethod.accountType, paymentMethod.accountData, paymentMethod.isDefault, paymentMethod.methodID),
iconFill: isMethodActive ? StyleUtils.getIconFillColor(CONST.BUTTON_STATES.PRESSED) : null,
wrapperStyle: isMethodActive ? [StyleUtils.getButtonBackgroundColorStyle(CONST.BUTTON_STATES.PRESSED)] : null,
disabled: paymentMethod.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
};
});

return combinedPaymentMethods;
}, [fundList, shouldShowAssignedCards, bankAccountList, filterType, network.isOffline, cardList, translate, actionPaymentMethodType, activePaymentMethodID, onPress]);
}, [shouldShowAssignedCards, fundList, bankAccountList, filterType, isOffline, cardList, translate, actionPaymentMethodType, activePaymentMethodID, onPress]);

/**
* Render placeholder when there are no payments methods
Expand Down Expand Up @@ -318,25 +333,27 @@ function PaymentMethodList({

return (
<>
<FlatList
data={filteredPaymentMethods}
renderItem={renderItem}
keyExtractor={(item) => item.key}
ListEmptyComponent={shouldShowEmptyListMessage ? renderListEmptyComponent : null}
ListHeaderComponent={listHeaderComponent}
ListFooterComponent={shouldShowAddBankAccount ? renderListFooterComponent : null}
onContentSizeChange={onListContentSizeChange}
scrollEnabled={shouldEnableScroll}
style={style}
/>
<View style={[style, {minHeight: variables.paymentMethodHeight}]}>
Copy link
Contributor

@jjcoffee jjcoffee Jan 22, 2024

Choose a reason for hiding this comment

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

I believe this may have caused #33075 as variables.paymentMethodHeight is only equal to 1 item's height, but we can have multiple cards. More details here.

<FlashList
estimatedItemSize={variables.paymentMethodHeight}
data={filteredPaymentMethods}
renderItem={renderItem}
keyExtractor={keyExtractor}
ListEmptyComponent={shouldShowEmptyListMessage ? renderListEmptyComponent : null}
ListHeaderComponent={listHeaderComponent}
ListFooterComponent={shouldShowAddBankAccount ? renderListFooterComponent : null}
onContentSizeChange={onListContentSizeChange}
scrollEnabled={shouldEnableScroll}
/>
</View>
{shouldShowAddPaymentMethodButton && (
<FormAlertWrapper>
{(isOffline) => (
{(isFormOffline) => (
<Button
text={translate('paymentMethodList.addPaymentMethod')}
icon={Expensicons.CreditCard}
onPress={onPress}
isDisabled={isLoadingPaymentMethods || isOffline}
isDisabled={isLoadingPaymentMethods || isFormOffline}
style={[styles.mh4, styles.buttonCTA]}
iconStyles={[styles.buttonCTAIcon]}
key="addPaymentMethodButton"
Expand All @@ -356,24 +373,20 @@ PaymentMethodList.propTypes = propTypes;
PaymentMethodList.defaultProps = defaultProps;
PaymentMethodList.displayName = 'PaymentMethodList';

export default compose(
withLocalize,
withNetwork(),
withOnyx({
bankAccountList: {
key: ONYXKEYS.BANK_ACCOUNT_LIST,
},
cardList: {
key: ONYXKEYS.CARD_LIST,
},
fundList: {
key: ONYXKEYS.FUND_LIST,
},
isLoadingPaymentMethods: {
key: ONYXKEYS.IS_LOADING_PAYMENT_METHODS,
},
userWallet: {
key: ONYXKEYS.USER_WALLET,
},
}),
)(PaymentMethodList);
export default withOnyx({
bankAccountList: {
key: ONYXKEYS.BANK_ACCOUNT_LIST,
},
cardList: {
key: ONYXKEYS.CARD_LIST,
},
fundList: {
key: ONYXKEYS.FUND_LIST,
},
isLoadingPaymentMethods: {
key: ONYXKEYS.IS_LOADING_PAYMENT_METHODS,
},
userWallet: {
key: ONYXKEYS.USER_WALLET,
},
})(PaymentMethodList);
2 changes: 1 addition & 1 deletion src/styles/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3150,7 +3150,7 @@ const styles = (theme: ThemeDefault) =>

paymentMethod: {
paddingHorizontal: 20,
height: 64,
height: variables.paymentMethodHeight,
},

archivedReportFooter: {
Expand Down
1 change: 1 addition & 0 deletions src/styles/variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export default {
anonymousReportFooterBreakpoint: 650,
dropDownButtonDividerHeight: 28,
addPaymentMethodLeftSpacing: 2,
paymentMethodHeight: 64,
Copy link
Contributor

Choose a reason for hiding this comment

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

@adhorodyski I just realized we already had a const for this optionRowHeight – in your next PR can you please get rid of this paymentMethodHeight and instead use optionRowHeight?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, will update in the morning! 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see my follow up PR #30486 :)

addBankAccountLeftSpacing: 3,
eReceiptThumbnailSmallBreakpoint: 110,
eReceiptThumbnailMediumBreakpoint: 335,
Expand Down
Loading