Skip to content

Revert "Fix Onboarding modal does not appear until clearing cache" #103645

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.

Sign up for GitHub

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

Closed
carlosmiceli wants to merge 1 commit into from
Closed

Revert "Fix Onboarding modal does not appear until clearing cache" #103645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 1 addition & 4 deletions src/libs/Navigation/AppNavigator/AuthScreens.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import useGPSInProgressModal from '@hooks/useGPSInProgressModal';
import useGPSTripStateChecker from '@hooks/useGPSTripStateChecker';
import useOnboardingFlowRouter from '@hooks/useOnboardingFlow';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useRootNavigationState from '@hooks/useRootNavigationState';
import useShouldSuppressPromotionalUI from '@hooks/useShouldSuppressPromotionalUI';
import {SidebarOrderedReportsContextProvider} from '@hooks/useSidebarOrderedReports';
import useStyleUtils from '@hooks/useStyleUtils';
Expand Down Expand Up @@ -138,8 +137,6 @@ function AuthScreens() {
const modalCardStyleInterpolator = useModalCardStyleInterpolator();
const {isVisitingSecureLink} = useOnboardingFlowRouter();
const shouldSuppressPromotionalUI = useShouldSuppressPromotionalUI();
// Subscribed to navigation state so the onboarding navigator gets registered as soon as the user leaves the validate login screen
const shouldHideOnboardingNavigator = useRootNavigationState(Navigation.shouldHideOnboardingNavigator);
useGPSTripStateChecker();
useGPSInProgressModal();

Expand Down Expand Up @@ -359,7 +356,7 @@ function AuthScreens() {
* screen in and out of the RootStack mid-session (which resets the navigator's state to its initial
* route). OnboardingGuard and useOnboardingFlowRouter gate whether/when a user actually lands here.
*/}
{!shouldSuppressPromotionalUI && !isVisitingSecureLink && !shouldHideOnboardingNavigator && (
{!shouldSuppressPromotionalUI && !isVisitingSecureLink && !Navigation.isValidateLoginFlow() && (

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Subscribe before gating onboarding on the active route

When AuthScreens initially renders with VALIDATE_LOGIN focused, this condition omits ONBOARDING_MODAL_NAVIGATOR, but the component no longer subscribes to root navigation-state changes. On a fresh sign-up where the onboarding data finishes loading before validation completes, leaving the validation screen alone therefore neither registers the navigator nor reruns useOnboardingFlowRouter, so onboarding can remain absent until an unrelated rerender or app-data reset. Restore the root-navigation-state subscription so completing validation immediately updates this gate.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve an existing onboarding route during validation

If a user already in onboarding opens a validation link and AuthScreens rerenders while VALIDATE_LOGIN is focused, this predicate unregisters the onboarding screen even though its route is already in the root stack. React Navigation then discards that route and its nested state, so returning from validation loses the user's in-progress onboarding position; the gate should hide onboarding only when validation is active and the onboarding route has not already been registered.

Useful? React with 👍 / 👎.

<RootStack.Screen
name={NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}
options={{...rootNavigatorScreenOptions.basicModalNavigator, gestureEnabled: false}}
Expand Down
15 changes: 4 additions & 11 deletions src/libs/Navigation/Navigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1127,16 +1127,10 @@ function isOnboardingFlow() {
return isOnboardingFlowName(currentFocusedRoute?.name);
}

function isValidateLoginFlow(state: NavigationState | undefined) {
return !!state && findFocusedRoute(state)?.name === SCREENS.VALIDATE_LOGIN;
}

/**
* Keeps onboarding from opening over the validate login screen. Once the onboarding navigator is in the root stack it stays
* registered, because unregistering it drops its route and the user's onboarding progress.
*/
function shouldHideOnboardingNavigator(state: NavigationState | undefined) {
return isValidateLoginFlow(state) && !state?.routes.some((route) => route.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR);
function isValidateLoginFlow() {
const state = navigationRef.getRootState();
const currentFocusedRoute = findFocusedRoute(state);
return currentFocusedRoute?.name === SCREENS.VALIDATE_LOGIN;
}

function clearPreloadedRoutes() {
Expand Down Expand Up @@ -1339,7 +1333,6 @@ export default {
isOnboardingFlow,
clearPreloadedRoutes,
isValidateLoginFlow,
shouldHideOnboardingNavigator,
dismissToPreviousRHP,
dismissToSuperWideRHP,
revealRouteBeforeDismissingModal,
Expand Down
48 changes: 1 addition & 47 deletions tests/navigation/NavigationTests.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,8 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout';

import getIsNarrowLayout from '@libs/getIsNarrowLayout';
import Navigation from '@libs/Navigation/Navigation';
import navigationRef from '@libs/Navigation/navigationRef';

import CONST from '@src/CONST';
import NAVIGATORS from '@src/NAVIGATORS';
import SCREENS from '@src/SCREENS';

import type {NavigationState} from '@react-navigation/native';

import React from 'react';

Expand All @@ -24,18 +19,6 @@ jest.mock('@pages/inbox/sidebar/NavigationTabBarAvatar');
const mockedGetIsNarrowLayout = jest.mocked(getIsNarrowLayout);
const mockedUseResponsiveLayout = jest.mocked(useResponsiveLayout);

/** Builds a root stack state with the given routes, focused on the last one */
function buildRootState(routeNames: string[]): NavigationState {
return {
key: 'root',
index: routeNames.length - 1,
routeNames,
routes: routeNames.map((name) => ({key: `${name}-key`, name})),
type: 'stack',
stale: false,
};
}

describe('Navigation', () => {
beforeEach(() => {
mockedGetIsNarrowLayout.mockReturnValue(false);
Expand All @@ -58,36 +41,7 @@ describe('Navigation', () => {
/>,
);

expect(Navigation.isValidateLoginFlow(navigationRef.getRootState())).toBe(true);
});
});

describe('shouldHideOnboardingNavigator', () => {
it('should hide the onboarding navigator on the validate login screen before onboarding has started', () => {
// Given a fresh magic link sign-in, where the validate login screen is focused and onboarding isn't in the root stack yet
const state = buildRootState([NAVIGATORS.TAB_NAVIGATOR, SCREENS.VALIDATE_LOGIN]);

// When checking whether the onboarding navigator should be registered
// Then it is hidden so onboarding doesn't open over the validate login screen
expect(Navigation.shouldHideOnboardingNavigator(state)).toBe(true);
});

it('should keep the onboarding navigator when the validate login screen is pushed over onboarding', () => {
// Given a user in the middle of onboarding who opens a magic link, pushing the validate login screen over the onboarding navigator
const state = buildRootState([NAVIGATORS.TAB_NAVIGATOR, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, SCREENS.VALIDATE_LOGIN]);

// When checking whether the onboarding navigator should be registered
// Then it stays registered, because unregistering it would drop the onboarding route and the user's progress
expect(Navigation.shouldHideOnboardingNavigator(state)).toBe(false);
});

it('should not hide the onboarding navigator once the user leaves the validate login screen', () => {
// Given the validate login screen was popped and the user is back on the tab navigator
const state = buildRootState([NAVIGATORS.TAB_NAVIGATOR]);

// When checking whether the onboarding navigator should be registered
// Then it is registered so startOnboardingFlow can reset into it
expect(Navigation.shouldHideOnboardingNavigator(state)).toBe(false);
expect(Navigation.isValidateLoginFlow()).toBe(true);
});
});
});
Loading