From 5af5792141af82ca7c2355429216a66341779b51 Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Fri, 25 Aug 2023 15:01:17 +0100 Subject: [PATCH 01/11] initial fix --- src/components/DownloadAppModal.js | 18 ++++++++++++++++-- src/libs/actions/Welcome.js | 11 ++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/components/DownloadAppModal.js b/src/components/DownloadAppModal.js index 1eeab1c72fd3..5542ceb92066 100644 --- a/src/components/DownloadAppModal.js +++ b/src/components/DownloadAppModal.js @@ -16,14 +16,25 @@ const propTypes = { /** ONYX PROP to hide banner for a user that has dismissed it */ // eslint-disable-next-line react/forbid-prop-types showDownloadAppBanner: PropTypes.bool, + + /** Session of currently logged in user */ + session: PropTypes.shape({ + /** Currently logged in user authToken */ + authToken: PropTypes.string, + }), }; const defaultProps = { showDownloadAppBanner: true, + session: { + authToken: null, + }, }; -function DownloadAppModal({showDownloadAppBanner}) { - const [shouldShowBanner, setshouldShowBanner] = useState(Browser.isMobile() && showDownloadAppBanner); +function DownloadAppModal({session, showDownloadAppBanner}) { + const userLoggedIn = !_.isEmpty(session.authToken); + console.log({userLoggedIn}) + const [shouldShowBanner, setshouldShowBanner] = useState(Browser.isMobile() && userLoggedIn && showDownloadAppBanner); const {translate} = useLocalize(); @@ -71,4 +82,7 @@ export default withOnyx({ showDownloadAppBanner: { key: ONYXKEYS.SHOW_DOWNLOAD_APP_BANNER, }, + session: { + key: ONYXKEYS.SESSION, + }, })(DownloadAppModal); diff --git a/src/libs/actions/Welcome.js b/src/libs/actions/Welcome.js index 6e6fe2512dff..2ce61bf03fed 100644 --- a/src/libs/actions/Welcome.js +++ b/src/libs/actions/Welcome.js @@ -15,6 +15,7 @@ let isReadyPromise = new Promise((resolve) => { }); let isFirstTimeNewExpensifyUser; +let shouldShowDownloadAppBanner; let isLoadingReportData = true; let currentUserAccountID; @@ -33,6 +34,14 @@ function checkOnReady() { resolveIsReadyPromise(); } +Onyx.connect({ + key: ONYXKEYS.SHOW_DOWNLOAD_APP_BANNER, + initWithStoredValues: false, + callback: (val) => { + shouldShowDownloadAppBanner = val; + }, +}); + Onyx.connect({ key: ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER, initWithStoredValues: false, @@ -106,7 +115,7 @@ Onyx.connect({ */ function show({routes, showCreateMenu = () => {}, showPopoverMenu = () => {}}) { isReadyPromise.then(() => { - if (!isFirstTimeNewExpensifyUser) { + if (!isFirstTimeNewExpensifyUser || shouldShowDownloadAppBanner) { return; } From 3b1362dde0cae2898933b7bdeb3f7bf1795f274e Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Mon, 28 Aug 2023 15:09:05 +0100 Subject: [PATCH 02/11] fix` --- src/Expensify.js | 2 +- src/components/DownloadAppModal.js | 5 +++-- src/libs/actions/Welcome.js | 13 +++---------- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/Expensify.js b/src/Expensify.js index ede42c2873dd..8b3ca7780505 100644 --- a/src/Expensify.js +++ b/src/Expensify.js @@ -196,10 +196,10 @@ function Expensify(props) { {shouldInit && ( <> + - {/* We include the modal for showing a new update at the top level so the option is always present. */} {props.updateAvailable ? : null} diff --git a/src/components/DownloadAppModal.js b/src/components/DownloadAppModal.js index 5542ceb92066..a0e220839891 100644 --- a/src/components/DownloadAppModal.js +++ b/src/components/DownloadAppModal.js @@ -1,6 +1,7 @@ import React, {useState} from 'react'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; +import _ from 'underscore'; import ONYXKEYS from '../ONYXKEYS'; import styles from '../styles/styles'; import CONST from '../CONST'; @@ -33,8 +34,8 @@ const defaultProps = { function DownloadAppModal({session, showDownloadAppBanner}) { const userLoggedIn = !_.isEmpty(session.authToken); - console.log({userLoggedIn}) - const [shouldShowBanner, setshouldShowBanner] = useState(Browser.isMobile() && userLoggedIn && showDownloadAppBanner); + console.log({userLoggedIn}); + const [shouldShowBanner, setshouldShowBanner] = useState((Browser.isMobile() || true) && userLoggedIn && showDownloadAppBanner); const {translate} = useLocalize(); diff --git a/src/libs/actions/Welcome.js b/src/libs/actions/Welcome.js index 2ce61bf03fed..20ff6b40ada6 100644 --- a/src/libs/actions/Welcome.js +++ b/src/libs/actions/Welcome.js @@ -15,7 +15,6 @@ let isReadyPromise = new Promise((resolve) => { }); let isFirstTimeNewExpensifyUser; -let shouldShowDownloadAppBanner; let isLoadingReportData = true; let currentUserAccountID; @@ -34,14 +33,6 @@ function checkOnReady() { resolveIsReadyPromise(); } -Onyx.connect({ - key: ONYXKEYS.SHOW_DOWNLOAD_APP_BANNER, - initWithStoredValues: false, - callback: (val) => { - shouldShowDownloadAppBanner = val; - }, -}); - Onyx.connect({ key: ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER, initWithStoredValues: false, @@ -115,7 +106,7 @@ Onyx.connect({ */ function show({routes, showCreateMenu = () => {}, showPopoverMenu = () => {}}) { isReadyPromise.then(() => { - if (!isFirstTimeNewExpensifyUser || shouldShowDownloadAppBanner) { + if (!isFirstTimeNewExpensifyUser) { return; } @@ -145,6 +136,8 @@ function show({routes, showCreateMenu = () => {}, showPopoverMenu = () => {}}) { if (shouldNavigateToWorkspaceChat && workspaceChatReport) { Navigation.navigate(ROUTES.getReportRoute(workspaceChatReport.reportID)); + + // If showPopoverMenu exists and returns true then it opened the Popover Menu successfully, and we can update isFirstTimeNewExpensifyUser // so the Welcome logic doesn't run again if (showPopoverMenu()) { From ac3dd949c31ab6dc88ae2d492964ea3e2c75fa38 Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Mon, 28 Aug 2023 15:17:03 +0100 Subject: [PATCH 03/11] remove spaces --- src/libs/actions/Welcome.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libs/actions/Welcome.js b/src/libs/actions/Welcome.js index 20ff6b40ada6..6e6fe2512dff 100644 --- a/src/libs/actions/Welcome.js +++ b/src/libs/actions/Welcome.js @@ -136,8 +136,6 @@ function show({routes, showCreateMenu = () => {}, showPopoverMenu = () => {}}) { if (shouldNavigateToWorkspaceChat && workspaceChatReport) { Navigation.navigate(ROUTES.getReportRoute(workspaceChatReport.reportID)); - - // If showPopoverMenu exists and returns true then it opened the Popover Menu successfully, and we can update isFirstTimeNewExpensifyUser // so the Welcome logic doesn't run again if (showPopoverMenu()) { From 84edf34ce3176bbcbb8ed946ddcb7a3989ddb182 Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Tue, 29 Aug 2023 12:32:39 +0100 Subject: [PATCH 04/11] Show download app modal BEFORE welcome message --- .../FloatingActionButtonAndPopover.js | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js index abade067f4fc..3fa13a5bafbd 100644 --- a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js +++ b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js @@ -61,6 +61,9 @@ const propTypes = { /** Indicated whether the report data is loading */ isLoading: PropTypes.bool, + /** For first time users, whether the download app banner should show */ + showDownloadAppBanner: PropTypes.bool, + /** Forwarded ref to FloatingActionButtonAndPopover */ innerRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]), }; @@ -71,6 +74,7 @@ const defaultProps = { betas: [], isLoading: false, innerRef: null, + showDownloadAppBanner: true, }; /** @@ -86,6 +90,8 @@ function FloatingActionButtonAndPopover(props) { const prevIsFocused = usePrevious(props.isFocused); + console.log({showDownloadAppBanner: props.showDownloadAppBanner}); + /** * Check if LHN status changed from active to inactive. * Used to close already opened FAB menu when open any other pages (i.e. Press Command + K on web). @@ -146,19 +152,17 @@ function FloatingActionButtonAndPopover(props) { } }; - useEffect( - () => { - const navigationState = props.navigation.getState(); - const routes = lodashGet(navigationState, 'routes', []); - const currentRoute = routes[navigationState.index]; - if (currentRoute && ![NAVIGATORS.CENTRAL_PANE_NAVIGATOR, SCREENS.HOME].includes(currentRoute.name)) { - return; - } + useEffect(() => { + const navigationState = props.navigation.getState(); + const routes = lodashGet(navigationState, 'routes', []); + const currentRoute = routes[navigationState.index]; + if (currentRoute && ![NAVIGATORS.CENTRAL_PANE_NAVIGATOR, SCREENS.HOME].includes(currentRoute.name)) { + return; + } + if (!props.showDownloadAppBanner) { Welcome.show({routes, showCreateMenu}); - }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [], - ); + } + }); useEffect(() => { if (!didScreenBecomeInactive()) { @@ -286,6 +290,9 @@ export default compose( isLoading: { key: ONYXKEYS.IS_LOADING_REPORT_DATA, }, + showDownloadAppBanner: { + key: ONYXKEYS.SHOW_DOWNLOAD_APP_BANNER, + }, }), )( forwardRef((props, ref) => ( From c39de0eb327744ecc603e797e99dee9aca0f156f Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Tue, 29 Aug 2023 12:34:18 +0100 Subject: [PATCH 05/11] remove debugging statements --- src/components/DownloadAppModal.js | 3 +-- .../sidebar/SidebarScreen/FloatingActionButtonAndPopover.js | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/components/DownloadAppModal.js b/src/components/DownloadAppModal.js index a0e220839891..0c887ad315f8 100644 --- a/src/components/DownloadAppModal.js +++ b/src/components/DownloadAppModal.js @@ -34,8 +34,7 @@ const defaultProps = { function DownloadAppModal({session, showDownloadAppBanner}) { const userLoggedIn = !_.isEmpty(session.authToken); - console.log({userLoggedIn}); - const [shouldShowBanner, setshouldShowBanner] = useState((Browser.isMobile() || true) && userLoggedIn && showDownloadAppBanner); + const [shouldShowBanner, setshouldShowBanner] = useState(Browser.isMobile() && userLoggedIn && showDownloadAppBanner); const {translate} = useLocalize(); diff --git a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js index 3fa13a5bafbd..e951ebf1b516 100644 --- a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js +++ b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js @@ -90,8 +90,6 @@ function FloatingActionButtonAndPopover(props) { const prevIsFocused = usePrevious(props.isFocused); - console.log({showDownloadAppBanner: props.showDownloadAppBanner}); - /** * Check if LHN status changed from active to inactive. * Used to close already opened FAB menu when open any other pages (i.e. Press Command + K on web). From 1a047323d1cb43852851a69d2a92aa6189f53345 Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Tue, 29 Aug 2023 14:14:59 +0100 Subject: [PATCH 06/11] ignore this for non mWeb cases --- .../sidebar/SidebarScreen/FloatingActionButtonAndPopover.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js index e951ebf1b516..6c36c4d1af42 100644 --- a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js +++ b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js @@ -5,6 +5,7 @@ import lodashGet from 'lodash/get'; import {View} from 'react-native'; import styles from '../../../../styles/styles'; import * as Expensicons from '../../../../components/Icon/Expensicons'; +import * as Browser from '../../../../libs/Browser'; import Navigation from '../../../../libs/Navigation/Navigation'; import ROUTES from '../../../../ROUTES'; import NAVIGATORS from '../../../../NAVIGATORS'; @@ -157,7 +158,7 @@ function FloatingActionButtonAndPopover(props) { if (currentRoute && ![NAVIGATORS.CENTRAL_PANE_NAVIGATOR, SCREENS.HOME].includes(currentRoute.name)) { return; } - if (!props.showDownloadAppBanner) { + if (!props.showDownloadAppBanner || !Browser.isMobile()) { Welcome.show({routes, showCreateMenu}); } }); From b3be15a6cec3b2a5ff9f783915605aa01a38f03f Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Tue, 29 Aug 2023 15:08:51 +0100 Subject: [PATCH 07/11] pass down isAuthenticated --- src/Expensify.js | 2 +- src/components/DownloadAppModal.js | 18 ++++-------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/Expensify.js b/src/Expensify.js index 8b3ca7780505..60ab9f756dff 100644 --- a/src/Expensify.js +++ b/src/Expensify.js @@ -196,7 +196,7 @@ function Expensify(props) { {shouldInit && ( <> - + diff --git a/src/components/DownloadAppModal.js b/src/components/DownloadAppModal.js index 0c887ad315f8..687cb872988d 100644 --- a/src/components/DownloadAppModal.js +++ b/src/components/DownloadAppModal.js @@ -18,23 +18,16 @@ const propTypes = { // eslint-disable-next-line react/forbid-prop-types showDownloadAppBanner: PropTypes.bool, - /** Session of currently logged in user */ - session: PropTypes.shape({ - /** Currently logged in user authToken */ - authToken: PropTypes.string, - }), + /** Whether the user is logged in */ + isAuthenticated: PropTypes.bool, }; const defaultProps = { showDownloadAppBanner: true, - session: { - authToken: null, - }, }; -function DownloadAppModal({session, showDownloadAppBanner}) { - const userLoggedIn = !_.isEmpty(session.authToken); - const [shouldShowBanner, setshouldShowBanner] = useState(Browser.isMobile() && userLoggedIn && showDownloadAppBanner); +function DownloadAppModal({isAuthenticated, showDownloadAppBanner}) { + const [shouldShowBanner, setshouldShowBanner] = useState(Browser.isMobile() && isAuthenticated && showDownloadAppBanner); const {translate} = useLocalize(); @@ -82,7 +75,4 @@ export default withOnyx({ showDownloadAppBanner: { key: ONYXKEYS.SHOW_DOWNLOAD_APP_BANNER, }, - session: { - key: ONYXKEYS.SESSION, - }, })(DownloadAppModal); From 2351c4102bef97798d175a07aab6014f57e47982 Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Tue, 29 Aug 2023 15:17:28 +0100 Subject: [PATCH 08/11] Fix lint --- src/components/DownloadAppModal.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/DownloadAppModal.js b/src/components/DownloadAppModal.js index 687cb872988d..ffa933708e4c 100644 --- a/src/components/DownloadAppModal.js +++ b/src/components/DownloadAppModal.js @@ -1,7 +1,6 @@ import React, {useState} from 'react'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; -import _ from 'underscore'; import ONYXKEYS from '../ONYXKEYS'; import styles from '../styles/styles'; import CONST from '../CONST'; @@ -19,7 +18,7 @@ const propTypes = { showDownloadAppBanner: PropTypes.bool, /** Whether the user is logged in */ - isAuthenticated: PropTypes.bool, + isAuthenticated: PropTypes.bool.isRequired, }; const defaultProps = { From 460b2e6d83cab77672fa1c7268349e88b52dc8a3 Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Tue, 29 Aug 2023 15:32:33 +0100 Subject: [PATCH 09/11] add dependency to useEffect --- .../sidebar/SidebarScreen/FloatingActionButtonAndPopover.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js index 6c36c4d1af42..da620e94ae27 100644 --- a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js +++ b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js @@ -161,7 +161,7 @@ function FloatingActionButtonAndPopover(props) { if (!props.showDownloadAppBanner || !Browser.isMobile()) { Welcome.show({routes, showCreateMenu}); } - }); + }, [props.showDownloadAppBanner]); useEffect(() => { if (!didScreenBecomeInactive()) { From 5df5de8e697ecc623ac0488ace571592bd1a92af Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Tue, 29 Aug 2023 16:26:37 +0100 Subject: [PATCH 10/11] lint --- .../sidebar/SidebarScreen/FloatingActionButtonAndPopover.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js index da620e94ae27..970725f8dd49 100644 --- a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js +++ b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js @@ -161,7 +161,7 @@ function FloatingActionButtonAndPopover(props) { if (!props.showDownloadAppBanner || !Browser.isMobile()) { Welcome.show({routes, showCreateMenu}); } - }, [props.showDownloadAppBanner]); + }, [props.showDownloadAppBanner, props.navigation, showCreateMenu]); useEffect(() => { if (!didScreenBecomeInactive()) { From c0e1409a7c65e2ec8319ad636078de6e44c1cca4 Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Tue, 29 Aug 2023 18:46:13 +0100 Subject: [PATCH 11/11] early return, rename to shouldShowDownloadAppBanner --- .../FloatingActionButtonAndPopover.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js index 970725f8dd49..ec4f33997d2d 100644 --- a/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js +++ b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js @@ -63,7 +63,7 @@ const propTypes = { isLoading: PropTypes.bool, /** For first time users, whether the download app banner should show */ - showDownloadAppBanner: PropTypes.bool, + shouldShowDownloadAppBanner: PropTypes.bool, /** Forwarded ref to FloatingActionButtonAndPopover */ innerRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]), @@ -75,7 +75,7 @@ const defaultProps = { betas: [], isLoading: false, innerRef: null, - showDownloadAppBanner: true, + shouldShowDownloadAppBanner: true, }; /** @@ -158,10 +158,12 @@ function FloatingActionButtonAndPopover(props) { if (currentRoute && ![NAVIGATORS.CENTRAL_PANE_NAVIGATOR, SCREENS.HOME].includes(currentRoute.name)) { return; } - if (!props.showDownloadAppBanner || !Browser.isMobile()) { - Welcome.show({routes, showCreateMenu}); + // Avoid rendering the create menu for first-time users until they have dismissed the download app banner (mWeb only). + if (props.shouldShowDownloadAppBanner || Browser.isMobile()) { + return; } - }, [props.showDownloadAppBanner, props.navigation, showCreateMenu]); + Welcome.show({routes, showCreateMenu}); + }, [props.shouldShowDownloadAppBanner, props.navigation, showCreateMenu]); useEffect(() => { if (!didScreenBecomeInactive()) { @@ -289,7 +291,7 @@ export default compose( isLoading: { key: ONYXKEYS.IS_LOADING_REPORT_DATA, }, - showDownloadAppBanner: { + shouldShowDownloadAppBanner: { key: ONYXKEYS.SHOW_DOWNLOAD_APP_BANNER, }, }),