From f781c0659694912ca247a86c5715404735c4272f Mon Sep 17 00:00:00 2001 From: Hans Date: Mon, 13 Nov 2023 14:07:22 +0700 Subject: [PATCH 1/5] fix showing notfound page when offline --- .../home/report/withReportAndPrivateNotesOrNotFound.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js index 3982dd5ab542..7394c5900e13 100644 --- a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js +++ b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js @@ -6,6 +6,7 @@ import _ from 'underscore'; import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator'; import networkPropTypes from '@components/networkPropTypes'; import {withNetwork} from '@components/OnyxProvider'; +import usePrevious from '@hooks/usePrevious'; import * as Report from '@libs/actions/Report'; import compose from '@libs/compose'; import getComponentDisplayName from '@libs/getComponentDisplayName'; @@ -56,6 +57,8 @@ export default function (WrappedComponent) { const {route, report, network, session} = props; const accountID = route.params.accountID; const isPrivateNotesFetchTriggered = !_.isUndefined(report.isLoadingPrivateNotes); + const prevIsOffline = usePrevious(network.isOffline); + const isReconnecting = prevIsOffline && !network.isOffline; useEffect(() => { // Do not fetch private notes if isLoadingPrivateNotes is already defined, or if network is offline. @@ -67,7 +70,7 @@ export default function (WrappedComponent) { // eslint-disable-next-line react-hooks/exhaustive-deps -- do not add report.isLoadingPrivateNotes to dependencies }, [report.reportID, network.isOffline, isPrivateNotesFetchTriggered]); - const isPrivateNotesEmpty = accountID ? _.isEmpty(lodashGet(report, ['privateNotes', accountID, 'note'], '')) : _.isEmpty(report.privateNotes); + const isPrivateNotesEmpty = accountID ? _.has(lodashGet(report, ['privateNotes', accountID, 'note'], '')) : _.isEmpty(report.privateNotes); const shouldShowFullScreenLoadingIndicator = !isPrivateNotesFetchTriggered || (isPrivateNotesEmpty && report.isLoadingPrivateNotes); // eslint-disable-next-line rulesdir/no-negated-variables @@ -78,13 +81,13 @@ export default function (WrappedComponent) { } // Don't show not found view if the notes are still loading, or if the notes are non-empty. - if (shouldShowFullScreenLoadingIndicator || !isPrivateNotesEmpty) { + if (shouldShowFullScreenLoadingIndicator || !isPrivateNotesEmpty || isReconnecting) { return false; } // As notes being empty and not loading is a valid case, show not found view only in offline mode. return network.isOffline; - }, [report, network.isOffline, accountID, session.accountID, isPrivateNotesEmpty, shouldShowFullScreenLoadingIndicator]); + }, [report, network.isOffline, accountID, session.accountID, isPrivateNotesEmpty, shouldShowFullScreenLoadingIndicator, isReconnecting]); if (shouldShowFullScreenLoadingIndicator) { return ; From 216db981321ed4cee2185341c2d5cb49abfe08d3 Mon Sep 17 00:00:00 2001 From: Hans Date: Mon, 11 Dec 2023 10:29:14 +0700 Subject: [PATCH 2/5] fix private note edge case --- .../home/report/withReportAndPrivateNotesOrNotFound.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js index 7394c5900e13..a3efd8edc2a0 100644 --- a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js +++ b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js @@ -59,19 +59,20 @@ export default function (WrappedComponent) { const isPrivateNotesFetchTriggered = !_.isUndefined(report.isLoadingPrivateNotes); const prevIsOffline = usePrevious(network.isOffline); const isReconnecting = prevIsOffline && !network.isOffline; + const isOtherUserNote = accountID && Number(session.accountID) !== Number(accountID); + const isPrivateNotesEmpty = accountID ? _.has(lodashGet(report, ['privateNotes', accountID, 'note'], '')) : _.isEmpty(report.privateNotes); useEffect(() => { // Do not fetch private notes if isLoadingPrivateNotes is already defined, or if network is offline. - if (isPrivateNotesFetchTriggered || network.isOffline) { + if ((isPrivateNotesFetchTriggered && !isReconnecting) || network.isOffline) { return; } Report.getReportPrivateNote(report.reportID); // eslint-disable-next-line react-hooks/exhaustive-deps -- do not add report.isLoadingPrivateNotes to dependencies - }, [report.reportID, network.isOffline, isPrivateNotesFetchTriggered]); + }, [report.reportID, network.isOffline, isPrivateNotesFetchTriggered, isReconnecting]); - const isPrivateNotesEmpty = accountID ? _.has(lodashGet(report, ['privateNotes', accountID, 'note'], '')) : _.isEmpty(report.privateNotes); - const shouldShowFullScreenLoadingIndicator = !isPrivateNotesFetchTriggered || (isPrivateNotesEmpty && report.isLoadingPrivateNotes); + const shouldShowFullScreenLoadingIndicator = !isPrivateNotesFetchTriggered || (isPrivateNotesEmpty && (report.isLoadingPrivateNotes || !isOtherUserNote)); // eslint-disable-next-line rulesdir/no-negated-variables const shouldShowNotFoundPage = useMemo(() => { From d351eb30ca67ead1bc1c43b4bdb4b8cc159ff942 Mon Sep 17 00:00:00 2001 From: Hans Date: Tue, 12 Dec 2023 14:25:47 +0700 Subject: [PATCH 3/5] add loading page --- src/pages/LoadingPage.js | 35 +++++++++++++++++++ .../withReportAndPrivateNotesOrNotFound.js | 4 +-- 2 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 src/pages/LoadingPage.js diff --git a/src/pages/LoadingPage.js b/src/pages/LoadingPage.js new file mode 100644 index 000000000000..b77757241cfd --- /dev/null +++ b/src/pages/LoadingPage.js @@ -0,0 +1,35 @@ +import PropTypes from 'prop-types'; +import React from 'react'; +import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator'; +import HeaderWithBackButton from '@components/HeaderWithBackButton'; +import ScreenWrapper from '@components/ScreenWrapper'; +import useThemeStyles from '@styles/useThemeStyles'; + +const propTypes = { + /** Method to trigger when pressing back button of the header */ + onBackButtonPress: PropTypes.func, +}; + +const defaultProps = { + onBackButtonPress: undefined, +}; + +// eslint-disable-next-line rulesdir/no-negated-variables +function LoadingPage(props) { + const styles = useThemeStyles(); + return ( + + + + + ); +} + +LoadingPage.displayName = 'NotFoundPage'; +LoadingPage.propTypes = propTypes; +LoadingPage.defaultProps = defaultProps; + +export default LoadingPage; diff --git a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js index a3efd8edc2a0..f057ce550d65 100644 --- a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js +++ b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js @@ -3,7 +3,6 @@ import PropTypes from 'prop-types'; import React, {useEffect, useMemo} from 'react'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; -import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator'; import networkPropTypes from '@components/networkPropTypes'; import {withNetwork} from '@components/OnyxProvider'; import usePrevious from '@hooks/usePrevious'; @@ -12,6 +11,7 @@ import compose from '@libs/compose'; import getComponentDisplayName from '@libs/getComponentDisplayName'; import * as ReportUtils from '@libs/ReportUtils'; import NotFoundPage from '@pages/ErrorPage/NotFoundPage'; +import LoadingPage from '@pages/LoadingPage'; import reportPropTypes from '@pages/reportPropTypes'; import ONYXKEYS from '@src/ONYXKEYS'; import withReportOrNotFound from './withReportOrNotFound'; @@ -91,7 +91,7 @@ export default function (WrappedComponent) { }, [report, network.isOffline, accountID, session.accountID, isPrivateNotesEmpty, shouldShowFullScreenLoadingIndicator, isReconnecting]); if (shouldShowFullScreenLoadingIndicator) { - return ; + return ; } if (shouldShowNotFoundPage) { From 48ab68e8ee0a33b16f2f1bf706800e1a000e31b9 Mon Sep 17 00:00:00 2001 From: Hans Date: Thu, 14 Dec 2023 09:55:45 +0700 Subject: [PATCH 4/5] add props title to HOCs --- src/pages/LoadingPage.js | 4 +- .../PrivateNotes/PrivateNotesEditPage.js | 2 +- .../PrivateNotes/PrivateNotesListPage.js | 2 +- .../PrivateNotes/PrivateNotesViewPage.js | 2 +- .../withReportAndPrivateNotesOrNotFound.js | 141 +++++++++--------- 5 files changed, 79 insertions(+), 72 deletions(-) diff --git a/src/pages/LoadingPage.js b/src/pages/LoadingPage.js index b77757241cfd..d65e3329703d 100644 --- a/src/pages/LoadingPage.js +++ b/src/pages/LoadingPage.js @@ -3,11 +3,12 @@ import React from 'react'; import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; import ScreenWrapper from '@components/ScreenWrapper'; -import useThemeStyles from '@styles/useThemeStyles'; +import useThemeStyles from '@hooks/useThemeStyles'; const propTypes = { /** Method to trigger when pressing back button of the header */ onBackButtonPress: PropTypes.func, + title: PropTypes.string.isRequired, }; const defaultProps = { @@ -22,6 +23,7 @@ function LoadingPage(props) { diff --git a/src/pages/PrivateNotes/PrivateNotesEditPage.js b/src/pages/PrivateNotes/PrivateNotesEditPage.js index c62fbe3edcb5..0d4bc2c3e7e1 100644 --- a/src/pages/PrivateNotes/PrivateNotesEditPage.js +++ b/src/pages/PrivateNotes/PrivateNotesEditPage.js @@ -182,7 +182,7 @@ PrivateNotesEditPage.defaultProps = defaultProps; export default compose( withLocalize, - withReportAndPrivateNotesOrNotFound, + withReportAndPrivateNotesOrNotFound('privateNotes.title'), withOnyx({ personalDetailsList: { key: ONYXKEYS.PERSONAL_DETAILS_LIST, diff --git a/src/pages/PrivateNotes/PrivateNotesListPage.js b/src/pages/PrivateNotes/PrivateNotesListPage.js index a34eb0ce596d..8e2f8c9f43e0 100644 --- a/src/pages/PrivateNotes/PrivateNotesListPage.js +++ b/src/pages/PrivateNotes/PrivateNotesListPage.js @@ -144,7 +144,7 @@ PrivateNotesListPage.displayName = 'PrivateNotesListPage'; export default compose( withLocalize, - withReportAndPrivateNotesOrNotFound, + withReportAndPrivateNotesOrNotFound('privateNotes.title'), withOnyx({ personalDetailsList: { key: ONYXKEYS.PERSONAL_DETAILS_LIST, diff --git a/src/pages/PrivateNotes/PrivateNotesViewPage.js b/src/pages/PrivateNotes/PrivateNotesViewPage.js index 1406dfd76748..f71259a2b685 100644 --- a/src/pages/PrivateNotes/PrivateNotesViewPage.js +++ b/src/pages/PrivateNotes/PrivateNotesViewPage.js @@ -103,7 +103,7 @@ PrivateNotesViewPage.defaultProps = defaultProps; export default compose( withLocalize, - withReportAndPrivateNotesOrNotFound, + withReportAndPrivateNotesOrNotFound('privateNotes.title'), withOnyx({ personalDetailsList: { key: ONYXKEYS.PERSONAL_DETAILS_LIST, diff --git a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js index f057ce550d65..de5f7d36299c 100644 --- a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js +++ b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.js @@ -5,6 +5,7 @@ import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import networkPropTypes from '@components/networkPropTypes'; import {withNetwork} from '@components/OnyxProvider'; +import useLocalize from '@hooks/useLocalize'; import usePrevious from '@hooks/usePrevious'; import * as Report from '@libs/actions/Report'; import compose from '@libs/compose'; @@ -51,84 +52,88 @@ const defaultProps = { }, }; -export default function (WrappedComponent) { +export default function (pageTitle) { // eslint-disable-next-line rulesdir/no-negated-variables - function WithReportAndPrivateNotesOrNotFound({forwardedRef, ...props}) { - const {route, report, network, session} = props; - const accountID = route.params.accountID; - const isPrivateNotesFetchTriggered = !_.isUndefined(report.isLoadingPrivateNotes); - const prevIsOffline = usePrevious(network.isOffline); - const isReconnecting = prevIsOffline && !network.isOffline; - const isOtherUserNote = accountID && Number(session.accountID) !== Number(accountID); - const isPrivateNotesEmpty = accountID ? _.has(lodashGet(report, ['privateNotes', accountID, 'note'], '')) : _.isEmpty(report.privateNotes); - - useEffect(() => { - // Do not fetch private notes if isLoadingPrivateNotes is already defined, or if network is offline. - if ((isPrivateNotesFetchTriggered && !isReconnecting) || network.isOffline) { - return; - } - - Report.getReportPrivateNote(report.reportID); - // eslint-disable-next-line react-hooks/exhaustive-deps -- do not add report.isLoadingPrivateNotes to dependencies - }, [report.reportID, network.isOffline, isPrivateNotesFetchTriggered, isReconnecting]); - - const shouldShowFullScreenLoadingIndicator = !isPrivateNotesFetchTriggered || (isPrivateNotesEmpty && (report.isLoadingPrivateNotes || !isOtherUserNote)); - + return (WrappedComponent) => { // eslint-disable-next-line rulesdir/no-negated-variables - const shouldShowNotFoundPage = useMemo(() => { - // Show not found view if the report is archived, or if the note is not of current user. - if (ReportUtils.isArchivedRoom(report) || (accountID && Number(session.accountID) !== Number(accountID))) { - return true; + function WithReportAndPrivateNotesOrNotFound({forwardedRef, ...props}) { + const {translate} = useLocalize(); + const {route, report, network, session} = props; + const accountID = route.params.accountID; + const isPrivateNotesFetchTriggered = !_.isUndefined(report.isLoadingPrivateNotes); + const prevIsOffline = usePrevious(network.isOffline); + const isReconnecting = prevIsOffline && !network.isOffline; + const isOtherUserNote = accountID && Number(session.accountID) !== Number(accountID); + const isPrivateNotesEmpty = accountID ? _.has(lodashGet(report, ['privateNotes', accountID, 'note'], '')) : _.isEmpty(report.privateNotes); + + useEffect(() => { + // Do not fetch private notes if isLoadingPrivateNotes is already defined, or if network is offline. + if ((isPrivateNotesFetchTriggered && !isReconnecting) || network.isOffline) { + return; + } + + Report.getReportPrivateNote(report.reportID); + // eslint-disable-next-line react-hooks/exhaustive-deps -- do not add report.isLoadingPrivateNotes to dependencies + }, [report.reportID, network.isOffline, isPrivateNotesFetchTriggered, isReconnecting]); + + const shouldShowFullScreenLoadingIndicator = !isPrivateNotesFetchTriggered || (isPrivateNotesEmpty && (report.isLoadingPrivateNotes || !isOtherUserNote)); + + // eslint-disable-next-line rulesdir/no-negated-variables + const shouldShowNotFoundPage = useMemo(() => { + // Show not found view if the report is archived, or if the note is not of current user. + if (ReportUtils.isArchivedRoom(report) || (accountID && Number(session.accountID) !== Number(accountID))) { + return true; + } + + // Don't show not found view if the notes are still loading, or if the notes are non-empty. + if (shouldShowFullScreenLoadingIndicator || !isPrivateNotesEmpty || isReconnecting) { + return false; + } + + // As notes being empty and not loading is a valid case, show not found view only in offline mode. + return network.isOffline; + }, [report, network.isOffline, accountID, session.accountID, isPrivateNotesEmpty, shouldShowFullScreenLoadingIndicator, isReconnecting]); + + if (shouldShowFullScreenLoadingIndicator) { + return ; } - // Don't show not found view if the notes are still loading, or if the notes are non-empty. - if (shouldShowFullScreenLoadingIndicator || !isPrivateNotesEmpty || isReconnecting) { - return false; + if (shouldShowNotFoundPage) { + return ; } - // As notes being empty and not loading is a valid case, show not found view only in offline mode. - return network.isOffline; - }, [report, network.isOffline, accountID, session.accountID, isPrivateNotesEmpty, shouldShowFullScreenLoadingIndicator, isReconnecting]); - - if (shouldShowFullScreenLoadingIndicator) { - return ; + return ( + + ); } - if (shouldShowNotFoundPage) { - return ; - } + WithReportAndPrivateNotesOrNotFound.propTypes = propTypes; + WithReportAndPrivateNotesOrNotFound.defaultProps = defaultProps; + WithReportAndPrivateNotesOrNotFound.displayName = `withReportAndPrivateNotesOrNotFound(${getComponentDisplayName(WrappedComponent)})`; - return ( - ( + - ); - } - - WithReportAndPrivateNotesOrNotFound.propTypes = propTypes; - WithReportAndPrivateNotesOrNotFound.defaultProps = defaultProps; - WithReportAndPrivateNotesOrNotFound.displayName = `withReportAndPrivateNotesOrNotFound(${getComponentDisplayName(WrappedComponent)})`; - - // eslint-disable-next-line rulesdir/no-negated-variables - const WithReportAndPrivateNotesOrNotFoundWithRef = React.forwardRef((props, ref) => ( - - )); - - WithReportAndPrivateNotesOrNotFoundWithRef.displayName = 'WithReportAndPrivateNotesOrNotFoundWithRef'; - - return compose( - withReportOrNotFound(), - withOnyx({ - session: { - key: ONYXKEYS.SESSION, - }, - }), - withNetwork(), - )(WithReportAndPrivateNotesOrNotFoundWithRef); + )); + + WithReportAndPrivateNotesOrNotFoundWithRef.displayName = 'WithReportAndPrivateNotesOrNotFoundWithRef'; + + return compose( + withReportOrNotFound(), + withOnyx({ + session: { + key: ONYXKEYS.SESSION, + }, + }), + withNetwork(), + )(WithReportAndPrivateNotesOrNotFoundWithRef); + }; } From 8036a5a747019fceed997e637d2dc64ebb8b6849 Mon Sep 17 00:00:00 2001 From: Hans Date: Thu, 14 Dec 2023 22:59:10 +0700 Subject: [PATCH 5/5] Update LoadingPage.js Co-authored-by: Joel Davies --- src/pages/LoadingPage.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/LoadingPage.js b/src/pages/LoadingPage.js index d65e3329703d..fc315495619a 100644 --- a/src/pages/LoadingPage.js +++ b/src/pages/LoadingPage.js @@ -15,7 +15,6 @@ const defaultProps = { onBackButtonPress: undefined, }; -// eslint-disable-next-line rulesdir/no-negated-variables function LoadingPage(props) { const styles = useThemeStyles(); return ( @@ -30,7 +29,7 @@ function LoadingPage(props) { ); } -LoadingPage.displayName = 'NotFoundPage'; +LoadingPage.displayName = 'LoadingPage'; LoadingPage.propTypes = propTypes; LoadingPage.defaultProps = defaultProps;