From 4a8a55554a4f8cf49b9d3c0ecebc95a6f8e17bb9 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Fri, 20 Dec 2024 23:24:21 +0700 Subject: [PATCH 1/4] fix: onboarding modal is not shown when redirecting from OD --- src/libs/actions/Welcome/OnboardingFlow.ts | 14 +++++++++----- src/pages/home/ReportScreen.tsx | 6 ------ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/libs/actions/Welcome/OnboardingFlow.ts b/src/libs/actions/Welcome/OnboardingFlow.ts index 9aa0f07dc59c..e8c8c3d1d42f 100644 --- a/src/libs/actions/Welcome/OnboardingFlow.ts +++ b/src/libs/actions/Welcome/OnboardingFlow.ts @@ -1,5 +1,6 @@ import {findFocusedRoute, getStateFromPath} from '@react-navigation/native'; import type {NavigationState, PartialState} from '@react-navigation/native'; +import {InteractionManager} from 'react-native'; import Onyx from 'react-native-onyx'; import linkingConfig from '@libs/Navigation/linkingConfig'; import getAdaptedStateFromPath from '@libs/Navigation/linkingConfig/getAdaptedStateFromPath'; @@ -44,11 +45,14 @@ function startOnboardingFlow(isPrivateDomain?: boolean) { if (focusedRoute?.name === currentRoute?.name) { return; } - navigationRef.resetRoot({ - ...navigationRef.getRootState(), - ...adaptedState, - stale: true, - } as PartialState); + // This should delay opening the onboarding modal so it does not interfere with the ongoing ReportScreen params changes + InteractionManager.runAfterInteractions(() => { + navigationRef.resetRoot({ + ...navigationRef.getRootState(), + ...adaptedState, + stale: true, + } as PartialState); + }); } function getOnboardingInitialPath(isPrivateDomain?: boolean): string { diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index e9771189bed2..d66a17bbe9b8 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -112,7 +112,6 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro const {isOffline} = useNetwork(); const {shouldUseNarrowLayout, isInNarrowPaneModal} = useResponsiveLayout(); const {activeWorkspaceID} = useActiveWorkspace(); - const lastAccessedReportIDRef = useRef(false); const [modal] = useOnyx(ONYXKEYS.MODAL); const [isComposerFullSize] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_IS_COMPOSER_FULL_SIZE}${reportIDFromRoute}`, {initialValue: false}); @@ -152,10 +151,6 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro return; } - if (lastAccessedReportIDRef.current) { - return; - } - const lastAccessedReportID = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, !!route.params.openOnAdminRoom, activeWorkspaceID)?.reportID; // It's possible that reports aren't fully loaded yet @@ -165,7 +160,6 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro } Log.info(`[ReportScreen] no reportID found in params, setting it to lastAccessedReportID: ${lastAccessedReportID}`); - lastAccessedReportIDRef.current = true; navigation.setParams({reportID: lastAccessedReportID}); }, [activeWorkspaceID, canUseDefaultRooms, navigation, route, finishedLoadingApp]); From fdddac404c8b99b55e0ce2e09d4c37f37a683991 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Fri, 20 Dec 2024 23:44:52 +0700 Subject: [PATCH 2/4] update implementation --- src/hooks/useOnboardingFlow.ts | 10 +++++++--- src/libs/actions/Welcome/OnboardingFlow.ts | 14 +++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/hooks/useOnboardingFlow.ts b/src/hooks/useOnboardingFlow.ts index 81796dae851d..048c7b72508f 100644 --- a/src/hooks/useOnboardingFlow.ts +++ b/src/hooks/useOnboardingFlow.ts @@ -1,5 +1,5 @@ import {useEffect} from 'react'; -import {NativeModules} from 'react-native'; +import {InteractionManager, NativeModules} from 'react-native'; import {useOnyx} from 'react-native-onyx'; import Navigation from '@libs/Navigation/Navigation'; import {hasCompletedGuidedSetupFlowSelector, tryNewDotOnyxSelector} from '@libs/onboardingSelectors'; @@ -62,13 +62,17 @@ function useOnboardingFlowRouter() { // But if the hybrid app onboarding is completed, but NewDot onboarding is not completed, we start NewDot onboarding flow // This is a special case when user created an account from NewDot without finishing the onboarding flow and then logged in from OldDot if (isHybridAppOnboardingCompleted === true && isOnboardingCompleted === false) { - OnboardingFlow.startOnboardingFlow(isPrivateDomain); + InteractionManager.runAfterInteractions(() => { + OnboardingFlow.startOnboardingFlow(isPrivateDomain); + }); } } // If the user is not transitioning from OldDot to NewDot, we should start NewDot onboarding flow if it's not completed yet if (!NativeModules.HybridAppModule && isOnboardingCompleted === false) { - OnboardingFlow.startOnboardingFlow(isPrivateDomain); + InteractionManager.runAfterInteractions(() => { + OnboardingFlow.startOnboardingFlow(isPrivateDomain); + }); } }, [ isOnboardingCompleted, diff --git a/src/libs/actions/Welcome/OnboardingFlow.ts b/src/libs/actions/Welcome/OnboardingFlow.ts index e8c8c3d1d42f..9aa0f07dc59c 100644 --- a/src/libs/actions/Welcome/OnboardingFlow.ts +++ b/src/libs/actions/Welcome/OnboardingFlow.ts @@ -1,6 +1,5 @@ import {findFocusedRoute, getStateFromPath} from '@react-navigation/native'; import type {NavigationState, PartialState} from '@react-navigation/native'; -import {InteractionManager} from 'react-native'; import Onyx from 'react-native-onyx'; import linkingConfig from '@libs/Navigation/linkingConfig'; import getAdaptedStateFromPath from '@libs/Navigation/linkingConfig/getAdaptedStateFromPath'; @@ -45,14 +44,11 @@ function startOnboardingFlow(isPrivateDomain?: boolean) { if (focusedRoute?.name === currentRoute?.name) { return; } - // This should delay opening the onboarding modal so it does not interfere with the ongoing ReportScreen params changes - InteractionManager.runAfterInteractions(() => { - navigationRef.resetRoot({ - ...navigationRef.getRootState(), - ...adaptedState, - stale: true, - } as PartialState); - }); + navigationRef.resetRoot({ + ...navigationRef.getRootState(), + ...adaptedState, + stale: true, + } as PartialState); } function getOnboardingInitialPath(isPrivateDomain?: boolean): string { From 3a590f2c74043cdd0f7a50d22989f51187eb20ff Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Fri, 20 Dec 2024 23:46:36 +0700 Subject: [PATCH 3/4] add a comment --- src/hooks/useOnboardingFlow.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hooks/useOnboardingFlow.ts b/src/hooks/useOnboardingFlow.ts index 048c7b72508f..b6753ce5b408 100644 --- a/src/hooks/useOnboardingFlow.ts +++ b/src/hooks/useOnboardingFlow.ts @@ -62,6 +62,7 @@ function useOnboardingFlowRouter() { // But if the hybrid app onboarding is completed, but NewDot onboarding is not completed, we start NewDot onboarding flow // This is a special case when user created an account from NewDot without finishing the onboarding flow and then logged in from OldDot if (isHybridAppOnboardingCompleted === true && isOnboardingCompleted === false) { + // This should delay opening the onboarding modal so it does not interfere with the ongoing ReportScreen params changes InteractionManager.runAfterInteractions(() => { OnboardingFlow.startOnboardingFlow(isPrivateDomain); }); @@ -70,6 +71,7 @@ function useOnboardingFlowRouter() { // If the user is not transitioning from OldDot to NewDot, we should start NewDot onboarding flow if it's not completed yet if (!NativeModules.HybridAppModule && isOnboardingCompleted === false) { + // This should delay opening the onboarding modal so it does not interfere with the ongoing ReportScreen params changes InteractionManager.runAfterInteractions(() => { OnboardingFlow.startOnboardingFlow(isPrivateDomain); }); From 830b02d0165c06c6a6b8761c5fa3235f122d8976 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Fri, 20 Dec 2024 23:49:31 +0700 Subject: [PATCH 4/4] wrap whole useEffect callback --- src/hooks/useOnboardingFlow.ts | 65 ++++++++++++++++------------------ 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/src/hooks/useOnboardingFlow.ts b/src/hooks/useOnboardingFlow.ts index b6753ce5b408..d322a4d52703 100644 --- a/src/hooks/useOnboardingFlow.ts +++ b/src/hooks/useOnboardingFlow.ts @@ -32,50 +32,47 @@ function useOnboardingFlowRouter() { const [isSingleNewDotEntry, isSingleNewDotEntryMetadata] = useOnyx(ONYXKEYS.IS_SINGLE_NEW_DOT_ENTRY); const [allBetas, allBetasMetadata] = useOnyx(ONYXKEYS.BETAS); useEffect(() => { - if (isLoadingOnyxValue(isOnboardingCompletedMetadata, tryNewDotdMetadata, dismissedProductTrainingMetadata, allBetasMetadata)) { - return; - } - - if (NativeModules.HybridAppModule && isLoadingOnyxValue(isSingleNewDotEntryMetadata)) { - return; - } - - if (hasBeenAddedToNudgeMigration && !dismissedProductTraining?.migratedUserWelcomeModal && Permissions.shouldShowProductTrainingElements(allBetas)) { - const defaultCannedQuery = SearchQueryUtils.buildCannedSearchQuery(); - const query = defaultCannedQuery; - Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query})); - Navigation.navigate(ROUTES.MIGRATED_USER_WELCOME_MODAL); - return; - } + // This should delay opening the onboarding modal so it does not interfere with the ongoing ReportScreen params changes + InteractionManager.runAfterInteractions(() => { + if (isLoadingOnyxValue(isOnboardingCompletedMetadata, tryNewDotdMetadata, dismissedProductTrainingMetadata, allBetasMetadata)) { + return; + } - if (NativeModules.HybridAppModule) { - // For single entries, such as using the Travel feature from OldDot, we don't want to show onboarding - if (isSingleNewDotEntry) { + if (NativeModules.HybridAppModule && isLoadingOnyxValue(isSingleNewDotEntryMetadata)) { return; } - // When user is transitioning from OldDot to NewDot, we usually show the explanation modal - if (isHybridAppOnboardingCompleted === false) { - Navigation.navigate(ROUTES.EXPLANATION_MODAL_ROOT); + if (hasBeenAddedToNudgeMigration && !dismissedProductTraining?.migratedUserWelcomeModal && Permissions.shouldShowProductTrainingElements(allBetas)) { + const defaultCannedQuery = SearchQueryUtils.buildCannedSearchQuery(); + const query = defaultCannedQuery; + Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query})); + Navigation.navigate(ROUTES.MIGRATED_USER_WELCOME_MODAL); + return; } - // But if the hybrid app onboarding is completed, but NewDot onboarding is not completed, we start NewDot onboarding flow - // This is a special case when user created an account from NewDot without finishing the onboarding flow and then logged in from OldDot - if (isHybridAppOnboardingCompleted === true && isOnboardingCompleted === false) { - // This should delay opening the onboarding modal so it does not interfere with the ongoing ReportScreen params changes - InteractionManager.runAfterInteractions(() => { + if (NativeModules.HybridAppModule) { + // For single entries, such as using the Travel feature from OldDot, we don't want to show onboarding + if (isSingleNewDotEntry) { + return; + } + + // When user is transitioning from OldDot to NewDot, we usually show the explanation modal + if (isHybridAppOnboardingCompleted === false) { + Navigation.navigate(ROUTES.EXPLANATION_MODAL_ROOT); + } + + // But if the hybrid app onboarding is completed, but NewDot onboarding is not completed, we start NewDot onboarding flow + // This is a special case when user created an account from NewDot without finishing the onboarding flow and then logged in from OldDot + if (isHybridAppOnboardingCompleted === true && isOnboardingCompleted === false) { OnboardingFlow.startOnboardingFlow(isPrivateDomain); - }); + } } - } - // If the user is not transitioning from OldDot to NewDot, we should start NewDot onboarding flow if it's not completed yet - if (!NativeModules.HybridAppModule && isOnboardingCompleted === false) { - // This should delay opening the onboarding modal so it does not interfere with the ongoing ReportScreen params changes - InteractionManager.runAfterInteractions(() => { + // If the user is not transitioning from OldDot to NewDot, we should start NewDot onboarding flow if it's not completed yet + if (!NativeModules.HybridAppModule && isOnboardingCompleted === false) { OnboardingFlow.startOnboardingFlow(isPrivateDomain); - }); - } + } + }); }, [ isOnboardingCompleted, isHybridAppOnboardingCompleted,