From b9199e1c41d9d0540d28c4dff4235ded9f137f4b Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Wed, 23 Feb 2022 23:18:05 -0800 Subject: [PATCH 01/10] fix crash --- 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 d31e1f9d0fd4..1659c195e1c8 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -167,7 +167,7 @@ class SidebarLinks extends React.Component { const recentReports = SidebarLinks.getRecentReports(nextProps); const orderedReports = shouldReorder ? recentReports - : _.map(prevState.orderedReports, + : _.filter(prevState.orderedReports, orderedReport => _.chain(recentReports) .filter(recentReport => orderedReport.reportID === recentReport.reportID) .first() From f3e59cbad0eb0713b6323e80bd8dd936aade64f5 Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Thu, 24 Feb 2022 11:38:20 -0800 Subject: [PATCH 02/10] always reorder when changing focus --- src/pages/home/sidebar/SidebarLinks.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 1659c195e1c8..28a98957b9dd 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -144,12 +144,6 @@ class SidebarLinks extends React.Component { return true; } - // Do not re-order if the active report has a draft and vice versa. - if (nextProps.currentlyViewedReportID) { - const hasActiveReportDraft = lodashGet(nextProps.reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`, false); - return !hasActiveReportDraft; - } - return true; } From 89eb09ce0f950b7c014a7dcb1b4d947f8bc46950 Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Sat, 26 Feb 2022 15:27:16 -0800 Subject: [PATCH 03/10] fix fix --- src/pages/home/sidebar/SidebarLinks.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 28a98957b9dd..c5e10462e1ed 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -161,11 +161,13 @@ class SidebarLinks extends React.Component { const recentReports = SidebarLinks.getRecentReports(nextProps); const orderedReports = shouldReorder ? recentReports - : _.filter(prevState.orderedReports, - orderedReport => _.chain(recentReports) - .filter(recentReport => orderedReport.reportID === recentReport.reportID) - .first() - .value()); + : _.chain(prevState.orderedReports) + .map(orderedReport => _.chain(recentReports) + .filter(recentReport => orderedReport.reportID === recentReport.reportID) + .first() + .value()) + .filter(orderedReport => orderedReport !== undefined) + .value(); return { orderedReports, From b88a9fa698798b66776ce2563328bb1157d300a6 Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Sat, 26 Feb 2022 15:38:12 -0800 Subject: [PATCH 04/10] add comments because it took me several tries --- src/pages/home/sidebar/SidebarLinks.js | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index c5e10462e1ed..ac10f069c579 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -158,16 +158,25 @@ class SidebarLinks extends React.Component { static getDerivedStateFromProps(nextProps, prevState) { const shouldReorder = SidebarLinks.shouldReorder(nextProps, prevState.orderedReports, prevState.currentlyViewedReportID, prevState.unreadReports); + + // Pull the reports we want to show on the left hand nav const recentReports = SidebarLinks.getRecentReports(nextProps); + + // Determine whether we want to re-order/sort the reports on the left hand nav const orderedReports = shouldReorder ? recentReports : _.chain(prevState.orderedReports) - .map(orderedReport => _.chain(recentReports) - .filter(recentReport => orderedReport.reportID === recentReport.reportID) - .first() - .value()) - .filter(orderedReport => orderedReport !== undefined) - .value(); + + // To preserve the order of the conversations, we pull the previous state's order of reports. + // Then match and replace them with the conversations we are supposed to show ('recentReports'). + .map(orderedReport => _.chain(recentReports) + .filter(recentReport => orderedReport.reportID === recentReport.reportID) + .first() + .value()) + + // We have to use map() because we need to replace the older reports objects with the newer ones in recentReports + .filter(orderedReport => orderedReport !== undefined) + .value(); return { orderedReports, From da4dc8a136afd07b4f7eca6b7de163b14a6d928d Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Sat, 26 Feb 2022 15:38:26 -0800 Subject: [PATCH 05/10] revert removing reorder condition --- src/pages/home/sidebar/SidebarLinks.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index ac10f069c579..405fc551ff4d 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -144,6 +144,12 @@ class SidebarLinks extends React.Component { return true; } + // Do not re-order if the active report has a draft and vice versa. + if (nextProps.currentlyViewedReportID) { + const hasActiveReportDraft = lodashGet(nextProps.reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`, false); + return !hasActiveReportDraft; + } + return true; } From edb7149a62a166d8de3d3d801c6c36825b329b47 Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Sun, 27 Feb 2022 23:38:32 -0800 Subject: [PATCH 06/10] clean it up a bit --- src/pages/home/sidebar/SidebarLinks.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 405fc551ff4d..79ac5865c21e 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -144,7 +144,8 @@ class SidebarLinks extends React.Component { return true; } - // Do not re-order if the active report has a draft and vice versa. + // Do not re-order if the active report has a draft + // Unless, we are switching priority modes from "#focus" to "Most Recent". Otherwise, we won't display any of the previously hidden conversations if (nextProps.currentlyViewedReportID) { const hasActiveReportDraft = lodashGet(nextProps.reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`, false); return !hasActiveReportDraft; @@ -173,14 +174,11 @@ class SidebarLinks extends React.Component { ? recentReports : _.chain(prevState.orderedReports) - // To preserve the order of the conversations, we pull the previous state's order of reports. - // Then match and replace them with the conversations we are supposed to show ('recentReports'). - .map(orderedReport => _.chain(recentReports) - .filter(recentReport => orderedReport.reportID === recentReport.reportID) - .first() - .value()) + // 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)) - // We have to use map() because we need to replace the older reports objects with the newer ones in recentReports + // Because we are using map, we have to filter out any undefined reports that may happen when switching priority modes .filter(orderedReport => orderedReport !== undefined) .value(); From d69f3bd3a91ca8e0c8e3833ae282d1537b938983 Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Mon, 28 Feb 2022 09:24:38 -0800 Subject: [PATCH 07/10] factor in priorityMode switching --- src/pages/home/sidebar/SidebarLinks.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 79ac5865c21e..3d0431d03d73 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -159,18 +159,20 @@ class SidebarLinks extends React.Component { this.state = { currentlyViewedReportID: props.currentlyViewedReportID, orderedReports: [], + priorityMode: props.priorityMode, unreadReports: SidebarLinks.getUnreadReports(props.reports || {}), }; } static getDerivedStateFromProps(nextProps, prevState) { const shouldReorder = SidebarLinks.shouldReorder(nextProps, prevState.orderedReports, prevState.currentlyViewedReportID, prevState.unreadReports); + const switchingPriorityModes = nextProps.priorityMode !== prevState.priorityMode; // Pull the reports we want to show on the left hand nav const recentReports = SidebarLinks.getRecentReports(nextProps); // Determine whether we want to re-order/sort the reports on the left hand nav - const orderedReports = shouldReorder + const orderedReports = shouldReorder || switchingPriorityModes ? recentReports : _.chain(prevState.orderedReports) @@ -184,6 +186,7 @@ class SidebarLinks extends React.Component { return { orderedReports, + priorityMode: nextProps.priorityMode, currentlyViewedReportID: nextProps.currentlyViewedReportID, unreadReports: SidebarLinks.getUnreadReports(nextProps.reports || {}), }; From 28d0309a25be43c8238e243eae45107248e3573c Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Mon, 28 Feb 2022 15:22:06 -0800 Subject: [PATCH 08/10] fix comment --- src/pages/home/sidebar/SidebarLinks.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 3d0431d03d73..870fbf92960b 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -145,7 +145,6 @@ class SidebarLinks extends React.Component { } // Do not re-order if the active report has a draft - // Unless, we are switching priority modes from "#focus" to "Most Recent". Otherwise, we won't display any of the previously hidden conversations if (nextProps.currentlyViewedReportID) { const hasActiveReportDraft = lodashGet(nextProps.reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`, false); return !hasActiveReportDraft; From 88ea587f659737791e55be41b508a825cc506f01 Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Mon, 28 Feb 2022 15:30:37 -0800 Subject: [PATCH 09/10] clarify --- src/pages/home/sidebar/SidebarLinks.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 870fbf92960b..fb62c94e26bf 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -167,10 +167,10 @@ class SidebarLinks extends React.Component { const shouldReorder = SidebarLinks.shouldReorder(nextProps, prevState.orderedReports, prevState.currentlyViewedReportID, prevState.unreadReports); const switchingPriorityModes = nextProps.priorityMode !== prevState.priorityMode; - // Pull the reports we want to show on the left hand nav + // Pull the reports we want to show on the left hand nav, ordered by priority const recentReports = SidebarLinks.getRecentReports(nextProps); - // Determine whether we want to re-order/sort the reports on the left hand nav + // Determine whether we need to keep the previous LHN order const orderedReports = shouldReorder || switchingPriorityModes ? recentReports : _.chain(prevState.orderedReports) @@ -179,7 +179,8 @@ class SidebarLinks extends React.Component { // 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 that may happen when switching priority modes + // 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(); From c9fcea4972bf7c9fde435cb85e36c4f67d555cb5 Mon Sep 17 00:00:00 2001 From: Kosuke Tseng Date: Mon, 28 Feb 2022 18:43:45 -0800 Subject: [PATCH 10/10] clarify 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 fb62c94e26bf..0532ccf3fc11 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -167,7 +167,7 @@ class SidebarLinks extends React.Component { const shouldReorder = SidebarLinks.shouldReorder(nextProps, prevState.orderedReports, prevState.currentlyViewedReportID, prevState.unreadReports); const switchingPriorityModes = nextProps.priorityMode !== prevState.priorityMode; - // Pull the reports we want to show on the left hand nav, ordered by priority + // Build the report options we want to show const recentReports = SidebarLinks.getRecentReports(nextProps); // Determine whether we need to keep the previous LHN order