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

Make onboarding continue from last visited onboarding page #48137

Merged
merged 6 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 4 additions & 0 deletions src/ONYXKEYS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,9 @@ const ONYXKEYS = {
/** Onboarding Purpose selected by the user during Onboarding flow */
ONBOARDING_ADMINS_CHAT_REPORT_ID: 'onboardingAdminsChatReportID',

// Stores onboarding last visited path
ONBOARDING_LAST_VISITED_PATH: 'onboardingLastVisitedPath',

// Max width supported for HTML <canvas> element
MAX_CANVAS_WIDTH: 'maxCanvasWidth',

Expand Down Expand Up @@ -885,6 +888,7 @@ type OnyxValuesMapping = {
[ONYXKEYS.ONBOARDING_ERROR_MESSAGE]: string;
[ONYXKEYS.ONBOARDING_POLICY_ID]: string;
[ONYXKEYS.ONBOARDING_ADMINS_CHAT_REPORT_ID]: string;
[ONYXKEYS.ONBOARDING_LAST_VISITED_PATH]: string;
[ONYXKEYS.IS_SEARCHING_FOR_REPORTS]: boolean;
[ONYXKEYS.LAST_VISITED_PATH]: string | undefined;
[ONYXKEYS.VERIFY_3DS_SUBSCRIPTION]: string;
Expand Down
4 changes: 2 additions & 2 deletions src/components/ExplanationModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import useLocalize from '@hooks/useLocalize';
import Navigation from '@libs/Navigation/Navigation';
import variables from '@styles/variables';
import * as Welcome from '@userActions/Welcome';
import * as OnboardingFlow from '@userActions/Welcome/OnboardingFlow';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import FeatureTrainingModal from './FeatureTrainingModal';

function ExplanationModal() {
Expand All @@ -18,7 +18,7 @@ function ExplanationModal() {
onNotCompleted: () => {
setTimeout(() => {
Navigation.isNavigationReady().then(() => {
Navigation.navigate(ROUTES.ONBOARDING_ROOT.route);
OnboardingFlow.startOnboardingFlow();
});
}, variables.welcomeVideoDelay);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import * as Session from '@libs/actions/Session';
import interceptAnonymousUser from '@libs/interceptAnonymousUser';
import linkingConfig from '@libs/Navigation/linkingConfig';
import getAdaptedStateFromPath from '@libs/Navigation/linkingConfig/getAdaptedStateFromPath';
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import Navigation from '@libs/Navigation/Navigation';
import type {RootStackParamList, State} from '@libs/Navigation/types';
import {isCentralPaneName} from '@libs/NavigationUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
Expand All @@ -26,6 +24,7 @@ import BottomTabAvatar from '@pages/home/sidebar/BottomTabAvatar';
import BottomTabBarFloatingActionButton from '@pages/home/sidebar/BottomTabBarFloatingActionButton';
import variables from '@styles/variables';
import * as Welcome from '@userActions/Welcome';
import * as OnboardingFlow from '@userActions/Welcome/OnboardingFlow';
import CONST from '@src/CONST';
import NAVIGATORS from '@src/NAVIGATORS';
import ONYXKEYS from '@src/ONYXKEYS';
Expand Down Expand Up @@ -95,10 +94,7 @@ function BottomTabBar({selectedTab}: BottomTabBarProps) {
}

Welcome.isOnboardingFlowCompleted({
onNotCompleted: () => {
const {adaptedState} = getAdaptedStateFromPath(ROUTES.ONBOARDING_ROOT.route, linkingConfig.config);
navigationRef.resetRoot(adaptedState);
},
onNotCompleted: () => OnboardingFlow.startOnboardingFlow(),
});

// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
Expand Down
7 changes: 6 additions & 1 deletion src/libs/Navigation/NavigationRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import hasCompletedGuidedSetupFlowSelector from '@libs/hasCompletedGuidedSetupFl
import Log from '@libs/Log';
import {getPathFromURL} from '@libs/Url';
import {updateLastVisitedPath} from '@userActions/App';
import {updateOnboardingLastVisitedPath} from '@userActions/Welcome';
import {getOnboardingInitialPath} from '@userActions/Welcome/OnboardingFlow';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Route} from '@src/ROUTES';
Expand Down Expand Up @@ -58,6 +60,9 @@ function parseAndLogRoute(state: NavigationState) {

if (focusedRoute && !CONST.EXCLUDE_FROM_LAST_VISITED_PATH.includes(focusedRoute?.name)) {
updateLastVisitedPath(currentPath);
if (currentPath.startsWith(`/${ROUTES.ONBOARDING_ROOT.route}`)) {
updateOnboardingLastVisitedPath(currentPath);
}
}

// Don't log the route transitions from OldDot because they contain authTokens
Expand Down Expand Up @@ -98,7 +103,7 @@ function NavigationRoot({authenticated, lastVisitedPath, initialUrl, onReady, sh
// If the user haven't completed the flow, we want to always redirect them to the onboarding flow.
// We also make sure that the user is authenticated.
if (!hasCompletedGuidedSetupFlow && authenticated && !shouldShowRequire2FAModal) {
const {adaptedState} = getAdaptedStateFromPath(ROUTES.ONBOARDING_ROOT.route, linkingConfig.config);
const {adaptedState} = getAdaptedStateFromPath(getOnboardingInitialPath(), linkingConfig.config);
return adaptedState;
}

Expand Down
3 changes: 2 additions & 1 deletion src/libs/Navigation/linkingConfig/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ const config: LinkingOptions<RootStackParamList>['config'] = {
},
},
[NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR]: {
// Don't set the initialRouteName, because when the user continues from the last visited onboarding page,
// the onboarding purpose page will be briefly visible.
path: ROUTES.ONBOARDING_ROOT.route,
initialRouteName: SCREENS.ONBOARDING.PURPOSE,
screens: {
[SCREENS.ONBOARDING.PURPOSE]: {
path: ROUTES.ONBOARDING_PURPOSE.route,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type GetAdaptedStateReturnType = {
metainfo: Metainfo;
};

type GetAdaptedStateFromPath = (...args: Parameters<typeof getStateFromPath>) => GetAdaptedStateReturnType;
type GetAdaptedStateFromPath = (...args: [...Parameters<typeof getStateFromPath>, shouldReplacePathInNestedState?: boolean]) => GetAdaptedStateReturnType;

// The function getPathFromState that we are using in some places isn't working correctly without defined index.
const getRoutesWithIndex = (routes: NavigationPartialRoute[]): PartialState<NavigationState> => ({routes, index: routes.length - 1});
Expand Down Expand Up @@ -365,7 +365,7 @@ function getAdaptedState(state: PartialState<NavigationState<RootStackParamList>
};
}

const getAdaptedStateFromPath: GetAdaptedStateFromPath = (path, options) => {
const getAdaptedStateFromPath: GetAdaptedStateFromPath = (path, options, shouldReplacePathInNestedState = true) => {
const normalizedPath = !path.startsWith('/') ? `/${path}` : path;
const pathWithoutPolicyID = getPathWithoutPolicyID(normalizedPath);
const isAnonymous = isAnonymousUser();
Expand All @@ -374,7 +374,9 @@ const getAdaptedStateFromPath: GetAdaptedStateFromPath = (path, options) => {
const policyID = isAnonymous ? undefined : extractPolicyIDFromPath(path);

const state = getStateFromPath(pathWithoutPolicyID, options) as PartialState<NavigationState<RootStackParamList>>;
replacePathInNestedState(state, path);
if (shouldReplacePathInNestedState) {
replacePathInNestedState(state, path);
}
if (state === undefined) {
throw new Error('Unable to parse path');
}
Expand Down
5 changes: 4 additions & 1 deletion src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ import * as Modal from './Modal';
import navigateFromNotification from './navigateFromNotification';
import * as Session from './Session';
import * as Welcome from './Welcome';
import * as OnboardingFlow from './Welcome/OnboardingFlow';

type SubscriberCallback = (isFromCurrentUser: boolean, reportActionID: string | undefined) => void;

Expand Down Expand Up @@ -2701,7 +2702,9 @@ function openReportFromDeepLink(url: string) {

// We need skip deeplinking if the user hasn't completed the guided setup flow.
if (!hasCompletedGuidedSetupFlow) {
Welcome.isOnboardingFlowCompleted({onNotCompleted: () => Navigation.navigate(ROUTES.ONBOARDING_ROOT.getRoute())});
Welcome.isOnboardingFlowCompleted({
onNotCompleted: () => OnboardingFlow.startOnboardingFlow(),
});
return;
}

Expand Down
127 changes: 127 additions & 0 deletions src/libs/actions/Welcome/OnboardingFlow.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import {findFocusedRoute, getStateFromPath} from '@react-navigation/native';
import type {NavigationState, PartialState} from '@react-navigation/native';
import Onyx from 'react-native-onyx';
import linkingConfig from '@libs/Navigation/linkingConfig';
import getAdaptedStateFromPath from '@libs/Navigation/linkingConfig/getAdaptedStateFromPath';
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import type {NavigationPartialRoute, RootStackParamList} from '@libs/Navigation/types';
import CONST from '@src/CONST';
import NAVIGATORS from '@src/NAVIGATORS';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import SCREENS from '@src/SCREENS';

let selectedPurpose: string | undefined = '';
Onyx.connect({
key: ONYXKEYS.ONBOARDING_PURPOSE_SELECTED,
callback: (value) => {
selectedPurpose = value;
},
});

let onboardingInitialPath = '';
const onboardingLastVisitedPathConnection = Onyx.connect({
key: ONYXKEYS.ONBOARDING_LAST_VISITED_PATH,
callback: (value) => {
if (value === undefined) {
return;
}
onboardingInitialPath = value;
Onyx.disconnect(onboardingLastVisitedPathConnection);
},
});

/**
* Build the correct stack order for `onboardingModalNavigator`,
* based on onboarding data (currently from the selected purpose).
* The correct stack order will ensure that navigation and
* the `goBack` navigatoin work properly.
*/
function adaptOnboardingRouteState() {
const currentRoute: NavigationPartialRoute | undefined = navigationRef.getCurrentRoute();
if (!currentRoute || currentRoute?.name === SCREENS.ONBOARDING.PURPOSE) {
return;
}

const rootState = navigationRef.getRootState();
const adaptedState = rootState;
const lastRouteIndex = (adaptedState?.routes?.length ?? 0) - 1;
const onBoardingModalNavigatorState = adaptedState?.routes[lastRouteIndex]?.state;
if (!onBoardingModalNavigatorState || onBoardingModalNavigatorState?.routes?.length > 1) {
return;
}

let adaptedOnboardingModalNavigatorState = {} as Readonly<PartialState<NavigationState>>;
if (currentRoute?.name === SCREENS.ONBOARDING.PERSONAL_DETAILS && selectedPurpose === CONST.ONBOARDING_CHOICES.MANAGE_TEAM) {
adaptedOnboardingModalNavigatorState = {
index: 2,
routes: [
{
name: SCREENS.ONBOARDING.PURPOSE,
params: currentRoute?.params,
},
{
name: SCREENS.ONBOARDING.WORK,
params: currentRoute?.params,
path: '/onboarding/works',
Copy link
Contributor

Choose a reason for hiding this comment

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

The path is work singular. I think it's better we use getRoute().

Suggested change
path: '/onboarding/works',
path: `/${ROUTES.ONBOARDING_WORK.route}`,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mollfpr actually we don’t need the path field, I forgot to remove it.

},
{...currentRoute},
],
} as Readonly<PartialState<NavigationState>>;
} else {
adaptedOnboardingModalNavigatorState = {
index: 1,
routes: [
{
name: SCREENS.ONBOARDING.PURPOSE,
params: currentRoute?.params,
},
{...currentRoute},
],
} as Readonly<PartialState<NavigationState>>;
}

adaptedState.routes[lastRouteIndex].state = adaptedOnboardingModalNavigatorState;
navigationRef.resetRoot(adaptedState);
}

/**
* Start a new onboarding flow or continue from the last visited onboarding page.
*/
function startOnboardingFlow() {
const currentRoute = navigationRef.getCurrentRoute();
const {adaptedState} = getAdaptedStateFromPath(getOnboardingInitialPath(), linkingConfig.config, false);
const focusedRoute = findFocusedRoute(adaptedState as PartialState<NavigationState<RootStackParamList>>);
if (focusedRoute?.name === currentRoute?.name) {
return;
}
navigationRef.resetRoot(adaptedState);
}

function getOnboardingInitialPath(): string {
const state = getStateFromPath(onboardingInitialPath, linkingConfig.config);
if (state?.routes?.at(-1)?.name !== NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR) {
return `/${ROUTES.ONBOARDING_ROOT.route}`;
}

return onboardingInitialPath;
}

function clearInitialPath() {
onboardingInitialPath = '';
}

/**
* Onboarding flow: Go back to the previous page.
* Since there is no `initialRoute` for `onBoardingModalNavigator`,
* firstly, adjust the current onboarding modal navigator to establish the correct stack order.
* Then, navigate to the previous onboarding page using the usual `goBack` function.
*/
function goBack() {
adaptOnboardingRouteState();
Navigation.isNavigationReady().then(() => {
Navigation.goBack();
Comment on lines +119 to +122
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from the BZ of #50177
This condition is missing handling for the case where the user navigates back using the hardware button on Android and the onboarding route state does not initialize properly. The solution for this issue is to reset the onboarding state when setting up the router.

});
}

export {getOnboardingInitialPath, startOnboardingFlow, clearInitialPath, goBack};
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type Onboarding from '@src/types/onyx/Onboarding';
import type TryNewDot from '@src/types/onyx/TryNewDot';
import * as OnboardingFlow from './OnboardingFlow';

type OnboardingData = Onboarding | [] | undefined;

Expand Down Expand Up @@ -46,15 +47,18 @@ function onServerDataReady(): Promise<void> {
return isServerDataReadyPromise;
}

let isOnboardingInProgress = false;
function isOnboardingFlowCompleted({onCompleted, onNotCompleted}: HasCompletedOnboardingFlowProps) {
isOnboardingFlowStatusKnownPromise.then(() => {
if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
return;
}

if (onboarding?.hasCompletedGuidedSetupFlow) {
isOnboardingInProgress = false;
onCompleted?.();
} else {
} else if (!isOnboardingInProgress) {
isOnboardingInProgress = true;
onNotCompleted?.();
}
});
Expand Down Expand Up @@ -97,7 +101,7 @@ function handleHybridAppOnboarding() {
isOnboardingFlowCompleted({
onNotCompleted: () =>
setTimeout(() => {
Navigation.navigate(ROUTES.ONBOARDING_ROOT.route);
OnboardingFlow.startOnboardingFlow();
}, variables.explanationModalDelay),
}),
});
Expand Down Expand Up @@ -152,6 +156,10 @@ function setOnboardingPolicyID(policyID?: string) {
Onyx.set(ONYXKEYS.ONBOARDING_POLICY_ID, policyID ?? null);
}

function updateOnboardingLastVisitedPath(path: string) {
Onyx.merge(ONYXKEYS.ONBOARDING_LAST_VISITED_PATH, path);
}

function completeHybridAppOnboarding() {
const optimisticData: OnyxUpdate[] = [
{
Expand Down Expand Up @@ -213,12 +221,15 @@ function resetAllChecks() {
resolveOnboardingFlowStatus = resolve;
});
isLoadingReportData = true;
isOnboardingInProgress = false;
OnboardingFlow.clearInitialPath();
}

export {
onServerDataReady,
isOnboardingFlowCompleted,
setOnboardingPurposeSelected,
updateOnboardingLastVisitedPath,
resetAllChecks,
setOnboardingAdminsChatReportID,
setOnboardingPolicyID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import * as ValidationUtils from '@libs/ValidationUtils';
import * as PersonalDetails from '@userActions/PersonalDetails';
import * as Report from '@userActions/Report';
import * as Welcome from '@userActions/Welcome';
import * as OnboardingFlow from '@userActions/Welcome/OnboardingFlow';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import INPUT_IDS from '@src/types/form/DisplayNameForm';
Expand Down Expand Up @@ -133,7 +134,7 @@ function BaseOnboardingPersonalDetails({
<HeaderWithBackButton
shouldShowBackButton
progressBarPercentage={75}
onBackButtonPress={Navigation.goBack}
onBackButtonPress={OnboardingFlow.goBack}
/>
<FormProvider
style={[styles.flexGrow1, isMediumOrLargerScreenWidth && styles.mt5, isMediumOrLargerScreenWidth ? styles.mh8 : styles.mh5]}
Expand Down
4 changes: 2 additions & 2 deletions src/pages/OnboardingWork/BaseOnboardingWork.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import Navigation from '@libs/Navigation/Navigation';
import * as ValidationUtils from '@libs/ValidationUtils';
import * as Policy from '@userActions/Policy/Policy';
import * as Welcome from '@userActions/Welcome';
import * as OnboardingFlow from '@userActions/Welcome/OnboardingFlow';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -81,7 +82,7 @@ function BaseOnboardingWork({shouldUseNativeStyles, onboardingPurposeSelected, o
<HeaderWithBackButton
shouldShowBackButton
progressBarPercentage={OPEN_WORK_PAGE_PURPOSES.includes(onboardingPurposeSelected ?? '') ? 50 : 75}
onBackButtonPress={Navigation.goBack}
onBackButtonPress={OnboardingFlow.goBack}
/>
<FormProvider
style={[styles.flexGrow1, isMediumOrLargerScreenWidth && styles.mt5, isMediumOrLargerScreenWidth ? styles.mh8 : styles.mh5]}
Expand Down Expand Up @@ -111,7 +112,6 @@ function BaseOnboardingWork({shouldUseNativeStyles, onboardingPurposeSelected, o
shouldSaveDraft
maxLength={CONST.TITLE_CHARACTER_LIMIT}
spellCheck={false}
autoFocus
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am removing this line because the autofocus is handled by useAutoFocusInput(); as it is in the personal details page. This line is causing issues with the work page animation (the animation doesn't run) and navigation problems on iOS.

/>
</View>
</FormProvider>
Expand Down
Loading