Skip to content

Commit

Permalink
perf: improve renderItem and use FlatList
Browse files Browse the repository at this point in the history
  • Loading branch information
hurali97 committed Feb 15, 2024
1 parent f33930e commit 6921051
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 155 deletions.
114 changes: 18 additions & 96 deletions src/components/LHNOptionsList/LHNOptionsList.tsx
Original file line number Diff line number Diff line change
@@ -1,41 +1,27 @@
import {FlashList} from '@shopify/flash-list';
import type {ReactElement} from 'react';
import React, {memo, useCallback} from 'react';
import {StyleSheet, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import withCurrentReportID from '@components/withCurrentReportID';
import usePermissions from '@hooks/usePermissions';
import {useReports} from '@hooks/useReports';
import React, {useCallback, memo} from 'react';
import {FlatList, StyleSheet, View} from 'react-native';
import useThemeStyles from '@hooks/useThemeStyles';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import OptionRowLHNData from './OptionRowLHNData';

Check failure on line 5 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

`./OptionRowLHNData` import should occur after import of `@shopify/flash-list`
import type {LHNOptionsListOnyxProps, LHNOptionsListProps, RenderItemProps} from './types';
import type {LHNOptionsListProps, RenderItemProps} from './types';

Check failure on line 6 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

`./types` type import should occur after import of `@shopify/flash-list`
import { OrderedReports } from '@libs/SidebarUtils';

Check failure on line 7 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

All imports in the declaration are only used as types. Use `import type`
import CONST from '@src/CONST';

Check failure on line 8 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

'CONST' is defined but never used
import variables from '@styles/variables';

Check failure on line 9 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

'variables' is defined but never used
import { FlashList } from '@shopify/flash-list';

Check failure on line 10 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

'FlashList' is defined but never used

const keyExtractor = (item: string) => `report_${item}`;
const keyExtractor = (item: OrderedReports) => `report_${item?.reportID}`;

function LHNOptionsList({
style,
contentContainerStyles,
data,
onSelectRow,
optionMode,
shouldDisableFocusOptions = false,
reportActions = {},
policy = {},
preferredLocale = CONST.LOCALES.DEFAULT,
personalDetails = {},
transactions = {},
currentReportID = '',
draftComments = {},
transactionViolations = {},
optionMode,

Check failure on line 21 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

'optionMode' is defined but never used
onFirstItemRendered = () => {},
}: LHNOptionsListProps) {
const reports = useReports();
const styles = useThemeStyles();
const {canUseViolations} = usePermissions();

// When the first item renders we want to call the onFirstItemRendered callback.
// At this point in time we know that the list is actually displaying items.
Expand All @@ -53,78 +39,38 @@ function LHNOptionsList({
* Function which renders a row in the list
*/
const renderItem = useCallback(
({item: reportID}: RenderItemProps): ReactElement => {
const itemFullReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? null;
const itemReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? null;
const itemParentReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`] ?? null;
const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? ''] ?? null;
const itemPolicy = policy?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null;
const transactionID = itemParentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? itemParentReportAction.originalMessage.IOUTransactionID ?? '' : '';
const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? null;
const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? '';
const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions);
const lastReportAction = sortedReportActions[0];

// Get the transaction for the last report action
let lastReportActionTransactionID = '';

if (lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU) {
lastReportActionTransactionID = lastReportAction.originalMessage?.IOUTransactionID ?? '';
}
const lastReportActionTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${lastReportActionTransactionID}`] ?? {};
({item}: RenderItemProps): ReactElement => {

Check failure on line 42 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected block statement surrounding arrow body; move the returned value immediately after the `=>`

return (
<OptionRowLHNData
reportID={reportID}
fullReport={itemFullReport}
reportActions={itemReportActions}
parentReportAction={itemParentReportAction}
policy={itemPolicy}
personalDetails={personalDetails ?? {}}
transaction={itemTransaction}
lastReportActionTransaction={lastReportActionTransaction}
receiptTransactions={transactions}
viewMode={optionMode}
isFocused={!shouldDisableFocusOptions && reportID === currentReportID}
reportID={item?.reportID}
isFocused={!shouldDisableFocusOptions && item?.reportID === currentReportID}
onSelectRow={onSelectRow}
preferredLocale={preferredLocale}
comment={itemComment}
transactionViolations={transactionViolations}
canUseViolations={canUseViolations}
comment={item?.comment}
optionItem={item?.optionItem}

Check failure on line 50 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Type '{ reportID: string; isFocused: boolean; onSelectRow: ((optionItem: OptionData, popoverAnchor: RefObject<View>) => void) | undefined; comment: string; optionItem: OptionData | undefined; onLayout: () => void; }' is not assignable to type 'IntrinsicAttributes & OptionRowLHNDataProps'.
onLayout={onLayoutItem}
/>
);
},
[

Check warning on line 55 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

React Hook useCallback has a missing dependency: 'shouldDisableFocusOptions'. Either include it or remove the dependency array
currentReportID,
draftComments,
onSelectRow,
optionMode,
personalDetails,
policy,
preferredLocale,
reportActions,
reports,
shouldDisableFocusOptions,
transactions,
transactionViolations,
canUseViolations,
onLayoutItem,
],
);

return (
<View style={style ?? styles.flex1}>
<FlashList
<FlatList
indicatorStyle="white"
keyboardShouldPersistTaps="always"
contentContainerStyle={StyleSheet.flatten(contentContainerStyles)}
data={data}

Check failure on line 68 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / typecheck

No overload matches this call.
testID="lhn-options-list"
keyExtractor={keyExtractor}
renderItem={renderItem}
estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight}
extraData={[currentReportID]}
// estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight}
// extraData={[currentReportID]}
showsVerticalScrollIndicator={false}
/>
</View>
Expand All @@ -133,30 +79,6 @@ function LHNOptionsList({

LHNOptionsList.displayName = 'LHNOptionsList';

export default withCurrentReportID(
withOnyx<LHNOptionsListProps, LHNOptionsListOnyxProps>({
reportActions: {
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
},
policy: {
key: ONYXKEYS.COLLECTION.POLICY,
},
preferredLocale: {
key: ONYXKEYS.NVP_PREFERRED_LOCALE,
},
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
transactions: {
key: ONYXKEYS.COLLECTION.TRANSACTION,
},
draftComments: {
key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT,
},
transactionViolations: {
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
},
})(memo(LHNOptionsList)),
);
export default memo(LHNOptionsList);

export type {LHNOptionsListProps};
51 changes: 1 addition & 50 deletions src/components/LHNOptionsList/OptionRowLHNData.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,61 +16,12 @@ import type {OptionRowLHNDataProps} from './types';
*/
function OptionRowLHNData({
isFocused = false,
fullReport,
reportActions,
personalDetails = {},
preferredLocale = CONST.LOCALES.DEFAULT,
comment,
policy,
receiptTransactions,
parentReportAction,
transaction,
lastReportActionTransaction = {},
transactionViolations,
canUseViolations,
optionItem,

Check failure on line 20 in src/components/LHNOptionsList/OptionRowLHNData.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Property 'optionItem' does not exist on type 'OptionRowLHNDataProps'.
...propsToForward
}: OptionRowLHNDataProps) {
const reportID = propsToForward.reportID;

const optionItemRef = useRef<OptionData>();

const hasViolations = canUseViolations && ReportUtils.doesTransactionThreadHaveViolations(fullReport, transactionViolations, parentReportAction ?? null);

const optionItem = useMemo(() => {
// Note: ideally we'd have this as a dependent selector in onyx!
const item = SidebarUtils.getOptionData({
report: fullReport,
reportActions,
personalDetails,
preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT,
policy,
parentReportAction,
hasViolations: !!hasViolations,
});
if (deepEqual(item, optionItemRef.current)) {
return optionItemRef.current;
}

optionItemRef.current = item;

return item;
// Listen parentReportAction to update title of thread report when parentReportAction changed
// Listen to transaction to update title of transaction report when transaction changed
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [
fullReport,
lastReportActionTransaction,
reportActions,
personalDetails,
preferredLocale,
policy,
parentReportAction,
transaction,
transactionViolations,
canUseViolations,
receiptTransactions,
]);

useEffect(() => {
if (!optionItem || !!optionItem.hasDraftComment || !comment || comment.length <= 0 || isFocused) {
return;
Expand Down
3 changes: 2 additions & 1 deletion src/components/LHNOptionsList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type CONST from '@src/CONST';
import type {OptionData} from '@src/libs/ReportUtils';
import type {Locale, PersonalDetailsList, Policy, Report, ReportAction, ReportActions, Transaction, TransactionViolation} from '@src/types/onyx';
import type {EmptyObject} from '@src/types/utils/EmptyObject';
import { OrderedReports } from '@libs/SidebarUtils';

type OptionMode = ValueOf<typeof CONST.OPTION_MODE>;

Expand Down Expand Up @@ -134,6 +135,6 @@ type OptionRowLHNProps = {
onLayout?: (event: LayoutChangeEvent) => void;
};

type RenderItemProps = {item: string};
type RenderItemProps = {item: OrderedReports};

export type {LHNOptionsListProps, OptionRowLHNDataProps, OptionRowLHNProps, LHNOptionsListOnyxProps, RenderItemProps};
10 changes: 9 additions & 1 deletion src/components/withCurrentReportID.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,15 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro
*/
const updateCurrentReportID = useCallback(
(state: NavigationState) => {
setCurrentReportID(Navigation.getTopmostReportId(state) ?? '');
const reportID = Navigation.getTopmostReportId(state) ?? '';
/**
* This is to make sure we don't set the undefined as reportID when
* switching between chat list and settings->workspaces tab.
* and doing so avoid unnecessary re-render of `useOrderedReportIDs`.
*/
if (reportID) {
setCurrentReportID(reportID);
}
},
[setCurrentReportID],
);
Expand Down
38 changes: 37 additions & 1 deletion src/hooks/useOrderedReportIDs.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, {createContext, useContext, useMemo} from 'react';
import {withOnyx} from 'react-native-onyx';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import {usePersonalDetails} from '@components/OnyxProvider';
import {getCurrentUserAccountID} from '@libs/actions/Report';
import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils';
import SidebarUtils from '@libs/SidebarUtils';
Expand All @@ -10,6 +11,7 @@ import type {Beta, Policy, PolicyMembers, ReportAction, ReportActions, Transacti
import type PriorityMode from '@src/types/onyx/PriorityMode';
import useActiveWorkspace from './useActiveWorkspace';
import useCurrentReportID from './useCurrentReportID';
import usePermissions from './usePermissions';
import {useReports} from './useReports';

type OnyxProps = {
Expand All @@ -31,6 +33,8 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP
const chatReports = useReports();
const currentReportIDValue = useCurrentReportID();
const {activeWorkspaceID} = useActiveWorkspace();
const personalDetails = usePersonalDetails();
const {canUseViolations} = usePermissions();

const policyMemberAccountIDs = useMemo(
() => getPolicyMembersByIdWithoutCurrentUser(props.policyMembers, activeWorkspaceID, getCurrentUserAccountID()),
Expand All @@ -49,8 +53,25 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP
props.transactionViolations,
activeWorkspaceID,
policyMemberAccountIDs,
personalDetails,
props.preferredLocale,

Check failure on line 57 in src/hooks/useOrderedReportIDs.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Property 'preferredLocale' does not exist on type 'WithOrderedReportIDsContextProviderProps'.
canUseViolations,

Check failure on line 58 in src/hooks/useOrderedReportIDs.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
props.draftComments,

Check failure on line 59 in src/hooks/useOrderedReportIDs.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Property 'draftComments' does not exist on type 'WithOrderedReportIDsContextProviderProps'.
),
[chatReports, props.betas, props.policies, props.priorityMode, props.allReportActions, props.transactionViolations, activeWorkspaceID, policyMemberAccountIDs],
[
chatReports,
props.betas,
props.policies,
props.priorityMode,
props.allReportActions,
props.transactionViolations,
activeWorkspaceID,
policyMemberAccountIDs,
personalDetails,
props.preferredLocale,

Check failure on line 71 in src/hooks/useOrderedReportIDs.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Property 'preferredLocale' does not exist on type 'WithOrderedReportIDsContextProviderProps'.
canUseViolations,
props.draftComments,

Check failure on line 73 in src/hooks/useOrderedReportIDs.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Property 'draftComments' does not exist on type 'WithOrderedReportIDsContextProviderProps'.
],
);

// We need to make sure the current report is in the list of reports, but we do not want
Expand All @@ -70,6 +91,10 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP
props.transactionViolations,
activeWorkspaceID,
policyMemberAccountIDs,
personalDetails,
props.preferredLocale,

Check failure on line 95 in src/hooks/useOrderedReportIDs.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Property 'preferredLocale' does not exist on type 'WithOrderedReportIDsContextProviderProps'.
canUseViolations,
props.draftComments,
);
}
return optionListItems;
Expand All @@ -84,6 +109,10 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP
props.policies,
props.priorityMode,
props.transactionViolations,
personalDetails,
props.preferredLocale,
canUseViolations,
props.draftComments,
]);

return <OrderedReportIDsContext.Provider value={optionListItemsWithCurrentReport}>{props.children}</OrderedReportIDsContext.Provider>;
Expand Down Expand Up @@ -136,6 +165,13 @@ const OrderedReportIDsContextProvider = withOnyx<WithOrderedReportIDsContextProv
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
initialValue: {},
},
draftComments: {
key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT,
initialValue: {},
},
preferredLocale: {
key: ONYXKEYS.NVP_PREFERRED_LOCALE,
},
})(WithOrderedReportIDsContextProvider);

function useOrderedReportIDs() {
Expand Down
Loading

0 comments on commit 6921051

Please sign in to comment.