Follow up to comments on PR1559

src/Expensify.js

@@ -42,7 +42,10 @@ const propTypes = {
 
     // Session info for the currently logged in user.
     session: PropTypes.shape({
+        // Currently logged in user authToken
         authToken: PropTypes.string,
+
+        // Currently logged in user accountID
         accountID: PropTypes.number,
     }),
 

src/ROUTES.js

@@ -2,22 +2,21 @@
  * This is a file containing constants for all of the routes we want to be able to go to
  */
 export default {
-    HOME: '/',
-    SETTINGS: '/settings',
-    SETTINGS_PROFILE: '/settings/profile',
-    SETTINGS_PREFERENCES: '/settings/preferences',
-    SETTINGS_PASSWORD: '/settings/password',
-    SETTINGS_PAYMENTS: '/settings/payments',
-    NEW_GROUP: '/new/group',
-    NEW_CHAT: '/new/chat',
-    REPORT: '/r',
-    IOU_REQUEST: '/iou/request',
-    IOU_BILL: '/iou/split',
-    getReportRoute: reportID => `/r/${reportID}`,
-    SEARCH: '/search',
-    SET_PASSWORD: '/setpassword/:validateCode',
-    SIGNIN: '/signin',
-    NOT_FOUND: '/404',
-    PROFILE: '/profile/:login',
-    getProfileRoute: login => `/profile/${login}`,
+    HOME: '',
+    SETTINGS: 'settings',
+    SETTINGS_PROFILE: 'settings/profile',
+    SETTINGS_PREFERENCES: 'settings/preferences',
+    SETTINGS_PASSWORD: 'settings/password',
+    SETTINGS_PAYMENTS: 'settings/payments',
+    NEW_GROUP: 'new/group',
+    NEW_CHAT: 'new/chat',
+    REPORT: 'r',
+    IOU_REQUEST: 'iou/request',
+    IOU_BILL: 'iou/split',
+    getReportRoute: reportID => `r/${reportID}`,
+    SEARCH: 'search',
+    SIGNIN: 'signin',
+    SET_PASSWORD: 'setpassword/:validateCode',
+    PROFILE: 'profile',
+    getProfileRoute: login => `profile/${login}`,
 };

src/components/ScreenWrapper.js

@@ -1,3 +1,4 @@
+import _ from 'underscore';
 import React from 'react';
 import PropTypes from 'prop-types';
 import {View} from 'react-native';
@@ -9,8 +10,11 @@ const propTypes = {
     // Array of additional styles to add
     style: PropTypes.arrayOf(PropTypes.object),
 
-    // Returns a function as a child to pass insets to
-    children: PropTypes.func.isRequired,
+    // Returns a function as a child to pass insets to or a node to render without insets
+    children: PropTypes.oneOfType([
+        PropTypes.node,
+        PropTypes.func,
+    ]).isRequired,
 
     // Whether to include padding bottom
     includePaddingBottom: PropTypes.bool,
@@ -47,7 +51,11 @@ const ScreenWrapper = props => (
                     ]}
                 >
                     <HeaderGap />
-                    {props.children(insets)}
+                    {// If props.children is a function, call it to provide the insets to the children.
+                        _.isFunction(props.children)
+                            ? props.children(insets)
+                            : props.children
+                    }
                 </View>
             );
         }}

src/libs/Navigation/AppNavigator/ModalStackNavigators.js

@@ -1,6 +1,7 @@
 import React from 'react';
 
 import styles from '../../../styles/styles';
+import ROUTES from '../../../ROUTES';
 import {
     SettingsModalStack,
     NewChatModalStack,
@@ -29,7 +30,7 @@ const defaultSubRouteOptions = {
 
 const IOUBillStackNavigator = () => (
     <IOUBillModalStack.Navigator
-        path="/iou/split"
+        path={ROUTES.IOU_BILL}
     >
         <IOUBillModalStack.Screen
             name="IOU_Bill_Root"
@@ -44,7 +45,7 @@ const IOUBillStackNavigator = () => (
 
 const IOURequestModalStackNavigator = () => (
     <IOURequestModalStack.Navigator
-        path="/iou/request"
+        path={ROUTES.IOU_REQUEST}
     >
         <IOURequestModalStack.Screen
             name="IOU_Request_Root"
@@ -59,7 +60,7 @@ const IOURequestModalStackNavigator = () => (
 
 const ProfileModalStackNavigator = () => (
     <ProfileModalStack.Navigator
-        path="/profile"
+        path={ROUTES.PROFILE}
     >
         <ProfileModalStack.Screen
             name="Profile_Root"
@@ -74,7 +75,7 @@ const ProfileModalStackNavigator = () => (
 
 const SearchModalStackNavigator = () => (
     <SearchModalStack.Navigator
-        path="/search"
+        path={ROUTES.SEARCH}
     >
         <SearchModalStack.Screen
             name="Search_Root"
@@ -89,7 +90,7 @@ const SearchModalStackNavigator = () => (
 
 const NewGroupModalStackNavigator = () => (
     <NewGroupModalStack.Navigator
-        path="/new/group"
+        path={ROUTES.NEW_GROUP}
     >
         <NewGroupModalStack.Screen
             name="NewGroup_Root"
@@ -104,7 +105,7 @@ const NewGroupModalStackNavigator = () => (
 
 const NewChatModalStackNavigator = () => (
     <NewChatModalStack.Navigator
-        path="/new/chat"
+        path={ROUTES.NEW_CHAT}
     >
         <NewChatModalStack.Screen
             name="NewChat_Root"
@@ -119,7 +120,7 @@ const NewChatModalStackNavigator = () => (
 
 const SettingsModalStackNavigator = () => (
     <SettingsModalStack.Navigator
-        path="/settings"
+        path={ROUTES.SETTINGS}
     >
         <SettingsModalStack.Screen
             name="Settings_Root"

src/libs/Navigation/AppNavigator/createCustomModalStackNavigator.js

@@ -12,16 +12,27 @@ import compose from '../../compose';
 import CONST from '../../../CONST';
 
 const propTypes = {
-    // Internal react-navigation stuff used to determine which view we should display
+    // Navigation state for this navigator
+    // See: https://reactnavigation.org/docs/navigation-state/
     state: PropTypes.shape({
+        // Index of the focused route object in the routes array
         index: PropTypes.number,
+
+        // List of route objects (screens) which are rendered in the navigator. It also represents the history in a
+        // stack navigator. There should be at least one item present in this array.
         routes: PropTypes.arrayOf(PropTypes.shape({
+
+            // A unique key name for a screen. Created automatically by react-nav.
             key: PropTypes.string,
         })),
     }).isRequired,
 
-    // The current view object that has a render method to call
+    // Object containing descriptors for each route with the route keys as its properties
+    // See: https://reactnavigation.org/docs/custom-navigators/#usenavigationbuilder
     descriptors: PropTypes.objectOf(PropTypes.shape({
+
+        // A function which can be used to render the actual screen. Calling descriptors[route.key].render() will return
+        // a React element containing the screen content.
         render: PropTypes.func,
     })).isRequired,
 
@@ -38,6 +49,13 @@ const defaultProps = {
 
 
 class ResponsiveView extends React.Component {
+    /**
+     * Returns the current descriptor for the focused screen in this navigators state. The descriptor has a function
+     * called render() that we must call each time this navigator updates. It's important to use this method to render
+     * a screen, otherwise any child navigators won't be connected to the navigation tree properly.
+     *
+     * @returns {Object}
+     */
     getCurrentViewDescriptor() {
         const currentRoute = this.props.state.routes[this.props.state.index];
         const currentRouteKey = currentRoute.key;

src/libs/Navigation/Navigation.js

@@ -32,11 +32,7 @@ function goBack() {
  * Main navigation method for redirecting to a route.
  * @param {String} route
  */
-function navigate(route) {
-    if (!route) {
-        return;
-    }
-
+function navigate(route = ROUTES.HOME) {
     if (route === ROUTES.HOME) {
         openDrawer();
         return;

src/libs/Navigation/NavigationRoot.js

@@ -7,7 +7,7 @@ import {
     getPathFromState,
     NavigationContainer,
 } from '@react-navigation/native';
-import Onyx, {withOnyx} from 'react-native-onyx';
+import {withOnyx} from 'react-native-onyx';
 import {navigationRef} from './Navigation';
 import linkingConfig from './linkingConfig';
 import AppNavigator from './AppNavigator';
@@ -16,9 +16,14 @@ import ONYXKEYS from '../../ONYXKEYS';
 import ROUTES from '../../ROUTES';
 import styles from '../../styles/styles';
 import themeColors from '../../styles/themes/default';
+import {updateCurrentlyViewedReportID} from '../actions/Report';
+import {setCurrentURL} from '../actions/App';
 
 const propTypes = {
+    // Whether the current user is logged in with an authToken
     authenticated: PropTypes.bool.isRequired,
+
+    // The current reportID that we are navigated to or should show in the ReportScreen
     currentlyViewedReportID: PropTypes.string,
 };
 
@@ -43,11 +48,11 @@ class NavigationRoot extends Component {
                 // hooked up
                 const path = getPathName(initialUrl);
                 let initialState = getStateFromPath(path, linkingConfig.config);
-                Onyx.set(ONYXKEYS.CURRENT_URL, path);
+                setCurrentURL(path);
 
                 // If we are landing on something other than the report screen or site root then we MUST set the
                 // initial route to the currently viewed report so there some history to navigate back from
-                if (path !== ROUTES.HOME && !path.includes(ROUTES.REPORT)) {
+                if (path !== `/${ROUTES.HOME}` && !path.includes(`/${ROUTES.REPORT}`)) {
                     const homeRoute = {
                         name: 'Home',
                     };
@@ -99,11 +104,11 @@ class NavigationRoot extends Component {
                     if (path.includes(ROUTES.REPORT)) {
                         const reportID = Number(_.last(path.split('/')));
                         if (!_.isNaN(reportID)) {
-                            Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, String(reportID));
+                            updateCurrentlyViewedReportID(reportID);
                         }
                     }
 
-                    Onyx.merge(ONYXKEYS.CURRENT_URL, path);
+                    setCurrentURL(path);
                 }}
                 ref={navigationRef}
                 linking={linkingConfig}

src/libs/Navigation/linkTo.js

@@ -5,36 +5,33 @@ import {
 import linkingConfig from './linkingConfig';
 
 export default function linkTo(navigation, path) {
-    if (!path.startsWith('/')) {
-        throw new Error(`The path must start with '/' (${path}).`);
-    }
-
+    const normalizedPath = !path.startsWith('/') ? `/${path}` : path;
     if (navigation === undefined) {
         throw new Error("Couldn't find a navigation object. Is your component inside a screen in a navigator?");
     }
 
-    const state = linkingConfig?.getStateFromPath
-        ? linkingConfig.getStateFromPath(path, linkingConfig.config)
-        : getStateFromPath(path, linkingConfig?.config);
+    const state = linkingConfig.getStateFromPath
+        ? linkingConfig.getStateFromPath(normalizedPath, linkingConfig.config)
+        : getStateFromPath(normalizedPath, linkingConfig.config);
 
-    if (state) {
-        let root = navigation;
-        let current;
+    if (!state) {
+        throw new Error('Failed to parse the path to a navigation state.');
+    }
 
-        // Traverse up to get the root navigation
-        // eslint-disable-next-line no-cond-assign
-        while ((current = root.dangerouslyGetParent())) {
-            root = current;
-        }
+    let root = navigation;
+    let current;
 
-        const action = getActionFromState(state, linkingConfig?.config);
+    // Traverse up to get the root navigation
+    // eslint-disable-next-line no-cond-assign
+    while ((current = root.dangerouslyGetParent())) {
+        root = current;
+    }
+
+    const action = getActionFromState(state, linkingConfig.config);
 
-        if (action !== undefined) {
-            root.dispatch(action);
-        } else {
-            root.reset(state);
-        }
+    if (action !== undefined) {
+        root.dispatch(action);
     } else {
-        throw new Error('Failed to parse the path to a navigation state.');
+        root.reset(state);
     }
 }