Skip to content

Commit

Permalink
Merge pull request #28615 from kacper-mikolajczak/chat-transition-opt…
Browse files Browse the repository at this point in the history
…imisation-proposal

Improve memoization of LHN by handling the isFocused prop better
  • Loading branch information
mountiny authored Oct 8, 2023
2 parents e0ab038 + 72ee59a commit fabff79
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/components/LHNOptionsList/LHNOptionsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {FlatList, View} from 'react-native';
import _ from 'underscore';
import CONST from '../../CONST';
import styles from '../../styles/styles';
import OptionRowLHNData from './OptionRowLHNData';
import variables from '../../styles/variables';
import OptionRowLHNDataWithFocus from './OptionRowLHNDataWithFocus';

const propTypes = {
/** Wrapper style for the section list */
Expand Down Expand Up @@ -63,7 +63,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
* @return {Component}
*/
const renderItem = ({item}) => (
<OptionRowLHNData
<OptionRowLHNDataWithFocus
reportID={item}
viewMode={optionMode}
shouldDisableFocusOptions={shouldDisableFocusOptions}
Expand Down
17 changes: 4 additions & 13 deletions src/components/LHNOptionsList/OptionRowLHNData.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {withReportCommentDrafts} from '../OnyxProvider';
import SidebarUtils from '../../libs/SidebarUtils';
import compose from '../../libs/compose';
import ONYXKEYS from '../../ONYXKEYS';
import withCurrentReportID, {withCurrentReportIDPropTypes, withCurrentReportIDDefaultProps} from '../withCurrentReportID';
import OptionRowLHN, {propTypes as basePropTypes, defaultProps as baseDefaultProps} from './OptionRowLHN';
import * as Report from '../../libs/actions/Report';
import * as UserUtils from '../../libs/UserUtils';
Expand All @@ -20,8 +19,8 @@ import CONST from '../../CONST';
import reportActionPropTypes from '../../pages/home/report/reportActionPropTypes';

const propTypes = {
/** If true will disable ever setting the OptionRowLHN to focused */
shouldDisableFocusOptions: PropTypes.bool,
/** Whether row should be focused */
isFocused: PropTypes.bool,

/** List of users' personal details */
personalDetails: PropTypes.objectOf(participantPropTypes),
Expand Down Expand Up @@ -51,20 +50,17 @@ const propTypes = {
/** The ID of the transaction */
transactionID: PropTypes.string,
}),

...withCurrentReportIDPropTypes,
...basePropTypes,
};

const defaultProps = {
shouldDisableFocusOptions: false,
isFocused: false,
personalDetails: {},
fullReport: {},
policy: {},
parentReportActions: {},
transaction: {},
preferredLocale: CONST.LOCALES.DEFAULT,
...withCurrentReportIDDefaultProps,
...baseDefaultProps,
};

Expand All @@ -75,8 +71,7 @@ const defaultProps = {
* re-render if the data really changed.
*/
function OptionRowLHNData({
shouldDisableFocusOptions,
currentReportID,
isFocused,
fullReport,
reportActions,
personalDetails,
Expand All @@ -89,9 +84,6 @@ function OptionRowLHNData({
...propsToForward
}) {
const reportID = propsToForward.reportID;
// We only want to pass a boolean to the memoized component,
// instead of a changing number (so we prevent unnecessary re-renders).
const isFocused = !shouldDisableFocusOptions && currentReportID === reportID;

const parentReportAction = parentReportActions[fullReport.parentReportActionID];

Expand Down Expand Up @@ -172,7 +164,6 @@ const personalDetailsSelector = (personalDetails) =>
*/
export default React.memo(
compose(
withCurrentReportID,
withReportCommentDrafts({
propName: 'comment',
transformValue: (drafts, props) => {
Expand Down
39 changes: 39 additions & 0 deletions src/components/LHNOptionsList/OptionRowLHNDataWithFocus.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import React from 'react';
import PropTypes from 'prop-types';
import withCurrentReportID, {withCurrentReportIDPropTypes, withCurrentReportIDDefaultProps} from '../withCurrentReportID';
import OptionRowLHNData from './OptionRowLHNData';

const propTypes = {
...withCurrentReportIDPropTypes,
shouldDisableFocusOptions: PropTypes.bool,
};

const defaultProps = {
...withCurrentReportIDDefaultProps,
shouldDisableFocusOptions: false,
};

/**
* Wrapper component for OptionRowLHNData that calculates isFocused prop based on currentReportID.
* This is extracted from OptionRowLHNData to prevent unnecessary re-renders when currentReportID changes.
* @returns {React.Component} OptionRowLHNData component with isFocused prop
*/
function OptionRowLHNDataWithFocus({currentReportID, shouldDisableFocusOptions, ...props}) {
// We only want to pass a boolean to the memoized component,
// instead of a changing number (so we prevent unnecessary re-renders).
const isFocused = !shouldDisableFocusOptions && currentReportID === props.reportID;

return (
<OptionRowLHNData
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
isFocused={isFocused}
/>
);
}

OptionRowLHNDataWithFocus.defaultProps = defaultProps;
OptionRowLHNDataWithFocus.propTypes = propTypes;
OptionRowLHNDataWithFocus.displayName = 'OptionRowLHNDataWithFocus';

export default withCurrentReportID(OptionRowLHNDataWithFocus);

0 comments on commit fabff79

Please sign in to comment.