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

[NoQA] Fix Virtualised List Error #49487

Merged
merged 9 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
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
allgandalf marked this conversation as resolved.
Show resolved Hide resolved

## Problem
allgandalf marked this conversation as resolved.
Show resolved Hide resolved

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:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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:
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 this error:


> "VirtualizedLists should never be nested inside plain ScrollViews with the same orientation."

## Solution
allgandalf marked this conversation as resolved.
Show resolved Hide resolved

The correct approach is to replace 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.
allgandalf marked this conversation as resolved.
Show resolved Hide resolved

## Wrong Approach (Using `SelectionList`)
allgandalf marked this conversation as resolved.
Show resolved Hide resolved

```jsx
<ScrollView>
<Text>Header Content</Text>
<SelectionList
sections={[{data}]}
ListItem={RadioListItem}
onSelectRow={handleSelect}
/>
<Button title="Submit" onPress={handleSubmit} />
</ScrollView>
```

## Correct Approach (Using `SelectionList`)
allgandalf marked this conversation as resolved.
Show resolved Hide resolved

```jsx
<SelectionList
sections={[{item}]}
ListItem={RadioListItem}
onSelectRow={handleSelect}
ListHeaderComponent={<Text>Header Content</Text>}
ListFooterComponent={<Button title="Submit" onPress={handleSubmit} />}
allgandalf marked this conversation as resolved.
Show resolved Hide resolved
/>
```

This ensures optimal performance and avoids layout issues.


Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docs looks like the below @mountiny

Screenshot 2024-09-20 at 9 53 48 PM

## 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 AccountListItem = ListItem & {
allgandalf marked this conversation as resolved.
Show resolved Hide resolved
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): AccountListItem => {
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>
allgandalf marked this conversation as resolved.
Show resolved Hide resolved
<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
Loading