Skip to content

Commit

Permalink
Merge pull request #49487 from allgandalf/collaboratewithfabio
Browse files Browse the repository at this point in the history
[NoQA] Fix Virtualised List Error
  • Loading branch information
mountiny authored Sep 20, 2024
2 parents 8383570 + 98e60bb commit eaa7969
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 47 deletions.
45 changes: 45 additions & 0 deletions contributingGuides/STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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?
Expand Down
58 changes: 32 additions & 26 deletions src/pages/settings/Wallet/ChooseTransferAccountPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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>
);
Expand Down

0 comments on commit eaa7969

Please sign in to comment.