From fed705a508371e463dc87f77fcbeb4c6d22549e2 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 25 Aug 2022 20:03:33 +0200 Subject: [PATCH 01/46] Remove the last pieces out of state --- src/pages/home/sidebar/SidebarLinks.js | 265 +++++++++++++++++-------- 1 file changed, 179 insertions(+), 86 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 6e4fdeefc63c..f315178b70a7 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -120,82 +120,131 @@ function getUnreadReports(reportsObject) { const memoizeGetUnreadReports = memoizeOne(getUnreadReports); class SidebarLinks extends React.Component { - static getRecentReports(props) { - const activeReportID = parseInt(props.currentlyViewedReportID, 10); - const sidebarOptions = OptionsListUtils.getSidebarOptions( - props.reports, - props.personalDetails, - activeReportID, - props.priorityMode, - props.betas, - props.reportActions, - ); - return sidebarOptions.recentReports; - } + // static getRecentReports(props) { + // const activeReportID = parseInt(props.currentlyViewedReportID, 10); + // const sidebarOptions = OptionsListUtils.getSidebarOptions( + // props.reports, + // props.personalDetails, + // activeReportID, + // 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; + // } - /** - * 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. + constructor(props) { + super(props); - // We don't need to limit draft comment flashing for small screen widths as LHN is not visible. - if (nextProps.isSmallScreenWidth) { - return true; - } + this.getFilteredReports = _.memoize(this.getFilteredReports); - // Always update if LHN is empty. - if (orderedReports.length === 0) { - return true; - } + this.activeReport = { + reportID: props.currentlyViewedReportID, + }; - const didActiveReportChange = currentlyViewedReportID !== nextProps.currentlyViewedReportID; + this.orderedReports = []; + this.priorityMode = props.priorityMode; + this.unreadReports = memoizeGetUnreadReports(props.reports || {}); + } - // Always re-order the list whenever the active report is changed - if (didActiveReportChange) { - return true; - } + static getDerivedStateFromProps(nextProps, prevState) { + // const isActiveReportSame = prevState.activeReport.reportID === nextProps.currentlyViewedReportID; + // const lastMessageTimestamp = lodashGet(nextProps.reports, `${ONYXKEYS.COLLECTION.REPORT}${nextProps.currentlyViewedReportID}.lastMessageTimestamp`, 0); - // If any reports have new unread messages, re-order the list - const nextUnreadReports = memoizeGetUnreadReports(nextProps.reports || {}); - if (memoizeCheckForNewUnreadReports(nextUnreadReports, unreadReports)) { - return true; - } + // Determines if the active report has a history of draft comments while active. + // let hasDraftHistory; - // 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; - } + // 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); + // } - return true; - } + // const shouldReorder = SidebarLinks.shouldReorder(nextProps, hasDraftHistory, prevState.orderedReports, prevState.activeReport.reportID, prevState.unreadReports); + // const switchingPriorityModes = nextProps.priorityMode !== prevState.priorityMode; - constructor(props) { - super(props); + // Build the report options we want to show + // const recentReports = SidebarLinks.getRecentReports(nextProps); - 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 || {}), - }; + // 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 + // .filter(orderedReport => orderedReport !== undefined) + // .value(); + + // return { + // orderedReports, + // priorityMode: nextProps.priorityMode, + // activeReport: { + // reportID: nextProps.currentlyViewedReportID, + // hasDraftHistory, + // lastMessageTimestamp, + // }, + // unreadReports: memoizeGetUnreadReports(nextProps.reports || {}), + // }; } - static getDerivedStateFromProps(nextProps, prevState) { - const isActiveReportSame = prevState.activeReport.reportID === nextProps.currentlyViewedReportID; - const lastMessageTimestamp = lodashGet(nextProps.reports, `${ONYXKEYS.COLLECTION.REPORT}${nextProps.currentlyViewedReportID}.lastMessageTimestamp`, 0); + getFilteredReports(unfilteredReports) { + const isActiveReportSame = this.activeReport.reportID === this.props.currentlyViewedReportID; + const lastMessageTimestamp = lodashGet(unfilteredReports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.lastMessageTimestamp`, 0); // Determines if the active report has a history of draft comments while active. let hasDraftHistory; @@ -203,44 +252,88 @@ class SidebarLinks extends React.Component { // 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) { + if (isActiveReportSame && this.activeReport.lastMessageTimestamp !== lastMessageTimestamp) { hasDraftHistory = false; - } else if (isActiveReportSame && prevState.activeReport.hasDraftHistory) { + } else if (isActiveReportSame && this.activeReport.hasDraftHistory) { hasDraftHistory = true; } else { - hasDraftHistory = lodashGet(nextProps.reports, `${ONYXKEYS.COLLECTION.REPORT}${nextProps.currentlyViewedReportID}.hasDraft`, false); + hasDraftHistory = lodashGet(this.props.reports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.hasDraft`, false); } - const shouldReorder = SidebarLinks.shouldReorder(nextProps, hasDraftHistory, prevState.orderedReports, prevState.activeReport.reportID, prevState.unreadReports); - const switchingPriorityModes = nextProps.priorityMode !== prevState.priorityMode; + const shouldReorder = this.shouldReorder(hasDraftHistory); + const switchingPriorityModes = this.props.priorityMode !== this.priorityMode; // Build the report options we want to show - const recentReports = SidebarLinks.getRecentReports(nextProps); + const recentReports = this.getRecentReports(); - // Determine whether we need to keep the previous LHN order - const orderedReports = shouldReorder || switchingPriorityModes + this.orderedReports = shouldReorder || switchingPriorityModes ? recentReports - : _.chain(prevState.orderedReports) + : _.chain(this.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 + // 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 + // 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 .filter(orderedReport => orderedReport !== undefined) .value(); - return { - orderedReports, - priorityMode: nextProps.priorityMode, - activeReport: { - reportID: nextProps.currentlyViewedReportID, - hasDraftHistory, - lastMessageTimestamp, - }, - unreadReports: memoizeGetUnreadReports(nextProps.reports || {}), + this.priorityMode = this.props.priorityMode; + this.activeReport = { + reportID: this.props.currentlyViewedReportID, + hasDraftHistory, + lastMessageTimestamp, }; + this.unreadReports = memoizeGetUnreadReports(unfilteredReports || {}); + } + + getRecentReports() { + const activeReportID = parseInt(this.props.currentlyViewedReportID, 10); + const sidebarOptions = OptionsListUtils.getSidebarOptions( + this.props.reports, + this.props.personalDetails, + activeReportID, + this.props.priorityMode, + this.props.betas, + this.props.reportActions, + ); + return sidebarOptions.recentReports; + } + + shouldReorder(hasDraftHistory) { + // 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 (this.props.isSmallScreenWidth) { + return true; + } + + // Always update if LHN is empty. + if (this.orderedReports.length === 0) { + return true; + } + + const didActiveReportChange = this.activeReport.reportID !== this.props.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(this.props.reports || {}); + if (memoizeCheckForNewUnreadReports(nextUnreadReports, this.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 (this.props.currentlyViewedReportID && hasDraftHistory) { + return false; + } + + return true; } showSearchPage() { From 44a2ff60ed4f00429dd6e1759a49c39fd16ef079 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 07:27:42 +0100 Subject: [PATCH 02/46] Simplify the check for unread reports --- src/pages/home/sidebar/SidebarLinks.js | 179 +++++++++++++------------ 1 file changed, 92 insertions(+), 87 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index f315178b70a7..b4fb87f8e254 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -93,31 +93,31 @@ const defaultProps = { isSyncingData: false, }; -/** - * @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 && report.unreadActionCount > 0); - return unreadReports; -} -const memoizeGetUnreadReports = memoizeOne(getUnreadReports); +// /** +// * @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 && report.unreadActionCount > 0); +// return unreadReports; +// } +// const memoizeGetUnreadReports = memoizeOne(getUnreadReports); class SidebarLinks extends React.Component { // static getRecentReports(props) { @@ -192,55 +192,55 @@ class SidebarLinks extends React.Component { this.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 - // .filter(orderedReport => orderedReport !== undefined) - // .value(); - - // return { - // orderedReports, - // priorityMode: nextProps.priorityMode, - // activeReport: { - // reportID: nextProps.currentlyViewedReportID, - // hasDraftHistory, - // lastMessageTimestamp, - // }, - // unreadReports: memoizeGetUnreadReports(nextProps.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 + // // .filter(orderedReport => orderedReport !== undefined) + // // .value(); + // + // // return { + // // orderedReports, + // // priorityMode: nextProps.priorityMode, + // // activeReport: { + // // reportID: nextProps.currentlyViewedReportID, + // // hasDraftHistory, + // // lastMessageTimestamp, + // // }, + // // unreadReports: memoizeGetUnreadReports(nextProps.reports || {}), + // // }; + // } getFilteredReports(unfilteredReports) { const isActiveReportSame = this.activeReport.reportID === this.props.currentlyViewedReportID; @@ -302,38 +302,43 @@ class SidebarLinks extends React.Component { } shouldReorder(hasDraftHistory) { - // 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. + // Because: TBD + // @TODO try and figure out why if (this.props.isSmallScreenWidth) { return true; } // Always update if LHN is empty. + // Because: TBD + // @TODO try and figure out why if (this.orderedReports.length === 0) { return true; } - const didActiveReportChange = this.activeReport.reportID !== this.props.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(this.props.reports || {}); - if (memoizeCheckForNewUnreadReports(nextUnreadReports, this.unreadReports)) { + // Because: TBD + // @TODO try and figure out why + if (this.activeReport.reportID !== this.props.currentlyViewedReportID) { return true; } // If there is an active report that either had or has a draft, we do not want to re-order the list + // because the position of the report in the list won't change if (this.props.currentlyViewedReportID && hasDraftHistory) { return false; } - return true; + // If any reports have new unread messages, the list needs to be reordered + // because the unread reports need to be placed at the top of the list + const hasNewUnreadReports = _.some(this.props.reports, (report) => { + return report.unreadActionCount > 0 && !this.unreadReports[report.reportID]; + }); + if (hasNewUnreadReports) { + return true; + } + + return false; } showSearchPage() { From 57a4c2e3bba88581e34315d50e80967fe280149e Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 07:38:43 +0100 Subject: [PATCH 03/46] Simplify the map of unread reports --- src/pages/home/sidebar/SidebarLinks.js | 32 ++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index b4fb87f8e254..382569468b64 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -189,7 +189,7 @@ class SidebarLinks extends React.Component { this.orderedReports = []; this.priorityMode = props.priorityMode; - this.unreadReports = memoizeGetUnreadReports(props.reports || {}); + this.unreadReports = this.getUnreadReports(props.reports); } // static getDerivedStateFromProps(nextProps, prevState) { @@ -285,7 +285,31 @@ class SidebarLinks extends React.Component { hasDraftHistory, lastMessageTimestamp, }; - this.unreadReports = memoizeGetUnreadReports(unfilteredReports || {}); + this.unreadReports = this.getUnreadReports(unfilteredReports); + } + + /** + * Create a map of unread reports that looks like this: + * { + * 1: true, + * 2: true, + * } + * This is so that when the new props are compared to the old props, it's + * fast to look up if there are any new unread reports. + * + * @param {Object[]} reports + * @returns {Object} + */ + getUnreadReports(reports) { + return _.reduce(unfilteredReports, (finalUnreadReportMap, report) => { + if (report.unreadActionCount > 0) { + return { + [report.reportID]: true, + ...finalUnreadReportMap, + }; + } + return finalUnreadReportMap; + }, {}); } getRecentReports() { @@ -331,9 +355,7 @@ class SidebarLinks extends React.Component { // If any reports have new unread messages, the list needs to be reordered // because the unread reports need to be placed at the top of the list - const hasNewUnreadReports = _.some(this.props.reports, (report) => { - return report.unreadActionCount > 0 && !this.unreadReports[report.reportID]; - }); + const hasNewUnreadReports = _.some(this.props.reports, report => report.unreadActionCount > 0 && !this.unreadReports[report.reportID]); if (hasNewUnreadReports) { return true; } From 4b40492308160b8eba91d27be6e3d591384225f6 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 07:39:30 +0100 Subject: [PATCH 04/46] Correct outdated comment --- src/pages/home/sidebar/SidebarLinks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 382569468b64..c0c0442009a7 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -368,7 +368,7 @@ class SidebarLinks extends React.Component { } render() { - // Wait until the reports and personalDetails are actually loaded before displaying the LHN + // Wait until the personalDetails are actually loaded before displaying the LHN if (_.isEmpty(this.props.personalDetails)) { return null; } From 00ebe57e4d931d93a4b40dbe9fbc4a0b5b6eac97 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 07:42:02 +0100 Subject: [PATCH 05/46] Rename method to be more apparent --- src/pages/home/sidebar/SidebarLinks.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index c0c0442009a7..60a99f90da8f 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -260,7 +260,7 @@ class SidebarLinks extends React.Component { hasDraftHistory = lodashGet(this.props.reports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.hasDraft`, false); } - const shouldReorder = this.shouldReorder(hasDraftHistory); + const shouldReorder = this.shouldReorderReports(hasDraftHistory); const switchingPriorityModes = this.props.priorityMode !== this.priorityMode; // Build the report options we want to show @@ -301,7 +301,7 @@ class SidebarLinks extends React.Component { * @returns {Object} */ getUnreadReports(reports) { - return _.reduce(unfilteredReports, (finalUnreadReportMap, report) => { + return _.reduce(reports, (finalUnreadReportMap, report) => { if (report.unreadActionCount > 0) { return { [report.reportID]: true, @@ -325,7 +325,7 @@ class SidebarLinks extends React.Component { return sidebarOptions.recentReports; } - shouldReorder(hasDraftHistory) { + shouldReorderReports(hasDraftHistory) { // We don't need to limit draft comment flashing for small screen widths as LHN is not visible. // Because: TBD // @TODO try and figure out why From 103b126e819c6112813cc64c9e18a72ebf96939d Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 07:49:52 +0100 Subject: [PATCH 06/46] Access filtered report options direction from render --- src/pages/home/sidebar/SidebarLinks.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 60a99f90da8f..1783f60faa73 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -264,7 +264,7 @@ class SidebarLinks extends React.Component { const switchingPriorityModes = this.props.priorityMode !== this.priorityMode; // Build the report options we want to show - const recentReports = this.getRecentReports(); + const recentReports = this.getRecentReportsOptionListItems(); this.orderedReports = shouldReorder || switchingPriorityModes ? recentReports @@ -286,6 +286,8 @@ class SidebarLinks extends React.Component { lastMessageTimestamp, }; this.unreadReports = this.getUnreadReports(unfilteredReports); + + return this.orderedReports; } /** @@ -312,7 +314,7 @@ class SidebarLinks extends React.Component { }, {}); } - getRecentReports() { + getRecentReportsOptionListItems() { const activeReportID = parseInt(this.props.currentlyViewedReportID, 10); const sidebarOptions = OptionsListUtils.getSidebarOptions( this.props.reports, @@ -377,7 +379,7 @@ class SidebarLinks extends React.Component { const sections = [{ title: '', indexOffset: 0, - data: this.state.orderedReports || [], + data: this.getFilteredReports(this.props.reports), shouldShow: true, }]; From bb57fb055365b9d173e0675adb377024af2aeccb Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 17:33:03 +0200 Subject: [PATCH 07/46] Remove the import of unused library --- src/pages/home/sidebar/SidebarLinks.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 1783f60faa73..1afda46fecba 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -4,7 +4,6 @@ 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'; import ONYXKEYS from '../../../ONYXKEYS'; @@ -431,7 +430,7 @@ class SidebarLinks extends React.Component { {paddingBottom: StyleUtils.getSafeAreaMargins(this.props.insets).marginBottom}, ]} sections={sections} - focusedIndex={_.findIndex(this.state.orderedReports, ( + focusedIndex={_.findIndex(this.orderedReports, ( option => option.reportID === activeReportID ))} onSelectRow={(option) => { From f3a7fa083fad9746ad174cf52748c129ee5c1529 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 17:33:13 +0200 Subject: [PATCH 08/46] Fix a prop warning --- src/pages/home/report/ReportActionsList.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index a78f672d3901..f5559585d2d3 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/src/pages/home/report/ReportActionsList.js @@ -50,7 +50,7 @@ const propTypes = { mostRecentIOUReportSequenceNumber: PropTypes.number, /** Are we loading more report actions? */ - isLoadingMoreReportActions: PropTypes.bool.isRequired, + isLoadingMoreReportActions: PropTypes.bool, /** Callback executed on list layout */ onLayout: PropTypes.func.isRequired, @@ -68,6 +68,7 @@ const propTypes = { const defaultProps = { personalDetails: {}, mostRecentIOUReportSequenceNumber: undefined, + isLoadingMoreReportActions: false, }; class ReportActionsList extends React.Component { From 35cfd235dae8e900b88760e7c5a8443e43f4e69a Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 17:49:03 +0200 Subject: [PATCH 09/46] Add some performance timing for filtering the reports --- src/CONST.js | 1 + src/pages/home/sidebar/SidebarLinks.js | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/CONST.js b/src/CONST.js index a6b7586c217b..0add0513eecc 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -343,6 +343,7 @@ 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/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 1afda46fecba..c0f3bc48cad0 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -27,6 +27,7 @@ import * as ReportUtils from '../../../libs/ReportUtils'; import networkPropTypes from '../../../components/networkPropTypes'; import {withNetwork} from '../../../components/OnyxProvider'; import withCurrentUserPersonalDetails from '../../../components/withCurrentUserPersonalDetails'; +import Timing from '../../../libs/actions/Timing'; const propTypes = { /** Toggles the navigation menu open and closed */ @@ -180,7 +181,7 @@ class SidebarLinks extends React.Component { constructor(props) { super(props); - this.getFilteredReports = _.memoize(this.getFilteredReports); + this.getFilteredReports = this.getFilteredReports.bind(this); this.activeReport = { reportID: props.currentlyViewedReportID, @@ -375,12 +376,14 @@ class SidebarLinks extends React.Component { } const activeReportID = parseInt(this.props.currentlyViewedReportID, 10); + Timing.start(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); const sections = [{ title: '', indexOffset: 0, data: this.getFilteredReports(this.props.reports), shouldShow: true, }]; + Timing.end(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); return ( From 8e8ba1f926819fe8bb3502f87bd3acb59a9e5db7 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 17:51:28 +0200 Subject: [PATCH 10/46] Rename method to be more clear --- src/pages/home/sidebar/SidebarLinks.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index c0f3bc48cad0..d09ff1192f05 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -181,7 +181,7 @@ class SidebarLinks extends React.Component { constructor(props) { super(props); - this.getFilteredReports = this.getFilteredReports.bind(this); + this.getFilteredAndOrderedReports = this.getFilteredAndOrderedReports.bind(this); this.activeReport = { reportID: props.currentlyViewedReportID, @@ -242,7 +242,7 @@ class SidebarLinks extends React.Component { // // }; // } - getFilteredReports(unfilteredReports) { + getFilteredAndOrderedReports(unfilteredReports) { const isActiveReportSame = this.activeReport.reportID === this.props.currentlyViewedReportID; const lastMessageTimestamp = lodashGet(unfilteredReports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.lastMessageTimestamp`, 0); @@ -266,7 +266,7 @@ class SidebarLinks extends React.Component { // Build the report options we want to show const recentReports = this.getRecentReportsOptionListItems(); - this.orderedReports = shouldReorder || switchingPriorityModes + const orderedReports = shouldReorder || switchingPriorityModes ? recentReports : _.chain(this.orderedReports) @@ -279,6 +279,10 @@ class SidebarLinks extends React.Component { .filter(orderedReport => orderedReport !== undefined) .value(); + // Store these pieces of data on the class so that the next time this method is called + // the previous values can be compared against to tell if something changed which would + // cause the reports to be reordered + this.orderedReports = orderedReports; this.priorityMode = this.props.priorityMode; this.activeReport = { reportID: this.props.currentlyViewedReportID, @@ -380,7 +384,7 @@ class SidebarLinks extends React.Component { const sections = [{ title: '', indexOffset: 0, - data: this.getFilteredReports(this.props.reports), + data: this.getFilteredAndOrderedReports(this.props.reports), shouldShow: true, }]; Timing.end(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); From 6a26a03708c7352b2473c1e76d862e0c365fac74 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 20:16:43 +0200 Subject: [PATCH 11/46] Remove mobile preoptimization --- src/pages/home/sidebar/SidebarLinks.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index d09ff1192f05..0ecee2ff2018 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -332,13 +332,6 @@ class SidebarLinks extends React.Component { } shouldReorderReports(hasDraftHistory) { - // We don't need to limit draft comment flashing for small screen widths as LHN is not visible. - // Because: TBD - // @TODO try and figure out why - if (this.props.isSmallScreenWidth) { - return true; - } - // Always update if LHN is empty. // Because: TBD // @TODO try and figure out why From d97377655124db1839a00dbbbd044acbdcb9a5f4 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Sat, 27 Aug 2022 20:25:22 +0200 Subject: [PATCH 12/46] Correct the language in a comment --- src/pages/home/sidebar/SidebarLinks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 0ecee2ff2018..e51054b6e072 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -270,12 +270,12 @@ class SidebarLinks extends React.Component { ? recentReports : _.chain(this.orderedReports) - // To preserve the order of the conversations, we map over the previous state's order of reports. + // To preserve the order of the conversations, we map over the previous ordered 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 + // does not have all the conversations in the previous set of orderedReports .filter(orderedReport => orderedReport !== undefined) .value(); From b770d0ce445cb7b11b9a1a90db9c9e082de2c9b2 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 30 Aug 2022 17:26:32 +0200 Subject: [PATCH 13/46] Remove old code --- src/pages/home/sidebar/SidebarLinks.js | 134 ------------------------- 1 file changed, 134 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index e51054b6e072..da5c9e4ab86c 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -93,91 +93,7 @@ const defaultProps = { isSyncingData: false, }; -// /** -// * @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 && report.unreadActionCount > 0); -// return unreadReports; -// } -// const memoizeGetUnreadReports = memoizeOne(getUnreadReports); - class SidebarLinks extends React.Component { - // static getRecentReports(props) { - // const activeReportID = parseInt(props.currentlyViewedReportID, 10); - // const sidebarOptions = OptionsListUtils.getSidebarOptions( - // props.reports, - // props.personalDetails, - // activeReportID, - // 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); @@ -192,56 +108,6 @@ class SidebarLinks extends React.Component { this.unreadReports = this.getUnreadReports(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 - // // .filter(orderedReport => orderedReport !== undefined) - // // .value(); - // - // // return { - // // orderedReports, - // // priorityMode: nextProps.priorityMode, - // // activeReport: { - // // reportID: nextProps.currentlyViewedReportID, - // // hasDraftHistory, - // // lastMessageTimestamp, - // // }, - // // unreadReports: memoizeGetUnreadReports(nextProps.reports || {}), - // // }; - // } - getFilteredAndOrderedReports(unfilteredReports) { const isActiveReportSame = this.activeReport.reportID === this.props.currentlyViewedReportID; const lastMessageTimestamp = lodashGet(unfilteredReports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.lastMessageTimestamp`, 0); From 962697294e61ef5625e90c0f582295a4571ad2f7 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 30 Aug 2022 17:33:51 +0200 Subject: [PATCH 14/46] Rename method to make more sense and add comments --- src/pages/home/sidebar/SidebarLinks.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index fb53b5e15021..31ad2b18b9b9 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -117,13 +117,16 @@ class SidebarLinks extends React.Component { hasDraftHistory = lodashGet(this.props.reports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.hasDraft`, false); } - const shouldReorder = this.shouldReorderReports(hasDraftHistory); const switchingPriorityModes = this.props.priorityMode !== this.priorityMode; // Build the report options we want to show const recentReports = this.getRecentReportsOptionListItems(); - const orderedReports = shouldReorder || switchingPriorityModes + // If the order of the reports is different from the last render (or if priority mode is changing) + // then orderedReports is the same as the freshly calculated recentReports. + // If the order of the reports is the same as the last render + // then the data for each report is updated from the data in the new props (not sure why this is necessary) + const orderedReports = this.isReportOrderDifferentThanLastRender(hasDraftHistory) || switchingPriorityModes ? recentReports : _.chain(this.orderedReports) @@ -188,7 +191,7 @@ class SidebarLinks extends React.Component { return sidebarOptions.recentReports; } - shouldReorderReports(hasDraftHistory) { + isReportOrderDifferentThanLastRender(hasDraftHistory) { // Always update if LHN is empty. // Because: TBD // @TODO try and figure out why From 6091608c2bd9270fbb0f39756eab70dbd16589b0 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 30 Aug 2022 17:40:08 +0200 Subject: [PATCH 15/46] Add comments to the logic for ordering reports --- src/pages/home/sidebar/SidebarLinks.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 31ad2b18b9b9..b5d73c968de9 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -192,33 +192,33 @@ class SidebarLinks extends React.Component { } isReportOrderDifferentThanLastRender(hasDraftHistory) { - // Always update if LHN is empty. - // Because: TBD - // @TODO try and figure out why - if (this.orderedReports.length === 0) { + // If the number of reports changed, then the report order is different + if (this.orderedReports.length !== this.props.reports.length) { return true; } - // Always re-order the list whenever the active report is changed - // Because: TBD - // @TODO try and figure out why + // If the active report changed, then the report order is different if (this.activeReport.reportID !== this.props.currentlyViewedReportID) { return true; } - // If there is an active report that either had or has a draft, we do not want to re-order the list - // because the position of the report in the list won't change + // If the active report has a draft, the order of the reports doesn't change + // because it would cause the reports to reorder when a user starts typing a comment + // and that is an annoying UX (too much stuff jumping around) if (this.props.currentlyViewedReportID && hasDraftHistory) { return false; } - // If any reports have new unread messages, the list needs to be reordered + // If the unread reports have changed, then the report order changes // because the unread reports need to be placed at the top of the list + // @TODO: This can probably be optimized const hasNewUnreadReports = _.some(this.props.reports, report => report.unreadActionCount > 0 && !this.unreadReports[report.reportID]); if (hasNewUnreadReports) { return true; } + // By default, assume that the order of the reports doesn't change + // in order to optimize the rendering return false; } From 56d6d3416125f9d36bc2af37a263e69cd6ea00ec Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 30 Aug 2022 20:45:12 +0200 Subject: [PATCH 16/46] Add a todo --- src/pages/home/sidebar/SidebarLinks.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index b5d73c968de9..392c74e9a50b 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -125,7 +125,8 @@ class SidebarLinks extends React.Component { // If the order of the reports is different from the last render (or if priority mode is changing) // then orderedReports is the same as the freshly calculated recentReports. // If the order of the reports is the same as the last render - // then the data for each report is updated from the data in the new props (not sure why this is necessary) + // then the data for each report is updated from the data in the new props + // @TODO: not sure why this is necessary and see if it can be removed or do something more intuitive const orderedReports = this.isReportOrderDifferentThanLastRender(hasDraftHistory) || switchingPriorityModes ? recentReports : _.chain(this.orderedReports) From 44e258cf9e7568629104055bf774fdad92f67448 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 31 Aug 2022 17:10:32 +0200 Subject: [PATCH 17/46] Fix proptype warnings --- src/pages/home/ReportScreen.js | 5 ++--- src/pages/home/report/ReportActionCompose.js | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 717963c3dd49..2f5c2742eb24 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -60,7 +60,7 @@ const propTypes = { isLoadingReportActions: PropTypes.bool, /** ID for the report */ - reportID: PropTypes.string, + reportID: PropTypes.number, }), /** Array of report actions for this report */ @@ -111,8 +111,7 @@ const defaultProps = { * @returns {Number} */ function getReportID(route) { - const params = route.params; - return Number.parseInt(params.reportID, 10); + return route.params.reportID.toString(); } class ReportScreen extends React.Component { diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 61369be57010..a82101de219f 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.number.isRequired, + reportID: PropTypes.string.isRequired, /** Details about any modals being used */ modal: PropTypes.shape({ From bd05cecc019cb26d946b57d55e7fc897e9fc1359 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 11:21:13 +0200 Subject: [PATCH 18/46] Remove unnecessary binding --- src/pages/home/sidebar/SidebarLinks.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 3ad2041655ce..d9365e5d2327 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -91,8 +91,6 @@ class SidebarLinks extends React.Component { constructor(props) { super(props); - this.getFilteredAndOrderedReports = this.getFilteredAndOrderedReports.bind(this); - this.activeReport = { reportID: props.currentlyViewedReportID, }; @@ -237,14 +235,15 @@ class SidebarLinks extends React.Component { } const activeReportID = parseInt(this.props.currentlyViewedReportID, 10); - Timing.start(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); + // Timing.start(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); const sections = [{ title: '', indexOffset: 0, data: this.getFilteredAndOrderedReports(this.props.reports), shouldShow: true, }]; - Timing.end(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); + // const sections = []; + // Timing.end(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); return ( From 475f0eddd25867b68df048bc8e0d42074587a149 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 11:22:00 +0200 Subject: [PATCH 19/46] Remove debug code that is not needed --- src/pages/home/sidebar/SidebarLinks.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index d9365e5d2327..3b5c9e8b5521 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -235,15 +235,14 @@ class SidebarLinks extends React.Component { } const activeReportID = parseInt(this.props.currentlyViewedReportID, 10); - // Timing.start(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); + Timing.start(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); const sections = [{ title: '', indexOffset: 0, data: this.getFilteredAndOrderedReports(this.props.reports), shouldShow: true, }]; - // const sections = []; - // Timing.end(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); + Timing.end(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); return ( From 80615c9446ef933bb36b1d35791de1cb9775c3cb Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 11:40:38 +0200 Subject: [PATCH 20/46] Fix proptype warning --- src/components/OptionsSelector/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/OptionsSelector/index.js b/src/components/OptionsSelector/index.js index 14495ecae24d..9415c4e3b279 100644 --- a/src/components/OptionsSelector/index.js +++ b/src/components/OptionsSelector/index.js @@ -1,6 +1,7 @@ import React, {forwardRef} from 'react'; import BaseOptionsSelector from './BaseOptionsSelector'; import {propTypes, defaultProps} from './optionsSelectorPropTypes'; +import withLocalize from '../withLocalize'; const OptionsSelector = forwardRef((props, ref) => ( Date: Thu, 1 Sep 2022 11:41:29 +0200 Subject: [PATCH 21/46] Don't cast activeReportID to string --- src/libs/OptionsListUtils.js | 4 ++-- src/pages/home/sidebar/SidebarLinks.js | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index d17c4bb4d330..0139a3ad247d 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -349,7 +349,7 @@ function isCurrentUser(userDetails) { * * @param {Object} reports * @param {Object} personalDetails - * @param {Number} activeReportID + * @param {String} activeReportID * @param {Object} options * @returns {Object} * @private @@ -428,7 +428,7 @@ function getOptions(reports, personalDetails, activeReportID, { const shouldFilterReportIfRead = hideReadReports && report.unreadActionCount === 0; const shouldFilterReport = shouldFilterReportIfEmpty || shouldFilterReportIfRead; - if (report.reportID !== activeReportID + if (report.reportID.toString() !== activeReportID.toString() && !report.isPinned && !hasDraftComment && shouldFilterReport diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 3b5c9e8b5521..1b0148e6b53c 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -101,6 +101,8 @@ class SidebarLinks extends React.Component { } getFilteredAndOrderedReports(unfilteredReports) { + return this.getRecentReportsOptionListItems(); + const isActiveReportSame = this.activeReport.reportID === this.props.currentlyViewedReportID; const lastMessageTimestamp = lodashGet(unfilteredReports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.lastMessageTimestamp`, 0); @@ -181,7 +183,7 @@ class SidebarLinks extends React.Component { } getRecentReportsOptionListItems() { - const activeReportID = parseInt(this.props.currentlyViewedReportID, 10); + const activeReportID = this.props.currentlyViewedReportID; const sidebarOptions = OptionsListUtils.getSidebarOptions( this.props.reports, this.props.personalDetails, From 8512aa094133c2155e74ff5a2e295f41a7439ca0 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 11:46:47 +0200 Subject: [PATCH 22/46] Remove all data processing --- src/pages/home/sidebar/SidebarLinks.js | 194 ++++++++++++------------- 1 file changed, 97 insertions(+), 97 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 1b0148e6b53c..41a49e37076b 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -91,71 +91,71 @@ class SidebarLinks extends React.Component { constructor(props) { super(props); - this.activeReport = { - reportID: props.currentlyViewedReportID, - }; - - this.orderedReports = []; - this.priorityMode = props.priorityMode; - this.unreadReports = this.getUnreadReports(props.reports); + // this.activeReport = { + // reportID: props.currentlyViewedReportID, + // }; + // + // this.orderedReports = []; + // this.priorityMode = props.priorityMode; + // this.unreadReports = this.getUnreadReports(props.reports); } getFilteredAndOrderedReports(unfilteredReports) { return this.getRecentReportsOptionListItems(); - const isActiveReportSame = this.activeReport.reportID === this.props.currentlyViewedReportID; - const lastMessageTimestamp = lodashGet(unfilteredReports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.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 && this.activeReport.lastMessageTimestamp !== lastMessageTimestamp) { - hasDraftHistory = false; - } else if (isActiveReportSame && this.activeReport.hasDraftHistory) { - hasDraftHistory = true; - } else { - hasDraftHistory = lodashGet(this.props.reports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.hasDraft`, false); - } - - const switchingPriorityModes = this.props.priorityMode !== this.priorityMode; - - // Build the report options we want to show - const recentReports = this.getRecentReportsOptionListItems(); - - // If the order of the reports is different from the last render (or if priority mode is changing) - // then orderedReports is the same as the freshly calculated recentReports. - // If the order of the reports is the same as the last render - // then the data for each report is updated from the data in the new props - // @TODO: not sure why this is necessary and see if it can be removed or do something more intuitive - const orderedReports = this.isReportOrderDifferentThanLastRender(hasDraftHistory) || switchingPriorityModes - ? recentReports - : _.chain(this.orderedReports) - - // To preserve the order of the conversations, we map over the previous ordered 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 the previous set of orderedReports - .compact() - .value(); - - // Store these pieces of data on the class so that the next time this method is called - // the previous values can be compared against to tell if something changed which would - // cause the reports to be reordered - this.orderedReports = orderedReports; - this.priorityMode = this.props.priorityMode; - this.activeReport = { - reportID: this.props.currentlyViewedReportID, - hasDraftHistory, - lastMessageTimestamp, - }; - this.unreadReports = this.getUnreadReports(unfilteredReports); - - return this.orderedReports; + // const isActiveReportSame = this.activeReport.reportID === this.props.currentlyViewedReportID; + // const lastMessageTimestamp = lodashGet(unfilteredReports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.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 && this.activeReport.lastMessageTimestamp !== lastMessageTimestamp) { + // hasDraftHistory = false; + // } else if (isActiveReportSame && this.activeReport.hasDraftHistory) { + // hasDraftHistory = true; + // } else { + // hasDraftHistory = lodashGet(this.props.reports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.hasDraft`, false); + // } + // + // const switchingPriorityModes = this.props.priorityMode !== this.priorityMode; + // + // // Build the report options we want to show + // const recentReports = this.getRecentReportsOptionListItems(); + // + // // If the order of the reports is different from the last render (or if priority mode is changing) + // // then orderedReports is the same as the freshly calculated recentReports. + // // If the order of the reports is the same as the last render + // // then the data for each report is updated from the data in the new props + // // @TODO: not sure why this is necessary and see if it can be removed or do something more intuitive + // const orderedReports = this.isReportOrderDifferentThanLastRender(hasDraftHistory) || switchingPriorityModes + // ? recentReports + // : _.chain(this.orderedReports) + // + // // To preserve the order of the conversations, we map over the previous ordered 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 the previous set of orderedReports + // .compact() + // .value(); + // + // // Store these pieces of data on the class so that the next time this method is called + // // the previous values can be compared against to tell if something changed which would + // // cause the reports to be reordered + // this.orderedReports = orderedReports; + // this.priorityMode = this.props.priorityMode; + // this.activeReport = { + // reportID: this.props.currentlyViewedReportID, + // hasDraftHistory, + // lastMessageTimestamp, + // }; + // this.unreadReports = this.getUnreadReports(unfilteredReports); + // + // return this.orderedReports; } /** @@ -171,15 +171,15 @@ class SidebarLinks extends React.Component { * @returns {Object} */ getUnreadReports(reports) { - return _.reduce(reports, (finalUnreadReportMap, report) => { - if (report.unreadActionCount > 0) { - return { - [report.reportID]: true, - ...finalUnreadReportMap, - }; - } - return finalUnreadReportMap; - }, {}); + // return _.reduce(reports, (finalUnreadReportMap, report) => { + // if (report.unreadActionCount > 0) { + // return { + // [report.reportID]: true, + // ...finalUnreadReportMap, + // }; + // } + // return finalUnreadReportMap; + // }, {}); } getRecentReportsOptionListItems() { @@ -196,34 +196,34 @@ class SidebarLinks extends React.Component { } isReportOrderDifferentThanLastRender(hasDraftHistory) { - // If the number of reports changed, then the report order is different - if (this.orderedReports.length !== this.props.reports.length) { - return true; - } - - // If the active report changed, then the report order is different - if (this.activeReport.reportID !== this.props.currentlyViewedReportID) { - return true; - } - - // If the active report has a draft, the order of the reports doesn't change - // because it would cause the reports to reorder when a user starts typing a comment - // and that is an annoying UX (too much stuff jumping around) - if (this.props.currentlyViewedReportID && hasDraftHistory) { - return false; - } - - // If the unread reports have changed, then the report order changes - // because the unread reports need to be placed at the top of the list - // @TODO: This can probably be optimized - const hasNewUnreadReports = _.some(this.props.reports, report => report.unreadActionCount > 0 && !this.unreadReports[report.reportID]); - if (hasNewUnreadReports) { - return true; - } - - // By default, assume that the order of the reports doesn't change - // in order to optimize the rendering - return false; + // // If the number of reports changed, then the report order is different + // if (this.orderedReports.length !== this.props.reports.length) { + // return true; + // } + // + // // If the active report changed, then the report order is different + // if (this.activeReport.reportID !== this.props.currentlyViewedReportID) { + // return true; + // } + // + // // If the active report has a draft, the order of the reports doesn't change + // // because it would cause the reports to reorder when a user starts typing a comment + // // and that is an annoying UX (too much stuff jumping around) + // if (this.props.currentlyViewedReportID && hasDraftHistory) { + // return false; + // } + // + // // If the unread reports have changed, then the report order changes + // // because the unread reports need to be placed at the top of the list + // // @TODO: This can probably be optimized + // const hasNewUnreadReports = _.some(this.props.reports, report => report.unreadActionCount > 0 && !this.unreadReports[report.reportID]); + // if (hasNewUnreadReports) { + // return true; + // } + // + // // By default, assume that the order of the reports doesn't change + // // in order to optimize the rendering + // return false; } showSearchPage() { From ee56ec42828b1440824922a210ba973809be48ae Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 11:53:10 +0200 Subject: [PATCH 23/46] Add report actions to the propTypes --- src/pages/home/sidebar/SidebarLinks.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 41a49e37076b..15d9652bb058 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -26,6 +26,7 @@ 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 */ @@ -53,6 +54,9 @@ 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), @@ -79,6 +83,7 @@ const propTypes = { const defaultProps = { reports: {}, + reportActions: {}, personalDetails: {}, currentUserPersonalDetails: { avatar: ReportUtils.getDefaultAvatar(), @@ -100,6 +105,27 @@ class SidebarLinks extends React.Component { // this.unreadReports = this.getUnreadReports(props.reports); } + componentDidUpdate(prevProps) { + if (!_.isEqual(prevProps.reports, this.props.reports)) { + console.log('!!! props.reports') + } + if (!_.isEqual(prevProps.personalDetails, this.props.personalDetails)) { + console.log('!!! props.personalDetails') + } + if (!_.isEqual(prevProps.currentUserPersonalDetails, this.props.currentUserPersonalDetails)) { + console.log('!!! props.currentUserPersonalDetails') + } + if (!_.isEqual(prevProps.currentlyViewedReportID, this.props.currentlyViewedReportID)) { + console.log('!!! props.currentlyViewedReportID') + } + if (!_.isEqual(prevProps.priorityMode, this.props.priorityMode)) { + console.log('!!! props.priorityMode') + } + if (!_.isEqual(prevProps.reportActions, this.props.reportActions)) { + console.log('!!! props.reportActions') + } + } + getFilteredAndOrderedReports(unfilteredReports) { return this.getRecentReportsOptionListItems(); From e7f7b338ad12650602c48add0d309f1d313a3774 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 11:59:35 +0200 Subject: [PATCH 24/46] WIP on profiling what makes things rerender when switching reports --- src/pages/home/sidebar/SidebarLinks.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 15d9652bb058..5017b8b9c25f 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -108,22 +108,45 @@ class SidebarLinks extends React.Component { componentDidUpdate(prevProps) { if (!_.isEqual(prevProps.reports, this.props.reports)) { console.log('!!! props.reports') + return; } if (!_.isEqual(prevProps.personalDetails, this.props.personalDetails)) { console.log('!!! props.personalDetails') + return; } if (!_.isEqual(prevProps.currentUserPersonalDetails, this.props.currentUserPersonalDetails)) { console.log('!!! props.currentUserPersonalDetails') + return; } if (!_.isEqual(prevProps.currentlyViewedReportID, this.props.currentlyViewedReportID)) { console.log('!!! props.currentlyViewedReportID') + return; } if (!_.isEqual(prevProps.priorityMode, this.props.priorityMode)) { console.log('!!! props.priorityMode') + return; } if (!_.isEqual(prevProps.reportActions, this.props.reportActions)) { console.log('!!! props.reportActions') + return; } + if (!_.isEqual(prevProps.isSmallScreenWidth, this.props.isSmallScreenWidth)) { + console.log('!!! props.isSmallScreenWidth') + return; + } + if (!_.isEqual(prevProps.insets, this.props.insets)) { + console.log('!!! props.insets') + return; + } + if (!_.isEqual(prevProps.onAvatarClick, this.props.onAvatarClick)) { + console.log('!!! props.onAvatarClick') + return; + } + if (!_.isEqual(prevProps.onLinkClick, this.props.onLinkClick)) { + console.log('!!! props.onLinkClick') + return; + } + console.log('!!! unknown') } getFilteredAndOrderedReports(unfilteredReports) { From 8057f213b78b3d40ef88086dd01bcb305be9ca2f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 12:10:42 +0200 Subject: [PATCH 25/46] Fix proptype warning --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index d2c40a74b477..3bfcc71fd4c9 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.isRequired, + shiftHorizontal: PropTypes.number, /** 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.isRequired, + shiftVertical: PropTypes.number, /** Text to be shown in the tooltip */ text: PropTypes.string.isRequired, @@ -43,6 +43,11 @@ 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. @@ -132,5 +137,6 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } TooltipRenderedOnPageBody.propTypes = propTypes; +TooltipRenderedOnPageBody.defaultProps = defaultProps; export default TooltipRenderedOnPageBody; From ff4994827b932856e3a4b99aa5e723c4dab6fec9 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 12:51:02 +0200 Subject: [PATCH 26/46] WIP optimizing create option --- src/libs/OptionsListUtils.js | 49 ++++++++++++++------------ src/libs/ReportUtils.js | 7 ++-- src/pages/home/sidebar/SidebarLinks.js | 44 ----------------------- 3 files changed, 31 insertions(+), 69 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 0139a3ad247d..bf208198dae4 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -224,29 +224,31 @@ function createOption(logins, personalDetails, report, reportActions = {}, { const isArchivedRoom = ReportUtils.isArchivedRoom(report); const hasMultipleParticipants = personalDetailList.length > 1 || isChatRoom || isPolicyExpenseChat; const personalDetail = personalDetailList[0]; - 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', '')); + const hasOutstandingIOU = report && report.hasOutstandingIOU || false; + const iouReport = hasOutstandingIOU && iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`] || {}; + const lastActorDetails = report && personalDetailMap[report.lastActorEmail] || null; + let lastMessageTextFromReport = ''; + if (report) { + if (ReportUtils.isReportMessageAttachment({text: report.lastMessageText, html: report.lastMessageHtml})) { + lastMessageTextFromReport = `[${Localize.translateLocal('common.attachment')}]`; + } else { + lastMessageTextFromReport = Str.htmlDecode(report ? 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); + 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: lodashGet(lastActorDetails, 'displayName', report.lastActorEmail), + displayName: archiveReason.displayName || report.lastActorEmail, policyName: ReportUtils.getPolicyName(report, policies), }); } - const tooltipText = ReportUtils.getReportParticipantsTitle(lodashGet(report, ['participants'], [])); + const tooltipText = report && ReportUtils.getReportParticipantsTitle(report.participants || []) || null; const subtitle = ReportUtils.getChatRoomSubtitle(report, policies); const reportName = ReportUtils.getReportName(report, personalDetailMap, policies); let alternateText; @@ -263,9 +265,9 @@ function createOption(logins, personalDetails, report, reportActions = {}, { text: reportName, alternateText, brickRoadIndicator: getBrickRoadIndicatorStatusForReport(report, reportActions), - icons: ReportUtils.getIcons(report, personalDetails, policies, lodashGet(personalDetail, ['avatar'])), + icons: ReportUtils.getIcons(report, personalDetails, policies, personalDetail.avatar), tooltipText, - ownerEmail: lodashGet(report, ['ownerEmail']), + ownerEmail: report ? report.ownerEmail : null, subtitle, participantsList: personalDetailList, @@ -276,14 +278,14 @@ function createOption(logins, personalDetails, report, reportActions = {}, { phoneNumber: !hasMultipleParticipants ? personalDetail.phoneNumber : null, payPalMeAddress: !hasMultipleParticipants ? personalDetail.payPalMeAddress : null, isUnread: report ? report.unreadActionCount > 0 : null, - hasDraftComment: lodashGet(report, 'hasDraft', false), + hasDraftComment: report ? report.hasDraft : false, keyForList: report ? String(report.reportID) : personalDetail.login, searchText: getSearchText(report, reportName, personalDetailList, isChatRoom || isPolicyExpenseChat), - isPinned: lodashGet(report, 'isPinned', false), + isPinned: report ? report.isPinned : false, hasOutstandingIOU, - iouReportID: lodashGet(report, 'iouReportID'), - isIOUReportOwner: lodashGet(iouReport, 'ownerEmail', '') === currentUserLogin, - iouReportAmount: lodashGet(iouReport, 'total', 0), + iouReportID: report ? report.iouReportID : null, + isIOUReportOwner: iouReport ? iouReport.ownerEmail === currentUserLogin : false, + iouReportAmount: iouReport ? iouReport.total : 0, isChatRoom, isArchivedRoom, shouldShowSubscript: isPolicyExpenseChat && !report.isOwnPolicyExpenseChat && !isArchivedRoom, @@ -405,7 +407,7 @@ function getOptions(reports, personalDetails, activeReportID, { const isChatRoom = ReportUtils.isChatRoom(report); const isDefaultRoom = ReportUtils.isDefaultRoom(report); const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report); - const logins = lodashGet(report, ['participants'], []); + const logins = 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; @@ -413,9 +415,10 @@ function getOptions(reports, personalDetails, activeReportID, { return; } - const hasDraftComment = lodashGet(report, 'hasDraft', false); - const iouReportOwner = lodashGet(report, 'hasOutstandingIOU', false) - ? lodashGet(iouReports, [`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`, 'ownerEmail'], '') + const hasDraftComment = report.hasDraft || false; + const iouReport = report.iouReportID && iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`]; + const iouReportOwner = report.hasOutstandingIOU && iouReport + ? iouReport.ownerEmail : ''; const reportContainsIOUDebt = iouReportOwner && iouReportOwner !== currentUserLogin; diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 5cef373dd1ad..a058a0bb7bf8 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -244,8 +244,11 @@ function isArchivedRoom(report) { * @returns {String} */ function getPolicyName(report, policies) { - const defaultValue = report.oldPolicyName || Localize.translateLocal('workspace.common.unavailable'); - return lodashGet(policies, [`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, 'name'], defaultValue); + const policyName = policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`] && policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`].name || false; + if (!policyName) { + return report.oldPolicyName || Localize.translateLocal('workspace.common.unavailable'); + } + return policyName; } /** diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 5017b8b9c25f..e35c1e9acce4 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -105,50 +105,6 @@ class SidebarLinks extends React.Component { // this.unreadReports = this.getUnreadReports(props.reports); } - componentDidUpdate(prevProps) { - if (!_.isEqual(prevProps.reports, this.props.reports)) { - console.log('!!! props.reports') - return; - } - if (!_.isEqual(prevProps.personalDetails, this.props.personalDetails)) { - console.log('!!! props.personalDetails') - return; - } - if (!_.isEqual(prevProps.currentUserPersonalDetails, this.props.currentUserPersonalDetails)) { - console.log('!!! props.currentUserPersonalDetails') - return; - } - if (!_.isEqual(prevProps.currentlyViewedReportID, this.props.currentlyViewedReportID)) { - console.log('!!! props.currentlyViewedReportID') - return; - } - if (!_.isEqual(prevProps.priorityMode, this.props.priorityMode)) { - console.log('!!! props.priorityMode') - return; - } - if (!_.isEqual(prevProps.reportActions, this.props.reportActions)) { - console.log('!!! props.reportActions') - return; - } - if (!_.isEqual(prevProps.isSmallScreenWidth, this.props.isSmallScreenWidth)) { - console.log('!!! props.isSmallScreenWidth') - return; - } - if (!_.isEqual(prevProps.insets, this.props.insets)) { - console.log('!!! props.insets') - return; - } - if (!_.isEqual(prevProps.onAvatarClick, this.props.onAvatarClick)) { - console.log('!!! props.onAvatarClick') - return; - } - if (!_.isEqual(prevProps.onLinkClick, this.props.onLinkClick)) { - console.log('!!! props.onLinkClick') - return; - } - console.log('!!! unknown') - } - getFilteredAndOrderedReports(unfilteredReports) { return this.getRecentReportsOptionListItems(); From 7c51a86bfc9dcb992f0add3bc5e2c65eb7d482d6 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 14:16:08 +0200 Subject: [PATCH 27/46] WIP optimizing create option --- src/libs/OptionsListUtils.js | 23 ++++++++++++----------- src/libs/ReportUtils.js | 10 ++++------ 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index bf208198dae4..919255e52df2 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -154,28 +154,29 @@ function getParticipantNames(personalDetailList) { * @return {String} */ function getSearchText(report, reportName, personalDetailList, isChatRoomOrPolicyExpenseChat) { - const searchTerms = []; + let searchTerms = []; if (!isChatRoomOrPolicyExpenseChat) { - _.each(personalDetailList, (personalDetail) => { - searchTerms.push(personalDetail.displayName); - searchTerms.push(personalDetail.login.replace(/\./g, '')); - }); + for (let i = 0; i < personalDetailList.length; i++) { + const personalDetail = personalDetailList[i]; + searchTerms = searchTerms.concat([personalDetail.displayName, personalDetail.login.replace(/\./g, '')]); + } } if (report) { - searchTerms.push(...reportName); - searchTerms.push(..._.map(reportName.split(','), name => name.trim())); + Array.prototype.push.apply(searchTerms, reportName.split('')); + Array.prototype.push.apply(searchTerms, reportName.split(',')); if (isChatRoomOrPolicyExpenseChat) { const chatRoomSubtitle = ReportUtils.getChatRoomSubtitle(report, policies); - searchTerms.push(...chatRoomSubtitle); - searchTerms.push(..._.map(chatRoomSubtitle.split(','), name => name.trim())); + Array.prototype.push.apply(searchTerms, chatRoomSubtitle.split('')); + Array.prototype.push.apply(searchTerms, chatRoomSubtitle.split(',')); } else { - searchTerms.push(...report.participants); + searchTerms = searchTerms.concat(report.participants); } } - return _.unique(searchTerms).join(' '); + const finalSearchTerms = _.unique(searchTerms).join(' '); + return finalSearchTerms; } /** diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index a058a0bb7bf8..55aa46ff2ea8 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -498,7 +498,7 @@ function getReportName(report, personalDetailsForParticipants = {}, policies = { } if (isPolicyExpenseChat(report)) { - const reportOwnerPersonalDetails = lodashGet(personalDetailsForParticipants, report.ownerEmail); + const reportOwnerPersonalDetails = personalDetailsForParticipants[report.ownerEmail]; const reportOwnerDisplayName = getDisplayNameForParticipant(reportOwnerPersonalDetails) || report.ownerEmail || report.reportName; formattedName = report.isOwnPolicyExpenseChat ? getPolicyName(report, policies) : reportOwnerDisplayName; } @@ -513,11 +513,9 @@ function getReportName(report, personalDetailsForParticipants = {}, policies = { // Not a room or PolicyExpenseChat, generate title from participants const participants = _.without(lodashGet(report, 'participants', []), sessionEmail); - const displayNamesWithTooltips = getDisplayNamesWithTooltips( - _.isEmpty(personalDetailsForParticipants) ? participants : personalDetailsForParticipants, - participants.length > 1, - ); - return _.map(displayNamesWithTooltips, ({displayName}) => displayName).join(', '); + const isMultipleParticipantReport = participants.length > 1; + const participantsToGetTheNamesOf = _.isEmpty(personalDetailsForParticipants) ? participants : personalDetailsForParticipants; + return _.map(participantsToGetTheNamesOf, participant => getDisplayNameForParticipant(participant, isMultipleParticipantReport)).join(', '); } /** From fd60c45f2d3728f53694b6ff1480ed036be371e2 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 14:34:29 +0200 Subject: [PATCH 28/46] WIP optimizing create option --- src/libs/OptionsListUtils.js | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 919255e52df2..a4eb25e9d6c8 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -143,6 +143,27 @@ 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. @@ -175,7 +196,7 @@ function getSearchText(report, reportName, personalDetailList, isChatRoomOrPolic } } - const finalSearchTerms = _.unique(searchTerms).join(' '); + const finalSearchTerms = uniqFast(searchTerms).join(' '); return finalSearchTerms; } From 9fad973fa9a0130be828a3bfbaddddeaf4debf6f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 15:29:51 +0200 Subject: [PATCH 29/46] Finished optimizing createOption --- src/libs/OptionsListUtils.js | 157 +++++++++++++++++++++-------------- 1 file changed, 96 insertions(+), 61 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index a4eb25e9d6c8..6351b6165bac 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -239,80 +239,115 @@ function createOption(logins, personalDetails, report, reportActions = {}, { showChatPreviewLine = false, forcePolicyNamePreview = false, }) { - const isChatRoom = ReportUtils.isChatRoom(report); - const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report); + 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, + isPinned: false, + hasOutstandingIOU: false, + iouReportID: null, + isIOUReportOwner: null, + iouReportAmount: 0, + isChatRoom: false, + isArchivedRoom: false, + shouldShowSubscript: false, + isPolicyExpenseChat: false, + }; + const personalDetailMap = getPersonalDetailsForLogins(logins, personalDetails); const personalDetailList = _.values(personalDetailMap); - const isArchivedRoom = ReportUtils.isArchivedRoom(report); - const hasMultipleParticipants = personalDetailList.length > 1 || isChatRoom || isPolicyExpenseChat; const personalDetail = personalDetailList[0]; - const hasOutstandingIOU = report && report.hasOutstandingIOU || false; - const iouReport = hasOutstandingIOU && iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`] || {}; - const lastActorDetails = report && personalDetailMap[report.lastActorEmail] || null; - let lastMessageTextFromReport = ''; + let hasMultipleParticipants = personalDetailList.length > 1; + let subtitle; + if (report) { + result.isChatRoom = ReportUtils.isChatRoom(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 = report.unreadActionCount > 0; + 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; } - let lastMessageText = report && hasMultipleParticipants && lastActorDetails - ? `${lastActorDetails.displayName}: ` - : ''; - lastMessageText += report ? lastMessageTextFromReport : ''; - - if (isPolicyExpenseChat && 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.hasOutstandingIOU) { + const iouReport = iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`] || null; + if (iouReport) { + result.isIOUReportOwner = iouReport.ownerEmail === currentUserLogin; + result.iouReportAmount = iouReport.total; + } } - const tooltipText = report && ReportUtils.getReportParticipantsTitle(report.participants || []) || null; - const subtitle = ReportUtils.getChatRoomSubtitle(report, policies); - const reportName = ReportUtils.getReportName(report, personalDetailMap, policies); - let alternateText; - if (isChatRoom || isPolicyExpenseChat) { - alternateText = (showChatPreviewLine && !forcePolicyNamePreview && lastMessageText) - ? lastMessageText - : subtitle; - } else { - alternateText = (showChatPreviewLine && lastMessageText) - ? lastMessageText - : Str.removeSMSDomain(personalDetail.login); + if (!hasMultipleParticipants) { + result.login = personalDetail.login; + result.phoneNumber = personalDetail.phoneNumber; + result.payPalMeAddress = personalDetail.payPalMeAddress; } - return { - text: reportName, - alternateText, - brickRoadIndicator: getBrickRoadIndicatorStatusForReport(report, reportActions), - icons: ReportUtils.getIcons(report, personalDetails, policies, personalDetail.avatar), - tooltipText, - ownerEmail: report ? report.ownerEmail : null, - 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: report ? report.unreadActionCount > 0 : null, - hasDraftComment: report ? report.hasDraft : false, - keyForList: report ? String(report.reportID) : personalDetail.login, - searchText: getSearchText(report, reportName, personalDetailList, isChatRoom || isPolicyExpenseChat), - isPinned: report ? report.isPinned : false, - hasOutstandingIOU, - iouReportID: report ? report.iouReportID : null, - isIOUReportOwner: iouReport ? iouReport.ownerEmail === currentUserLogin : false, - iouReportAmount: iouReport ? iouReport.total : 0, - isChatRoom, - isArchivedRoom, - shouldShowSubscript: isPolicyExpenseChat && !report.isOwnPolicyExpenseChat && !isArchivedRoom, - isPolicyExpenseChat, - }; + + 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; } /** From ceb2ea954cec4810eab77506a8794a146d9080cf Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 15:32:11 +0200 Subject: [PATCH 30/46] Remove memoization and imports --- src/libs/OptionsListUtils.js | 5 +---- src/pages/home/sidebar/SidebarLinks.js | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 6351b6165bac..80f726988832 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -3,7 +3,6 @@ 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'; @@ -817,7 +816,7 @@ function getMemberInviteOptions( * @param {Object} reportActions * @returns {Object} */ -function calculateSidebarOptions(reports, personalDetails, activeReportID, priorityMode, betas, reportActions) { +function getSidebarOptions(reports, personalDetails, activeReportID, priorityMode, betas, reportActions) { let sideBarOptions = { prioritizeIOUDebts: true, prioritizeReportsWithDraftComments: true, @@ -841,8 +840,6 @@ function calculateSidebarOptions(reports, personalDetails, activeReportID, prior }); } -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/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index e35c1e9acce4..fa202bbc36c2 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -3,7 +3,6 @@ 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 styles from '../../../styles/styles'; import * as StyleUtils from '../../../styles/StyleUtils'; import ONYXKEYS from '../../../ONYXKEYS'; From 2ef3622b00336316e72acf4789ab8424079308dd Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 16:10:08 +0200 Subject: [PATCH 31/46] Memoize getting the option list items --- src/pages/home/sidebar/SidebarLinks.js | 149 +++---------------------- 1 file changed, 18 insertions(+), 131 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index fa202bbc36c2..f4bdda7ec179 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 memoizeOne from 'memoize-one'; import styles from '../../../styles/styles'; import * as StyleUtils from '../../../styles/StyleUtils'; import ONYXKEYS from '../../../ONYXKEYS'; @@ -94,142 +95,21 @@ const defaultProps = { class SidebarLinks extends React.Component { constructor(props) { super(props); - - // this.activeReport = { - // reportID: props.currentlyViewedReportID, - // }; - // - // this.orderedReports = []; - // this.priorityMode = props.priorityMode; - // this.unreadReports = this.getUnreadReports(props.reports); - } - - getFilteredAndOrderedReports(unfilteredReports) { - return this.getRecentReportsOptionListItems(); - - // const isActiveReportSame = this.activeReport.reportID === this.props.currentlyViewedReportID; - // const lastMessageTimestamp = lodashGet(unfilteredReports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.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 && this.activeReport.lastMessageTimestamp !== lastMessageTimestamp) { - // hasDraftHistory = false; - // } else if (isActiveReportSame && this.activeReport.hasDraftHistory) { - // hasDraftHistory = true; - // } else { - // hasDraftHistory = lodashGet(this.props.reports, `${ONYXKEYS.COLLECTION.REPORT}${this.props.currentlyViewedReportID}.hasDraft`, false); - // } - // - // const switchingPriorityModes = this.props.priorityMode !== this.priorityMode; - // - // // Build the report options we want to show - // const recentReports = this.getRecentReportsOptionListItems(); - // - // // If the order of the reports is different from the last render (or if priority mode is changing) - // // then orderedReports is the same as the freshly calculated recentReports. - // // If the order of the reports is the same as the last render - // // then the data for each report is updated from the data in the new props - // // @TODO: not sure why this is necessary and see if it can be removed or do something more intuitive - // const orderedReports = this.isReportOrderDifferentThanLastRender(hasDraftHistory) || switchingPriorityModes - // ? recentReports - // : _.chain(this.orderedReports) - // - // // To preserve the order of the conversations, we map over the previous ordered 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 the previous set of orderedReports - // .compact() - // .value(); - // - // // Store these pieces of data on the class so that the next time this method is called - // // the previous values can be compared against to tell if something changed which would - // // cause the reports to be reordered - // this.orderedReports = orderedReports; - // this.priorityMode = this.props.priorityMode; - // this.activeReport = { - // reportID: this.props.currentlyViewedReportID, - // hasDraftHistory, - // lastMessageTimestamp, - // }; - // this.unreadReports = this.getUnreadReports(unfilteredReports); - // - // return this.orderedReports; - } - - /** - * Create a map of unread reports that looks like this: - * { - * 1: true, - * 2: true, - * } - * This is so that when the new props are compared to the old props, it's - * fast to look up if there are any new unread reports. - * - * @param {Object[]} reports - * @returns {Object} - */ - getUnreadReports(reports) { - // return _.reduce(reports, (finalUnreadReportMap, report) => { - // if (report.unreadActionCount > 0) { - // return { - // [report.reportID]: true, - // ...finalUnreadReportMap, - // }; - // } - // return finalUnreadReportMap; - // }, {}); + this.getRecentReportsOptionListItems = memoizeOne(this.getRecentReportsOptionListItems.bind(this)); } - getRecentReportsOptionListItems() { - const activeReportID = this.props.currentlyViewedReportID; + getRecentReportsOptionListItems(activeReportID, priorityMode, unorderedReports, personalDetails, betas, reportActions) { const sidebarOptions = OptionsListUtils.getSidebarOptions( - this.props.reports, - this.props.personalDetails, + unorderedReports, + personalDetails, activeReportID, - this.props.priorityMode, - this.props.betas, - this.props.reportActions, + priorityMode, + betas, + reportActions, ); return sidebarOptions.recentReports; } - isReportOrderDifferentThanLastRender(hasDraftHistory) { - // // If the number of reports changed, then the report order is different - // if (this.orderedReports.length !== this.props.reports.length) { - // return true; - // } - // - // // If the active report changed, then the report order is different - // if (this.activeReport.reportID !== this.props.currentlyViewedReportID) { - // return true; - // } - // - // // If the active report has a draft, the order of the reports doesn't change - // // because it would cause the reports to reorder when a user starts typing a comment - // // and that is an annoying UX (too much stuff jumping around) - // if (this.props.currentlyViewedReportID && hasDraftHistory) { - // return false; - // } - // - // // If the unread reports have changed, then the report order changes - // // because the unread reports need to be placed at the top of the list - // // @TODO: This can probably be optimized - // const hasNewUnreadReports = _.some(this.props.reports, report => report.unreadActionCount > 0 && !this.unreadReports[report.reportID]); - // if (hasNewUnreadReports) { - // return true; - // } - // - // // By default, assume that the order of the reports doesn't change - // // in order to optimize the rendering - // return false; - } - showSearchPage() { Navigation.navigate(ROUTES.SEARCH); } @@ -240,12 +120,19 @@ class SidebarLinks extends React.Component { return null; } - const activeReportID = parseInt(this.props.currentlyViewedReportID, 10); 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 sections = [{ title: '', indexOffset: 0, - data: this.getFilteredAndOrderedReports(this.props.reports), + data: optionListItems, shouldShow: true, }]; Timing.end(CONST.TIMING.SIDEBAR_LINKS_FILTER_REPORTS); @@ -297,7 +184,7 @@ class SidebarLinks extends React.Component { ]} sections={sections} focusedIndex={_.findIndex(this.orderedReports, ( - option => option.reportID === activeReportID + option => option.reportID.toString() === this.props.currentlyViewedReportID.toString() ))} onSelectRow={(option) => { Navigation.navigate(ROUTES.getReportRoute(option.reportID)); From 9ceb909b0c8c13553d4ad9d39d608af513cfa141 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 1 Sep 2022 16:30:05 +0200 Subject: [PATCH 32/46] Fix lint issues --- src/libs/OptionsListUtils.js | 3 ++- src/libs/ReportUtils.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 80f726988832..a1e5027bf9fd 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -305,7 +305,8 @@ function createOption(logins, personalDetails, report, reportActions = {}, { 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; + 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), diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 55aa46ff2ea8..ac36f15649a3 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -244,7 +244,7 @@ function isArchivedRoom(report) { * @returns {String} */ function getPolicyName(report, policies) { - const policyName = policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`] && policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`].name || false; + const policyName = (policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`] && policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`].name) || false; if (!policyName) { return report.oldPolicyName || Localize.translateLocal('workspace.common.unavailable'); } From 6eb03cf72146f02bd06d7c2367f983d0c9a1d4bd Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 5 Sep 2022 09:36:20 +0100 Subject: [PATCH 33/46] Remove unused HOC --- src/components/OptionsSelector/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/OptionsSelector/index.js b/src/components/OptionsSelector/index.js index eea20274561e..9f7c924e427f 100644 --- a/src/components/OptionsSelector/index.js +++ b/src/components/OptionsSelector/index.js @@ -11,4 +11,4 @@ const OptionsSelector = forwardRef((props, ref) => ( OptionsSelector.displayName = 'OptionsSelector'; -export default withLocalize(OptionsSelector); +export default OptionsSelector; From 42ece5b3f399e8d2d563768f7ef0af1326d78dbf Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 5 Sep 2022 09:50:36 +0100 Subject: [PATCH 34/46] Add error boundary to rendered component in tests --- tests/unit/LHNOrderTest.js | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/unit/LHNOrderTest.js b/tests/unit/LHNOrderTest.js index 3e411f7ca251..e085d6001289 100644 --- a/tests/unit/LHNOrderTest.js +++ b/tests/unit/LHNOrderTest.js @@ -158,18 +158,37 @@ Onyx.init({ }); function getDefaultRenderedSidebarLinks() { + 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} + /> + )); } From 66d147d3987747860caf2eaf5b32634dd66a4b01 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 5 Sep 2022 16:20:35 +0100 Subject: [PATCH 35/46] Add an early return to protect against null reports --- src/libs/OptionsListUtils.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index e3505ad53a67..fb969a157c48 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -462,6 +462,9 @@ function getOptions(reports, personalDetails, activeReportID, { const allReportOptions = []; _.each(orderedReports, (report) => { + if (!report) { + return null; + } const isChatRoom = ReportUtils.isChatRoom(report); const isDefaultRoom = ReportUtils.isDefaultRoom(report); const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report); @@ -469,7 +472,7 @@ function getOptions(reports, personalDetails, activeReportID, { // 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 || !report.reportID || shouldFilterNoParticipants) { + if (!report.reportID || shouldFilterNoParticipants) { return; } From d03e6fbed7c5720cac8c6d7a569ba50af6611064 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 5 Sep 2022 16:29:03 +0100 Subject: [PATCH 36/46] Add a method comment about performance --- src/libs/OptionsListUtils.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index fb969a157c48..e2a0567cdd4b 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -167,6 +167,10 @@ function uniqFast(items) { * 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 From 47aa0b8f3ab00f50c1381aad0d9b4221f7da037f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 5 Sep 2022 16:31:37 +0100 Subject: [PATCH 37/46] Keep policy name as a string and simplify the return --- src/libs/ReportUtils.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 53e80e0ab32f..45d317e355b4 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -246,11 +246,11 @@ function isArchivedRoom(report) { * @returns {String} */ function getPolicyName(report, policies) { - const policyName = (policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`] && policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`].name) || false; - if (!policyName) { - return report.oldPolicyName || Localize.translateLocal('workspace.common.unavailable'); - } - return policyName; + const policyName = ( + policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`] + && policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`].name + ) || ''; + return policyName || report.oldPolicyName || Localize.translateLocal('workspace.common.unavailable'); } /** From 99532c20083b3d648c715a978826f727fbd6b60f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 5 Sep 2022 16:59:45 +0100 Subject: [PATCH 38/46] Simplify the return statement --- src/libs/OptionsListUtils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index e2a0567cdd4b..a4c4d93642a5 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -199,8 +199,7 @@ function getSearchText(report, reportName, personalDetailList, isChatRoomOrPolic } } - const finalSearchTerms = uniqFast(searchTerms).join(' '); - return finalSearchTerms; + return uniqFast(searchTerms).join(' '); } /** @@ -259,6 +258,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, { hasDraftComment: false, keyForList: null, searchText: null, + isDefaultRoom: false, isPinned: false, hasOutstandingIOU: false, iouReportID: null, From 988e7c6d3b40e6068fcc2a30922b718f565b27c2 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 5 Sep 2022 17:20:16 +0100 Subject: [PATCH 39/46] Fix the check for unread reports --- src/libs/OptionsListUtils.js | 2 +- src/libs/ReportUtils.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index a4c4d93642a5..96e9eb92d895 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -285,7 +285,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, { result.brickRoadIndicator = getBrickRoadIndicatorStatusForReport(report, reportActions); result.ownerEmail = report.ownerEmail; result.reportID = report.reportID; - result.isUnread = report.unreadActionCount > 0; + result.isUnread = ReportUtils.isUnread(report); result.hasDraftComment = report.hasDraft; result.isPinned = report.isPinned; result.iouReportID = report.iouReportID; diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 45d317e355b4..4ce8ecf45def 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -626,8 +626,8 @@ function buildOptimisticIOUReportAction(type, amount, comment, paymentType = '', * @returns {Boolean} */ function isUnread(report) { - const lastReadSequenceNumber = lodashGet(report, 'lastReadSequenceNumber', 0); - const maxSequenceNumber = lodashGet(report, 'maxSequenceNumber', 0); + const lastReadSequenceNumber = report.lastReadSequenceNumber || 0; + const maxSequenceNumber = report.maxSequenceNumber || 0; return lastReadSequenceNumber < maxSequenceNumber; } From 403632528ff64bf7ff00aa4349dbd1d287f99bea Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 6 Sep 2022 05:01:37 -0600 Subject: [PATCH 40/46] Update src/libs/OptionsListUtils.js Co-authored-by: Marc Glasser --- 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 96e9eb92d895..d9f0c658cfaf 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -467,7 +467,7 @@ function getOptions(reports, personalDetails, activeReportID, { const allReportOptions = []; _.each(orderedReports, (report) => { if (!report) { - return null; + return; } const isChatRoom = ReportUtils.isChatRoom(report); const isDefaultRoom = ReportUtils.isDefaultRoom(report); From a41a30a15ad8d44087d554b6f92750d37600d66c Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 8 Sep 2022 08:38:34 +0100 Subject: [PATCH 41/46] Fix the active report equality check --- 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 d9f0c658cfaf..55640f8bfbbc 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -497,7 +497,7 @@ function getOptions(reports, personalDetails, activeReportID, { const shouldFilterReportIfRead = hideReadReports && !ReportUtils.isUnread(report); const shouldFilterReport = shouldFilterReportIfEmpty || shouldFilterReportIfRead; - if (report.reportID !== activeReportID + if (report.reportID.toString() !== activeReportID.toString() && (!report.isPinned || isDefaultRoom) && !hasDraftComment && shouldFilterReport From 6b87f562e4d4879e03ba6b4179b028d400b08e70 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 8 Sep 2022 11:43:18 +0100 Subject: [PATCH 42/46] Fix the active report equality check --- 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 55640f8bfbbc..483e58ea9668 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -497,7 +497,7 @@ function getOptions(reports, personalDetails, activeReportID, { const shouldFilterReportIfRead = hideReadReports && !ReportUtils.isUnread(report); const shouldFilterReport = shouldFilterReportIfEmpty || shouldFilterReportIfRead; - if (report.reportID.toString() !== activeReportID.toString() + if (report.reportID.toString() !== activeReportID && (!report.isPinned || isDefaultRoom) && !hasDraftComment && shouldFilterReport From 629c4c4a66963ed36752edf4de81b140d73e22f8 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 8 Sep 2022 11:46:53 +0100 Subject: [PATCH 43/46] Add a comment about why we use an error boundary --- tests/unit/LHNOrderTest.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/LHNOrderTest.js b/tests/unit/LHNOrderTest.js index e085d6001289..3a0194f0f480 100644 --- a/tests/unit/LHNOrderTest.js +++ b/tests/unit/LHNOrderTest.js @@ -158,6 +158,11 @@ 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. From 55bd67fa48766351c6bb2bbd5633312814f1039e Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 8 Sep 2022 11:47:55 +0100 Subject: [PATCH 44/46] Fix return data type --- src/pages/home/ReportScreen.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 86bcc92bdd36..5c5182a6880e 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -104,7 +104,7 @@ const defaultProps = { * @param {Object} route * @param {Object} route.params * @param {String} route.params.reportID - * @returns {Number} + * @returns {String} */ function getReportID(route) { return route.params.reportID.toString(); From 84b1be42fb202270ef7cc81960cd0a2f1c88d148 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 8 Sep 2022 12:00:00 +0100 Subject: [PATCH 45/46] Remove an unnecessary string cast --- src/pages/home/sidebar/SidebarLinks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index be36fef880e4..1257d29cbdb8 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -180,8 +180,8 @@ class SidebarLinks extends React.Component { {paddingBottom: StyleUtils.getSafeAreaMargins(this.props.insets).marginBottom}, ]} sections={sections} - focusedIndex={_.findIndex(this.orderedReports, ( - option => option.reportID.toString() === this.props.currentlyViewedReportID.toString() + focusedIndex={_.findIndex(optionListItems, ( + option => option.reportID.toString() === this.props.currentlyViewedReportID ))} onSelectRow={(option) => { Navigation.navigate(ROUTES.getReportRoute(option.reportID)); From 5ca3b5ebb7b53c236a0376e3069c2a92ff9ed00f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 8 Sep 2022 12:06:28 +0100 Subject: [PATCH 46/46] Add JSDocs --- src/pages/home/sidebar/SidebarLinks.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 1257d29cbdb8..462d14e9f3ba 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -95,6 +95,15 @@ class SidebarLinks extends React.Component { this.getRecentReportsOptionListItems = memoizeOne(this.getRecentReportsOptionListItems.bind(this)); } + /** + * @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) { const sidebarOptions = OptionsListUtils.getSidebarOptions( unorderedReports,