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

[Search v1] Fix desynchronized bottom tab and central pane after goBack to the Search #43998

Merged
Show file tree
Hide file tree
Changes from all 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
46 changes: 36 additions & 10 deletions src/libs/Navigation/Navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ import type {Report} from '@src/types/onyx';
import originalCloseRHPFlow from './closeRHPFlow';
import originalDismissModal from './dismissModal';
import originalDismissModalWithReport from './dismissModalWithReport';
import getTopmostBottomTabRoute from './getTopmostBottomTabRoute';
import getTopmostCentralPaneRoute from './getTopmostCentralPaneRoute';
import originalGetTopmostReportActionId from './getTopmostReportActionID';
import originalGetTopmostReportId from './getTopmostReportId';
import linkingConfig from './linkingConfig';
import getMatchingBottomTabRouteForState from './linkingConfig/getMatchingBottomTabRouteForState';
import linkTo from './linkTo';
import navigationRef from './navigationRef';
import setNavigationActionToMicrotaskQueue from './setNavigationActionToMicrotaskQueue';
Expand All @@ -38,8 +40,8 @@ let shouldPopAllStateOnUP = false;
/**
* Inform the navigation that next time user presses UP we should pop all the state back to LHN.
*/
function setShouldPopAllStateOnUP() {
shouldPopAllStateOnUP = true;
function setShouldPopAllStateOnUP(shouldPopAllStateFlag: boolean) {
shouldPopAllStateOnUP = shouldPopAllStateFlag;
}

function canNavigate(methodName: string, params: Record<string, unknown> = {}): boolean {
Expand Down Expand Up @@ -229,16 +231,40 @@ function goBack(fallbackRoute?: Route, shouldEnforceFallback = false, shouldPopT
const isCentralPaneFocused = isCentralPaneName(findFocusedRoute(navigationRef.current.getState())?.name);
const distanceFromPathInRootNavigator = getDistanceFromPathInRootNavigator(fallbackRoute ?? '');

// Allow CentralPane to use UP with fallback route if the path is not found in root navigator.
if (isCentralPaneFocused && fallbackRoute && distanceFromPathInRootNavigator === -1) {
navigate(fallbackRoute, CONST.NAVIGATION.TYPE.UP);
return;
if (isCentralPaneFocused && fallbackRoute) {
// Allow CentralPane to use UP with fallback route if the path is not found in root navigator.
if (distanceFromPathInRootNavigator === -1) {
navigate(fallbackRoute, CONST.NAVIGATION.TYPE.UP);
return;
}

// Add possibility to go back more than one screen in root navigator if that screen is on the stack.
if (distanceFromPathInRootNavigator > 0) {
navigationRef.current.dispatch(StackActions.pop(distanceFromPathInRootNavigator));
return;
}
Comment on lines -232 to +245
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is cosmetic only. It won't change behaviour. I just realised that the ifs are similar.

}

// Add possibility to go back more than one screen in root navigator if that screen is on the stack.
if (isCentralPaneFocused && fallbackRoute && distanceFromPathInRootNavigator > 0) {
navigationRef.current.dispatch(StackActions.pop(distanceFromPathInRootNavigator));
return;
// If the central pane is focused, it's possible that we navigated from other central pane with different matching bottom tab.
if (isCentralPaneFocused) {
const rootState = navigationRef.getRootState();
const stateAfterPop = {routes: rootState.routes.slice(0, -1)} as State<RootStackParamList>;
const topmostCentralPaneRouteAfterPop = getTopmostCentralPaneRoute(stateAfterPop);

const topmostBottomTabRoute = getTopmostBottomTabRoute(rootState as State<RootStackParamList>);
const matchingBottomTabRoute = getMatchingBottomTabRouteForState(stateAfterPop);

// If the central pane is defined after the pop action, we need to check if it's synced with the bottom tab screen.
// If not, we need to pop to the bottom tab screen/screens to sync it with the new central pane.
if (topmostCentralPaneRouteAfterPop && topmostBottomTabRoute?.name !== matchingBottomTabRoute.name) {
const bottomTabNavigator = rootState.routes.find((item: NavigationStateRoute) => item.name === NAVIGATORS.BOTTOM_TAB_NAVIGATOR)?.state;

if (bottomTabNavigator && bottomTabNavigator.index) {
const matchingIndex = bottomTabNavigator.routes.findLastIndex((item) => item.name === matchingBottomTabRoute.name);
const indexToPop = matchingIndex !== -1 ? bottomTabNavigator.index - matchingIndex : undefined;
navigationRef.current.dispatch({...StackActions.pop(indexToPop), target: bottomTabNavigator?.key});
}
}
}

navigationRef.current.goBack();
Expand Down
6 changes: 2 additions & 4 deletions src/libs/Navigation/NavigationRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,8 @@ function NavigationRoot({authenticated, lastVisitedPath, initialUrl, onReady}: N
firstRenderRef.current = false;
return;
}
if (!isSmallScreenWidth) {
return;
}
Navigation.setShouldPopAllStateOnUP();

Navigation.setShouldPopAllStateOnUP(!isSmallScreenWidth);
}, [isSmallScreenWidth]);

const handleStateChange = (state: NavigationState | undefined) => {
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ function ReportScreen({
}
Navigation.dismissModal();
if (Navigation.getTopmostReportId() === prevOnyxReportID) {
Navigation.setShouldPopAllStateOnUP();
Navigation.setShouldPopAllStateOnUP(true);
Navigation.goBack(undefined, false, true);
}
if (prevReport.parentReportID) {
Expand Down
Loading