From 4da6777105649ef3946b0963fd23cd5279179026 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Wed, 16 Aug 2023 19:12:09 -0500 Subject: [PATCH 01/13] fix: show existing reports while loading --- src/components/LHNOptionsList/LHNOptionsList.js | 9 +++++++-- src/pages/home/sidebar/SidebarLinks.js | 17 ++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 13de5a5e8c52..82861397c51a 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -8,6 +8,10 @@ import OptionRowLHNData from './OptionRowLHNData'; import variables from '../../styles/variables'; const propTypes = { + /** Wrapper style for the section list */ + // eslint-disable-next-line react/forbid-prop-types + style: PropTypes.arrayOf(PropTypes.object), + /** Extra styles for the section list container */ // eslint-disable-next-line react/forbid-prop-types contentContainerStyles: PropTypes.arrayOf(PropTypes.object).isRequired, @@ -26,10 +30,11 @@ const propTypes = { }; const defaultProps = { + style: [styles.flex1], shouldDisableFocusOptions: false, }; -function LHNOptionsList({contentContainerStyles, data, onSelectRow, optionMode, shouldDisableFocusOptions}) { +function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optionMode, shouldDisableFocusOptions}) { /** * This function is used to compute the layout of any given item in our list. Since we know that each item will have the exact same height, this is a performance optimization * so that the heights can be determined before the options are rendered. Otherwise, the heights are determined when each option is rendering and it causes a lot of overhead on large @@ -67,7 +72,7 @@ function LHNOptionsList({contentContainerStyles, data, onSelectRow, optionMode, ); return ( - + {this.props.isLoading ? ( <> - {lodashGet(this.props.report, 'reportID') && ( - - )} + ) : ( From f36ca39b74d4e070f4936ac09144260c40f0a4a7 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Wed, 16 Aug 2023 20:29:01 -0500 Subject: [PATCH 02/13] fix: minor refactoring --- src/pages/home/sidebar/SidebarLinks.js | 31 +++++++++----------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index f56e54cfcff0..38cb1e060a39 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -227,27 +227,16 @@ class SidebarLinks extends React.PureComponent { )} - {this.props.isLoading ? ( - <> - - - - ) : ( - - )} + + + {this.props.isLoading && } ); } From de433a46df48fd4f36b0516750611e33fdbcdeb2 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Wed, 23 Aug 2023 12:23:18 -0500 Subject: [PATCH 03/13] fix: LHN glitch --- src/pages/home/sidebar/SidebarLinksData.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index a919607938c7..f41db7e415db 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -67,16 +67,23 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr const {translate} = useLocalize(); const reportIDsRef = useRef([]); + const isLoading = SessionUtils.didUserLogInDuringSession() && isLoadingReportData; const optionListItems = useMemo(() => { + if (_.isEmpty(chatReports) || _.isEmpty(betas) || _.isEmpty(policies) || _.isEmpty(priorityMode) || _.isEmpty(allReportActions)) { + return []; + } const reportIDs = SidebarUtils.getOrderedReportIDs(currentReportID, chatReports, betas, policies, priorityMode, allReportActions); if (deepEqual(reportIDsRef.current, reportIDs)) { return reportIDsRef.current; } - reportIDsRef.current = reportIDs; - return reportIDs; - }, [allReportActions, betas, chatReports, currentReportID, policies, priorityMode]); - - const isLoading = SessionUtils.didUserLogInDuringSession() && isLoadingReportData; + if (isLoading && reportIDsRef.current.length === 0 && !currentReportID) { + reportIDsRef.current = reportIDs; + } + if (!isLoading) { + reportIDsRef.current = reportIDs; + } + return reportIDsRef.current; + }, [allReportActions, betas, chatReports, currentReportID, policies, priorityMode, isLoading]); return ( Date: Wed, 23 Aug 2023 19:52:56 -0500 Subject: [PATCH 04/13] fix: remove unnecessary condition' --- src/pages/home/sidebar/SidebarLinksData.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index f41db7e415db..6040ffea91c5 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -69,9 +69,6 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr const reportIDsRef = useRef([]); const isLoading = SessionUtils.didUserLogInDuringSession() && isLoadingReportData; const optionListItems = useMemo(() => { - if (_.isEmpty(chatReports) || _.isEmpty(betas) || _.isEmpty(policies) || _.isEmpty(priorityMode) || _.isEmpty(allReportActions)) { - return []; - } const reportIDs = SidebarUtils.getOrderedReportIDs(currentReportID, chatReports, betas, policies, priorityMode, allReportActions); if (deepEqual(reportIDsRef.current, reportIDs)) { return reportIDsRef.current; From b35a98f84f69b5d1ba1cf4d179707c3c02dc6328 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Wed, 23 Aug 2023 20:38:53 -0500 Subject: [PATCH 05/13] fix: initial value to null --- src/pages/home/sidebar/SidebarLinksData.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 6040ffea91c5..df8c37f92966 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -66,20 +66,20 @@ const defaultProps = { function SidebarLinksData({isFocused, allReportActions, betas, chatReports, currentReportID, insets, isLoadingReportData, isSmallScreenWidth, onLinkClick, policies, priorityMode}) { const {translate} = useLocalize(); - const reportIDsRef = useRef([]); + const reportIDsRef = useRef(null); const isLoading = SessionUtils.didUserLogInDuringSession() && isLoadingReportData; const optionListItems = useMemo(() => { const reportIDs = SidebarUtils.getOrderedReportIDs(currentReportID, chatReports, betas, policies, priorityMode, allReportActions); if (deepEqual(reportIDsRef.current, reportIDs)) { return reportIDsRef.current; } - if (isLoading && reportIDsRef.current.length === 0 && !currentReportID) { + if (isLoading && !reportIDsRef.current && !currentReportID) { reportIDsRef.current = reportIDs; } if (!isLoading) { reportIDsRef.current = reportIDs; } - return reportIDsRef.current; + return reportIDsRef.current || []; }, [allReportActions, betas, chatReports, currentReportID, policies, priorityMode, isLoading]); return ( From e8322ca166b801892da5cd37058e7734e044e78c Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Thu, 24 Aug 2023 16:40:17 -0500 Subject: [PATCH 06/13] fix: lint error --- 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 5daed20db009..0c19816b17a7 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -1,5 +1,4 @@ /* eslint-disable rulesdir/onyx-props-must-have-default */ -import lodashGet from 'lodash/get'; import React from 'react'; import {View} from 'react-native'; import _ from 'underscore'; From 7ffa7938ccae8bfe6abca38b7c652c55f554284f Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Thu, 24 Aug 2023 17:20:21 -0500 Subject: [PATCH 07/13] fix: add comments --- src/pages/home/sidebar/SidebarLinksData.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index df8c37f92966..55804177a3e5 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -73,6 +73,9 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr if (deepEqual(reportIDsRef.current, reportIDs)) { return reportIDsRef.current; } + + // We need to update existing reports only once while loading because they are updated several times during loading and causes this reguression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 + // Also we should not update options list when currentReportID is not null: https://github.com/Expensify/App/issues/23735 if (isLoading && !reportIDsRef.current && !currentReportID) { reportIDsRef.current = reportIDs; } From bce37385dbc60a4dcade3b357fd346ac683d5547 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Fri, 25 Aug 2023 07:38:12 -0500 Subject: [PATCH 08/13] fix: default style of LHNOptionsList --- src/components/LHNOptionsList/LHNOptionsList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js index 82861397c51a..b1430676afc0 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.js +++ b/src/components/LHNOptionsList/LHNOptionsList.js @@ -30,7 +30,7 @@ const propTypes = { }; const defaultProps = { - style: [styles.flex1], + style: styles.flex1, shouldDisableFocusOptions: false, }; From 2734584bfc8938f13d56291cfbe00673ee547dd1 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Mon, 28 Aug 2023 15:16:56 -0500 Subject: [PATCH 09/13] fix: same behavior on all platforms --- src/pages/home/sidebar/SidebarLinksData.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 55804177a3e5..918538283cc4 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -75,8 +75,7 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr } // We need to update existing reports only once while loading because they are updated several times during loading and causes this reguression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 - // Also we should not update options list when currentReportID is not null: https://github.com/Expensify/App/issues/23735 - if (isLoading && !reportIDsRef.current && !currentReportID) { + if ((isLoading && !reportIDsRef.current) || (_.isEmpty(reportIDsRef.current) && currentReportID)) { reportIDsRef.current = reportIDs; } if (!isLoading) { From 7c0f96b87213023e28bd9fc0f3bbf8c1ed32a947 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Mon, 28 Aug 2023 15:29:21 -0500 Subject: [PATCH 10/13] fix: lint error --- src/pages/iou/WaypointEditor.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pages/iou/WaypointEditor.js b/src/pages/iou/WaypointEditor.js index c0c621e353cb..9c0bc5b74e40 100644 --- a/src/pages/iou/WaypointEditor.js +++ b/src/pages/iou/WaypointEditor.js @@ -36,6 +36,9 @@ const propTypes = { }), }), + /** Recent waypoints array */ + recentWaypoints: PropTypes.array, + /** The optimistic transaction for this request */ transaction: transactionPropTypes, From 52070c8a61a11f81206d13186a3ef8794d2a6f23 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Mon, 28 Aug 2023 18:37:14 -0500 Subject: [PATCH 11/13] fix: lint error --- src/pages/iou/WaypointEditor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/iou/WaypointEditor.js b/src/pages/iou/WaypointEditor.js index 9c0bc5b74e40..91ea394de838 100644 --- a/src/pages/iou/WaypointEditor.js +++ b/src/pages/iou/WaypointEditor.js @@ -37,7 +37,7 @@ const propTypes = { }), /** Recent waypoints array */ - recentWaypoints: PropTypes.array, + recentWaypoints: PropTypes.arrayOf(PropTypes.object), /** The optimistic transaction for this request */ transaction: transactionPropTypes, From 2d85e653f5ef773ade38303ebd76d11660d01ad1 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Mon, 28 Aug 2023 22:36:51 -0500 Subject: [PATCH 12/13] fix: update comments --- src/pages/home/sidebar/SidebarLinksData.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 918538283cc4..c990d6a0d6fb 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -74,7 +74,7 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr return reportIDsRef.current; } - // We need to update existing reports only once while loading because they are updated several times during loading and causes this reguression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 + // We need to update existing reports only once while loading because they are updated several times during loading and causes this regression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 if ((isLoading && !reportIDsRef.current) || (_.isEmpty(reportIDsRef.current) && currentReportID)) { reportIDsRef.current = reportIDs; } From 243844e7aaf346cef29006732f45ea482c64d319 Mon Sep 17 00:00:00 2001 From: s-alves10 Date: Tue, 29 Aug 2023 07:08:56 -0500 Subject: [PATCH 13/13] fix: simplify conditions --- src/pages/home/sidebar/SidebarLinksData.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index c990d6a0d6fb..3eca506f7591 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -75,10 +75,7 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr } // We need to update existing reports only once while loading because they are updated several times during loading and causes this regression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 - if ((isLoading && !reportIDsRef.current) || (_.isEmpty(reportIDsRef.current) && currentReportID)) { - reportIDsRef.current = reportIDs; - } - if (!isLoading) { + if (!isLoading || !reportIDsRef.current || (_.isEmpty(reportIDsRef.current) && currentReportID)) { reportIDsRef.current = reportIDs; } return reportIDsRef.current || [];