Skip to content

Commit 17fd0cb

Browse files
committed
fix: eliminate destination flash on native RHP swipe-dismiss
1 parent 6019e1d commit 17fd0cb

6 files changed

Lines changed: 200 additions & 5 deletions

File tree

src/SCREENS.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,8 @@ const SCREENS = {
315315
CHRONOS_SCHEDULE_OOO: 'Chronos_Schedule_OOO',
316316
AVATAR_CROP: 'AvatarCrop',
317317
},
318+
// Neutral placeholder screen shown under the RHP while a destination is pre-mounted.
319+
PRE_MOUNT_BUFFER: 'PreMountBuffer',
318320
REPORT_CARD_ACTIVATE: 'Report_Card_Activate_Root',
319321
SAML_SIGN_IN: 'SAMLSignIn',
320322
WORKSPACE_JOIN_USER: 'WorkspaceJoinUser',

src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ const loadLogOutPreviousUserPage = () => require<ReactComponentModule>('../../..
8484
const loadConciergePage = () => require<ReactComponentModule>('../../../pages/ConciergePage').default;
8585
const loadTrackExpensePage = () => require<ReactComponentModule>('../../../pages/TrackExpensePage').default;
8686
const loadSubmitExpensePage = () => require<ReactComponentModule>('../../../pages/SubmitExpensePage').default;
87+
const loadPreMountBufferPage = () => require<ReactComponentModule>('../../../pages/PreMountBufferPage').default;
8788
const loadWorkspaceJoinUser = () => require<ReactComponentModule>('@pages/workspace/WorkspaceJoinUserPage').default;
8889

8990
const loadSearchRouterPage = () => require<ReactComponentModule>('../../../components/Search/SearchRouter/SearchRouterPage').default;
@@ -227,6 +228,13 @@ function AuthScreens() {
227228
options={defaultScreenOptions}
228229
getComponent={loadSubmitExpensePage}
229230
/>
231+
{/* No linking path, not deep-linkable. animation: none - it's a fake screen,
232+
insert/remove must be instant, not slide. */}
233+
<RootStack.Screen
234+
name={SCREENS.PRE_MOUNT_BUFFER}
235+
options={{...defaultScreenOptions, animation: 'none'}}
236+
getComponent={loadPreMountBufferPage}
237+
/>
230238
<RootStack.Screen
231239
name={SCREENS.REPORT_ATTACHMENTS}
232240
options={attachmentModalScreenOptions}

src/libs/Navigation/AppNavigator/createRootStackNavigator/GetStateForActionHandlers.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {CommonActions, NavigationState, PartialState, RouterConfigOptions,
1414
import type {ParamListBase, Router} from '@react-navigation/routers';
1515

1616
import {StackActions} from '@react-navigation/native';
17+
import {Platform} from 'react-native';
1718

1819
import type {
1920
PushActionType,
@@ -458,7 +459,18 @@ function handleReplaceFullscreenUnderRHP(
458459
preInsertedOriginalTabRoute = existingTabState?.routes?.length
459460
? existingTabRoute
460461
: ({...existingTabRoute, state: buildTabNavigatorNestedState({name: TAB_SCREENS[0]})} as StackNavigationState<ParamListBase>['routes'][number]);
461-
const newRoutes = [...routesWithoutRHP.slice(0, tabNavIndex), updatedTabRoute, ...routesWithoutRHP.slice(tabNavIndex + 1), rhpRoute];
462+
// Build Buffer into this same dispatch. A separate follow-up dispatch races the stale-state
463+
// rehydration this action just computed and can freeze a pre-rehydration snapshot, so
464+
// Buffer must land atomically.
465+
const bufferRouteForTab =
466+
Platform.OS !== 'web' ? ({name: SCREENS.PRE_MOUNT_BUFFER, key: `pre-mount-buffer-${rhpRoute.key}`} as StackNavigationState<ParamListBase>['routes'][number]) : undefined;
467+
const newRoutes = [
468+
...routesWithoutRHP.slice(0, tabNavIndex),
469+
updatedTabRoute,
470+
...routesWithoutRHP.slice(tabNavIndex + 1),
471+
...(bufferRouteForTab ? [bufferRouteForTab] : []),
472+
rhpRoute,
473+
];
462474
return stackRouter.getRehydratedState({...state, routes: newRoutes, index: newRoutes.length - 1}, configOptions);
463475
}
464476

@@ -485,10 +497,13 @@ function handleReplaceFullscreenUnderRHP(
485497
}
486498

487499
const rehydratedStateAfterPush = stackRouter.getRehydratedState(stateAfterPush, configOptions);
500+
// Build Buffer into this same dispatch (same reasoning as the tab branch above).
501+
const bufferRouteForPush =
502+
Platform.OS !== 'web' ? ({name: SCREENS.PRE_MOUNT_BUFFER, key: `pre-mount-buffer-${rhpRoute.key}`} as StackNavigationState<ParamListBase>['routes'][number]) : undefined;
488503
return {
489504
...rehydratedStateAfterPush,
490-
routes: [...rehydratedStateAfterPush.routes, rhpRoute],
491-
index: rehydratedStateAfterPush.routes.length,
505+
routes: [...rehydratedStateAfterPush.routes, ...(bufferRouteForPush ? [bufferRouteForPush] : []), rhpRoute],
506+
index: rehydratedStateAfterPush.routes.length + (bufferRouteForPush ? 1 : 0),
492507
};
493508
}
494509

@@ -561,7 +576,8 @@ function handleDismissModalAction(
561576
return null;
562577
}
563578

564-
return stackRouter.getStateForAction(state, newAction, configOptions);
579+
const result = stackRouter.getStateForAction(state, newAction, configOptions);
580+
return result;
565581
}
566582

567583
/**

src/libs/Navigation/Navigation.ts

Lines changed: 145 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {CommonActions, StackActions, TabActions} from '@react-navigation/native'
2929
import {Str} from 'expensify-common';
3030
// eslint-disable-next-line you-dont-need-lodash-underscore/omit
3131
import omit from 'lodash/omit';
32-
import {DeviceEventEmitter, Dimensions} from 'react-native';
32+
import {DeviceEventEmitter, Dimensions, Platform} from 'react-native';
3333
import Onyx from 'react-native-onyx';
3434

3535
import type {LinkToOptions} from './helpers/linkTo/types';
@@ -1103,6 +1103,134 @@ function revealRouteBeforeDismissingModal(route: Route, options?: {afterTransiti
11031103
let isFullscreenPreInsertedUnderRHP = false;
11041104
let preInsertedFullscreenRouteName: string | undefined;
11051105

1106+
// Keyed native-stack buffer transaction. Covers push-path (destination pushed as its own route)
1107+
// and tab-switch (destination is the focused tab in TAB_NAVIGATOR) pre-inserts. When set, a
1108+
// neutral SCREENS.PRE_MOUNT_BUFFER route sits directly under the RHP so a native swipe-dismiss
1109+
// paints Buffer instead of the destination. Cleaned up by commitBufferReveal (confirm),
1110+
// cancelBufferTransactionWhileRHPPresent (back out while RHP still open), or
1111+
// handleRHPClosedForBuffer (RHP removed some other way, e.g. native swipe/predictive-back).
1112+
//
1113+
// Gated on the root 'state' event, not transitionEnd: transitionEnd(closing:true) never fires on
1114+
// Android while animation is NONE (active for the whole pre-insert window), for any dismiss path.
1115+
// 'state' fires reliably on both platforms regardless. It used to leak a destination frame on
1116+
// iOS, but that was a race, not a timing problem: removePreInsertedFullscreenIfNeeded's
1117+
// already-dismissed branch ran a separate, rAF-deferred tab restore that could land after an
1118+
// early buffer-strip. Fixed by making that branch back off when a buffer transaction is live (see
1119+
// removePreInsertedFullscreenIfNeeded), so handleRHPClosedForBuffer's single atomic reset is the
1120+
// only thing that runs.
1121+
let bufferTransaction: {rhpRouteKey: string; bufferRouteKey: string; mode: 'push' | 'tab'; destinationRouteKey?: string} | undefined;
1122+
let bufferStateListenerUnsubscribe: (() => void) | undefined;
1123+
1124+
function clearBufferStateListener() {
1125+
bufferStateListenerUnsubscribe?.();
1126+
bufferStateListenerUnsubscribe = undefined;
1127+
}
1128+
1129+
function handleRHPClosedForBuffer() {
1130+
if (!bufferTransaction) {
1131+
return;
1132+
}
1133+
const {rhpRouteKey, bufferRouteKey, mode, destinationRouteKey} = bufferTransaction;
1134+
const rootState = navigationRef.getRootState();
1135+
if (!rootState) {
1136+
return;
1137+
}
1138+
const stillHasRHP = rootState.routes.some((r) => r.key === rhpRouteKey);
1139+
if (stillHasRHP) {
1140+
return;
1141+
}
1142+
1143+
// RHP is gone but our transaction wasn't cleared by commit/cancel - something else removed it
1144+
// (native swipe, external dismissal). End state must be the origin, same as a normal cancel.
1145+
bufferTransaction = undefined;
1146+
clearBufferStateListener();
1147+
isFullscreenPreInsertedUnderRHP = false;
1148+
preInsertedFullscreenRouteName = undefined;
1149+
1150+
if (mode === 'tab') {
1151+
const originalTabRoute = getPreInsertedOriginalTabRoute();
1152+
clearPreInsertedOriginalTabRoute();
1153+
const tabNavIndex = rootState.routes.findLastIndex((r) => r.name === NAVIGATORS.TAB_NAVIGATOR);
1154+
if (!originalTabRoute || tabNavIndex < 0) {
1155+
const fallbackRoutes = rootState.routes.filter((r) => r.key !== bufferRouteKey);
1156+
navigationRef.current?.dispatch(CommonActions.reset({...rootState, routes: fallbackRoutes, index: fallbackRoutes.length - 1}));
1157+
return;
1158+
}
1159+
const newRoutes = rootState.routes
1160+
.filter((r) => r.key !== bufferRouteKey)
1161+
.map((r, i, arr) => (i === arr.findLastIndex((rr) => rr.name === NAVIGATORS.TAB_NAVIGATOR) ? originalTabRoute : r));
1162+
navigationRef.current?.dispatch(CommonActions.reset({...rootState, routes: newRoutes, index: newRoutes.length - 1}));
1163+
return;
1164+
}
1165+
1166+
// Push mode: strip both the speculative destination and Buffer atomically.
1167+
const newRoutes = rootState.routes.filter((r) => r.key !== bufferRouteKey && r.key !== destinationRouteKey);
1168+
navigationRef.current?.dispatch(
1169+
CommonActions.reset({
1170+
...rootState,
1171+
routes: newRoutes,
1172+
index: newRoutes.length - 1,
1173+
}),
1174+
);
1175+
}
1176+
1177+
/**
1178+
* Buffer is built directly into REPLACE_FULLSCREEN_UNDER_RHP's own dispatch
1179+
* (GetStateForActionHandlers.ts), not a separate follow-up dispatch - a second dispatch was
1180+
* racing the stale-state rehydration that action performs and could freeze a pre-rehydration
1181+
* snapshot (tab index computed correctly, then clobbered back to the wrong value by the second
1182+
* reset). This function only reads the already-landed state to
1183+
* capture the transaction; it never dispatches.
1184+
*/
1185+
function captureBufferTransaction(stateAfter: ReturnType<typeof navigationRef.getRootState>, wasTabSwitched: boolean) {
1186+
if (Platform.OS === 'web' || !stateAfter) {
1187+
return;
1188+
}
1189+
const rhpRoute = stateAfter.routes.at(-1);
1190+
const bufferRoute = stateAfter.routes.at(-2);
1191+
if (rhpRoute?.name !== NAVIGATORS.RIGHT_MODAL_NAVIGATOR || bufferRoute?.name !== SCREENS.PRE_MOUNT_BUFFER) {
1192+
return;
1193+
}
1194+
1195+
if (wasTabSwitched) {
1196+
bufferTransaction = {rhpRouteKey: rhpRoute.key, bufferRouteKey: bufferRoute.key, mode: 'tab'};
1197+
} else {
1198+
const destinationRoute = stateAfter.routes.at(-3);
1199+
if (!destinationRoute) {
1200+
return;
1201+
}
1202+
bufferTransaction = {rhpRouteKey: rhpRoute.key, bufferRouteKey: bufferRoute.key, mode: 'push', destinationRouteKey: destinationRoute.key};
1203+
}
1204+
1205+
clearBufferStateListener();
1206+
bufferStateListenerUnsubscribe = navigationRef.current?.addListener('state', handleRHPClosedForBuffer);
1207+
}
1208+
1209+
function removeBufferRouteOnly() {
1210+
if (!bufferTransaction) {
1211+
return;
1212+
}
1213+
const {bufferRouteKey} = bufferTransaction;
1214+
bufferTransaction = undefined;
1215+
clearBufferStateListener();
1216+
1217+
const rootState = navigationRef.getRootState();
1218+
if (!rootState) {
1219+
return;
1220+
}
1221+
const newRoutes = rootState.routes.filter((r) => r.key !== bufferRouteKey);
1222+
if (newRoutes.length === rootState.routes.length) {
1223+
return;
1224+
}
1225+
navigationRef.current?.dispatch(
1226+
CommonActions.reset({
1227+
...rootState,
1228+
routes: newRoutes,
1229+
index: newRoutes.length - 1,
1230+
}),
1231+
);
1232+
}
1233+
11061234
/**
11071235
* Pre-inserts a fullscreen route (e.g. Search) underneath the currently open RHP on narrow layout.
11081236
* The route renders behind the fullscreen RHP so that when the user later submits,
@@ -1155,6 +1283,8 @@ function preInsertFullscreenUnderRHP(route: Route) {
11551283
preInsertedFullscreenRouteName = targetRouteName;
11561284

11571285
DeviceEventEmitter.emit(CONST.MODAL_EVENTS.DISABLE_RHP_ANIMATION);
1286+
1287+
captureBufferTransaction(stateAfter, wasTabSwitched);
11581288
}
11591289

11601290
function getIsFullscreenPreInsertedUnderRHP() {
@@ -1166,6 +1296,8 @@ function getPreInsertedFullscreenRouteName() {
11661296
}
11671297

11681298
function clearFullscreenPreInsertedFlag() {
1299+
// Confirm path: keep the destination, only remove Buffer.
1300+
removeBufferRouteOnly();
11691301
isFullscreenPreInsertedUnderRHP = false;
11701302
preInsertedFullscreenRouteName = undefined;
11711303
clearPreInsertedOriginalTabRoute();
@@ -1198,13 +1330,25 @@ function removePreInsertedFullscreenIfNeeded() {
11981330
const isRHPStillOnTop = topRoute?.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR;
11991331

12001332
if (isRHPStillOnTop && routeNameToRemove) {
1333+
// Cancel-before-dismissal: strip Buffer first so the destination is directly under RHP again,
1334+
// matching what REMOVE_FULLSCREEN_UNDER_RHP expects.
1335+
removeBufferRouteOnly();
12011336
navigationRef.current?.dispatch({
12021337
type: CONST.NAVIGATION.ACTION_TYPE.REMOVE_FULLSCREEN_UNDER_RHP,
12031338
payload: {expectedRouteName: routeNameToRemove},
12041339
});
12051340
return;
12061341
}
12071342

1343+
// RHP already dismissed elsewhere (native gesture, hardware back, predictive-back). If a buffer
1344+
// transaction is live, hand off entirely to handleRHPClosedForBuffer's atomic reset (restores
1345+
// the tab + strips Buffer in ONE dispatch) instead of doing the separate, rAF-deferred restore
1346+
// below - two independent dispatches here raced each other and leaked a destination frame
1347+
// (confirmed via manual testing), the same shape as the earlier tab-index-clobbering bug.
1348+
if (bufferTransaction) {
1349+
return;
1350+
}
1351+
12081352
// RHP already dismissed. For the tab-switch path, jump back to the original tab.
12091353
// For the push path, pop the pre-inserted route directly.
12101354
const originalTabRoute = getPreInsertedOriginalTabRoute();

src/libs/Navigation/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3283,6 +3283,7 @@ type AuthScreensParamList = SharedScreensParamList &
32833283
[NAVIGATORS.SHARE_MODAL_NAVIGATOR]: NavigatorScreenParams<ShareNavigatorParamList>;
32843284
[SCREENS.BANK_CONNECTION_COMPLETE]: undefined;
32853285
[NAVIGATORS.TEST_TOOLS_MODAL_NAVIGATOR]: NavigatorScreenParams<TestToolsModalModalNavigatorParamList>;
3286+
[SCREENS.PRE_MOUNT_BUFFER]: undefined;
32863287
};
32873288

32883289
type SearchReportActionsParamList = {

src/pages/PreMountBufferPage.tsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import ActivityIndicator from '@components/ActivityIndicator';
2+
3+
import useThemeStyles from '@hooks/useThemeStyles';
4+
5+
import React from 'react';
6+
import {View} from 'react-native';
7+
8+
// Neutral placeholder shown under the RHP while a destination is pre-mounted. A native
9+
// swipe-dismiss reveals this instead of the pre-inserted destination. Never linked/deep-linkable.
10+
function PreMountBufferPage() {
11+
const styles = useThemeStyles();
12+
return (
13+
<View style={[styles.flex1, styles.appBG, styles.alignItemsCenter, styles.justifyContentCenter]}>
14+
<ActivityIndicator
15+
size="large"
16+
reasonAttributes={{context: 'PreMountBufferPage'}}
17+
/>
18+
</View>
19+
);
20+
}
21+
22+
PreMountBufferPage.displayName = 'PreMountBufferPage';
23+
24+
export default PreMountBufferPage;

0 commit comments

Comments
 (0)