Skip to content

Commit

Permalink
Merge pull request #30242 from adhorodyski/lhn-dataflow-rework
Browse files Browse the repository at this point in the history
refactor: LHN dataflow rework
  • Loading branch information
tgolen authored Nov 6, 2023
2 parents d2acc8c + 9f7b886 commit d75acaf
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 142 deletions.
160 changes: 140 additions & 20 deletions src/components/LHNOptionsList/LHNOptionsList.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React from 'react';
import React, {useCallback} from 'react';
import {FlatList, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import participantPropTypes from '@components/participantPropTypes';
import withCurrentReportID, {withCurrentReportIDDefaultProps, withCurrentReportIDPropTypes} from '@components/withCurrentReportID';
import compose from '@libs/compose';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import reportActionPropTypes from '@pages/home/report/reportActionPropTypes';
import reportPropTypes from '@pages/reportPropTypes';
import styles from '@styles/styles';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import OptionRowLHNDataWithFocus from './OptionRowLHNDataWithFocus';
import ONYXKEYS from '@src/ONYXKEYS';
import OptionRowLHNData from './OptionRowLHNData';

const propTypes = {
/** Wrapper style for the section list */
Expand All @@ -27,14 +36,72 @@ const propTypes = {

/** Whether to allow option focus or not */
shouldDisableFocusOptions: PropTypes.bool,

/** The policy which the user has access to and which the report could be tied to */
policy: PropTypes.shape({
/** The ID of the policy */
id: PropTypes.string,
/** Name of the policy */
name: PropTypes.string,
/** Avatar of the policy */
avatar: PropTypes.string,
}),

/** All reports shared with the user */
reports: PropTypes.objectOf(reportPropTypes),

/** Array of report actions for this report */
reportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)),

/** Indicates which locale the user currently has selected */
preferredLocale: PropTypes.string,

/** List of users' personal details */
personalDetails: PropTypes.objectOf(participantPropTypes),

/** The transaction from the parent report action */
transactions: PropTypes.objectOf(
PropTypes.shape({
/** The ID of the transaction */
transactionID: PropTypes.string,
}),
),
/** List of draft comments */
draftComments: PropTypes.objectOf(PropTypes.string),
...withCurrentReportIDPropTypes,
};

const defaultProps = {
style: styles.flex1,
shouldDisableFocusOptions: false,
reportActions: {},
reports: {},
policy: {},
preferredLocale: CONST.LOCALES.DEFAULT,
personalDetails: {},
transactions: {},
draftComments: {},
...withCurrentReportIDDefaultProps,
};

function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optionMode, shouldDisableFocusOptions}) {
const keyExtractor = (item) => item;

function LHNOptionsList({
style,
contentContainerStyles,
data,
onSelectRow,
optionMode,
shouldDisableFocusOptions,
reports,
reportActions,
policy,
preferredLocale,
personalDetails,
transactions,
draftComments,
currentReportID,
}) {
/**
* This function is used to compute the layout of any given item in our list. Since we know that each item will have the exact same height, this is a performance optimization
* so that the heights can be determined before the options are rendered. Otherwise, the heights are determined when each option is rendering and it causes a lot of overhead on large
Expand All @@ -45,14 +112,17 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
*
* @returns {Object}
*/
const getItemLayout = (itemData, index) => {
const optionHeight = optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight;
return {
length: optionHeight,
offset: index * optionHeight,
index,
};
};
const getItemLayout = useCallback(
(itemData, index) => {
const optionHeight = optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight;
return {
length: optionHeight,
offset: index * optionHeight,
index,
};
},
[optionMode],
);

/**
* Function which renders a row in the list
Expand All @@ -62,13 +132,38 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
*
* @return {Component}
*/
const renderItem = ({item}) => (
<OptionRowLHNDataWithFocus
reportID={item}
viewMode={optionMode}
shouldDisableFocusOptions={shouldDisableFocusOptions}
onSelectRow={onSelectRow}
/>
const renderItem = useCallback(
({item: reportID}) => {
const itemFullReport = reports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] || {};
const itemReportActions = reportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`];
const itemParentReportActions = reportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport.parentReportID}`];
const itemPolicy = policy[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport.policyID}`];
const itemTransaction = `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(
itemParentReportActions,
[itemFullReport.parentReportActionID, 'originalMessage', 'IOUTransactionID'],
'',
)}`;
const itemComment = draftComments[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] || '';
const participantPersonalDetailList = _.values(OptionsListUtils.getPersonalDetailsForAccountIDs(itemFullReport.participantAccountIDs, personalDetails));
return (
<OptionRowLHNData
reportID={reportID}
fullReport={itemFullReport}
reportActions={itemReportActions}
parentReportActions={itemParentReportActions}
policy={itemPolicy}
personalDetails={participantPersonalDetailList}
transaction={itemTransaction}
receiptTransactions={transactions}
viewMode={optionMode}
isFocused={!shouldDisableFocusOptions && reportID === currentReportID}
onSelectRow={onSelectRow}
preferredLocale={preferredLocale}
comment={itemComment}
/>
);
},
[currentReportID, draftComments, onSelectRow, optionMode, personalDetails, policy, preferredLocale, reportActions, reports, shouldDisableFocusOptions, transactions],
);

return (
Expand All @@ -80,7 +175,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
showsVerticalScrollIndicator={false}
data={data}
testID="lhn-options-list"
keyExtractor={(item) => item}
keyExtractor={keyExtractor}
stickySectionHeadersEnabled={false}
renderItem={renderItem}
getItemLayout={getItemLayout}
Expand All @@ -96,4 +191,29 @@ LHNOptionsList.propTypes = propTypes;
LHNOptionsList.defaultProps = defaultProps;
LHNOptionsList.displayName = 'LHNOptionsList';

export default LHNOptionsList;
export default compose(
withCurrentReportID,
withOnyx({
reports: {
key: ONYXKEYS.COLLECTION.REPORT,
},
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,
},
}),
)(LHNOptionsList);
90 changes: 7 additions & 83 deletions src/components/LHNOptionsList/OptionRowLHNData.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,22 @@
import {deepEqual} from 'fast-equals';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React, {useEffect, useMemo, useRef} from 'react';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import participantPropTypes from '@components/participantPropTypes';
import compose from '@libs/compose';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import SidebarUtils from '@libs/SidebarUtils';
import * as TransactionUtils from '@libs/TransactionUtils';
import * as UserUtils from '@libs/UserUtils';
import reportActionPropTypes from '@pages/home/report/reportActionPropTypes';
import * as Report from '@userActions/Report';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import OptionRowLHN, {defaultProps as baseDefaultProps, propTypes as basePropTypes} from './OptionRowLHN';

const propTypes = {
/** Whether row should be focused */
isFocused: PropTypes.bool,

/** List of users' personal details */
personalDetails: PropTypes.objectOf(participantPropTypes),
personalDetails: PropTypes.arrayOf(participantPropTypes),

/** The preferred language for the app */
preferredLocale: PropTypes.string,
Expand All @@ -44,10 +39,8 @@ const propTypes = {
parentReportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)),

/** The transaction from the parent report action */
transaction: PropTypes.shape({
/** The ID of the transaction */
transactionID: PropTypes.string,
}),
transactionID: PropTypes.string,

...basePropTypes,
};

Expand All @@ -57,7 +50,7 @@ const defaultProps = {
fullReport: {},
policy: {},
parentReportActions: {},
transaction: {},
transactionID: undefined,
preferredLocale: CONST.LOCALES.DEFAULT,
...baseDefaultProps,
};
Expand All @@ -78,11 +71,10 @@ function OptionRowLHNData({
policy,
receiptTransactions,
parentReportActions,
transaction,
transactionID,
...propsToForward
}) {
const reportID = propsToForward.reportID;

const parentReportAction = parentReportActions[fullReport.parentReportActionID];

const optionItemRef = useRef();
Expand All @@ -105,7 +97,7 @@ function OptionRowLHNData({
// 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, linkedTransaction, reportActions, personalDetails, preferredLocale, policy, parentReportAction, transaction]);
}, [fullReport, linkedTransaction, reportActions, personalDetails, preferredLocale, policy, parentReportAction, transactionID]);

useEffect(() => {
if (!optionItem || optionItem.hasDraftComment || !comment || comment.length <= 0 || isFocused) {
Expand All @@ -129,79 +121,11 @@ OptionRowLHNData.propTypes = propTypes;
OptionRowLHNData.defaultProps = defaultProps;
OptionRowLHNData.displayName = 'OptionRowLHNData';

/**
* @param {Object} [personalDetails]
* @returns {Object|undefined}
*/
const personalDetailsSelector = (personalDetails) =>
_.reduce(
personalDetails,
(finalPersonalDetails, personalData, accountID) => {
// It's OK to do param-reassignment in _.reduce() because we absolutely know the starting state of finalPersonalDetails
// eslint-disable-next-line no-param-reassign
finalPersonalDetails[accountID] = {
accountID: Number(accountID),
login: personalData.login,
displayName: personalData.displayName,
firstName: personalData.firstName,
status: personalData.status,
avatar: UserUtils.getAvatar(personalData.avatar, personalData.accountID),
fallbackIcon: personalData.fallbackIcon,
};
return finalPersonalDetails;
},
{},
);

/**
* This component is rendered in a list.
* On scroll we want to avoid that a item re-renders
* just because the list has to re-render when adding more items.
* Thats also why the React.memo is used on the outer component here, as we just
* use it to prevent re-renders from parent re-renders.
*/
export default React.memo(
compose(
withOnyx({
comment: {
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`,
},
fullReport: {
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
},
reportActions: {
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
canEvict: false,
},
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
selector: personalDetailsSelector,
},
preferredLocale: {
key: ONYXKEYS.NVP_PREFERRED_LOCALE,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
parentReportActions: {
key: ({fullReport}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${fullReport.parentReportID}`,
canEvict: false,
},
policy: {
key: ({fullReport}) => `${ONYXKEYS.COLLECTION.POLICY}${fullReport.policyID}`,
},
// Ideally, we aim to access only the last transaction for the current report by listening to changes in reportActions.
// In some scenarios, a transaction might be created after reportActions have been modified.
// This can lead to situations where `lastTransaction` doesn't update and retains the previous value.
// However, performance overhead of this is minimized by using memos inside the component.
receiptTransactions: {key: ONYXKEYS.COLLECTION.TRANSACTION},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
transaction: {
key: ({fullReport, parentReportActions}) =>
`${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(parentReportActions, [fullReport.parentReportActionID, 'originalMessage', 'IOUTransactionID'], '')}`,
},
}),
)(OptionRowLHNData),
);
export default React.memo(OptionRowLHNData);
Loading

0 comments on commit d75acaf

Please sign in to comment.