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

Dismissing onboarding fix #45327

Merged
Merged
Show file tree
Hide file tree
Changes from 5 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/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5310,6 +5310,10 @@ const CONST = {
DATE: 'date',
LIST: 'dropdown',
},

NAVIGATION_ACTIONS: {
RESET: 'RESET',
},
} as const;

type Country = keyof typeof CONST.ALL_COUNTRIES;
Expand Down
4 changes: 4 additions & 0 deletions src/ONYXKEYS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,9 @@ const ONYXKEYS = {
/** Onboarding Purpose selected by the user during Onboarding flow */
ONBOARDING_PURPOSE_SELECTED: 'onboardingPurposeSelected',

/** Onboarding error message to be displayed to the user */
ONBOARDING_ERROR_MESSAGE: 'onboardingErrorMessage',

/** Onboarding policyID selected by the user during Onboarding flow */
ONBOARDING_POLICY_ID: 'onboardingPolicyID',

Expand Down Expand Up @@ -798,6 +801,7 @@ type OnyxValuesMapping = {
[ONYXKEYS.MAX_CANVAS_HEIGHT]: number;
[ONYXKEYS.MAX_CANVAS_WIDTH]: number;
[ONYXKEYS.ONBOARDING_PURPOSE_SELECTED]: string;
[ONYXKEYS.ONBOARDING_ERROR_MESSAGE]: string;
[ONYXKEYS.ONBOARDING_POLICY_ID]: string;
[ONYXKEYS.ONBOARDING_ADMINS_CHAT_REPORT_ID]: string;
[ONYXKEYS.IS_SEARCHING_FOR_REPORTS]: boolean;
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,7 @@ export default {
title: 'What do you want to do today?',
errorSelection: 'Please make a selection to continue.',
errorContinue: 'Please press continue to get set up.',
errorBackButton: 'Please finish the setup questions to start using the app.',
[CONST.ONBOARDING_CHOICES.EMPLOYER]: 'Get paid back by my employer',
[CONST.ONBOARDING_CHOICES.MANAGE_TEAM]: "Manage my team's expenses",
[CONST.ONBOARDING_CHOICES.PERSONAL_SPEND]: 'Track and budget expenses',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,7 @@ export default {
title: '¿Qué quieres hacer hoy?',
errorSelection: 'Por favor selecciona una opción para continuar.',
errorContinue: 'Por favor, haz click en continuar para configurar tu cuenta.',
errorBackButton: 'Por favor, finaliza las preguntas de configuración para empezar a utilizar la aplicación.',
[CONST.ONBOARDING_CHOICES.EMPLOYER]: 'Cobrar de mi empresa',
[CONST.ONBOARDING_CHOICES.MANAGE_TEAM]: 'Gestionar los gastos de mi equipo',
[CONST.ONBOARDING_CHOICES.PERSONAL_SPEND]: 'Controlar y presupuestar gastos',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import {View} from 'react-native';
import {useOnyx} from 'react-native-onyx';
import NoDropZone from '@components/DragAndDrop/NoDropZone';
import FocusTrapForScreens from '@components/FocusTrap/FocusTrapForScreen';
import useDisableModalDismissOnEscape from '@hooks/useDisableModalDismissOnEscape';
import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
import useOnboardingLayout from '@hooks/useOnboardingLayout';
import useThemeStyles from '@hooks/useThemeStyles';
import hasCompletedGuidedSetupFlowSelector from '@libs/hasCompletedGuidedSetupFlowSelector';
import OnboardingModalNavigatorScreenOptions from '@libs/Navigation/AppNavigator/OnboardingModalNavigatorScreenOptions';
import Navigation from '@libs/Navigation/Navigation';
import type {OnboardingModalNavigatorParamList} from '@libs/Navigation/types';
Expand All @@ -26,15 +28,11 @@ function OnboardingModalNavigator() {
const styles = useThemeStyles();
const {shouldUseNarrowLayout} = useOnboardingLayout();
const [hasCompletedGuidedSetupFlow] = useOnyx(ONYXKEYS.NVP_ONBOARDING, {
selector: (onboarding) => {
// onboarding is an array for old accounts and accounts created from olddot
if (Array.isArray(onboarding)) {
return true;
}
return onboarding?.hasCompletedGuidedSetupFlow;
},
selector: hasCompletedGuidedSetupFlowSelector,
});

useDisableModalDismissOnEscape();

useEffect(() => {
if (!hasCompletedGuidedSetupFlow) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import * as Session from '@libs/actions/Session';
import interceptAnonymousUser from '@libs/interceptAnonymousUser';
import getTopmostBottomTabRoute from '@libs/Navigation/getTopmostBottomTabRoute';
import getTopmostCentralPaneRoute from '@libs/Navigation/getTopmostCentralPaneRoute';
import Navigation from '@libs/Navigation/Navigation';
import linkingConfig from '@libs/Navigation/linkingConfig';
import getAdaptedStateFromPath from '@libs/Navigation/linkingConfig/getAdaptedStateFromPath';
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import type {RootStackParamList, State} from '@libs/Navigation/types';
import {isCentralPaneName, isHomeTabName, isSearchTabName, isSettingTabName} from '@libs/NavigationUtils';
import {getChatTabBrickRoad} from '@libs/WorkspacesSettingsUtils';
Expand Down Expand Up @@ -56,7 +58,12 @@ function BottomTabBar({isLoadingApp = false}: PurposeForUsingExpensifyModalProps
return;
}

Welcome.isOnboardingFlowCompleted({onNotCompleted: () => Navigation.navigate(ROUTES.ONBOARDING_ROOT)});
Welcome.isOnboardingFlowCompleted({
onNotCompleted: () => {
const {adaptedState} = getAdaptedStateFromPath(ROUTES.ONBOARDING_ROOT, linkingConfig.config);
navigationRef.resetRoot(adaptedState);
},
});
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [isLoadingApp]);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import type {RouterConfigOptions, StackNavigationState} from '@react-navigation/native';
import {getPathFromState, StackRouter} from '@react-navigation/native';
import type {CommonActions, RouterConfigOptions, StackActionType, StackNavigationState} from '@react-navigation/native';
import {findFocusedRoute, getPathFromState, StackRouter} from '@react-navigation/native';
import type {ParamListBase} from '@react-navigation/routers';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
import * as Localize from '@libs/Localize';
import getTopmostBottomTabRoute from '@libs/Navigation/getTopmostBottomTabRoute';
import getTopmostCentralPaneRoute from '@libs/Navigation/getTopmostCentralPaneRoute';
import linkingConfig from '@libs/Navigation/linkingConfig';
import getAdaptedStateFromPath from '@libs/Navigation/linkingConfig/getAdaptedStateFromPath';
import type {NavigationPartialRoute, RootStackParamList, State} from '@libs/Navigation/types';
import {isCentralPaneName} from '@libs/NavigationUtils';
import {isCentralPaneName, isOnboardingFlowName} from '@libs/NavigationUtils';
import * as Welcome from '@userActions/Welcome';
import CONST from '@src/CONST';
import NAVIGATORS from '@src/NAVIGATORS';
import SCREENS from '@src/SCREENS';
import type {ResponsiveStackNavigatorRouterOptions} from './types';
Expand Down Expand Up @@ -97,6 +100,23 @@ function compareAndAdaptState(state: StackNavigationState<RootStackParamList>) {
}
}

function shouldPreventReset(state: StackNavigationState<ParamListBase>, action: CommonActions.Action | StackActionType) {
if (action.type !== CONST.NAVIGATION_ACTIONS.RESET || !action?.payload) {
return false;
}
const currentFocusedRoute = findFocusedRoute(state);
const targetFocusedRoute = findFocusedRoute(action?.payload);

// We want to prevent the user from navigating back to a non-onboarding screen if they are currently on an onboarding screen
if (isOnboardingFlowName(currentFocusedRoute?.name) && !isOnboardingFlowName(targetFocusedRoute?.name)) {
Welcome.setOnboardingErrorMessage(Localize.translateLocal('onboarding.purpose.errorBackButton'));
// We reset the URL as the browser sets it in a way that doesn't match the navigation state
// eslint-disable-next-line no-restricted-globals
history.replaceState({}, '', getPathFromState(state, linkingConfig.config));
return true;
Comment on lines +113 to +115
Copy link
Contributor

Choose a reason for hiding this comment

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

history does not exist on native. I am not sure if a real user could land into this code but I got a crash while testing another PR

Screenshot 2024-10-09 at 8 07 01 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we definetly should make this part platform-specific.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, should we make a issue for this @adamgrzybowski @s77rt ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Somebody from SWM can take care of this issue if you want

Copy link
Contributor

Choose a reason for hiding this comment

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

@mountiny Could you please create an issue for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

}
}

function CustomRouter(options: ResponsiveStackNavigatorRouterOptions) {
const stackRouter = StackRouter(options);

Expand All @@ -107,6 +127,12 @@ function CustomRouter(options: ResponsiveStackNavigatorRouterOptions) {
const state = stackRouter.getRehydratedState(partialState, {routeNames, routeParamList, routeGetIdList});
return state;
},
getStateForAction(state: StackNavigationState<ParamListBase>, action: CommonActions.Action | StackActionType, configOptions: RouterConfigOptions) {
if (shouldPreventReset(state, action)) {
return state;
}
return stackRouter.getStateForAction(state, action, configOptions);
},
};
}

Expand Down
52 changes: 37 additions & 15 deletions src/libs/Navigation/NavigationRoot.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
import type {NavigationState} from '@react-navigation/native';
import {DefaultTheme, findFocusedRoute, NavigationContainer} from '@react-navigation/native';
import React, {useContext, useEffect, useMemo, useRef} from 'react';
import {useOnyx} from 'react-native-onyx';
import HybridAppMiddleware from '@components/HybridAppMiddleware';
import {ScrollOffsetContext} from '@components/ScrollOffsetContextProvider';
import useActiveWorkspace from '@hooks/useActiveWorkspace';
import useCurrentReportID from '@hooks/useCurrentReportID';
import useTheme from '@hooks/useTheme';
import useWindowDimensions from '@hooks/useWindowDimensions';
import {FSPage} from '@libs/Fullstory';
import hasCompletedGuidedSetupFlowSelector from '@libs/hasCompletedGuidedSetupFlowSelector';
import Log from '@libs/Log';
import {getPathFromURL} from '@libs/Url';
import {updateLastVisitedPath} from '@userActions/App';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Route} from '@src/ROUTES';
import ROUTES from '@src/ROUTES';
import AppNavigator from './AppNavigator';
import getPolicyIDFromState from './getPolicyIDFromState';
import linkingConfig from './linkingConfig';
Expand Down Expand Up @@ -76,26 +80,44 @@ function NavigationRoot({authenticated, lastVisitedPath, initialUrl, onReady}: N
const currentReportIDValue = useCurrentReportID();
const {isSmallScreenWidth} = useWindowDimensions();
const {setActiveWorkspaceID} = useActiveWorkspace();
const [user] = useOnyx(ONYXKEYS.USER);

const initialState = useMemo(
() => {
if (!lastVisitedPath) {
return undefined;
}
const [hasCompletedGuidedSetupFlow] = useOnyx(ONYXKEYS.NVP_ONBOARDING, {
selector: hasCompletedGuidedSetupFlowSelector,
});

const path = initialUrl ? getPathFromURL(initialUrl) : null;

// For non-nullable paths we don't want to set initial state
if (path) {
return;
}
const initialState = useMemo(() => {
if (!user || user.isFromPublicDomain) {
return;
}
Comment on lines +89 to +92
Copy link
Contributor

Choose a reason for hiding this comment

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

This return is too early and prevented public domain users from using the lastVisitedPath feature.

Repro steps:

  1. Go to settings
  2. Open https://dev.new.expensify.com:8082/
  3. Verify that you land on settings

This only works with private domain users. cc @mountiny

Screen.Recording.2024-12-19.at.2.16.36.AM.mov


const {adaptedState} = getAdaptedStateFromPath(lastVisitedPath, linkingConfig.config);
// 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) {
const {adaptedState} = getAdaptedStateFromPath(ROUTES.ONBOARDING_ROOT, linkingConfig.config);
return adaptedState;
},
}

// If there is no lastVisitedPath, we can do early return. We won't modify the default behavior.
if (!lastVisitedPath) {
return undefined;
}

const path = initialUrl ? getPathFromURL(initialUrl) : null;

// If the user opens the root of app "/" it will be parsed to empty string "".
// If the path is defined and different that empty string we don't want to modify the default behavior.
if (path) {
return;
}

// Otherwise we want to redirect the user to the last visited path.
const {adaptedState} = getAdaptedStateFromPath(lastVisitedPath, linkingConfig.config);
return adaptedState;

// The initialState value is relevant only on the first render.
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
[],
);
}, []);

// https://reactnavigation.org/docs/themes
const navigationTheme = useMemo(
Expand Down
3 changes: 3 additions & 0 deletions src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,8 @@ type FullScreenName = keyof FullScreenNavigatorParamList;

type CentralPaneName = keyof CentralPaneScreensParamList;

type OnboardingFlowName = keyof OnboardingModalNavigatorParamList;

type SwitchPolicyIDParams = {
policyID?: string;
route?: Routes;
Expand Down Expand Up @@ -1274,6 +1276,7 @@ export type {
NewChatNavigatorParamList,
NewTaskNavigatorParamList,
OnboardingModalNavigatorParamList,
OnboardingFlowName,
ParticipantsNavigatorParamList,
PrivateNotesNavigatorParamList,
ProfileNavigatorParamList,
Expand Down
47 changes: 28 additions & 19 deletions src/libs/NavigationUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type {TupleToUnion} from 'type-fest';
import {flattenObject} from '@src/languages/translations';
import SCREENS from '@src/SCREENS';
import getTopmostBottomTabRoute from './Navigation/getTopmostBottomTabRoute';
import type {CentralPaneName, RootStackParamList, State} from './Navigation/types';
import type {CentralPaneName, OnboardingFlowName, RootStackParamList, State} from './Navigation/types';

const CENTRAL_PANE_SCREEN_NAMES = new Set([
SCREENS.SETTINGS.WORKSPACES,
Expand All @@ -19,23 +19,7 @@ const CENTRAL_PANE_SCREEN_NAMES = new Set([
SCREENS.REPORT,
]);

function isCentralPaneName(screen: string | undefined): screen is CentralPaneName {
if (!screen) {
return false;
}

return CENTRAL_PANE_SCREEN_NAMES.has(screen as CentralPaneName);
}

const removePolicyIDParamFromState = (state: State<RootStackParamList>) => {
const stateCopy = cloneDeep(state);
const bottomTabRoute = getTopmostBottomTabRoute(stateCopy);
if (bottomTabRoute?.params && 'policyID' in bottomTabRoute.params) {
delete bottomTabRoute.params.policyID;
}
return stateCopy;
};

const ONBOARDING_SCREEN_NAMES = new Set([SCREENS.ONBOARDING.PERSONAL_DETAILS, SCREENS.ONBOARDING.PURPOSE, SCREENS.ONBOARDING.WORK, SCREENS.ONBOARDING_MODAL.ONBOARDING]);
const SETTINGS_SCREENS = Object.values(flattenObject(SCREENS.SETTINGS));
const SEARCH_SCREENS = Object.values(flattenObject(SCREENS.SEARCH));
const HOME_SCREENS = [SCREENS.HOME, SCREENS.REPORT];
Expand All @@ -47,6 +31,14 @@ const SEARCH_TAB_SCREEN_NAMES = new Set(SEARCH_SCREENS);

const HOME_SCREEN_NAMES = new Set(HOME_SCREENS);

function isCentralPaneName(screen: string | undefined): screen is CentralPaneName {
if (!screen) {
return false;
}

return CENTRAL_PANE_SCREEN_NAMES.has(screen as CentralPaneName);
}

function isBottomTabName(screen: TupleToUnion<typeof SETTINGS_SCREENS> | undefined) {
if (!screen) {
return false;
Expand Down Expand Up @@ -75,4 +67,21 @@ function isHomeTabName(screen: TupleToUnion<typeof HOME_SCREENS> | undefined) {
return HOME_SCREEN_NAMES.has(screen);
}

export {isCentralPaneName, isBottomTabName, isSearchTabName, isSettingTabName, isHomeTabName, removePolicyIDParamFromState};
function isOnboardingFlowName(screen: string | undefined): screen is OnboardingFlowName {
if (!screen) {
return false;
}

return ONBOARDING_SCREEN_NAMES.has(screen as OnboardingFlowName);
}

const removePolicyIDParamFromState = (state: State<RootStackParamList>) => {
const stateCopy = cloneDeep(state);
const bottomTabRoute = getTopmostBottomTabRoute(stateCopy);
if (bottomTabRoute?.params && 'policyID' in bottomTabRoute.params) {
delete bottomTabRoute.params.policyID;
}
return stateCopy;
};

export {isCentralPaneName, isBottomTabName, isSearchTabName, isSettingTabName, isHomeTabName, removePolicyIDParamFromState, isOnboardingFlowName};
Loading
Loading