Skip to content

Commit

Permalink
Revert "Merge pull request #10784 from Expensify/tgolen-optimize-side…
Browse files Browse the repository at this point in the history 
…barordering"

This reverts commit 23f4fc8, reversing
changes made to c614dfc.
  • Loading branch information
@francoisl
francoisl committed Sep 15, 2022
1 parent fe631e1 commit bfdc7bb
Show file tree
Hide file tree
Showing 8 changed files with 260 additions and 246 deletions.
1 change: 0 additions & 1 deletion src/CONST.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ const CONST = {
SWITCH_REPORT: 'switch_report',
SIDEBAR_LOADED: 'sidebar_loaded',
PERSONAL_DETAILS_FORMATTED: 'personal_details_formatted',
SIDEBAR_LINKS_FILTER_REPORTS: 'sidebar_links_filter_reports',
COLD: 'cold',
REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME: 1500,
TOOLTIP_SENSE: 1000,
Expand Down
10 changes: 2 additions & 8 deletions src/components/Tooltip/TooltipRenderedOnPageBody.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ const propTypes = {

/** Any additional amount to manually adjust the horizontal position of the tooltip.
A positive value shifts the tooltip to the right, and a negative value shifts it to the left. */
shiftHorizontal: PropTypes.number,
shiftHorizontal: PropTypes.number.isRequired,

/** Any additional amount to manually adjust the vertical position of the tooltip.
A positive value shifts the tooltip down, and a negative value shifts it up. */
shiftVertical: PropTypes.number,
shiftVertical: PropTypes.number.isRequired,

/** Text to be shown in the tooltip */
text: PropTypes.string.isRequired,
Expand All @@ -43,11 +43,6 @@ const propTypes = {
numberOfLines: PropTypes.number.isRequired,
};

const defaultProps = {
shiftHorizontal: 0,
shiftVertical: 0,
};

// Props will change frequently.
// On every tooltip hover, we update the position in state which will result in re-rendering.
// We also update the state on layout changes which will be triggered often.
Expand Down Expand Up @@ -137,6 +132,5 @@ class TooltipRenderedOnPageBody extends React.PureComponent {
}

TooltipRenderedOnPageBody.propTypes = propTypes;
TooltipRenderedOnPageBody.defaultProps = defaultProps;

export default TooltipRenderedOnPageBody;
244 changes: 90 additions & 154 deletions src/libs/OptionsListUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import _ from 'underscore';
import Onyx from 'react-native-onyx';
import lodashGet from 'lodash/get';
import lodashOrderBy from 'lodash/orderBy';
import memoizeOne from 'memoize-one';
import Str from 'expensify-common/lib/str';
import ONYXKEYS from '../ONYXKEYS';
import CONST from '../CONST';
Expand Down Expand Up @@ -142,64 +143,39 @@ function getParticipantNames(personalDetailList) {
return participantNames;
}

/**
* A very optimized method to remove unique items from an array.
* Taken from https://stackoverflow.com/a/9229821/9114791
*
* @param {Array} items
* @returns {Array}
*/
function uniqFast(items) {
const seenItems = {};
const result = [];
let j = 0;
for (let i = 0; i < items.length; i++) {
const item = items[i];
if (seenItems[item] !== 1) {
seenItems[item] = 1;
result[j++] = item;
}
}
return result;
}

/**
* Returns a string with all relevant search terms.
* Default should be serachable by policy/domain name but not by participants.
*
* This method must be incredibly performant. It was found to be a big performance bottleneck
* when dealing with accounts that have thousands of reports. For loops are more efficient than _.each
* Array.prototype.push.apply is faster than using the spread operator, and concat() is faster than push().
*
* @param {Object} report
* @param {String} reportName
* @param {Array} personalDetailList
* @param {Boolean} isChatRoomOrPolicyExpenseChat
* @return {String}
*/
function getSearchText(report, reportName, personalDetailList, isChatRoomOrPolicyExpenseChat) {
let searchTerms = [];
const searchTerms = [];

if (!isChatRoomOrPolicyExpenseChat) {
for (let i = 0; i < personalDetailList.length; i++) {
const personalDetail = personalDetailList[i];
searchTerms = searchTerms.concat([personalDetail.displayName, personalDetail.login.replace(/\./g, '')]);
}
_.each(personalDetailList, (personalDetail) => {
searchTerms.push(personalDetail.displayName);
searchTerms.push(personalDetail.login.replace(/\./g, ''));
});
}
if (report) {
Array.prototype.push.apply(searchTerms, reportName.split(''));
Array.prototype.push.apply(searchTerms, reportName.split(','));
searchTerms.push(...reportName);
searchTerms.push(..._.map(reportName.split(','), name => name.trim()));

if (isChatRoomOrPolicyExpenseChat) {
const chatRoomSubtitle = ReportUtils.getChatRoomSubtitle(report, policies);
Array.prototype.push.apply(searchTerms, chatRoomSubtitle.split(''));
Array.prototype.push.apply(searchTerms, chatRoomSubtitle.split(','));
searchTerms.push(...chatRoomSubtitle);
searchTerms.push(..._.map(chatRoomSubtitle.split(','), name => name.trim()));
} else {
searchTerms = searchTerms.concat(report.participants);
searchTerms.push(...report.participants);
}
}

return uniqFast(searchTerms).join(' ');
return _.unique(searchTerms).join(' ');
}

/**
Expand Down Expand Up @@ -241,118 +217,80 @@ function createOption(logins, personalDetails, report, reportActions = {}, {
showChatPreviewLine = false,
forcePolicyNamePreview = false,
}) {
const result = {
text: null,
alternateText: null,
brickRoadIndicator: null,
icons: null,
tooltipText: null,
ownerEmail: null,
subtitle: null,
participantsList: null,
login: null,
reportID: null,
phoneNumber: null,
payPalMeAddress: null,
isUnread: null,
hasDraftComment: false,
keyForList: null,
searchText: null,
isDefaultRoom: false,
isPinned: false,
hasOutstandingIOU: false,
iouReportID: null,
isIOUReportOwner: null,
iouReportAmount: 0,
isChatRoom: false,
isArchivedRoom: false,
shouldShowSubscript: false,
isPolicyExpenseChat: false,
};

const isChatRoom = ReportUtils.isChatRoom(report);
const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report);
const personalDetailMap = getPersonalDetailsForLogins(logins, personalDetails);
const personalDetailList = _.values(personalDetailMap);
const isArchivedRoom = ReportUtils.isArchivedRoom(report);
const isDefaultRoom = ReportUtils.isDefaultRoom(report);
const hasMultipleParticipants = personalDetailList.length > 1 || isChatRoom || isPolicyExpenseChat;
const personalDetail = personalDetailList[0];
let hasMultipleParticipants = personalDetailList.length > 1;
let subtitle;

if (report) {
result.isChatRoom = ReportUtils.isChatRoom(report);
result.isDefaultRoom = ReportUtils.isDefaultRoom(report);
result.isArchivedRoom = ReportUtils.isArchivedRoom(report);
result.isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report);
result.shouldShowSubscript = result.isPolicyExpenseChat && !report.isOwnPolicyExpenseChat && !result.isArchivedRoom;
result.brickRoadIndicator = getBrickRoadIndicatorStatusForReport(report, reportActions);
result.ownerEmail = report.ownerEmail;
result.reportID = report.reportID;
result.isUnread = ReportUtils.isUnread(report);
result.hasDraftComment = report.hasDraft;
result.isPinned = report.isPinned;
result.iouReportID = report.iouReportID;
result.keyForList = String(report.reportID);
result.tooltipText = ReportUtils.getReportParticipantsTitle(report.participants || []);
result.hasOutstandingIOU = report.hasOutstandingIOU;

hasMultipleParticipants = personalDetailList.length > 1 || result.isChatRoom || result.isPolicyExpenseChat;
subtitle = ReportUtils.getChatRoomSubtitle(report, policies);

let lastMessageTextFromReport = '';
if (ReportUtils.isReportMessageAttachment({text: report.lastMessageText, html: report.lastMessageHtml})) {
lastMessageTextFromReport = `[${Localize.translateLocal('common.attachment')}]`;
} else {
lastMessageTextFromReport = Str.htmlDecode(report ? report.lastMessageText : '');
}

const lastActorDetails = personalDetailMap[report.lastActorEmail] || null;
let lastMessageText = hasMultipleParticipants && lastActorDetails
? `${lastActorDetails.displayName}: `
: '';
lastMessageText += report ? lastMessageTextFromReport : '';

if (result.isPolicyExpenseChat && result.isArchivedRoom) {
const archiveReason = (lastReportActions[report.reportID] && lastReportActions[report.reportID].originalMessage && lastReportActions[report.reportID].originalMessage.reason)
|| CONST.REPORT.ARCHIVE_REASON.DEFAULT;
lastMessageText = Localize.translate(preferredLocale, `reportArchiveReasons.${archiveReason}`, {
displayName: archiveReason.displayName || report.lastActorEmail,
policyName: ReportUtils.getPolicyName(report, policies),
});
}

if (result.isChatRoom || result.isPolicyExpenseChat) {
result.alternateText = (showChatPreviewLine && !forcePolicyNamePreview && lastMessageText)
? lastMessageText
: subtitle;
} else {
result.alternateText = (showChatPreviewLine && lastMessageText)
? lastMessageText
: Str.removeSMSDomain(personalDetail.login);
}
} else {
result.keyForList = personalDetail.login;
}

if (result.hasOutstandingIOU) {
const iouReport = iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`] || null;
if (iouReport) {
result.isIOUReportOwner = iouReport.ownerEmail === currentUserLogin;
result.iouReportAmount = iouReport.total;
}
}

if (!hasMultipleParticipants) {
result.login = personalDetail.login;
result.phoneNumber = personalDetail.phoneNumber;
result.payPalMeAddress = personalDetail.payPalMeAddress;
const hasOutstandingIOU = lodashGet(report, 'hasOutstandingIOU', false);
const iouReport = hasOutstandingIOU
? lodashGet(iouReports, `${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`, {})
: {};

const lastActorDetails = report ? _.find(personalDetailList, {login: report.lastActorEmail}) : null;
const lastMessageTextFromReport = ReportUtils.isReportMessageAttachment({text: lodashGet(report, 'lastMessageText', ''), html: lodashGet(report, 'lastMessageHtml', '')})
? `[${Localize.translateLocal('common.attachment')}]`
: Str.htmlDecode(lodashGet(report, 'lastMessageText', ''));
let lastMessageText = report && hasMultipleParticipants && lastActorDetails
? `${lastActorDetails.displayName}: `
: '';
lastMessageText += report ? lastMessageTextFromReport : '';

if (isPolicyExpenseChat && isArchivedRoom) {
const archiveReason = lodashGet(lastReportActions[report.reportID], 'originalMessage.reason', CONST.REPORT.ARCHIVE_REASON.DEFAULT);
lastMessageText = Localize.translate(preferredLocale, `reportArchiveReasons.${archiveReason}`, {
displayName: lodashGet(lastActorDetails, 'displayName', report.lastActorEmail),
policyName: ReportUtils.getPolicyName(report, policies),
});
}

const tooltipText = ReportUtils.getReportParticipantsTitle(lodashGet(report, ['participants'], []));
const subtitle = ReportUtils.getChatRoomSubtitle(report, policies);
const reportName = ReportUtils.getReportName(report, personalDetailMap, policies);
result.text = reportName;
result.subtitle = subtitle;
result.participantsList = personalDetailList;
result.icons = ReportUtils.getIcons(report, personalDetails, policies, personalDetail.avatar);
result.searchText = getSearchText(report, reportName, personalDetailList, result.isChatRoom || result.isPolicyExpenseChat);

return result;
let alternateText;
if (isChatRoom || isPolicyExpenseChat) {
alternateText = (showChatPreviewLine && !forcePolicyNamePreview && lastMessageText)
? lastMessageText
: subtitle;
} else {
alternateText = (showChatPreviewLine && lastMessageText)
? lastMessageText
: Str.removeSMSDomain(personalDetail.login);
}
return {
text: reportName,
alternateText,
brickRoadIndicator: getBrickRoadIndicatorStatusForReport(report, reportActions),
icons: ReportUtils.getIcons(report, personalDetails, policies, lodashGet(personalDetail, ['avatar'])),
tooltipText,
ownerEmail: lodashGet(report, ['ownerEmail']),
subtitle,
participantsList: personalDetailList,

// It doesn't make sense to provide a login in the case of a report with multiple participants since
// there isn't any one single login to refer to for a report.
login: !hasMultipleParticipants ? personalDetail.login : null,
reportID: report ? report.reportID : null,
phoneNumber: !hasMultipleParticipants ? personalDetail.phoneNumber : null,
payPalMeAddress: !hasMultipleParticipants ? personalDetail.payPalMeAddress : null,
isUnread: ReportUtils.isUnread(report),
hasDraftComment: lodashGet(report, 'hasDraft', false),
keyForList: report ? String(report.reportID) : personalDetail.login,
searchText: getSearchText(report, reportName, personalDetailList, isChatRoom || isPolicyExpenseChat),
isPinned: lodashGet(report, 'isPinned', false),
hasOutstandingIOU,
iouReportID: lodashGet(report, 'iouReportID'),
isIOUReportOwner: lodashGet(iouReport, 'ownerEmail', '') === currentUserLogin,
iouReportAmount: lodashGet(iouReport, 'total', 0),
isChatRoom,
isArchivedRoom,
isDefaultRoom,
shouldShowSubscript: isPolicyExpenseChat && !report.isOwnPolicyExpenseChat && !isArchivedRoom,
isPolicyExpenseChat,
};
}

/**
Expand Down Expand Up @@ -413,7 +351,7 @@ function isCurrentUser(userDetails) {
*
* @param {Object} reports
* @param {Object} personalDetails
* @param {String} activeReportID
* @param {Number} activeReportID
* @param {Object} options
* @returns {Object}
* @private
Expand Down Expand Up @@ -466,24 +404,20 @@ function getOptions(reports, personalDetails, activeReportID, {

const allReportOptions = [];
_.each(orderedReports, (report) => {
if (!report) {
return;
}
const isChatRoom = ReportUtils.isChatRoom(report);
const isDefaultRoom = ReportUtils.isDefaultRoom(report);
const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report);
const logins = report.participants || [];
const logins = lodashGet(report, ['participants'], []);

// Report data can sometimes be incomplete. If we have no logins or reportID then we will skip this entry.
const shouldFilterNoParticipants = _.isEmpty(logins) && !isChatRoom && !isDefaultRoom && !isPolicyExpenseChat;
if (!report.reportID || shouldFilterNoParticipants) {
if (!report || !report.reportID || shouldFilterNoParticipants) {
return;
}

const hasDraftComment = report.hasDraft || false;
const iouReport = report.iouReportID && iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`];
const iouReportOwner = report.hasOutstandingIOU && iouReport
? iouReport.ownerEmail
const hasDraftComment = lodashGet(report, 'hasDraft', false);
const iouReportOwner = lodashGet(report, 'hasOutstandingIOU', false)
? lodashGet(iouReports, [`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`, 'ownerEmail'], '')
: '';

const reportContainsIOUDebt = iouReportOwner && iouReportOwner !== currentUserLogin;
Expand All @@ -497,7 +431,7 @@ function getOptions(reports, personalDetails, activeReportID, {
const shouldFilterReportIfRead = hideReadReports && !ReportUtils.isUnread(report);
const shouldFilterReport = shouldFilterReportIfEmpty || shouldFilterReportIfRead;

if (report.reportID.toString() !== activeReportID
if (report.reportID !== activeReportID
&& (!report.isPinned || isDefaultRoom)
&& !hasDraftComment
&& shouldFilterReport
Expand Down Expand Up @@ -834,7 +768,7 @@ function getMemberInviteOptions(
* @param {Object} reportActions
* @returns {Object}
*/
function getSidebarOptions(reports, personalDetails, activeReportID, priorityMode, betas, reportActions) {
function calculateSidebarOptions(reports, personalDetails, activeReportID, priorityMode, betas, reportActions) {
let sideBarOptions = {
prioritizeIOUDebts: true,
prioritizeReportsWithDraftComments: true,
Expand All @@ -858,6 +792,8 @@ function getSidebarOptions(reports, personalDetails, activeReportID, priorityMod
});
}

const getSidebarOptions = memoizeOne(calculateSidebarOptions);

/**
* Helper method that returns the text to be used for the header's message and title (if any)
*
Expand Down
Loading

0 comments on commit bfdc7bb

Please sign in to comment.