From fe631e1f64eea5d457f97e2f6635d61c083f80ae Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Thu, 15 Sep 2022 11:10:23 -0700 Subject: [PATCH 1/2] Revert "Merge pull request #11006 from Expensify/jules-fixAvatarDeployBlocker" This reverts commit 17b237867d731d7d6af5ccf6b9d2086ba8242660, reversing changes made to ddd34f8945d7cd74b03373b38bac6e9634d9de34. --- src/libs/OptionsListUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index b56c08f7ca5a..2ab2d89ee9c8 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -349,7 +349,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, { result.text = reportName; result.subtitle = subtitle; result.participantsList = personalDetailList; - result.icons = ReportUtils.getIcons(report, personalDetails, policies); + result.icons = ReportUtils.getIcons(report, personalDetails, policies, personalDetail.avatar); result.searchText = getSearchText(report, reportName, personalDetailList, result.isChatRoom || result.isPolicyExpenseChat); return result; From bfdc7bb626f5a908b916acdc82d37bfdf0df8acf Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Thu, 15 Sep 2022 11:10:28 -0700 Subject: [PATCH 2/2] Revert "Merge pull request #10784 from Expensify/tgolen-optimize-sidebarordering" This reverts commit 23f4fc80ca4a2df155a58dfc51eef301125a1fc3, reversing changes made to c614dfce08272544fafb5466773cc0080e2f4ad9. --- src/CONST.js | 1 - .../Tooltip/TooltipRenderedOnPageBody.js | 10 +- src/libs/OptionsListUtils.js | 244 +++++++----------- src/libs/ReportUtils.js | 21 +- src/pages/home/ReportScreen.js | 5 +- src/pages/home/report/ReportActionCompose.js | 2 +- src/pages/home/sidebar/SidebarLinks.js | 187 +++++++++++--- tests/unit/LHNOrderTest.js | 36 +-- 8 files changed, 260 insertions(+), 246 deletions(-) diff --git a/src/CONST.js b/src/CONST.js index bc443e5e3fba..268b2fa3b214 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -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, diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 3bfcc71fd4c9..d2c40a74b477 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -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, @@ -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. @@ -137,6 +132,5 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } TooltipRenderedOnPageBody.propTypes = propTypes; -TooltipRenderedOnPageBody.defaultProps = defaultProps; export default TooltipRenderedOnPageBody; diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 2ab2d89ee9c8..c54690403d85 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -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'; @@ -142,35 +143,10 @@ 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 @@ -178,28 +154,28 @@ function uniqFast(items) { * @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(' '); } /** @@ -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, + }; } /** @@ -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 @@ -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; @@ -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 @@ -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, @@ -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) * diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 375a4be558f5..e48939c5e7e9 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -258,11 +258,8 @@ function isArchivedRoom(report) { * @returns {String} */ function getPolicyName(report, policies) { - const policyName = ( - policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`] - && policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`].name - ) || ''; - return policyName || report.oldPolicyName || Localize.translateLocal('workspace.common.unavailable'); + const defaultValue = report.oldPolicyName || Localize.translateLocal('workspace.common.unavailable'); + return lodashGet(policies, [`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, 'name'], defaultValue); } /** @@ -512,7 +509,7 @@ function getReportName(report, personalDetailsForParticipants = {}, policies = { } if (isPolicyExpenseChat(report)) { - const reportOwnerPersonalDetails = personalDetailsForParticipants[report.ownerEmail]; + const reportOwnerPersonalDetails = lodashGet(personalDetailsForParticipants, report.ownerEmail); const reportOwnerDisplayName = getDisplayNameForParticipant(reportOwnerPersonalDetails) || report.ownerEmail || report.reportName; formattedName = report.isOwnPolicyExpenseChat ? getPolicyName(report, policies) : reportOwnerDisplayName; } @@ -527,9 +524,11 @@ function getReportName(report, personalDetailsForParticipants = {}, policies = { // Not a room or PolicyExpenseChat, generate title from participants const participants = _.without(lodashGet(report, 'participants', []), sessionEmail); - const isMultipleParticipantReport = participants.length > 1; - const participantsToGetTheNamesOf = _.isEmpty(personalDetailsForParticipants) ? participants : personalDetailsForParticipants; - return _.map(participantsToGetTheNamesOf, participant => getDisplayNameForParticipant(participant, isMultipleParticipantReport)).join(', '); + const displayNamesWithTooltips = getDisplayNamesWithTooltips( + _.isEmpty(personalDetailsForParticipants) ? participants : personalDetailsForParticipants, + participants.length > 1, + ); + return _.map(displayNamesWithTooltips, ({displayName}) => displayName).join(', '); } /** @@ -661,8 +660,8 @@ function buildOptimisticIOUReportAction(type, amount, comment, paymentType = '', * @returns {Boolean} */ function isUnread(report) { - const lastReadSequenceNumber = report.lastReadSequenceNumber || 0; - const maxSequenceNumber = report.maxSequenceNumber || 0; + const lastReadSequenceNumber = lodashGet(report, 'lastReadSequenceNumber', 0); + const maxSequenceNumber = lodashGet(report, 'maxSequenceNumber', 0); return lastReadSequenceNumber < maxSequenceNumber; } diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 5728117b4c01..f852dfb2626f 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -104,10 +104,11 @@ const defaultProps = { * @param {Object} route * @param {Object} route.params * @param {String} route.params.reportID - * @returns {String} + * @returns {Number} */ function getReportID(route) { - return route.params.reportID.toString(); + const params = route.params; + return Number.parseInt(params.reportID, 10); } class ReportScreen extends React.Component { diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index a82101de219f..61369be57010 100755 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -57,7 +57,7 @@ const propTypes = { comment: PropTypes.string, /** The ID of the report actions will be created for */ - reportID: PropTypes.string.isRequired, + reportID: PropTypes.number.isRequired, /** Details about any modals being used */ modal: PropTypes.shape({ diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 462d14e9f3ba..7028b796a0ea 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -3,6 +3,7 @@ import {View, TouchableOpacity} from 'react-native'; import _ from 'underscore'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; +import lodashGet from 'lodash/get'; import memoizeOne from 'memoize-one'; import styles from '../../../styles/styles'; import * as StyleUtils from '../../../styles/StyleUtils'; @@ -25,8 +26,6 @@ import withLocalize, {withLocalizePropTypes} from '../../../components/withLocal import * as App from '../../../libs/actions/App'; import * as ReportUtils from '../../../libs/ReportUtils'; import withCurrentUserPersonalDetails from '../../../components/withCurrentUserPersonalDetails'; -import Timing from '../../../libs/actions/Timing'; -import reportActionPropTypes from '../report/reportActionPropTypes'; const propTypes = { /** Toggles the navigation menu open and closed */ @@ -51,9 +50,6 @@ const propTypes = { hasDraft: PropTypes.bool, })), - /** All report actions for all reports */ - reportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)), - /** List of users' personal details */ personalDetails: PropTypes.objectOf(participantPropTypes), @@ -80,7 +76,6 @@ const propTypes = { const defaultProps = { reports: {}, - reportActions: {}, personalDetails: {}, currentUserPersonalDetails: { avatar: ReportUtils.getDefaultAvatar(), @@ -89,59 +84,173 @@ const defaultProps = { priorityMode: CONST.PRIORITY_MODE.DEFAULT, }; -class SidebarLinks extends React.Component { - constructor(props) { - super(props); - this.getRecentReportsOptionListItems = memoizeOne(this.getRecentReportsOptionListItems.bind(this)); +/** + * @param {Object} nextUnreadReports + * @param {Object} unreadReports + * @returns {Boolean} + */ +function checkForNewUnreadReports(nextUnreadReports, unreadReports) { + return nextUnreadReports.length > 0 + && _.some(nextUnreadReports, + nextUnreadReport => !_.some(unreadReports, unreadReport => unreadReport.reportID === nextUnreadReport.reportID)); +} +const memoizeCheckForNewUnreadReports = memoizeOne(checkForNewUnreadReports); + +/** + * @param {Object} reportsObject + * @returns {Array} + */ +function getUnreadReports(reportsObject) { + const reports = _.values(reportsObject); + if (reports.length === 0) { + return []; } + const unreadReports = _.filter(reports, report => report && ReportUtils.isUnread(report)); + return unreadReports; +} +const memoizeGetUnreadReports = memoizeOne(getUnreadReports); - /** - * @param {String} activeReportID - * @param {String} priorityMode - * @param {Object[]} unorderedReports - * @param {Object} personalDetails - * @param {String[]} betas - * @param {Object} reportActions - * @returns {Object[]} - */ - getRecentReportsOptionListItems(activeReportID, priorityMode, unorderedReports, personalDetails, betas, reportActions) { +class SidebarLinks extends React.Component { + static getRecentReports(props) { + const activeReportID = parseInt(props.currentlyViewedReportID, 10); const sidebarOptions = OptionsListUtils.getSidebarOptions( - unorderedReports, - personalDetails, + props.reports, + props.personalDetails, activeReportID, - priorityMode, - betas, - reportActions, + props.priorityMode, + props.betas, + props.reportActions, ); return sidebarOptions.recentReports; } + /** + * Returns true if the sidebar list should be re-ordered + * + * @param {Object} nextProps + * @param {Boolean} hasActiveDraftHistory + * @param {Array} orderedReports + * @param {String} currentlyViewedReportID + * @param {Array} unreadReports + * @returns {Boolean} + */ + static shouldReorder(nextProps, hasActiveDraftHistory, orderedReports, currentlyViewedReportID, unreadReports) { + // We do not want to re-order reports in the LHN if the only change is the draft comment in the + // current report. + + // We don't need to limit draft comment flashing for small screen widths as LHN is not visible. + if (nextProps.isSmallScreenWidth) { + return true; + } + + // Always update if LHN is empty. + if (orderedReports.length === 0) { + return true; + } + + const didActiveReportChange = currentlyViewedReportID !== nextProps.currentlyViewedReportID; + + // Always re-order the list whenever the active report is changed + if (didActiveReportChange) { + return true; + } + + // If any reports have new unread messages, re-order the list + const nextUnreadReports = memoizeGetUnreadReports(nextProps.reports || {}); + if (memoizeCheckForNewUnreadReports(nextUnreadReports, unreadReports)) { + return true; + } + + // If there is an active report that either had or has a draft, we do not want to re-order the list + if (nextProps.currentlyViewedReportID && hasActiveDraftHistory) { + return false; + } + + return true; + } + + constructor(props) { + super(props); + + this.state = { + activeReport: { + reportID: props.currentlyViewedReportID, + hasDraftHistory: lodashGet(props.reports, `${ONYXKEYS.COLLECTION.REPORT}${props.currentlyViewedReportID}.hasDraft`, false), + lastMessageTimestamp: lodashGet(props.reports, `${ONYXKEYS.COLLECTION.REPORT}${props.currentlyViewedReportID}.lastMessageTimestamp`, 0), + }, + orderedReports: [], + priorityMode: props.priorityMode, + unreadReports: memoizeGetUnreadReports(props.reports || {}), + }; + } + + static getDerivedStateFromProps(nextProps, prevState) { + const isActiveReportSame = prevState.activeReport.reportID === nextProps.currentlyViewedReportID; + const lastMessageTimestamp = lodashGet(nextProps.reports, `${ONYXKEYS.COLLECTION.REPORT}${nextProps.currentlyViewedReportID}.lastMessageTimestamp`, 0); + + // Determines if the active report has a history of draft comments while active. + let hasDraftHistory; + + // If the active report has not changed and the message has been sent, set the draft history flag to false so LHN can reorder. + // Otherwise, if the active report has not changed and the flag was previously true, preserve the state so LHN cannot reorder. + // Otherwise, update the flag from the prop value. + if (isActiveReportSame && prevState.activeReport.lastMessageTimestamp !== lastMessageTimestamp) { + hasDraftHistory = false; + } else if (isActiveReportSame && prevState.activeReport.hasDraftHistory) { + hasDraftHistory = true; + } else { + hasDraftHistory = lodashGet(nextProps.reports, `${ONYXKEYS.COLLECTION.REPORT}${nextProps.currentlyViewedReportID}.hasDraft`, false); + } + + const shouldReorder = SidebarLinks.shouldReorder(nextProps, hasDraftHistory, prevState.orderedReports, prevState.activeReport.reportID, prevState.unreadReports); + const switchingPriorityModes = nextProps.priorityMode !== prevState.priorityMode; + + // Build the report options we want to show + const recentReports = SidebarLinks.getRecentReports(nextProps); + + // Determine whether we need to keep the previous LHN order + const orderedReports = shouldReorder || switchingPriorityModes + ? recentReports + : _.chain(prevState.orderedReports) + + // To preserve the order of the conversations, we map over the previous state's order of reports. + // Then match and replace older reports with the newer report conversations from recentReports + .map(orderedReport => _.find(recentReports, recentReport => orderedReport.reportID === recentReport.reportID)) + + // Because we are using map, we have to filter out any undefined reports. This happens if recentReports + // does not have all the conversations in prevState.orderedReports + .compact() + .value(); + + return { + orderedReports, + priorityMode: nextProps.priorityMode, + activeReport: { + reportID: nextProps.currentlyViewedReportID, + hasDraftHistory, + lastMessageTimestamp, + }, + unreadReports: memoizeGetUnreadReports(nextProps.reports || {}), + }; + } + showSearchPage() { Navigation.navigate(ROUTES.SEARCH); } render() { - // Wait until the personalDetails are actually loaded before displaying the LHN + // Wait until the reports and personalDetails are actually loaded before displaying the LHN if (_.isEmpty(this.props.personalDetails)) { return null; } - Timing.start(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); - const optionListItems = this.getRecentReportsOptionListItems( - this.props.currentlyViewedReportID, - this.props.priorityMode, - this.props.reports, - this.props.personalDetails, - this.props.betas, - this.props.reportActions, - ); + const activeReportID = parseInt(this.props.currentlyViewedReportID, 10); const sections = [{ title: '', indexOffset: 0, - data: optionListItems, + data: this.state.orderedReports || [], shouldShow: true, }]; - Timing.end(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); return ( @@ -189,8 +298,8 @@ class SidebarLinks extends React.Component { {paddingBottom: StyleUtils.getSafeAreaMargins(this.props.insets).marginBottom}, ]} sections={sections} - focusedIndex={_.findIndex(optionListItems, ( - option => option.reportID.toString() === this.props.currentlyViewedReportID + focusedIndex={_.findIndex(this.state.orderedReports, ( + option => option.reportID === activeReportID ))} onSelectRow={(option) => { Navigation.navigate(ROUTES.getReportRoute(option.reportID)); diff --git a/tests/unit/LHNOrderTest.js b/tests/unit/LHNOrderTest.js index 3a0194f0f480..3e411f7ca251 100644 --- a/tests/unit/LHNOrderTest.js +++ b/tests/unit/LHNOrderTest.js @@ -158,42 +158,18 @@ Onyx.init({ }); function getDefaultRenderedSidebarLinks() { - // An ErrorBoundary needs to be added to the rendering so that any errors that happen while the component - // renders are logged to the console. Without an error boundary, Jest only reports the error like "The above error - // occurred in your component", except, there is no "above error". It's just swallowed up by Jest somewhere. - // With the ErrorBoundary, those errors are caught and logged to the console so you can find exactly which error - // might be causing a rendering issue when developing tests. - class ErrorBoundary extends React.Component { - // Error boundaries have to implement this method. It's for providing a fallback UI, but - // we don't need that for unit testing, so this is basically a no-op. - static getDerivedStateFromError(error) { - return {error}; - } - - componentDidCatch(error, errorInfo) { - console.error(error, errorInfo); - } - - render() { - // eslint-disable-next-line react/prop-types - return this.props.children; - } - } - // Wrap the SideBarLinks inside of LocaleContextProvider so that all the locale props // are passed to the component. If this is not done, then all the locale props are missing // and there are a lot of render warnings. It needs to be done like this because normally in // our app (App.js) is when the react application is wrapped in the context providers return render(( - - {}} - insets={fakeInsets} - onAvatarClick={() => {}} - isSmallScreenWidth={false} - /> - + {}} + insets={fakeInsets} + onAvatarClick={() => {}} + isSmallScreenWidth={false} + /> )); }