Skip to content

[CP Staging] Revert "Fix reveal navigation under RHP + re-land reveal workspace route under RHP before dismissing on workspace creation " #90982

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
mountiny wants to merge 1 commit into from
Closed

[CP Staging] Revert "Fix reveal navigation under RHP + re-land reveal workspace route under RHP before dismissing on workspace creation " #90982

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
3 changes: 0 additions & 3 deletions src/CONST/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6479,9 +6479,6 @@ const CONST = {

NAVIGATION: {
CUSTOM_HISTORY_ENTRY_SIDE_PANEL: 'CUSTOM_HISTORY-SIDE_PANEL',
// Fake history entry used to keep browser Back behavior correct after revealing a route under an RHP.
// addRootHistoryRouterExtension owns when this is added, carried forward, and removed.
CUSTOM_HISTORY_ENTRY_REVEAL_PADDING: 'CUSTOM_HISTORY-REVEAL_PADDING',
ACTION_TYPE: {
REPLACE: 'REPLACE',
PUSH: 'PUSH',
Expand Down
41 changes: 2 additions & 39 deletions src/libs/Navigation/AppNavigator/createRootStackNavigator/GetStateForActionHandlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,39 +51,6 @@ const MODAL_ROUTES_TO_DISMISS = new Set<string>([

const screensWithEnteringAnimation = new Set<string>();

// RN's deep-link initial-state hint keys, per `getStateFromParams` in
// @react-navigation/core/src/useNavigationBuilder.tsx. Stripped only when `params.screen` is
// set so legitimate user keys (e.g. `path`, `initial`) on non-hydrated routes survive.
const STALE_DEEP_LINK_PARAM_KEYS = new Set(['state', 'screen', 'params', 'path', 'initial']);

/** Removes the RN deep-link hint chain from `route.params` when triggered by `params.screen`. */
function withSanitizedDeepLinkParams<R extends {params?: unknown}>(route: R, focusParams: Record<string, unknown> | undefined): R {
const rParamsRecord = route.params as Record<string, unknown> | undefined;

// RN stores nested deep-link instructions under params.screen/params.params.
const looksLikeDeepLinkInitialState = !!rParamsRecord && typeof rParamsRecord.screen === 'string';
const shouldSanitizeExistingParams = looksLikeDeepLinkInitialState && !!rParamsRecord;

// Remove only RN's hint keys; keep any real params that were stored next to them.
const sanitizedExistingParams = shouldSanitizeExistingParams ? Object.fromEntries(Object.entries(rParamsRecord).filter(([key]) => !STALE_DEEP_LINK_PARAM_KEYS.has(key))) : rParamsRecord;
const hasSanitizedExistingParams = !!sanitizedExistingParams && Object.keys(sanitizedExistingParams).length > 0;
const fallbackParams = hasSanitizedExistingParams ? sanitizedExistingParams : undefined;

// The new focused tab params win; otherwise keep the cleaned existing params.
const nextParams = focusParams ?? fallbackParams;

if (nextParams !== undefined) {
return {...route, params: nextParams};
}
if (looksLikeDeepLinkInitialState) {
// If params only contained stale RN hints, remove params entirely.
const routeWithoutParams = {...route};
delete (routeWithoutParams as {params?: unknown}).params;
return routeWithoutParams;
}
return {...route};
}

/**
* Stores the original TAB_NAVIGATOR route before a tab-switch pre-insertion
* (handleReplaceFullscreenUnderRHP). Restored on cancel by handleRemoveFullscreenUnderRHP,
Expand Down Expand Up @@ -413,11 +380,9 @@ function handleReplaceFullscreenUnderRHP(
const prependedRoutes = [sidebarRoute, ...(newNestedRoutes ?? [])];
mergedNestedState = {...focusedTargetTab.state, routes: prependedRoutes, index: prependedRoutes.length - 1};
}
// Strip any RN deep-link hint chain from `r.params`; otherwise RN would run a
// follow-up NAVIGATE from it and override the `state` we splice below.
const sanitizedRoute = withSanitizedDeepLinkParams(r, focusedTargetTab.params as Record<string, unknown> | undefined);
return {
...sanitizedRoute,
...r,
...(focusedTargetTab.params !== undefined ? {params: focusedTargetTab.params} : {}),
...(mergedNestedState !== undefined ? {state: mergedNestedState as typeof r.state} : {}),
Comment on lines +384 to 386
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 18, 2026

Choose a reason for hiding this comment

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

P2 Badge Drop stale deep-link params before replacing tab state

For tab-target reveals, spreading r keeps any existing params.screen/params.params deep-link hints on the tab route when the new focused tab has no params. If the tab was previously hydrated from a deep link, React Navigation will process those stale params after mount and issue a nested navigate that can override the freshly spliced state, so revealing a workspace/search tab under an RHP can land back on the old nested screen instead of the requested route.

Useful? React with 👍 / 👎.

};
});
Expand Down Expand Up @@ -577,6 +542,4 @@ export {
handleToggleSidePanelWithHistoryAction,
getPreInsertedOriginalTabRoute,
clearPreInsertedOriginalTabRoute,
// Exported for unit-test access; not used outside of testing.
withSanitizedDeepLinkParams,
};
97 changes: 35 additions & 62 deletions src/libs/Navigation/AppNavigator/routerExtensions/addRootHistoryRouterExtension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,37 @@
import type {ParamListBase, PartialState, Router, RouterConfigOptions} from '@react-navigation/native';
import Log from '@libs/Log';
import type {RootStackNavigatorAction} from '@libs/Navigation/AppNavigator/createRootStackNavigator/types';
import type {CommonActions, ParamListBase, PartialState, Router, RouterConfigOptions, StackActionType} from '@react-navigation/native';
import type {RemoveFullscreenUnderRHPActionType, ReplaceFullscreenUnderRHPActionType, RootStackNavigatorAction} from '@libs/Navigation/AppNavigator/createRootStackNavigator/types';
import type {PlatformStackNavigationState, PlatformStackRouterFactory, PlatformStackRouterOptions} from '@libs/Navigation/PlatformStackNavigation/types';
import CONST from '@src/CONST';
import {
applyRevealPaddingOffset,
getFrozenHistoryStateForRemoveFullscreenUnderRHP,
getFrozenHistoryStateForReplaceFullscreenUnderRHP,
getRevealDismissState,
isDismissModalAction,
isRemoveFullscreenUnderRHPAction,
isReplaceFullscreenUnderRHPAction,
} from './addRootHistoryRouterExtensionUtils';
import type {PendingReveal, RootHistoryState} from './addRootHistoryRouterExtensionUtils';
import {enhanceStateWithHistory} from './utils';

/** Manages root `state.history` for side-panel + reveal flows; per-branch rationale inline. */
function isReplaceFullscreenUnderRHPAction(action: RootStackNavigatorAction): action is ReplaceFullscreenUnderRHPActionType {
return action.type === CONST.NAVIGATION.ACTION_TYPE.REPLACE_FULLSCREEN_UNDER_RHP;
}

function isRemoveFullscreenUnderRHPAction(action: RootStackNavigatorAction): action is RemoveFullscreenUnderRHPActionType {
return action.type === CONST.NAVIGATION.ACTION_TYPE.REMOVE_FULLSCREEN_UNDER_RHP;
}

/**
* Higher-order function that extends a React Navigation stack router with history
* management for the root stack navigator.
*
* It maintains a `history` array mirroring the routes and handles two concerns:
*
* 1. **Side panel** – preserves the CUSTOM_HISTORY_ENTRY_SIDE_PANEL entry through
* rehydration so the side panel open/close state survives navigation state rebuilds.
*
* 2. **REPLACE/REMOVE_FULLSCREEN_UNDER_RHP** - freezes the history array for these
* actions so that useLinking sees historyDelta=0 and does NOT push/pop any browser
* history entries for these intermediate state changes. The correct browser history
* update happens later when DISMISS_MODAL pops the RHP in the next animation frame.
*/
function addRootHistoryRouterExtension<RouterOptions extends PlatformStackRouterOptions = PlatformStackRouterOptions>(
originalRouter: PlatformStackRouterFactory<ParamListBase, RouterOptions>,
) {
return (options: RouterOptions): Router<PlatformStackNavigationState<ParamListBase>, RootStackNavigatorAction> => {
return (options: RouterOptions): Router<PlatformStackNavigationState<ParamListBase>, CommonActions.Action | StackActionType> => {
const router = originalRouter(options);

// RHP snapshot taken on REPLACE; matching DISMISS must equal all three fields (key,
// routes depth, history depth) to commit the reveal freeze.
let pendingReveal: PendingReveal | null = null;

const getInitialState = (configOptions: RouterConfigOptions) => {
const state = router.getInitialState(configOptions);
return enhanceStateWithHistory(state);
Expand All @@ -35,7 +41,6 @@ function addRootHistoryRouterExtension<RouterOptions extends PlatformStackRouter
const state = router.getRehydratedState(partialState, configOptions);
const stateWithInitialHistory = enhanceStateWithHistory(state);

// Preserve trailing side-panel sentinel through state rebuilds.
if (state.history?.at(-1) === CONST.NAVIGATION.CUSTOM_HISTORY_ENTRY_SIDE_PANEL) {
stateWithInitialHistory.history = [...stateWithInitialHistory.history, CONST.NAVIGATION.CUSTOM_HISTORY_ENTRY_SIDE_PANEL];
return stateWithInitialHistory;
Expand All @@ -44,56 +49,24 @@ function addRootHistoryRouterExtension<RouterOptions extends PlatformStackRouter
return stateWithInitialHistory;
};

// Centralizes the `PartialState | FullState` cast to `getRehydratedState`'s input shape.
function rehydrate(newState: PartialState<RootHistoryState> | RootHistoryState, configOptions: RouterConfigOptions) {
return getRehydratedState(newState as PartialState<RootHistoryState>, configOptions);
}

const getStateForAction = (state: RootHistoryState, action: RootStackNavigatorAction, configOptions: RouterConfigOptions) => {
// Snapshot is stale if its RHP key vanished via a non-DISMISS path.
if (pendingReveal && !state.routes.some((r) => r.key === pendingReveal?.rhpKey)) {
Log.hmmm('[addRootHistoryRouterExtension] pending reveal RHP no longer in routes; clearing snapshot', {pendingReveal});
pendingReveal = null;
}

const getStateForAction = (state: PlatformStackNavigationState<ParamListBase>, action: CommonActions.Action | StackActionType, configOptions: RouterConfigOptions) => {
const newState = router.getStateForAction(state, action, configOptions);

if (!newState) {
return null;
}

// REPLACE: capture pending reveal + freeze history (intermediate frame; useLinking historyDelta=0).
if (isReplaceFullscreenUnderRHPAction(action)) {
const result = getFrozenHistoryStateForReplaceFullscreenUnderRHP(state, newState, configOptions, pendingReveal, rehydrate);
pendingReveal = result.pendingReveal;
return result.state;
}

// REMOVE: cancel path; clear snapshot + freeze history (same rationale as REPLACE).
if (isRemoveFullscreenUnderRHPAction(action)) {
const result = getFrozenHistoryStateForRemoveFullscreenUnderRHP(state, newState, configOptions, rehydrate);
if (state.history) {
pendingReveal = null;
}
return result;
}

// DISMISS that completes the reveal: pad new history to pre-DISMISS length so
// useLinking sees historyDelta=0 and just `history.replace`s the current entry,
// preserving the prior fullscreen browser entry. (RN 7.x useLinking semantics.)
if (isDismissModalAction(action) && pendingReveal && state.history) {
const result = getRevealDismissState(state, newState, configOptions, pendingReveal, rehydrate);
pendingReveal = result.pendingReveal;
if (result.state) {
return result.state;
}
// For REPLACE/REMOVE_FULLSCREEN_UNDER_RHP we intentionally preserve the original
// history array so that useLinking sees historyDelta=0 and does NOT push/pop any
// browser history entries for these intermediate state changes.
if ((isReplaceFullscreenUnderRHPAction(action) || isRemoveFullscreenUnderRHPAction(action)) && state.history) {
// @ts-expect-error newState can be partial but getRehydratedState handles it correctly.
const rehydrated = getRehydratedState(newState, configOptions);
return {...rehydrated, history: state.history};
}

// Default: re-apply the offset (single source of truth = leading sentinels in
// state.history). addPushParamsRouterExtension keeps all string entries, so
// reveal-padding sentinels survive PUSH_PARAMS / GO_BACK / POP / RESET dispatches.
const rehydrated = rehydrate(newState, configOptions);
return applyRevealPaddingOffset(state, rehydrated);
// @ts-expect-error newState may be partial, but getRehydratedState handles both partial and full states correctly.
return getRehydratedState(newState, configOptions);
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 18, 2026

Choose a reason for hiding this comment

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

P2 Badge Preserve reveal history through modal dismissal

When Navigation.revealRouteBeforeDismissingModal() runs on web, it dispatches REPLACE_FULLSCREEN_UNDER_RHP and then DISMISS_MODAL; this branch freezes history only for the replace, but the following dismiss falls through here and rebuilds history from the shortened route stack. In the common [fullscreen, RHP] -> [target] reveal flow, useLinking observes a negative history delta and pops browser history instead of replacing the current RHP entry, so the browser Back stack skips or loses the previous fullscreen entry after submitting/navigating from an RHP.

Useful? React with 👍 / 👎.

};

return {
Expand Down
169 changes: 0 additions & 169 deletions src/libs/Navigation/AppNavigator/routerExtensions/addRootHistoryRouterExtensionUtils.ts
View file
Open in desktop

This file was deleted.

Loading
Loading