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

Improve performance of optionslist #13398

Merged
merged 10 commits into from
Dec 8, 2022
29 changes: 6 additions & 23 deletions src/components/OptionRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ const propTypes = {
/** Whether to show the title tooltip */
showTitleTooltip: PropTypes.bool,

/** Toggle between compact and default view */
mode: PropTypes.oneOf(_.values(CONST.OPTION_MODE)),

/** Whether this option should be disabled */
isDisabled: PropTypes.bool,

Expand All @@ -71,7 +68,6 @@ const defaultProps = {
isSelected: false,
boldStyle: false,
showTitleTooltip: false,
mode: 'default',
onSelectRow: () => {},
isDisabled: false,
optionIsFocused: false,
Expand All @@ -86,22 +82,10 @@ const OptionRow = (props) => {
: styles.sidebarLinkText;
const textUnreadStyle = (props.boldStyle)
? [textStyle, styles.sidebarLinkTextBold] : [textStyle];
const displayNameStyle = StyleUtils.combineStyles(props.mode === CONST.OPTION_MODE.COMPACT
? [styles.optionDisplayName, ...textUnreadStyle, styles.optionDisplayNameCompact, styles.mr2]
: [styles.optionDisplayName, ...textUnreadStyle], props.style);
const alternateTextStyle = StyleUtils.combineStyles(props.mode === CONST.OPTION_MODE.COMPACT
? [textStyle, styles.optionAlternateText, styles.textLabelSupporting, styles.optionAlternateTextCompact]
: [textStyle, styles.optionAlternateText, styles.textLabelSupporting], props.style);
const contentContainerStyles = props.mode === CONST.OPTION_MODE.COMPACT
? [styles.flex1, styles.flexRow, styles.overflowHidden, styles.alignItemsCenter]
: [styles.flex1];
const sidebarInnerRowStyle = StyleSheet.flatten(props.mode === CONST.OPTION_MODE.COMPACT ? [
styles.chatLinkRowPressable,
styles.flexGrow1,
styles.optionItemAvatarNameWrapper,
styles.optionRowCompact,
styles.justifyContentCenter,
] : [
const displayNameStyle = [styles.optionDisplayName, ...textUnreadStyle, props.style];
const alternateTextStyle = [textStyle, styles.optionAlternateText, styles.textLabelSupporting, props.style];
const contentContainerStyles = [styles.flex1];
Comment on lines -89 to +87
Copy link
Contributor

Choose a reason for hiding this comment

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

The removal of StyleUtils.combineStyles caused this #13508

(following BugZero Checklist)

const sidebarInnerRowStyle = StyleSheet.flatten([
styles.chatLinkRowPressable,
styles.flexGrow1,
styles.optionItemAvatarNameWrapper,
Expand Down Expand Up @@ -169,12 +153,12 @@ const OptionRow = (props) => {
secondaryAvatar={props.option.icons[1]}
mainTooltip={props.option.ownerEmail}
secondaryTooltip={props.option.subtitle}
size={props.mode === CONST.OPTION_MODE.COMPACT ? CONST.AVATAR_SIZE.SMALL : CONST.AVATAR_SIZE.DEFAULT}
size={CONST.AVATAR_SIZE.DEFAULT}
/>
) : (
<MultipleAvatars
icons={props.option.icons}
size={props.mode === CONST.OPTION_MODE.COMPACT ? CONST.AVATAR_SIZE.SMALL : CONST.AVATAR_SIZE.DEFAULT}
size={CONST.AVATAR_SIZE.DEFAULT}
secondAvatarStyle={[
props.optionIsFocused
? StyleUtils.getBackgroundAndBorderStyle(focusedBackgroundColor)
Expand Down Expand Up @@ -256,7 +240,6 @@ OptionRow.displayName = 'OptionRow';
// It it very important to use React.memo here so SectionList items will not unnecessarily re-render
export default withLocalize(memo(OptionRow, (prevProps, nextProps) => prevProps.optionIsFocused === nextProps.optionIsFocused
&& prevProps.isSelected === nextProps.isSelected
&& prevProps.mode === nextProps.mode
&& prevProps.option.alternateText === nextProps.option.alternateText
&& prevProps.option.descriptiveText === nextProps.option.descriptiveText
&& _.isEqual(prevProps.option.icons, nextProps.option.icons)
Expand Down
7 changes: 2 additions & 5 deletions src/components/OptionsList/BaseOptionsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import _ from 'underscore';
import React, {forwardRef, Component} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import CONST from '../../CONST';
import styles from '../../styles/styles';
import variables from '../../styles/variables';
import OptionRow from '../OptionRow';
Expand Down Expand Up @@ -40,7 +39,6 @@ class BaseOptionsList extends Component {
this.buildFlatSectionArray = this.buildFlatSectionArray.bind(this);
this.extractKey = this.extractKey.bind(this);
this.onViewableItemsChanged = this.onViewableItemsChanged.bind(this);
this.viewabilityConfig = {viewAreaCoveragePercentThreshold: 95};
this.didLayout = false;

this.flattenedData = this.buildFlatSectionArray();
Expand Down Expand Up @@ -106,7 +104,7 @@ class BaseOptionsList extends Component {
* @returns {Array<Object>}
*/
buildFlatSectionArray() {
const optionHeight = this.props.optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight;
const optionHeight = variables.optionRowHeight;
let offset = 0;

// Start with just an empty list header
Expand Down Expand Up @@ -159,7 +157,6 @@ class BaseOptionsList extends Component {
return (
<OptionRow
option={item}
mode={this.props.optionMode}
showTitleTooltip={this.props.showTitleTooltip}
hoverStyle={this.props.optionHoveredStyle}
optionIsFocused={!this.props.disableFocusOptions
Expand Down Expand Up @@ -232,7 +229,7 @@ class BaseOptionsList extends Component {
initialNumToRender={5}
maxToRenderPerBatch={5}
windowSize={5}
viewabilityConfig={this.viewabilityConfig}
viewabilityConfig={{viewAreaCoveragePercentThreshold: 95}}
onViewableItemsChanged={this.onViewableItemsChanged}
/>
</View>
Expand Down
6 changes: 0 additions & 6 deletions src/components/OptionsList/optionsListPropTypes.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import PropTypes from 'prop-types';
import _ from 'underscore';
import SectionList from '../SectionList';
import styles from '../../styles/styles';
import optionPropTypes from '../optionPropTypes';
import CONST from '../../CONST';

const propTypes = {
/** option flexStyle for the options list container */
Expand Down Expand Up @@ -64,9 +62,6 @@ const propTypes = {
/** Whether to show the title tooltip */
showTitleTooltip: PropTypes.bool,

/** Toggle between compact and default view of the option */
optionMode: PropTypes.oneOf(_.values(CONST.OPTION_MODE)),

/** Whether to disable the interactivity of the list's option row(s) */
isDisabled: PropTypes.bool,

Expand All @@ -92,7 +87,6 @@ const defaultProps = {
headerMessage: '',
innerRef: null,
showTitleTooltip: false,
optionMode: undefined,
isDisabled: false,
onLayout: undefined,
shouldHaveOptionSeparator: false,
Expand Down
10 changes: 0 additions & 10 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,6 @@ function getOptions(reports, personalDetails, {
includeMultipleParticipantReports = false,
includePersonalDetails = false,
includeRecentReports = false,
prioritizeDefaultRoomsInSearch = false,

// When sortByReportTypeInSearch flag is true, recentReports will include the personalDetails options as well.
sortByReportTypeInSearch = false,
Expand Down Expand Up @@ -556,12 +555,6 @@ function getOptions(reports, personalDetails, {
}
}

// If we are prioritizing default rooms in search, do it only once we started something
if (prioritizeDefaultRoomsInSearch && searchValue !== '') {
const reportsSplitByDefaultChatRoom = _.partition(recentReportOptions, option => option.isChatRoom);
recentReportOptions = reportsSplitByDefaultChatRoom[0].concat(reportsSplitByDefaultChatRoom[1]);
}

if (includePersonalDetails) {
// Next loop over all personal details removing any that are selectedUsers or recentChats
_.each(allPersonalDetailsOptions, (personalDetailOption) => {
Expand Down Expand Up @@ -649,11 +642,8 @@ function getSearchOptions(
includeRecentReports: true,
includeMultipleParticipantReports: true,
maxRecentReportsToShow: 0, // Unlimited
prioritizePinnedReports: false,
prioritizeDefaultRoomsInSearch: false,
sortByReportTypeInSearch: true,
showChatPreviewLine: true,
showReportsWithNoComments: true,
includePersonalDetails: true,
forcePolicyNamePreview: true,
});
Expand Down
54 changes: 39 additions & 15 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ Onyx.connect({
callback: val => allReports = val,
});

function getChatType(report) {
return report ? report.chatType : '';
}

/**
* Returns the concatenated title for the PrimaryLogins of a report
*
Expand Down Expand Up @@ -150,7 +154,7 @@ function canDeleteReportAction(reportAction) {
* @returns {Boolean}
*/
function isAdminRoom(report) {
return lodashGet(report, ['chatType'], '') === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS;
return getChatType(report) === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS;
}

/**
Expand All @@ -160,7 +164,7 @@ function isAdminRoom(report) {
* @returns {Boolean}
*/
function isAnnounceRoom(report) {
return lodashGet(report, ['chatType'], '') === CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE;
return getChatType(report) === CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE;
}

/**
Expand All @@ -170,11 +174,11 @@ function isAnnounceRoom(report) {
* @returns {Boolean}
*/
function isDefaultRoom(report) {
return _.contains([
return [
CONST.REPORT.CHAT_TYPE.POLICY_ADMINS,
CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE,
CONST.REPORT.CHAT_TYPE.DOMAIN_ALL,
], lodashGet(report, ['chatType'], ''));
].indexOf(getChatType(report)) > -1;
}

/**
Expand All @@ -184,7 +188,7 @@ function isDefaultRoom(report) {
* @returns {Boolean}
*/
function isDomainRoom(report) {
return lodashGet(report, ['chatType'], '') === CONST.REPORT.CHAT_TYPE.DOMAIN_ALL;
return getChatType(report) === CONST.REPORT.CHAT_TYPE.DOMAIN_ALL;
}

/**
Expand All @@ -194,7 +198,7 @@ function isDomainRoom(report) {
* @returns {Boolean}
*/
function isUserCreatedPolicyRoom(report) {
return lodashGet(report, ['chatType'], '') === CONST.REPORT.CHAT_TYPE.POLICY_ROOM;
return getChatType(report) === CONST.REPORT.CHAT_TYPE.POLICY_ROOM;
}

/**
Expand All @@ -204,7 +208,7 @@ function isUserCreatedPolicyRoom(report) {
* @returns {Boolean}
*/
function isPolicyExpenseChat(report) {
return lodashGet(report, ['chatType'], '') === CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT;
return getChatType(report) === CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT;
}

/**
Expand Down Expand Up @@ -301,7 +305,7 @@ function getChatRoomSubtitle(report, policiesMap) {
if (!isDefaultRoom(report) && !isUserCreatedPolicyRoom(report) && !isPolicyExpenseChat(report)) {
return '';
}
if (report.chatType === CONST.REPORT.CHAT_TYPE.DOMAIN_ALL) {
if (getChatType(report) === CONST.REPORT.CHAT_TYPE.DOMAIN_ALL) {
// The domainAll rooms are just #domainName, so we ignore the prefix '#' to get the domainName
return report.reportName.substring(1);
}
Expand Down Expand Up @@ -468,12 +472,26 @@ function getIcons(report, personalDetails, policies, defaultIcon = null) {
];
}

// Return avatar sources for Group chats
const sortedParticipants = _.map(report.participants, dmParticipant => ({
firstName: lodashGet(personalDetails, [dmParticipant, 'firstName'], ''),
avatar: lodashGet(personalDetails, [dmParticipant, 'avatar']) || getDefaultAvatar(dmParticipant),
})).sort((first, second) => first.firstName - second.firstName);
return _.map(sortedParticipants, item => item.avatar);
const participantDetails = [];
for (let i = 0; i < report.participants.length; i++) {
const login = report.participants[i];
participantDetails.push([
login,
lodashGet(personalDetails, [login, 'firstName'], ''),
lodashGet(personalDetails, [login, 'avatar']) || getDefaultAvatar(login),
]);
}

// Sort all logins by first name (which is the second element in the array)
const sortedParticipantDetails = participantDetails.sort((a, b) => a[1] - b[1]);

// Now that things are sorted, gather only the avatars (third element in the array) and return those
const avatars = [];
for (let i = 0; i < sortedParticipantDetails.length; i++) {
avatars.push(sortedParticipantDetails[i][2]);
}

return avatars;
}

/**
Expand Down Expand Up @@ -570,7 +588,13 @@ function getReportName(report, policies = {}) {
const participants = (report && report.participants) || [];
const participantsWithoutCurrentUser = _.without(participants, sessionEmail);
const isMultipleParticipantReport = participantsWithoutCurrentUser.length > 1;
return _.map(participantsWithoutCurrentUser, login => getDisplayNameForParticipant(login, isMultipleParticipantReport)).join(', ');

const displayNames = [];
for (let i = 0; i < participantsWithoutCurrentUser.length; i++) {
const login = participantsWithoutCurrentUser[i];
displayNames.push(getDisplayNameForParticipant(login, isMultipleParticipantReport));
}
return displayNames.join(', ');
}

/**
Expand Down
1 change: 0 additions & 1 deletion src/pages/ReportParticipantsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ const ReportParticipantsPage = (props) => {
hideSectionHeaders
showTitleTooltip
disableFocusOptions
optionMode="default"
boldStyle
optionHoveredStyle={styles.hoveredComponentBG}
/>
Expand Down