Open lanitochka17 opened 1 day ago
melvin-bot[bot]
commented
1 day ago Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
lanitochka17
commented
1 day ago @strepanier03 FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
huult
commented
16 hours ago Edited by proposal-police: This proposal was edited at 2024-10-22 18:02:29 UTC.
Different navigation when closing report via app back button and device back navigation
When navigating back from a chat opened in the Chats section—either by swiping (iOS) or using the back button (Android)—the topmost screen of the BOTTOM_TAB_NAVIGATOR is HOME. This occurs because when we swipe, the navigation stack goes back through the screens in the following order: Report → Setting Workspace → BOTTOM_TAB_NAVIGATOR, resulting in the topmost screen in the BOTTOM_TAB_NAVIGATOR being Home. Therefore, when we swipe, we are taken to the Home page. Here is the log for this issue using const state = navigationRef.current?.getRootState() as State<RootStackParamList>;
This issue does not occur when using the back button, as we use pop for navigation in that case https://github.com/Expensify/App/blob/21f7842a445f4494806486ec8f741d3a25026a08/src/pages/home/ReportScreen.tsx#L283 https://github.com/Expensify/App/blob/21f7842a445f4494806486ec8f741d3a25026a08/src/libs/Navigation/Navigation.ts#L265-L276
This issue using solution same with PR 48990
When navigating back from a chat opened in the Chats section to a chat opened in the Search section, we need to update the condition to pop Home from the Bottom Tab Navigator when the case of Report → Setting Workspace → BOTTOM_TAB_NAVIGATOR has Home as the topmost screen, ensuring the same behavior as when using the back button. Something like this:
//.src/libs/Navigation/AppNavigator/beforeRemoveReportOpenedFromSearchRHP/index.native.ts#L25
function beforeRemoveReportOpenedFromSearchRHP(event: EventArg<'beforeRemove', true>) {
...
- const shouldPopHome =
- state.routes?.length >= 3 &&
- state.routes.at(-1)?.name === SCREENS.REPORT &&
- state.routes.at(-2)?.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR &&
- state.routes.at(-3)?.name === SCREENS.SEARCH.CENTRAL_PANE &&
- getTopmostBottomTabRoute(state)?.name === SCREENS.HOME;
+ const shouldPopHome =
+ (state.routes?.length >= 3 &&
+ state.routes.at(-1)?.name === SCREENS.REPORT &&
+ state.routes.at(-2)?.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR &&
+ state.routes.at(-3)?.name === SCREENS.SEARCH.CENTRAL_PANE &&
+ getTopmostBottomTabRoute(state)?.name === SCREENS.HOME) ||
+ (state.routes?.length >= 3 &&
+ state.routes.at(-1)?.name === SCREENS.REPORT &&
+ state.routes.at(-2)?.name === SCREENS.SETTINGS.WORKSPACES &&
+ state.routes.at(-3)?.name === NAVIGATORS.BOTTOM_TAB_NAVIGATOR &&
+ getTopmostBottomTabRoute(state)?.name === SCREENS.HOME);
...
}Here is test branch
c3024
commented
2 hours ago Edited by proposal-police: This proposal was edited at 2024-10-23 09:49:34 UTC.
Back button/gesture behaviour does not match with on screen back button press behaviour on native apps.
When we go to the workspaces list page, we have "Home" and "Settings Root" in BottomTabNavigator. When we open a report from there, we push another "Home" route to the BottomTabNavigator because the Report screen is a center pane and target screen is different.
https://github.com/Expensify/App/blob/45c44ee0dafff1f1878568443ec109e0de1209ea/src/libs/Navigation/linkTo/index.ts#L95-L112
When, we go back with back gesture/button on native apps, the routes are popped from the state. So, once the "Report" and "Settings Workspace" screens are popped, the "Home" screen is next. So, LHN shows up.
But, when click on screen back button when on "Settings Workspace", we readjust the state popping the recently added "Home" screen above from the "BottomTabNavigator" to reach to "Settings Root", so it navigates to Settings page. https://github.com/Expensify/App/blob/e2d99b4416410a47655a6cba6edb8787dd3c000d/src/libs/Navigation/Navigation.ts#L265-L266
Earlier, we can open a report in "central pane" on narrow layouts only from Home or Report screens because reports could be accessed only from LHN or from FAB or from a report. In all these cases, the BottomTabNavigator topMostRoute was always Home. So, this condition never executed when the screen navigated to is a report https://github.com/Expensify/App/blob/e2d99b4416410a47655a6cba6edb8787dd3c000d/src/libs/Navigation/linkTo/index.ts#L107 so, the push action of another Home never happened.
But, now, we can open report from other screens such as the workspace list page thanks to Search Router. Since, we can open report from any such screen, we would only want the report screen to be added to the state. We do not want to push another "Home" into BottomTabNavigator in these cases on narrow layouts because when we pop the last route before reaching BottomTabNavigator, we expect to navigate to the the bottom tab corresponding to this last route popped. We need pushing this "Home" only on normal screens and not on narrow screens. So, we can change this https://github.com/Expensify/App/blob/e2d99b4416410a47655a6cba6edb8787dd3c000d/src/libs/Navigation/linkTo/index.ts#L108 to
const bottomTabNavigatorRoute = rootState?.routes.findLast((route) => route.name === NAVIGATORS.BOTTOM_TAB_NAVIGATOR);
const homeExistsInBottomTab = bottomTabNavigatorRoute?.state?.routes.find((route) => route.name === SCREENS.HOME);
if (!(isNarrowLayout && homeExistsInBottomTab)) {
root.dispatch({I think there is always "Home" in bottom tab even when begin from a deeplink, so we can omit the homeExistsInBottomTab check. Since, this happens with the new Search Router we can also add this check only if bottom tab route is "Home" like
if (!(.... && matchingBottomTabRoute === SCREENS.HOME)
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.51-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A If this was caught during regression testing, add the test name, ID and link from TestRail:N/A Email or phone of affected tester (no customers): applausetester+kh081006@applause.expensifail.com Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
App will return to Account settings when the report is opened in Workspaces and swiped to the right twice
Actual Result:
App will return to LHN when the report is opened in Workspaces and swiped to the right twice. When performing similar steps using app back button, it returns to Account settings (Step 7)
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/dc49e122-f72b-4abe-a02c-1453c7457515
View all open jobs on GitHub