Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Blank screen after creating an account #2575

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
529f0fb
feat: Navigation navigateLater and enableNavigation
kidroca Apr 26, 2021
78490e7
refactor: AuthScreens empty string is a valid initial value for initi…
kidroca Apr 26, 2021
8aeafcc
fix: Report.fetchAllReports - fetchChatReportsByIDs would be called w…
kidroca Apr 26, 2021
da36c16
fix: Report.fetchAllReports - new report created by `fetchOrCreateCha…
kidroca Apr 26, 2021
c4d25f5
fix: Report.fetchAllReports - Navigation.navigate does not work when …
kidroca Apr 26, 2021
d6596c5
refactor: Move initialReportID from AuthScreens to MainDrawerNavigator
kidroca Apr 27, 2021
1591d76
refactor: Mount the Report Screen only when initial data is resolved
kidroca Apr 27, 2021
f394b98
refactor: Reuse `updateCurrentlyViewedReportID`
kidroca Apr 27, 2021
9faf5e4
Revert "feat: Navigation navigateLater and enableNavigation"
kidroca Apr 27, 2021
fdc8259
Revert "fix: Report.fetchAllReports - Navigation.navigate does not wo…
kidroca Apr 27, 2021
d0a952f
refactor: Bring back the cast logic to fetchAllReports
kidroca Apr 28, 2021
bfdbb5f
refactor: Move Onyx updating code to promise finally
kidroca Apr 28, 2021
c71967f
fix: Remove `loading` from the URL path in the address bar
kidroca Apr 28, 2021
5c91d54
update: Keep the Report Screen mounted since the first time it's reso…
kidroca Apr 28, 2021
b20db83
refactor: Replace inline Screen names with values from SCREENS
kidroca Apr 28, 2021
a2aa335
refactor: convert MainDrawerNavigator to class to avoid `route.state`…
kidroca Apr 28, 2021
6b224e1
fix: The logic moved to finally was not correctly translated
kidroca Apr 28, 2021
6326456
refactor: remove memo usage
kidroca Apr 29, 2021
3395cda
Revert "refactor: convert MainDrawerNavigator to class to avoid `rout…
kidroca Apr 29, 2021
8bc3104
refactor: base home navigator mounting on whether there are any repor…
kidroca Apr 29, 2021
14df6cb
Revert "refactor: Reuse `updateCurrentlyViewedReportID`"
kidroca Apr 29, 2021
fa3eaa2
refactor: remove the `shouldRedirectToReport` and lastReportID setting
kidroca Apr 29, 2021
558babb
feat: findLastAccessedReport a function to derive the last viewed report
kidroca Apr 29, 2021
f0050f6
refactor: move `findLastAccessedReport` to `reportUtils`
kidroca Apr 29, 2021
48f2427
refactor: remove `initialReportID` no longer needed
kidroca Apr 29, 2021
13435af
fix: When `fetchAllReports` creates a chat with concierge it don't ha…
kidroca Apr 29, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/SCREENS.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
export default {
HOME: 'Home',
LOADING: 'Loading',
REPORT: 'Report',
SIGN_IN: 'SignIn',
};
31 changes: 1 addition & 30 deletions src/libs/Navigation/AppNavigator/AuthScreens.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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() {
Expand All @@ -119,7 +113,7 @@ class AuthScreens extends React.Component {
PersonalDetails.fetch();
User.getUserDetails();
User.getBetas();
fetchAllReports(true, true, true);
fetchAllReports(true, true);
fetchCountryCodeByRequestIP();
UnreadIndicatorUpdater.listenForReportChanges();

Expand Down Expand Up @@ -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 (!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;
}

Expand All @@ -168,11 +153,6 @@ class AuthScreens extends React.Component {
}

render() {
// Wait to resolve initial Home route params.
if (!this.initialReportID) {
return null;
}

const modalScreenOptions = {
headerShown: false,
cardStyle: getNavigationModalCardStyle(this.props.isSmallScreenWidth),
Expand All @@ -196,12 +176,6 @@ class AuthScreens extends React.Component {
headerShown: false,
title: 'Expensify.cash',
}}
initialParams={{
screen: SCREENS.REPORT,
params: {
reportID: this.initialReportID,
},
}}
component={MainDrawerNavigator}
/>
<RootStack.Screen
Expand Down Expand Up @@ -278,8 +252,5 @@ export default compose(
network: {
key: ONYXKEYS.NETWORK,
},
initialReportID: {
key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
},
}),
)(AuthScreens);
86 changes: 62 additions & 24 deletions src/libs/Navigation/AppNavigator/MainDrawerNavigator.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,87 @@
import React from 'react';
import PropTypes from 'prop-types';
import _ from 'underscore';
import {createDrawerNavigator} from '@react-navigation/drawer';
import {withOnyx} from 'react-native-onyx';

import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
import FullScreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator';
import {getLastAccessedReport} from '../../reportUtils';
import styles, {
getNavigationDrawerType,
getNavigationDrawerStyle,
} from '../../../styles/styles';
import ONYXKEYS from '../../../ONYXKEYS';
import compose from '../../compose';
import SCREENS from '../../../SCREENS';

// Screens
import SidebarScreen from '../../../pages/home/sidebar/SidebarScreen';
import ReportScreen from '../../../pages/home/ReportScreen';

const propTypes = {
// Available reports that would be displayed in this navigator
reports: PropTypes.objectOf(PropTypes.shape({
reportID: PropTypes.number,
})),

...windowDimensionsPropTypes,
};

const defaultProps = {
reports: {},
};

const Drawer = createDrawerNavigator();

const MainDrawerNavigator = props => (
<Drawer.Navigator
openByDefault
drawerType={getNavigationDrawerType(props.isSmallScreenWidth)}
drawerStyle={getNavigationDrawerStyle(
props.windowWidth,
props.isSmallScreenWidth,
)}
sceneContainerStyle={styles.navigationSceneContainer}
edgeWidth={500}
drawerContent={() => <SidebarScreen />}
>
<Drawer.Screen
name="Report"
component={ReportScreen}

// Providing an empty string here will ensure that the ReportScreen does not show as '/r/undefined'
// eslint-disable-next-line react/jsx-props-no-multi-spaces
initialParams={{reportID: ''}}
options={{
// Decorated to always returning the result of the first call - keeps Screen initialParams from changing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is a little hard to follow and I might change it to something like:

We are decorating getLastAccessedReport so that it caches the first result once the reports load. This will ensure the initialParams are only set once for the ReportScreen.

Which also begs the question... what happens if the initialParams change? And why do we care? Maybe we can explain that as well here just so it is very obvious?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing will happen if we pass different initialParams after the screen is mounted. And that's why we want to stop calculating them each time MainDrawerNavigator re-renders

It might be better to just remove this _.once optimization and address it if it ever becomes a problem

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one, yep I think this is fine in any case but took me a second to get why so a small comment would have been helpful.

const getInitialReport = _.once(getLastAccessedReport);
kidroca marked this conversation as resolved.
Show resolved Hide resolved

const MainDrawerNavigator = (props) => {
// When there are no reports there's no point to render the empty navigator
if (_.size(props.reports) === 0) {
return <FullScreenLoadingIndicator visible />;
}

const initialReportID = getInitialReport(props.reports).reportID;
kidroca marked this conversation as resolved.
Show resolved Hide resolved

/* 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 (
<Drawer.Navigator
openByDefault
drawerType={getNavigationDrawerType(props.isSmallScreenWidth)}
drawerStyle={getNavigationDrawerStyle(
props.windowWidth,
props.isSmallScreenWidth,
)}
sceneContainerStyle={styles.navigationSceneContainer}
edgeWidth={500}
drawerContent={() => <SidebarScreen />}
screenOptions={{
cardStyle: styles.navigationScreenCardStyle,
headerShown: false,
}}
/>
</Drawer.Navigator>
);
>
<Drawer.Screen
name={SCREENS.REPORT}
component={ReportScreen}
initialParams={{reportID: initialReportID.toString()}}
/>
</Drawer.Navigator>
);
};

MainDrawerNavigator.propTypes = propTypes;
MainDrawerNavigator.defaultProps = defaultProps;
MainDrawerNavigator.displayName = 'MainDrawerNavigator';
export default withWindowDimensions(MainDrawerNavigator);

export default compose(
withWindowDimensions,
withOnyx({
reports: {
key: ONYXKEYS.COLLECTION.REPORT,
},
}),
)(MainDrawerNavigator);
1 change: 1 addition & 0 deletions src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default {
screens: {
// Report route
[SCREENS.REPORT]: ROUTES.REPORT_WITH_ID,
[SCREENS.LOADING]: ROUTES.REPORT,
},
},

Expand Down
27 changes: 10 additions & 17 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
) {
Expand All @@ -727,15 +725,20 @@ function fetchAllReports(
return;
}

// The string cast here is necessary as Get rvl='chatList' may return an int
reportIDs = String(response.chatList).split(',');
kidroca marked this conversation as resolved.
Show resolved Hide resolved
// The cast here is necessary as Get rvl='chatList' may return an int or Array
reportIDs = String(response.chatList)
.split(',')
.filter(_.identity);
kidroca marked this conversation as resolved.
Show resolved Hide resolved

// Get all the chat reports if they have any, otherwise create one with concierge
if (reportIDs.length) {
if (reportIDs.length > 0) {
return fetchChatReportsByIDs(reportIDs);
}

return fetchOrCreateChatReport([currentUserEmail, 'concierge@expensify.com']);
return fetchOrCreateChatReport([currentUserEmail, 'concierge@expensify.com'], false)
.then((createdReportID) => {
reportIDs = [createdReportID];
});
kidroca marked this conversation as resolved.
Show resolved Hide resolved
})
.then(() => {
Onyx.set(ONYXKEYS.INITIAL_REPORT_DATA_LOADED, true);
Expand All @@ -754,16 +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);

// 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);
kidroca marked this conversation as resolved.
Show resolved Hide resolved
});
}

Expand Down Expand Up @@ -960,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 {
Expand Down
15 changes: 15 additions & 0 deletions src/libs/reportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,22 @@ function isReportMessageAttachment(reportMessageText) {
return reportMessageText === '[Attachment]';
}

/**
* Given a collection of reports returns the most recently accessed one
*
* @param {Record<String, {lastVisitedTimestamp}>|Array<{lastVisitedTimestamp}>} reports
* @returns {Object}
*/
function getLastAccessedReport(reports) {
return _.chain(reports)
.toArray()
.sortBy('lastVisitedTimestamp')
.last()
.value();
}

export {
getReportParticipantsTitle,
isReportMessageAttachment,
getLastAccessedReport,
};