Merge pull request #49487 from allgandalf/collaboratewithfabio

[NoQA] Fix Virtualised List Error
Expensify · Sep 20, 2024 · eaa7969 · eaa7969
2 parents 8383570 + 98e60bb
commit eaa7969
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 47 deletions.
diff --git a/contributingGuides/STYLE.md b/contributingGuides/STYLE.md
@@ -50,6 +50,9 @@
     - [Stateless components vs Pure Components vs Class based components vs Render Props](#stateless-components-vs-pure-components-vs-class-based-components-vs-render-props---when-to-use-what)
     - [Use Refs Appropriately](#use-refs-appropriately)
     - [Are we allowed to use [insert brand new React feature]?](#are-we-allowed-to-use-insert-brand-new-react-feature-why-or-why-not)
+- [Handling Scroll Issues with Nested Lists in React Native](#handling-scroll-issues-with-nested-lists-in-react-native)
+    - [Wrong Approach (Using SelectionList)](#wrong-approach-using-selectionlist)
+    - [Correct Approach (Using SelectionList)](#correct-approach-using-selectionlist)
 - [React Hooks: Frequently Asked Questions](#react-hooks-frequently-asked-questions)
 - [Onyx Best Practices](#onyx-best-practices)
     - [Collection Keys](#collection-keys)
@@ -1105,6 +1108,48 @@ There are several ways to use and declare refs and we prefer the [callback metho
 
 We love React and learning about all the new features that are regularly being added to the API. However, we try to keep our organization's usage of React limited to the most stable set of features that React offers. We do this mainly for **consistency** and so our engineers don't have to spend extra time trying to figure out how everything is working. That said, if you aren't sure if we have adopted something, please ask us first.
 
+
+## Handling Scroll Issues with Nested Lists in React Native
+
+### Problem
+
+When using `SelectionList` alongside other components (e.g., `Text`, `Button`), wrapping them inside a `ScrollView` can lead to alignment and performance issues. Additionally, using `ScrollView` with nested `FlatList` or `SectionList` causes the error:
+
+> "VirtualizedLists should never be nested inside plain ScrollViews with the same orientation."
+
+### Solution
+
+The correct approach is avoid using `ScrollView`. You can add props like `listHeaderComponent` and `listFooterComponent` to add other components before or after the list while keeping the layout scrollable.
+
+### Wrong Approach (Using `SelectionList`)
+
+```jsx
+<ScrollView>
+    <Text>Header Content</Text>
+    <SelectionList
+        sections={[{data}]}
+        ListItem={RadioListItem}
+        onSelectRow={handleSelect}
+    />
+    <Button title="Submit" onPress={handleSubmit} />
+</ScrollView>
+```
+
+### Correct Approach (Using `SelectionList`)
+
+```jsx
+<SelectionList 
+    sections={[{item}]} 
+    ListItem={RadioListItem} 
+    onSelectRow={handleSelect}
+    listHeaderComponent={<Text>Header Content</Text>}
+    listFooterComponent={<Button title="Submit" onPress={handleSubmit} />}
+/>
+```
+
+This ensures optimal performance and avoids layout issues.
+
+
 ## React Hooks: Frequently Asked Questions
 
 ### Are Hooks a Replacement for HOCs or Render Props?

diff --git a/src/pages/settings/Wallet/ChooseTransferAccountPage.tsx b/src/pages/settings/Wallet/ChooseTransferAccountPage.tsx
@@ -8,9 +8,9 @@ import getBankIcon from '@components/Icon/BankIcons';
 import * as Expensicons from '@components/Icon/Expensicons';
 import MenuItem from '@components/MenuItem';
 import ScreenWrapper from '@components/ScreenWrapper';
-import ScrollView from '@components/ScrollView';
 import SelectionList from '@components/SelectionList';
 import RadioListItem from '@components/SelectionList/RadioListItem';
+import type {ListItem} from '@components/SelectionList/types';
 import useLocalize from '@hooks/useLocalize';
 import useThemeStyles from '@hooks/useThemeStyles';
 import {getLastFourDigits} from '@libs/BankAccountUtils';
@@ -20,10 +20,15 @@ import * as PaymentMethods from '@userActions/PaymentMethods';
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
-import type {AccountData} from '@src/types/onyx';
+import type {AccountData, BankAccount} from '@src/types/onyx';
 import type {BankName} from '@src/types/onyx/Bank';
 import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
 
+type BankAccountListItem = ListItem & {
+    value?: number;
+    bankAccount: BankAccount;
+};
+
 function ChooseTransferAccountPage() {
     const [walletTransfer, walletTransferResult] = useOnyx(ONYXKEYS.WALLET_TRANSFER);
 
@@ -54,7 +59,7 @@ function ChooseTransferAccountPage() {
     const [bankAccountsList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST);
     const selectedAccountID = walletTransfer?.selectedAccountID;
     const data = useMemo(() => {
-        const options = Object.values(bankAccountsList ?? {}).map((bankAccount) => {
+        const options = Object.values(bankAccountsList ?? {}).map((bankAccount): BankAccountListItem => {
             const bankName = (bankAccount.accountData?.additionalData?.bankName ?? '') as BankName;
             const bankAccountNumber = bankAccount.accountData?.accountNumber ?? '';
             const bankAccountID = bankAccount.accountData?.bankAccountID ?? bankAccount.methodID;
@@ -91,29 +96,30 @@ function ChooseTransferAccountPage() {
                 title={translate('chooseTransferAccountPage.chooseAccount')}
                 onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS_WALLET_TRANSFER_BALANCE)}
             />
-            <ScrollView>
-                <SelectionList
-                    sections={[{data}]}
-                    ListItem={RadioListItem}
-                    onSelectRow={(value) => {
-                        const accountType = value?.bankAccount?.accountType;
-                        const accountData = value?.bankAccount?.accountData;
-                        selectAccountAndNavigateBack(accountType, accountData);
-                    }}
-                    shouldSingleExecuteRowSelect
-                    shouldUpdateFocusedIndex
-                    initiallyFocusedOptionKey={walletTransfer?.selectedAccountID?.toString()}
-                />
-                <MenuItem
-                    onPress={navigateToAddPaymentMethodPage}
-                    title={
-                        walletTransfer?.filterPaymentMethodType === CONST.PAYMENT_METHODS.PERSONAL_BANK_ACCOUNT
-                            ? translate('paymentMethodList.addNewBankAccount')
-                            : translate('paymentMethodList.addNewDebitCard')
-                    }
-                    icon={Expensicons.Plus}
-                />
-            </ScrollView>
+
+            <SelectionList
+                sections={[{data}]}
+                ListItem={RadioListItem}
+                onSelectRow={(value) => {
+                    const accountType = value?.bankAccount?.accountType;
+                    const accountData = value?.bankAccount?.accountData;
+                    selectAccountAndNavigateBack(accountType, accountData);
+                }}
+                shouldSingleExecuteRowSelect
+                shouldUpdateFocusedIndex
+                initiallyFocusedOptionKey={walletTransfer?.selectedAccountID?.toString()}
+                listFooterContent={
+                    <MenuItem
+                        onPress={navigateToAddPaymentMethodPage}
+                        title={
+                            walletTransfer?.filterPaymentMethodType === CONST.PAYMENT_METHODS.PERSONAL_BANK_ACCOUNT
+                                ? translate('paymentMethodList.addNewBankAccount')
+                                : translate('paymentMethodList.addNewDebitCard')
+                        }
+                        icon={Expensicons.Plus}
+                    />
+                }
+            />
         </ScreenWrapper>
     );
 }

diff --git a/src/pages/workspace/expensifyCard/WorkspaceSettlementAccountPage.tsx b/src/pages/workspace/expensifyCard/WorkspaceSettlementAccountPage.tsx
@@ -6,7 +6,6 @@ import HeaderWithBackButton from '@components/HeaderWithBackButton';
 import Icon from '@components/Icon';
 import getBankIcon from '@components/Icon/BankIcons';
 import ScreenWrapper from '@components/ScreenWrapper';
-import ScrollView from '@components/ScrollView';
 import SelectionList from '@components/SelectionList';
 import RadioListItem from '@components/SelectionList/RadioListItem';
 import Text from '@components/Text';
@@ -98,26 +97,28 @@ function WorkspaceSettlementAccountPage({route}: WorkspaceSettlementAccountPageP
                     title={translate('workspace.expensifyCard.settlementAccount')}
                     onBackButtonPress={() => Navigation.goBack(ROUTES.WORKSPACE_EXPENSIFY_CARD_SETTINGS.getRoute(policyID))}
                 />
-                <ScrollView>
-                    <Text style={[styles.mh5, styles.mv4]}>{translate('workspace.expensifyCard.settlementAccountDescription')}</Text>
-                    {isUsedContinuousReconciliation && (
-                        <Text style={[styles.mh5, styles.mb6]}>
-                            <Text>{translate('workspace.expensifyCard.settlementAccountInfoPt1')}</Text>{' '}
-                            <TextLink onPress={() => Navigation.navigate(ROUTES.WORKSPACE_ACCOUNTING_RECONCILIATION_ACCOUNT_SETTINGS.getRoute(policyID, connectionParam))}>
-                                {translate('workspace.expensifyCard.reconciliationAccount')}
-                            </TextLink>{' '}
-                            <Text>{`(${CONST.MASKED_PAN_PREFIX}${getLastFourDigits(paymentBankAccountNumber)}) `}</Text>
-                            <Text>{translate('workspace.expensifyCard.settlementAccountInfoPt2')}</Text>
-                        </Text>
-                    )}
-                    <SelectionList
-                        sections={[{data}]}
-                        ListItem={RadioListItem}
-                        onSelectRow={({value}) => updateSettlementAccount(value ?? 0)}
-                        shouldSingleExecuteRowSelect
-                        initiallyFocusedOptionKey={paymentBankAccountID.toString()}
-                    />
-                </ScrollView>
+                <SelectionList
+                    sections={[{data}]}
+                    ListItem={RadioListItem}
+                    onSelectRow={({value}) => updateSettlementAccount(value ?? 0)}
+                    shouldSingleExecuteRowSelect
+                    initiallyFocusedOptionKey={paymentBankAccountID.toString()}
+                    listHeaderContent={
+                        <>
+                            <Text style={[styles.mh5, styles.mv4]}>{translate('workspace.expensifyCard.settlementAccountDescription')}</Text>
+                            {isUsedContinuousReconciliation && (
+                                <Text style={[styles.mh5, styles.mb6]}>
+                                    <Text>{translate('workspace.expensifyCard.settlementAccountInfoPt1')}</Text>{' '}
+                                    <TextLink onPress={() => Navigation.navigate(ROUTES.WORKSPACE_ACCOUNTING_RECONCILIATION_ACCOUNT_SETTINGS.getRoute(policyID, connectionParam))}>
+                                        {translate('workspace.expensifyCard.reconciliationAccount')}
+                                    </TextLink>{' '}
+                                    <Text>{`(${CONST.MASKED_PAN_PREFIX}${getLastFourDigits(paymentBankAccountNumber)}) `}</Text>
+                                    <Text>{translate('workspace.expensifyCard.settlementAccountInfoPt2')}</Text>
+                                </Text>
+                            )}
+                        </>
+                    }
+                />
             </ScreenWrapper>
         </AccessOrNotFoundWrapper>
     );