[HOLD PR #49539][$250] Search-Different navigation when closing report via app back button and device back navigation #51192

lanitochka17 · 2024-10-21T17:50:34Z

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:

Launch ND (hybrid or standalone)
Go to Account settings
Tap Workspaces
Tap on the search icon on the top right
Open any report
Tap on the app back button twice
Note that app returns to Account settings
Go back to Workspaces and tap search icon
Open any report
Swipe to the right twice using device navigation (swipe on iOS, back or swipe on Android)

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

Bug6641270_1729532054815.ScreenRecording_10-22-2024_01-27-58_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021851013558310537161
Upwork Job ID: 1851013558310537161
Last Price Increase: 2024-12-09

Issue Owner

Current Issue Owner: @ntdiary

melvin-bot · 2024-10-21T17:50:43Z

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 · 2024-10-21T17:51:16Z

@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 · 2024-10-22T17:44:44Z

Edited by proposal-police: This proposal was edited at 2024-10-22 18:02:29 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Different navigation when closing report via app back button and device back navigation

What is the root cause of that problem?

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

App/src/pages/home/ReportScreen.tsx

Line 283 in 21f7842

Navigation.goBack(undefined, false, true);

App/src/libs/Navigation/Navigation.ts

Lines 265 to 276 in 21f7842

    
               // 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}); 
        
                   } 
        
               } 
        
           }

What changes do you think we should make in order to solve the problem?

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);

        ...
        }

What alternative solutions did you explore? (Optional)

For the general case, not a specific one, we don't need to check state.routes.at(-2)?.name so that other cases can use:

//.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(-3)?.name === NAVIGATORS.BOTTOM_TAB_NAVIGATOR &&
+                getTopmostBottomTabRoute(state)?.name === SCREENS.HOME);

        ...
        }

Here is test branch

POC

Screen.Recording.2024-10-23.at.00.41.39.mov

c3024 · 2024-10-23T08:01:46Z

Edited by proposal-police: This proposal was edited at 2024-10-23 09:49:34 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Back button/gesture behaviour does not match with on screen back button press behaviour on native apps.

What is the root cause of that problem?

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.

App/src/libs/Navigation/linkTo/index.ts

Lines 95 to 112 in 45c44ee

    
           if (isCentralPaneName(action.payload.name) && (isTargetScreenDifferentThanCurrent || areParamsDifferent)) { 
        
               // We need to push a tab if the tab doesn't match the central pane route that we are going to push. 
        
               const topmostBottomTabRoute = getTopmostBottomTabRoute(rootState); 
        
               const focusedRoute = findFocusedRoute(stateFromPath); 
        
               const policyIDFromQuery = extractPolicyIDFromQuery(focusedRoute); 
        
               const matchingBottomTabRoute = getMatchingBottomTabRouteForState(stateFromPath, policyID ?? policyIDFromQuery); 
        
               const isOpeningSearch = matchingBottomTabRoute.name === SCREENS.SEARCH.BOTTOM_TAB; 
        
               const isNewPolicyID = 
        
                   ((topmostBottomTabRoute?.params as Record<string, string | undefined>)?.policyID ?? '') !== 
        
                   ((matchingBottomTabRoute?.params as Record<string, string | undefined>)?.policyID ?? ''); 
        
               if (topmostBottomTabRoute && (topmostBottomTabRoute.name !== matchingBottomTabRoute.name || isNewPolicyID || isOpeningSearch)) { 
        
                   root.dispatch({ 
        
                       type: CONST.NAVIGATION.ACTION_TYPE.PUSH, 
        
                       payload: matchingBottomTabRoute, 
        
                   }); 
        
               }

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.

App/src/libs/Navigation/Navigation.ts

Lines 265 to 266 in e2d99b4

    
           // 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.

What changes do you think we should make in order to solve the problem?

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

App/src/libs/Navigation/linkTo/index.ts

Line 107 in e2d99b4

    
           if (topmostBottomTabRoute && (topmostBottomTabRoute.name !== matchingBottomTabRoute.name || isNewPolicyID || isOpeningSearch)) {

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

App/src/libs/Navigation/linkTo/index.ts

Line 108 in e2d99b4

root.dispatch({

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)

What alternative solutions did you explore? (Optional)

melvin-bot · 2024-10-24T18:15:51Z

@strepanier03 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot · 2024-10-28T18:37:26Z

@strepanier03 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

strepanier03 · 2024-10-28T21:29:38Z

Got this tested and repro just fine. Looking at project and priority now.

melvin-bot · 2024-10-28T21:29:58Z

Job added to Upwork: https://www.upwork.com/jobs/~021851013558310537161

melvin-bot · 2024-10-28T21:30:03Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary (External)

ntdiary · 2024-10-29T15:31:27Z

test.mp4

@c3024 if we didn't push the Home, the LHN will not display correctly once we stretch the browser. 😂

As for @huult's proposal, I need more time to test some cases, will update tomorrow. :)

c3024 · 2024-10-29T16:16:47Z

Oh, I missed considering that. 😃

We can consider restricting this only to native apps like this

if (!(matchingBottomTabRoute === SCREENS.HOME && [CONST.PLATFORM.IOS, CONST.PLATFORM.ANDROID].includes(getPlatform()))

Since clicking on browser back button adjusts state based on path, it adds suitable BottomTabNavigator to the state and this is not an issue for browsers. The OP also says that this bug exists only on native apps.

ntdiary · 2024-10-30T15:26:26Z

Regarding @huult 's proposal, while it can address this issue, Search can actually be opened from many pages, so I would prefer to wait for a more universal solution. :)

test.mp4

huult · 2024-10-30T15:51:25Z

Proposal updated

Update with alternative solutions

@ntdiary Could you check it again? I removed thestate.routes.at(-2)?.name === SCREENS.SETTINGS.WORKSPACES condition so that other different settings pages can work. Thanks!

ntdiary · 2024-10-31T15:10:14Z

@ntdiary Could you check it again? I removed thestate.routes.at(-2)?.name === SCREENS.SETTINGS.WORKSPACES condition so that other different settings pages can work. Thanks!

@huult, seems to work well, will continue to test some prev cases to prevent regressions. :)

melvin-bot · 2024-11-04T16:00:19Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

ntdiary · 2024-11-04T16:14:51Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

There isn't a perfect solution at the moment. I started a discussion in #49122, will provide an update in the next couple of days. Also, I'm still open to get a more perfect solution here. :)

melvin-bot · 2024-11-18T09:04:40Z

@ntdiary @strepanier03 this issue is now 4 weeks old, please consider:

Finding a contributor to fix the bug
Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

melvin-bot · 2024-11-18T09:20:03Z

@ntdiary, @strepanier03 Huh... This is 4 days overdue. Who can take care of this?

melvin-bot · 2024-11-18T16:00:15Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot · 2024-11-20T09:08:00Z

@ntdiary, @strepanier03 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

melvin-bot · 2024-11-22T09:10:55Z

@ntdiary, @strepanier03 Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

ntdiary · 2024-11-22T12:40:06Z

not overdue, waiting for navigation refactoring. :)

melvin-bot · 2024-11-25T16:00:26Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot · 2024-11-26T09:12:30Z

@ntdiary, @strepanier03 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

ntdiary · 2024-11-26T13:32:02Z

the refactoring PR #49539 is still in progress, maybe we can set this issue to Weekly. :)

huult · 2024-11-26T13:34:05Z

If you don't want to wait, then let's go with my proposal.

melvin-bot · 2024-12-02T09:15:46Z

@ntdiary, @strepanier03 Eep! 4 days overdue now. Issues have feelings too...

melvin-bot · 2024-12-02T16:00:31Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot · 2024-12-04T09:05:30Z

@ntdiary, @strepanier03 Still overdue 6 days?! Let's take care of this!

ntdiary · 2024-12-05T12:57:09Z

Personally, I'm ok to hold this for navigation refactoring. :)

melvin-bot · 2024-12-09T09:15:09Z

@ntdiary, @strepanier03 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot · 2024-12-09T16:00:50Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

mallenexpensify · 2024-12-09T19:36:13Z

Put on hold pending PR #49539
This might be related or a dupe, it's also on hold, pending the same PR

[HOLD App#49539] [$250] iOS - Inbox becomes blank briefly after several navigation from workspace settings #53348

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 21, 2024

melvin-bot bot assigned strepanier03 Oct 21, 2024

melvin-bot bot added the Overdue label Oct 23, 2024

melvin-bot bot removed the Overdue label Oct 28, 2024

strepanier03 added the External Added to denote the issue can be worked on by a contributor label Oct 28, 2024

melvin-bot bot changed the title ~~Search-Different navigation when closing report via app back button and device back navigation~~ [$250] Search-Different navigation when closing report via app back button and device back navigation Oct 28, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 28, 2024

melvin-bot bot assigned ntdiary Oct 28, 2024

strepanier03 added this to [#whatsnext] #quality Oct 28, 2024

muttmuure moved this to LOW in [#whatsnext] #quality Oct 29, 2024

ntdiary mentioned this issue Nov 1, 2024

[HOLD for payment 2024-10-04][$250] [Search v2.4] [App] Extend the 🔍icon and search router to all pages by adding “Search” #49122

Closed

melvin-bot bot added the Overdue label Nov 4, 2024

melvin-bot bot removed the Overdue label Nov 4, 2024

melvin-bot bot added the Overdue label Nov 18, 2024

melvin-bot bot removed the Overdue label Nov 22, 2024

melvin-bot bot added the Overdue label Nov 25, 2024

melvin-bot bot removed the Overdue label Nov 26, 2024

melvin-bot bot added the Overdue label Dec 2, 2024

melvin-bot bot removed the Overdue label Dec 5, 2024

c3024 mentioned this issue Dec 5, 2024

[HOLD App#49539] [$250] iOS - Inbox becomes blank briefly after several navigation from workspace settings #53348

Open

8 tasks

melvin-bot bot added the Overdue label Dec 9, 2024

mallenexpensify added Weekly KSv2 and removed Daily KSv2 labels Dec 9, 2024

melvin-bot bot removed the Overdue label Dec 9, 2024

mallenexpensify changed the title ~~[$250] Search-Different navigation when closing report via app back button and device back navigation~~ [HOLD PR #49539][$250] Search-Different navigation when closing report via app back button and device back navigation Dec 9, 2024

mallenexpensify removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD PR #49539][$250] Search-Different navigation when closing report via app back button and device back navigation #51192

[HOLD PR #49539][$250] Search-Different navigation when closing report via app back button and device back navigation #51192

lanitochka17 commented Oct 21, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Oct 21, 2024

lanitochka17 commented Oct 21, 2024

huult commented Oct 22, 2024 •

edited

Loading

c3024 commented Oct 23, 2024 •

edited by github-actions bot

Loading

melvin-bot bot commented Oct 24, 2024

melvin-bot bot commented Oct 28, 2024

strepanier03 commented Oct 28, 2024

melvin-bot bot commented Oct 28, 2024

melvin-bot bot commented Oct 28, 2024

ntdiary commented Oct 29, 2024

c3024 commented Oct 29, 2024 •

edited

Loading

ntdiary commented Oct 30, 2024

huult commented Oct 30, 2024

ntdiary commented Oct 31, 2024

melvin-bot bot commented Nov 4, 2024

ntdiary commented Nov 4, 2024 •

edited

Loading

melvin-bot bot commented Nov 18, 2024

melvin-bot bot commented Nov 18, 2024

melvin-bot bot commented Nov 18, 2024

melvin-bot bot commented Nov 20, 2024

melvin-bot bot commented Nov 22, 2024

ntdiary commented Nov 22, 2024

melvin-bot bot commented Nov 25, 2024

melvin-bot bot commented Nov 26, 2024

ntdiary commented Nov 26, 2024

huult commented Nov 26, 2024

melvin-bot bot commented Dec 2, 2024

melvin-bot bot commented Dec 2, 2024

melvin-bot bot commented Dec 4, 2024

ntdiary commented Dec 5, 2024

melvin-bot bot commented Dec 9, 2024

melvin-bot bot commented Dec 9, 2024

mallenexpensify commented Dec 9, 2024

[HOLD PR #49539][$250] Search-Different navigation when closing report via app back button and device back navigation #51192

[HOLD PR #49539][$250] Search-Different navigation when closing report via app back button and device back navigation #51192

Comments

lanitochka17 commented Oct 21, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Oct 21, 2024

lanitochka17 commented Oct 21, 2024

huult commented Oct 22, 2024 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

c3024 commented Oct 23, 2024 • edited by github-actions bot Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Oct 24, 2024

melvin-bot bot commented Oct 28, 2024

strepanier03 commented Oct 28, 2024

melvin-bot bot commented Oct 28, 2024

melvin-bot bot commented Oct 28, 2024

ntdiary commented Oct 29, 2024

c3024 commented Oct 29, 2024 • edited Loading

ntdiary commented Oct 30, 2024

huult commented Oct 30, 2024

Proposal updated

ntdiary commented Oct 31, 2024

melvin-bot bot commented Nov 4, 2024

ntdiary commented Nov 4, 2024 • edited Loading

melvin-bot bot commented Nov 18, 2024

melvin-bot bot commented Nov 18, 2024

melvin-bot bot commented Nov 18, 2024

melvin-bot bot commented Nov 20, 2024

melvin-bot bot commented Nov 22, 2024

ntdiary commented Nov 22, 2024

melvin-bot bot commented Nov 25, 2024

melvin-bot bot commented Nov 26, 2024

ntdiary commented Nov 26, 2024

huult commented Nov 26, 2024

melvin-bot bot commented Dec 2, 2024

melvin-bot bot commented Dec 2, 2024

melvin-bot bot commented Dec 4, 2024

ntdiary commented Dec 5, 2024

melvin-bot bot commented Dec 9, 2024

melvin-bot bot commented Dec 9, 2024

mallenexpensify commented Dec 9, 2024

lanitochka17 commented Oct 21, 2024 •

edited by melvin-bot bot

Loading

huult commented Oct 22, 2024 •

edited

Loading

c3024 commented Oct 23, 2024 •

edited by github-actions bot

Loading

c3024 commented Oct 29, 2024 •

edited

Loading

ntdiary commented Nov 4, 2024 •

edited

Loading