Skip to content

Revert #10784 and #11006 #11018

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.

Sign up for GitHub

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

Closed
francoisl wants to merge 2 commits into from
Closed

Revert #10784 and #11006 #11018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe631e1
Revert "Merge pull request #11006 from Expensify/jules-fixAvatarDeplo…
francoisl Sep 15, 2022
bfdc7bb
Revert "Merge pull request #10784 from Expensify/tgolen-optimize-side…
francoisl Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/CONST.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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);
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