From 529f0fb6ff0472beee208de6a5ab6320917a4dc8 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 26 Apr 2021 18:16:21 +0300 Subject: [PATCH 01/26] feat: Navigation navigateLater and enableNavigation Wait for the NavigationContainer and all its children finish mounting for the first time before allowing/performing navigation If there were navigation attempts - the last navigation attempt would be automatically applied when the navigation is enabled --- src/libs/Navigation/Navigation.js | 29 +++++++++++++++++++++++++-- src/libs/Navigation/NavigationRoot.js | 3 ++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js index 28dbfe330da4..a5bd681c2115 100644 --- a/src/libs/Navigation/Navigation.js +++ b/src/libs/Navigation/Navigation.js @@ -9,6 +9,7 @@ import SCREENS from '../../SCREENS'; import CustomActions from './CustomActions'; export const navigationRef = React.createRef(); +let lastAttemptedRoute; /** * Opens the LHN drawer. @@ -91,6 +92,27 @@ function dismissModal(shouldOpenDrawer = false) { openDrawer(); } +/** + * Capture any navigation attempts until the navigation container and all it's children finish mounting + * + * @param {String} route + */ +function navigateLater(route) { + lastAttemptedRoute = route; +} + +/** + * Enable navigation after the navigation tree becomes ready + * Executes the last captured navigation command if attempts were made prior being ready + */ +function enableNavigation() { + if (lastAttemptedRoute) { + navigate(lastAttemptedRoute); + } + // eslint-disable-next-line no-use-before-define + exports.navigate = navigate; +} + /** * Determines whether the drawer is currently open. * @@ -100,9 +122,12 @@ function isDrawerOpen() { return getIsDrawerOpenFromState(navigationRef.current.getRootState().routes[0].state); } -export default { - navigate, +const exports = { + navigate: navigateLater, + enableNavigation, dismissModal, isDrawerOpen, goBack, }; + +export default exports; diff --git a/src/libs/Navigation/NavigationRoot.js b/src/libs/Navigation/NavigationRoot.js index 322b7036914f..7cb7be9e8027 100644 --- a/src/libs/Navigation/NavigationRoot.js +++ b/src/libs/Navigation/NavigationRoot.js @@ -1,7 +1,7 @@ import React, {Component} from 'react'; import PropTypes from 'prop-types'; import {getPathFromState, NavigationContainer} from '@react-navigation/native'; -import {navigationRef} from './Navigation'; +import Navigation, {navigationRef} from './Navigation'; import linkingConfig from './linkingConfig'; import AppNavigator from './AppNavigator'; import {setCurrentURL} from '../actions/App'; @@ -37,6 +37,7 @@ class NavigationRoot extends Component { } onStateChange={this.parseAndStoreRoute} + onReady={Navigation.enableNavigation} ref={navigationRef} linking={linkingConfig} documentTitle={{ From 78490e78e22353cd3042cdf3e444107cbb2b3cba Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 26 Apr 2021 19:27:31 +0300 Subject: [PATCH 02/26] refactor: AuthScreens empty string is a valid initial value for initialReportID Sometimes `currentReportID` might resolve as empty '' and prevent the RootStack navigator from mounting `const currentReportID = firstReportID ? String(firstReportID) : '';` https://github.com/Expensify/Expensify.cash/blob/0da357c30bc1ace6d354a81aabe075f87c12144c/src/libs/actions/Report.js#L688 --- src/libs/Navigation/AppNavigator/AuthScreens.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index d136fdf2cdf6..24baff9b36ba 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -149,7 +149,7 @@ class AuthScreens extends React.Component { } // Skip when `this.initialReportID` is already assigned. We no longer want to update it - if (!this.initialReportID) { + if (!_.isString(this.initialReportID)) { // Either we have a reportID or fetchAllReports resolved with no reports. Otherwise keep waiting if (nextProps.initialReportID || nextProps.initialReportID === '') { this.initialReportID = nextProps.initialReportID; @@ -169,7 +169,7 @@ class AuthScreens extends React.Component { render() { // Wait to resolve initial Home route params. - if (!this.initialReportID) { + if (!_.isString(this.initialReportID)) { return null; } From 8aeafccb99cf7cbc6c93807db45680e2890adc91 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 26 Apr 2021 19:42:17 +0300 Subject: [PATCH 03/26] fix: Report.fetchAllReports - fetchChatReportsByIDs would be called when there are no reports When `response.chatList` is an empty array (new users) - `reportIDs = String(response.chatList).split(',');` Results in `reportIDs` being equal to `['']` and the former control flow `if (reportIDs.length)` evaluated to `true` Not casting the chat list IDs to string is not longer an issue because all the following usages are working with strings as well as with numbers --- src/libs/actions/Report.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index d4769920c343..7a84822e5660 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -727,11 +727,9 @@ function fetchAllReports( return; } - // The string cast here is necessary as Get rvl='chatList' may return an int - reportIDs = String(response.chatList).split(','); - // Get all the chat reports if they have any, otherwise create one with concierge - if (reportIDs.length) { + if (response.chatList && response.chatList.length > 0) { + reportIDs = response.chatList; return fetchChatReportsByIDs(reportIDs); } From da36c16ad77df62d92ad0f8662c4c8a823cc1bfb Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 26 Apr 2021 19:43:42 +0300 Subject: [PATCH 04/26] fix: Report.fetchAllReports - new report created by `fetchOrCreateChatReport` is not used in `reportIDs` --- src/libs/actions/Report.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 7a84822e5660..585e5218d746 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -733,7 +733,10 @@ function fetchAllReports( return fetchChatReportsByIDs(reportIDs); } - return fetchOrCreateChatReport([currentUserEmail, 'concierge@expensify.com']); + return fetchOrCreateChatReport([currentUserEmail, 'concierge@expensify.com']) + .then((createdReportID) => { + reportIDs = [createdReportID]; + }); }) .then(() => { Onyx.set(ONYXKEYS.INITIAL_REPORT_DATA_LOADED, true); From c4d25f5ed8c92131d48adcfa9affeee9baf75c86 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 26 Apr 2021 20:02:07 +0300 Subject: [PATCH 05/26] fix: Report.fetchAllReports - Navigation.navigate does not work when no router is mounted --- src/libs/actions/Report.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 585e5218d746..936cbb3e6a28 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -659,9 +659,15 @@ function fetchOrCreateChatReport(participants, shouldNavigate = true) { Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${data.reportID}`, {reportID: data.reportID}); if (shouldNavigate) { - // Redirect the logged in person to the new report - Navigation.navigate(ROUTES.getReportRoute(data.reportID)); + try { + // Redirect the logged in person to the new report + Navigation.navigate(ROUTES.getReportRoute(data.reportID)); + } catch (e) { + // When switching between the Auth and Public routers there's a brief time of no navigator mounted + console.error(e); + } } + return data.reportID; }); } From d6596c52f8535dc86f16bcb0ad5d7154b7caa59b Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 27 Apr 2021 17:50:16 +0300 Subject: [PATCH 06/26] refactor: Move initialReportID from AuthScreens to MainDrawerNavigator --- .../Navigation/AppNavigator/AuthScreens.js | 29 ------------------- .../AppNavigator/MainDrawerNavigator.js | 26 +++++++++++++---- 2 files changed, 21 insertions(+), 34 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 24baff9b36ba..0ff7a55c5875 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -84,15 +84,11 @@ const propTypes = { isOffline: PropTypes.bool, }), - // The initial report for the home screen - initialReportID: PropTypes.string, - ...windowDimensionsPropTypes, }; const defaultProps = { network: {isOffline: true}, - initialReportID: null, }; class AuthScreens extends React.Component { @@ -101,8 +97,6 @@ class AuthScreens extends React.Component { Timing.start(CONST.TIMING.HOMEPAGE_INITIAL_RENDER); Timing.start(CONST.TIMING.HOMEPAGE_REPORTS_LOADED); - - this.initialReportID = props.initialReportID; } componentDidMount() { @@ -148,15 +142,6 @@ class AuthScreens extends React.Component { return true; } - // Skip when `this.initialReportID` is already assigned. We no longer want to update it - if (!_.isString(this.initialReportID)) { - // Either we have a reportID or fetchAllReports resolved with no reports. Otherwise keep waiting - if (nextProps.initialReportID || nextProps.initialReportID === '') { - this.initialReportID = nextProps.initialReportID; - return true; - } - } - return false; } @@ -168,11 +153,6 @@ class AuthScreens extends React.Component { } render() { - // Wait to resolve initial Home route params. - if (!_.isString(this.initialReportID)) { - return null; - } - const modalScreenOptions = { headerShown: false, cardStyle: getNavigationModalCardStyle(this.props.isSmallScreenWidth), @@ -196,12 +176,6 @@ class AuthScreens extends React.Component { headerShown: false, title: 'Expensify.cash', }} - initialParams={{ - screen: SCREENS.REPORT, - params: { - reportID: this.initialReportID, - }, - }} component={MainDrawerNavigator} /> ( @@ -32,10 +43,7 @@ const MainDrawerNavigator = props => ( ( ); MainDrawerNavigator.propTypes = propTypes; +MainDrawerNavigator.defaultProps = defaultProps; MainDrawerNavigator.displayName = 'MainDrawerNavigator'; -export default withWindowDimensions(MainDrawerNavigator); +export default compose( + withWindowDimensions, + withOnyx({ + initialReportID: { + key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID, + }, + }), +)(MainDrawerNavigator); From 1591d76dffce5506d5638cce0dc22760f0b1fa78 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 27 Apr 2021 18:40:25 +0300 Subject: [PATCH 07/26] refactor: Mount the Report Screen only when initial data is resolved --- .../AppNavigator/MainDrawerNavigator.js | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index 627a29d14b29..502b53de2b4f 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -1,5 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; +import _ from 'underscore'; import {createDrawerNavigator} from '@react-navigation/drawer'; import {withOnyx} from 'react-native-onyx'; @@ -14,6 +15,7 @@ import compose from '../../compose'; // Screens import SidebarScreen from '../../../pages/home/sidebar/SidebarScreen'; import ReportScreen from '../../../pages/home/ReportScreen'; +import FullScreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; const propTypes = { // Initial report to be used if nothing else is specified by routing @@ -23,7 +25,7 @@ const propTypes = { }; const defaultProps = { - initialReportID: '', + initialReportID: null, }; const Drawer = createDrawerNavigator(); @@ -39,16 +41,26 @@ const MainDrawerNavigator = props => ( sceneContainerStyle={styles.navigationSceneContainer} edgeWidth={500} drawerContent={() => } + screenOptions={{ + cardStyle: styles.navigationScreenCardStyle, + headerShown: false, + }} > - + { + _.isString(props.initialReportID) + ? ( + + ) + : ( + + {() => } + + ) + } ); From f394b98be841fd314dade5cdba0e04198b8ade24 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 27 Apr 2021 20:19:54 +0300 Subject: [PATCH 08/26] refactor: Reuse `updateCurrentlyViewedReportID` --- src/libs/actions/Report.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 936cbb3e6a28..63f92b6fbfa7 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -769,8 +769,8 @@ function fetchAllReports( } const firstReportID = _.first(reportIDs); - const currentReportID = firstReportID ? String(firstReportID) : ''; - Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, currentReportID); + // eslint-disable-next-line no-use-before-define + updateCurrentlyViewedReportID(firstReportID); }); } @@ -957,7 +957,7 @@ function handleReportChanged(report) { * @param {Number} reportID */ function updateCurrentlyViewedReportID(reportID) { - Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, String(reportID)); + Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, String(reportID || '')); } Onyx.connect({ From 9faf5e4f58a1922622ec1f382419d26f68e4bc89 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 27 Apr 2021 20:21:10 +0300 Subject: [PATCH 09/26] Revert "feat: Navigation navigateLater and enableNavigation" This reverts commit 529f0fb6 These changes didn't prove as useful as intended --- src/libs/Navigation/Navigation.js | 29 ++------------------------- src/libs/Navigation/NavigationRoot.js | 3 +-- 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js index a5bd681c2115..28dbfe330da4 100644 --- a/src/libs/Navigation/Navigation.js +++ b/src/libs/Navigation/Navigation.js @@ -9,7 +9,6 @@ import SCREENS from '../../SCREENS'; import CustomActions from './CustomActions'; export const navigationRef = React.createRef(); -let lastAttemptedRoute; /** * Opens the LHN drawer. @@ -92,27 +91,6 @@ function dismissModal(shouldOpenDrawer = false) { openDrawer(); } -/** - * Capture any navigation attempts until the navigation container and all it's children finish mounting - * - * @param {String} route - */ -function navigateLater(route) { - lastAttemptedRoute = route; -} - -/** - * Enable navigation after the navigation tree becomes ready - * Executes the last captured navigation command if attempts were made prior being ready - */ -function enableNavigation() { - if (lastAttemptedRoute) { - navigate(lastAttemptedRoute); - } - // eslint-disable-next-line no-use-before-define - exports.navigate = navigate; -} - /** * Determines whether the drawer is currently open. * @@ -122,12 +100,9 @@ function isDrawerOpen() { return getIsDrawerOpenFromState(navigationRef.current.getRootState().routes[0].state); } -const exports = { - navigate: navigateLater, - enableNavigation, +export default { + navigate, dismissModal, isDrawerOpen, goBack, }; - -export default exports; diff --git a/src/libs/Navigation/NavigationRoot.js b/src/libs/Navigation/NavigationRoot.js index 7cb7be9e8027..322b7036914f 100644 --- a/src/libs/Navigation/NavigationRoot.js +++ b/src/libs/Navigation/NavigationRoot.js @@ -1,7 +1,7 @@ import React, {Component} from 'react'; import PropTypes from 'prop-types'; import {getPathFromState, NavigationContainer} from '@react-navigation/native'; -import Navigation, {navigationRef} from './Navigation'; +import {navigationRef} from './Navigation'; import linkingConfig from './linkingConfig'; import AppNavigator from './AppNavigator'; import {setCurrentURL} from '../actions/App'; @@ -37,7 +37,6 @@ class NavigationRoot extends Component { } onStateChange={this.parseAndStoreRoute} - onReady={Navigation.enableNavigation} ref={navigationRef} linking={linkingConfig} documentTitle={{ From fdc82590b98af0cae75770418660645b69648fdb Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 27 Apr 2021 20:47:33 +0300 Subject: [PATCH 10/26] Revert "fix: Report.fetchAllReports - Navigation.navigate does not work when no router is mounted" This reverts commit c4d25f5e This fix is no longer needed --- src/libs/actions/Report.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 63f92b6fbfa7..c97c739b8092 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -659,15 +659,9 @@ function fetchOrCreateChatReport(participants, shouldNavigate = true) { Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${data.reportID}`, {reportID: data.reportID}); if (shouldNavigate) { - try { - // Redirect the logged in person to the new report - Navigation.navigate(ROUTES.getReportRoute(data.reportID)); - } catch (e) { - // When switching between the Auth and Public routers there's a brief time of no navigator mounted - console.error(e); - } + // Redirect the logged in person to the new report + Navigation.navigate(ROUTES.getReportRoute(data.reportID)); } - return data.reportID; }); } From d0a952f71a001fb4297f51abc7ac0256fa1b7c61 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 28 Apr 2021 10:35:26 +0300 Subject: [PATCH 11/26] refactor: Bring back the cast logic to fetchAllReports --- src/libs/actions/Report.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index c97c739b8092..3be457abf86d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -727,9 +727,13 @@ function fetchAllReports( return; } + // The cast here is necessary as Get rvl='chatList' may return an int or Array + reportIDs = String(response.chatList) + .split(',') + .filter(_.identity); + // Get all the chat reports if they have any, otherwise create one with concierge - if (response.chatList && response.chatList.length > 0) { - reportIDs = response.chatList; + if (reportIDs.length > 0) { return fetchChatReportsByIDs(reportIDs); } From bfdbb5fcdb2e46ba122e64fa60657386c5a43438 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 28 Apr 2021 10:41:46 +0300 Subject: [PATCH 12/26] refactor: Move Onyx updating code to promise finally In the event of an error a currently viewed report would never get set This would result in a invalid initial state of the app Moving the logic to execute in `finally` would cover both success and error cases and allow the app to continue and recover --- src/libs/actions/Report.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 3be457abf86d..be43edb953c5 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -759,16 +759,15 @@ function fetchAllReports( // We are waiting 8 seconds since this provides a good time window to allow the UI to finish loading before // bogging it down with more requests and operations. }, shouldDelayActionsFetch ? 8000 : 0); - + }) + .finally(() => { // Update currentlyViewedReportID to be our first reportID from our report collection if we don't have // one already. - if (!shouldRedirectToReport || lastViewedReportID) { - return; + if (shouldRedirectToReport || !lastViewedReportID) { + const firstReportID = _.first(reportIDs); + // eslint-disable-next-line no-use-before-define + updateCurrentlyViewedReportID(firstReportID); } - - const firstReportID = _.first(reportIDs); - // eslint-disable-next-line no-use-before-define - updateCurrentlyViewedReportID(firstReportID); }); } From c71967f2f029665cc36dd4c5d87de0be65e578e0 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 28 Apr 2021 10:52:32 +0300 Subject: [PATCH 13/26] fix: Remove `loading` from the URL path in the address bar --- src/SCREENS.js | 1 + .../Navigation/AppNavigator/MainDrawerNavigator.js | 10 ++++------ src/libs/Navigation/linkingConfig.js | 1 + 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/SCREENS.js b/src/SCREENS.js index 664b2129dcbc..7970f25d418f 100644 --- a/src/SCREENS.js +++ b/src/SCREENS.js @@ -4,6 +4,7 @@ */ export default { HOME: 'Home', + LOADING: 'Loading', REPORT: 'Report', SIGN_IN: 'SignIn', }; diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index 502b53de2b4f..babd907af373 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -5,6 +5,7 @@ import {createDrawerNavigator} from '@react-navigation/drawer'; import {withOnyx} from 'react-native-onyx'; import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions'; +import FullScreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; import styles, { getNavigationDrawerType, getNavigationDrawerStyle, @@ -15,7 +16,8 @@ import compose from '../../compose'; // Screens import SidebarScreen from '../../../pages/home/sidebar/SidebarScreen'; import ReportScreen from '../../../pages/home/ReportScreen'; -import FullScreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; + +const LoadingScreen = React.memo(() => , () => true); const propTypes = { // Initial report to be used if nothing else is specified by routing @@ -55,11 +57,7 @@ const MainDrawerNavigator = props => ( initialParams={{reportID: props.initialReportID}} /> ) - : ( - - {() => } - - ) + : } ); diff --git a/src/libs/Navigation/linkingConfig.js b/src/libs/Navigation/linkingConfig.js index e6df75231b6c..0afed9758df4 100644 --- a/src/libs/Navigation/linkingConfig.js +++ b/src/libs/Navigation/linkingConfig.js @@ -18,6 +18,7 @@ export default { screens: { // Report route [SCREENS.REPORT]: ROUTES.REPORT_WITH_ID, + [SCREENS.LOADING]: ROUTES.REPORT, }, }, From 5c91d5418e3596fbcd29c16ae70361065ed4590c Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 28 Apr 2021 11:46:30 +0300 Subject: [PATCH 14/26] update: Keep the Report Screen mounted since the first time it's resolved The Report Screen is setup to mount when: - routing information is already available - initial params provide fallback when routing information is missing This would keep the Report Screen mounted in case the `initialReportID` is ever reset to `null` like in the event of clearing Onyx data --- .../AppNavigator/MainDrawerNavigator.js | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index babd907af373..b432d327828f 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -23,6 +23,14 @@ const propTypes = { // Initial report to be used if nothing else is specified by routing initialReportID: PropTypes.string, + // Route data passed by react navigation + route: PropTypes.shape({ + state: PropTypes.shape({ + // A list of mounted screen names + routeNames: PropTypes.arrayOf(PropTypes.string), + }), + }).isRequired, + ...windowDimensionsPropTypes, }; @@ -32,6 +40,22 @@ const defaultProps = { const Drawer = createDrawerNavigator(); +/** + * Derives whether it's time to mount the ReportScreen from current component props + * + * @param {String} initialReportID + * @param {Object} route + * @returns {boolean} + */ +const shouldMountReportScreen = ({initialReportID, route}) => { + if (_.isString(initialReportID)) { + return true; + } + + // After the ReportScreen is mounted it should stay mounted no matter initial report ID + return Boolean(route.state) && route.state.routeNames.includes('Report'); +}; + const MainDrawerNavigator = props => ( ( }} > { - _.isString(props.initialReportID) + shouldMountReportScreen(props) ? ( Date: Wed, 28 Apr 2021 11:57:16 +0300 Subject: [PATCH 15/26] refactor: Replace inline Screen names with values from SCREENS --- src/libs/Navigation/AppNavigator/MainDrawerNavigator.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index b432d327828f..7fff6b6f2e9a 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -12,6 +12,7 @@ import styles, { } from '../../../styles/styles'; import ONYXKEYS from '../../../ONYXKEYS'; import compose from '../../compose'; +import SCREENS from '../../../SCREENS'; // Screens import SidebarScreen from '../../../pages/home/sidebar/SidebarScreen'; @@ -53,7 +54,7 @@ const shouldMountReportScreen = ({initialReportID, route}) => { } // After the ReportScreen is mounted it should stay mounted no matter initial report ID - return Boolean(route.state) && route.state.routeNames.includes('Report'); + return Boolean(route.state) && route.state.routeNames.includes(SCREENS.REPORT); }; const MainDrawerNavigator = props => ( @@ -76,12 +77,12 @@ const MainDrawerNavigator = props => ( shouldMountReportScreen(props) ? ( ) - : + : } ); From a2aa335b475f5c2a658b046238f9a86e10ad11f3 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 28 Apr 2021 19:05:46 +0300 Subject: [PATCH 16/26] refactor: convert MainDrawerNavigator to class to avoid `route.state` usage Keep the Report Screen mounted since the first time it's resolved The Report Screen is setup to mount when: - routing information is already available - initial params provide fallback when routing information is missing This would keep the Report Screen mounted in case the `initialReportID` is ever reset to `null` like in the event of clearing Onyx data --- .../AppNavigator/MainDrawerNavigator.js | 108 +++++++++--------- 1 file changed, 56 insertions(+), 52 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index 7fff6b6f2e9a..6f8c7c3ec701 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -18,20 +18,10 @@ import SCREENS from '../../../SCREENS'; import SidebarScreen from '../../../pages/home/sidebar/SidebarScreen'; import ReportScreen from '../../../pages/home/ReportScreen'; -const LoadingScreen = React.memo(() => , () => true); - const propTypes = { // Initial report to be used if nothing else is specified by routing initialReportID: PropTypes.string, - // Route data passed by react navigation - route: PropTypes.shape({ - state: PropTypes.shape({ - // A list of mounted screen names - routeNames: PropTypes.arrayOf(PropTypes.string), - }), - }).isRequired, - ...windowDimensionsPropTypes, }; @@ -40,56 +30,70 @@ const defaultProps = { }; const Drawer = createDrawerNavigator(); +const LoadingScreen = React.memo(() => , () => true); -/** - * Derives whether it's time to mount the ReportScreen from current component props - * - * @param {String} initialReportID - * @param {Object} route - * @returns {boolean} - */ -const shouldMountReportScreen = ({initialReportID, route}) => { - if (_.isString(initialReportID)) { - return true; +class MainDrawerNavigator extends React.Component { + constructor(props) { + super(props); + this.shouldMountReportScreen = _.isString(props.initialReportID); } - // After the ReportScreen is mounted it should stay mounted no matter initial report ID - return Boolean(route.state) && route.state.routeNames.includes(SCREENS.REPORT); -}; + shouldComponentUpdate(nextProps) { + // Once the report screen is mounted we don't unmount it + if (!this.shouldMountReportScreen) { + this.shouldMountReportScreen = _.isString(nextProps.initialReportID); -const MainDrawerNavigator = props => ( - } - screenOptions={{ - cardStyle: styles.navigationScreenCardStyle, - headerShown: false, - }} - > - { - shouldMountReportScreen(props) - ? ( - - ) - : + if (this.shouldMountReportScreen) { + return true; + } } - -); + + // Re-render the component only for these changes + const shouldUpdateForProps = ['windowWidth', 'isSmallScreenWidth']; + + const areEqual = _.isEqual( + _.pick(this.props, ...shouldUpdateForProps), + _.pick(nextProps, ...shouldUpdateForProps), + ); + + return !areEqual; + } + + render() { + return ( + } + screenOptions={{ + cardStyle: styles.navigationScreenCardStyle, + headerShown: false, + }} + > + { + this.shouldMountReportScreen + ? ( + + ) + : + } + + ); + } +} MainDrawerNavigator.propTypes = propTypes; MainDrawerNavigator.defaultProps = defaultProps; -MainDrawerNavigator.displayName = 'MainDrawerNavigator'; export default compose( withWindowDimensions, withOnyx({ From 6b224e1b9d217a8901fa0b83e39e2a415f98b586 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 28 Apr 2021 23:49:19 +0300 Subject: [PATCH 17/26] fix: The logic moved to finally was not correctly translated --- src/libs/actions/Report.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index be43edb953c5..2e0fe2380ad4 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -763,11 +763,13 @@ function fetchAllReports( .finally(() => { // Update currentlyViewedReportID to be our first reportID from our report collection if we don't have // one already. - if (shouldRedirectToReport || !lastViewedReportID) { - const firstReportID = _.first(reportIDs); - // eslint-disable-next-line no-use-before-define - updateCurrentlyViewedReportID(firstReportID); + if (!shouldRedirectToReport || lastViewedReportID) { + return; } + + const firstReportID = _.first(reportIDs); + // eslint-disable-next-line no-use-before-define + updateCurrentlyViewedReportID(firstReportID); }); } From 63264569c7a1f9dee4f50bcb4c7c2a869e393d76 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 29 Apr 2021 11:55:05 +0300 Subject: [PATCH 18/26] refactor: remove memo usage --- src/libs/Navigation/AppNavigator/MainDrawerNavigator.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index 6f8c7c3ec701..a1f039ebff22 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -30,7 +30,6 @@ const defaultProps = { }; const Drawer = createDrawerNavigator(); -const LoadingScreen = React.memo(() => , () => true); class MainDrawerNavigator extends React.Component { constructor(props) { @@ -85,7 +84,11 @@ class MainDrawerNavigator extends React.Component { initialParams={{reportID: this.props.initialReportID}} /> ) - : + : ( + + {() => } + + ) } ); From 3395cda07737d46a491ae04be7be2d4468d2357a Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 29 Apr 2021 12:06:21 +0300 Subject: [PATCH 19/26] Revert "refactor: convert MainDrawerNavigator to class to avoid `route.state` usage" This reverts commit a2aa335b --- .../AppNavigator/MainDrawerNavigator.js | 113 +++++++++--------- 1 file changed, 54 insertions(+), 59 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index a1f039ebff22..9fbaded6ea65 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -22,6 +22,14 @@ const propTypes = { // Initial report to be used if nothing else is specified by routing initialReportID: PropTypes.string, + // Route data passed by react navigation + route: PropTypes.shape({ + state: PropTypes.shape({ + // A list of mounted screen names + routeNames: PropTypes.arrayOf(PropTypes.string), + }), + }).isRequired, + ...windowDimensionsPropTypes, }; @@ -31,72 +39,59 @@ const defaultProps = { const Drawer = createDrawerNavigator(); -class MainDrawerNavigator extends React.Component { - constructor(props) { - super(props); - this.shouldMountReportScreen = _.isString(props.initialReportID); +/** + * Derives whether it's time to mount the ReportScreen from current component props + * + * @param {String} initialReportID + * @param {Object} route + * @returns {boolean} + */ +const shouldMountReportScreen = ({initialReportID, route}) => { + if (_.isString(initialReportID)) { + return true; } - shouldComponentUpdate(nextProps) { - // Once the report screen is mounted we don't unmount it - if (!this.shouldMountReportScreen) { - this.shouldMountReportScreen = _.isString(nextProps.initialReportID); + // After the ReportScreen is mounted it should stay mounted no matter initial report ID + return Boolean(route.state) && route.state.routeNames.includes(SCREENS.REPORT); +}; - if (this.shouldMountReportScreen) { - return true; - } +const MainDrawerNavigator = props => ( + } + screenOptions={{ + cardStyle: styles.navigationScreenCardStyle, + headerShown: false, + }} + > + { + shouldMountReportScreen(props) + ? ( + + ) + : ( + + {() => } + + ) } - - // Re-render the component only for these changes - const shouldUpdateForProps = ['windowWidth', 'isSmallScreenWidth']; - - const areEqual = _.isEqual( - _.pick(this.props, ...shouldUpdateForProps), - _.pick(nextProps, ...shouldUpdateForProps), - ); - - return !areEqual; - } - - render() { - return ( - } - screenOptions={{ - cardStyle: styles.navigationScreenCardStyle, - headerShown: false, - }} - > - { - this.shouldMountReportScreen - ? ( - - ) - : ( - - {() => } - - ) - } - - ); - } -} + +); MainDrawerNavigator.propTypes = propTypes; MainDrawerNavigator.defaultProps = defaultProps; +MainDrawerNavigator.displayName = 'MainDrawerNavigator'; export default compose( withWindowDimensions, withOnyx({ From 8bc31042d2412ddde20e1b2a153eae04ef4d2941 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 29 Apr 2021 12:47:41 +0300 Subject: [PATCH 20/26] refactor: base home navigator mounting on whether there are any reports ready There's no point in mounting the actual navigator before it has anything to route Added a fallback logic here so that even if `initialReportID` is never set the component would still resolve and function correctly --- .../AppNavigator/MainDrawerNavigator.js | 95 ++++++++----------- 1 file changed, 42 insertions(+), 53 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index 9fbaded6ea65..c98540480d03 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -22,81 +22,70 @@ const propTypes = { // Initial report to be used if nothing else is specified by routing initialReportID: PropTypes.string, - // Route data passed by react navigation - route: PropTypes.shape({ - state: PropTypes.shape({ - // A list of mounted screen names - routeNames: PropTypes.arrayOf(PropTypes.string), - }), - }).isRequired, + // Available reports that would be displayed in this navigator + reports: PropTypes.objectOf(PropTypes.shape({ + reportID: PropTypes.number, + })), ...windowDimensionsPropTypes, }; const defaultProps = { initialReportID: null, + reports: {}, }; const Drawer = createDrawerNavigator(); -/** - * Derives whether it's time to mount the ReportScreen from current component props - * - * @param {String} initialReportID - * @param {Object} route - * @returns {boolean} - */ -const shouldMountReportScreen = ({initialReportID, route}) => { - if (_.isString(initialReportID)) { - return true; +const MainDrawerNavigator = (props) => { + // When there are no reports there's no point to render the empty navigator + if (_.size(props.reports) === 0) { + return ; } - // After the ReportScreen is mounted it should stay mounted no matter initial report ID - return Boolean(route.state) && route.state.routeNames.includes(SCREENS.REPORT); -}; + // Use the provided initialReport or fallback to the first available + const initialReportId = props.initialReportID || _.first(_.values(props.reports)).reportID; -const MainDrawerNavigator = props => ( - } - screenOptions={{ - cardStyle: styles.navigationScreenCardStyle, - headerShown: false, - }} - > - { - shouldMountReportScreen(props) - ? ( - - ) - : ( - - {() => } - - ) - } - -); + /* After the app initializes and reports are available the home navigation is mounted + * This way routing information is updated (if needed) based on the initial report ID resolved. + * This is usually needed after login/create account and re-launches */ + return ( + } + screenOptions={{ + cardStyle: styles.navigationScreenCardStyle, + headerShown: false, + }} + > + + + ); +}; MainDrawerNavigator.propTypes = propTypes; MainDrawerNavigator.defaultProps = defaultProps; MainDrawerNavigator.displayName = 'MainDrawerNavigator'; + export default compose( withWindowDimensions, withOnyx({ initialReportID: { key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID, }, + reports: { + key: ONYXKEYS.COLLECTION.REPORT, + }, }), )(MainDrawerNavigator); From 14df6cbfccc5944c46d7d193934b7364917e7687 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 29 Apr 2021 12:49:15 +0300 Subject: [PATCH 21/26] Revert "refactor: Reuse `updateCurrentlyViewedReportID`" This reverts commit f394b98b --- src/libs/actions/Report.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 2e0fe2380ad4..b113a07423eb 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -768,8 +768,8 @@ function fetchAllReports( } const firstReportID = _.first(reportIDs); - // eslint-disable-next-line no-use-before-define - updateCurrentlyViewedReportID(firstReportID); + const currentReportID = firstReportID ? String(firstReportID) : ''; + Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, currentReportID); }); } @@ -956,7 +956,7 @@ function handleReportChanged(report) { * @param {Number} reportID */ function updateCurrentlyViewedReportID(reportID) { - Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, String(reportID || '')); + Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, String(reportID)); } Onyx.connect({ From fa3eaa2f7024abdd96eb7504ec46cbf1b2de1293 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 29 Apr 2021 12:56:16 +0300 Subject: [PATCH 22/26] refactor: remove the `shouldRedirectToReport` and lastReportID setting This handling is no longer needed Initial routing data in `MainDrawerNavigator` would resolve with a reportID Then the ReportScreen will store it in Onyx when it is mounted: didMount -> storeCurrentlyViewedReport --- src/libs/Navigation/AppNavigator/AuthScreens.js | 2 +- src/libs/actions/Report.js | 15 +-------------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 0ff7a55c5875..a366c7c67113 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -113,7 +113,7 @@ class AuthScreens extends React.Component { PersonalDetails.fetch(); User.getUserDetails(); User.getBetas(); - fetchAllReports(true, true, true); + fetchAllReports(true, true); fetchCountryCodeByRequestIP(); UnreadIndicatorUpdater.listenForReportChanges(); diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index b113a07423eb..f67249d96b7d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -708,12 +708,10 @@ function fetchActions(reportID, offset) { /** * Get all of our reports * - * @param {Boolean} shouldRedirectToReport this is set to false when the network reconnect code runs * @param {Boolean} shouldRecordHomePageTiming whether or not performance timing should be measured * @param {Boolean} shouldDelayActionsFetch when the app loads we want to delay the fetching of additional actions */ function fetchAllReports( - shouldRedirectToReport = true, shouldRecordHomePageTiming = false, shouldDelayActionsFetch = false, ) { @@ -759,17 +757,6 @@ function fetchAllReports( // We are waiting 8 seconds since this provides a good time window to allow the UI to finish loading before // bogging it down with more requests and operations. }, shouldDelayActionsFetch ? 8000 : 0); - }) - .finally(() => { - // Update currentlyViewedReportID to be our first reportID from our report collection if we don't have - // one already. - if (!shouldRedirectToReport || lastViewedReportID) { - return; - } - - const firstReportID = _.first(reportIDs); - const currentReportID = firstReportID ? String(firstReportID) : ''; - Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, currentReportID); }); } @@ -966,7 +953,7 @@ Onyx.connect({ // When the app reconnects from being offline, fetch all of the reports and their actions NetworkConnection.onReconnect(() => { - fetchAllReports(false); + fetchAllReports(); }); export { From 558babb7b19dbfa46a0bd71991ea1bb762d36ff4 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 29 Apr 2021 13:19:36 +0300 Subject: [PATCH 23/26] feat: findLastAccessedReport a function to derive the last viewed report When `initialReportId` is not available we can fall back to the last viewed report. BTW `initialReportId` might not be needed at all since it would be storing the last accessed/viewed report --- .../AppNavigator/MainDrawerNavigator.js | 7 ++++--- src/libs/actions/Report.js | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index c98540480d03..19d3ddfdd42f 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -6,6 +6,7 @@ import {withOnyx} from 'react-native-onyx'; import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions'; import FullScreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; +import {findLastAccessedReport} from '../../actions/Report'; import styles, { getNavigationDrawerType, getNavigationDrawerStyle, @@ -43,8 +44,8 @@ const MainDrawerNavigator = (props) => { return ; } - // Use the provided initialReport or fallback to the first available - const initialReportId = props.initialReportID || _.first(_.values(props.reports)).reportID; + // Use the provided initialReport or fallback to the last accessed + const initialReportID = props.initialReportID || findLastAccessedReport(props.reports).reportID; /* After the app initializes and reports are available the home navigation is mounted * This way routing information is updated (if needed) based on the initial report ID resolved. @@ -68,7 +69,7 @@ const MainDrawerNavigator = (props) => { ); diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index f67249d96b7d..9568e48b84bc 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -946,6 +946,20 @@ function updateCurrentlyViewedReportID(reportID) { Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, String(reportID)); } +/** + * Given a collection of reports finds the most recently accessed one + * + * @param {Record|Array<{lastVisitedTimestamp}>} reports + * @returns {Object} + */ +function findLastAccessedReport(reports) { + return _.chain(reports) + .toArray() + .sortBy('lastVisitedTimestamp') + .last() + .value(); +} + Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT, callback: handleReportChanged, @@ -960,6 +974,7 @@ export { fetchAllReports, fetchActions, fetchOrCreateChatReport, + findLastAccessedReport, addAction, updateLastReadActionID, subscribeToReportCommentEvents, From f0050f68389df139ceaba9a364f3be80eac97a35 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 29 Apr 2021 13:54:30 +0300 Subject: [PATCH 24/26] refactor: move `findLastAccessedReport` to `reportUtils` When `initialReportId` is not available we can fall back to the last viewed report. BTW `initialReportId` might not be needed at all since it would be storing the last accessed/viewed report --- .../AppNavigator/MainDrawerNavigator.js | 4 ++-- src/libs/actions/Report.js | 15 --------------- src/libs/reportUtils.js | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index 19d3ddfdd42f..f81a47f0e5fb 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -6,7 +6,7 @@ import {withOnyx} from 'react-native-onyx'; import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions'; import FullScreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; -import {findLastAccessedReport} from '../../actions/Report'; +import {getLastAccessedReport} from '../../reportUtils'; import styles, { getNavigationDrawerType, getNavigationDrawerStyle, @@ -45,7 +45,7 @@ const MainDrawerNavigator = (props) => { } // Use the provided initialReport or fallback to the last accessed - const initialReportID = props.initialReportID || findLastAccessedReport(props.reports).reportID; + const initialReportID = props.initialReportID || getLastAccessedReport(props.reports).reportID; /* After the app initializes and reports are available the home navigation is mounted * This way routing information is updated (if needed) based on the initial report ID resolved. diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 9568e48b84bc..f67249d96b7d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -946,20 +946,6 @@ function updateCurrentlyViewedReportID(reportID) { Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, String(reportID)); } -/** - * Given a collection of reports finds the most recently accessed one - * - * @param {Record|Array<{lastVisitedTimestamp}>} reports - * @returns {Object} - */ -function findLastAccessedReport(reports) { - return _.chain(reports) - .toArray() - .sortBy('lastVisitedTimestamp') - .last() - .value(); -} - Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT, callback: handleReportChanged, @@ -974,7 +960,6 @@ export { fetchAllReports, fetchActions, fetchOrCreateChatReport, - findLastAccessedReport, addAction, updateLastReadActionID, subscribeToReportCommentEvents, diff --git a/src/libs/reportUtils.js b/src/libs/reportUtils.js index a8e5575059b4..668aca5c41b7 100644 --- a/src/libs/reportUtils.js +++ b/src/libs/reportUtils.js @@ -21,7 +21,22 @@ function isReportMessageAttachment(reportMessageText) { return reportMessageText === '[Attachment]'; } +/** + * Given a collection of reports returns the most recently accessed one + * + * @param {Record|Array<{lastVisitedTimestamp}>} reports + * @returns {Object} + */ +function getLastAccessedReport(reports) { + return _.chain(reports) + .toArray() + .sortBy('lastVisitedTimestamp') + .last() + .value(); +} + export { getReportParticipantsTitle, isReportMessageAttachment, + getLastAccessedReport, }; From 48f2427380d586f681b03e3162aa9f190044234a Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 29 Apr 2021 14:07:00 +0300 Subject: [PATCH 25/26] refactor: remove `initialReportID` no longer needed `initialReportID` is always the same as the ID returned by `getLastAccessedReport` --- .../Navigation/AppNavigator/MainDrawerNavigator.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js index f81a47f0e5fb..7dd353a5927e 100644 --- a/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js +++ b/src/libs/Navigation/AppNavigator/MainDrawerNavigator.js @@ -20,9 +20,6 @@ import SidebarScreen from '../../../pages/home/sidebar/SidebarScreen'; import ReportScreen from '../../../pages/home/ReportScreen'; const propTypes = { - // Initial report to be used if nothing else is specified by routing - initialReportID: PropTypes.string, - // Available reports that would be displayed in this navigator reports: PropTypes.objectOf(PropTypes.shape({ reportID: PropTypes.number, @@ -32,20 +29,21 @@ const propTypes = { }; const defaultProps = { - initialReportID: null, reports: {}, }; const Drawer = createDrawerNavigator(); +// Decorated to always returning the result of the first call - keeps Screen initialParams from changing +const getInitialReport = _.once(getLastAccessedReport); + const MainDrawerNavigator = (props) => { // When there are no reports there's no point to render the empty navigator if (_.size(props.reports) === 0) { return ; } - // Use the provided initialReport or fallback to the last accessed - const initialReportID = props.initialReportID || getLastAccessedReport(props.reports).reportID; + const initialReportID = getInitialReport(props.reports).reportID; /* After the app initializes and reports are available the home navigation is mounted * This way routing information is updated (if needed) based on the initial report ID resolved. @@ -82,9 +80,6 @@ MainDrawerNavigator.displayName = 'MainDrawerNavigator'; export default compose( withWindowDimensions, withOnyx({ - initialReportID: { - key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID, - }, reports: { key: ONYXKEYS.COLLECTION.REPORT, }, From 13435af592e4f5ef3361100b8e21789db27781f2 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 29 Apr 2021 14:48:23 +0300 Subject: [PATCH 26/26] fix: When `fetchAllReports` creates a chat with concierge it don't have to navigate to it This would be resolved by initial params Otherwise it still work, but an error is printed out that navigation failed The ReportScreen is not mounted as the change that mounts it happens a few lines before the call to navigate in `fetchOrCreateChatReport` --- src/libs/actions/Report.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index f67249d96b7d..584c69c4a368 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -735,7 +735,7 @@ function fetchAllReports( return fetchChatReportsByIDs(reportIDs); } - return fetchOrCreateChatReport([currentUserEmail, 'concierge@expensify.com']) + return fetchOrCreateChatReport([currentUserEmail, 'concierge@expensify.com'], false) .then((createdReportID) => { reportIDs = [createdReportID]; });