[$250] Hybrid - Android - WS Switcher - App is closed when switching workspace and using device back button.

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.71-0
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: No, reproducible on hybrid only
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): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Launch the Expensify app.
2. Tap on the workspace switcher on the top left corner.
3. Select any workspace.
4. Use the device back button.
5. Verify that now all the chats are displayed again on inbox.

Expected Result:

After switching workspaces and using device back button, all the chats should be displayed again on inbox.

Actual Result:

The app is closed instead of displaying all chats again, when the user switch workspaces and uses device back button. When the app is reopened, all chats are displayed in inbox.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6683901_1733291014764.WS_Switch.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021864885051135538956
• Upwork Job ID: 1864885051135538956
• Last Price Increase: 2024-12-06

Issue Owner

Current Issue Owner: @eh2077

[melvin-bot]

Triggered auto assignment to @RachCHopkins (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.

[RachCHopkins]

Can Repro - When I hit the device back button it closes the app entirely. I am on 9.0.71-1

[melvin-bot]

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

[melvin-bot]

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

[ikevin127]

Proposal

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

When the user switches to a specific workspace chats and then uses device back button, the app is closed instead of displaying all chats again (Expensify).

What is the root cause of that problem?

Note

The issue is present on Android: Native as well as on Android: HybridApp (more details below).

In the WorkspaceSwitcherPage we have the selectPolicy function which is called when we switch to a specific workspace chats. If we follow Navigation.navigateWithSwitchPolicyID down to source we arrive to the switchPolicyID function where at the bottom we get to the root cause of why, when we use the device back button the app will close instead of returning back to all chats (Expensify):

App/src/libs/Navigation/switchPolicyID.ts

Lines 150 to 156 in b595f5d

	} else {
	// If the layout is small we need to pop everything from the central pane so the bottom tab navigator is visible.
	root.dispatch({
	type: 'POP_TO_TOP',
	target: rootState.key,
	});
	}

as this block comes into action on narrow layout devices because of this condition:

App/src/libs/Navigation/switchPolicyID.ts

Lines 115 to 116 in b595f5d

	// The correct route for SearchPage is located in the CentralPane
	const shouldAddToCentralPane = !getIsNarrowLayout() || isOpeningSearchFromBottomTab;

Because that block pops all central pane routes out in order for the bottom tab navigator to be visible, this means that the only route present in the stack is BottomTabNavigator which is the same as when we open the app -> which when pressing hardware / device back button it will close the app as expected because there's no other route to go back to.

What's the code responsible for handling the hardware / device back button and close the app on Android ?

We have this android platform specific function setupCustomAndroidBackHandler which is used to dictate the behaviour and if we look at its code, there is this block which is Android: HybridApp specific that tells the app to close when isLastScreenOnStack is true which is the case for our issue:

App/src/libs/Navigation/setupCustomAndroidBackHandler/index.android.ts

Lines 27 to 29 in b595f5d

	if (NativeModules.HybridAppModule && isLastScreenOnStack) {
	NativeModules.HybridAppModule.exitApp();
	}

For Android: Native though we don't have any code in this function to specifically handle behaviour, therefore it defaults to the standard behaviour (close the app) at the end of the function:

App/src/libs/Navigation/setupCustomAndroidBackHandler/index.android.ts

Lines 66 to 67 in b595f5d

	// Handle all other cases with default handler.
	return false;

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

To solve our issue on both Android: Native and HybridApp we have to implement the following changes:

1. Under:

   App/src/libs/Navigation/setupCustomAndroidBackHandler/index.android.ts

   Line 25 in b595f5d

   const isLastScreenOnStack = bottomTabRoutes.length === 1 && rootState?.routes?.length === 1;

add the following logic:

const isWorkspaceHomeOnlyRoute = bottomTabRoutes.every((route) => route.name === SCREENS.HOME && !!(route.params as {policyID?: string})?.policyID);

if (isLastScreenOnStack && isWorkspaceHomeOnlyRoute) {
    navigationRef.dispatch({...StackActions.replace(SCREENS.HOME)});
    return true;
}

Explanation

1. isWorkspaceHomeOnlyRoute checks whether in bottomTabRoutes (array) every Home route has the policyID param
2. If isLastScreenOnStack and isWorkspaceHomeOnlyRoute are both true - this will always be the case when we are on the specific workspace chats home screen since there is only one route in the stack and it is the Home route with policyID param
3. When we go back with device back button and this condition is true -> we will replace the Home screen with policyID param with the Home screen without any params, which is the Expensify home page with all chats.

This will fix the issue as required by the Expected result (see video below) because when we will perform the go back action we will have the Home screen in the stack and will be navigated to Home (all chats), then if we go back again from Home then the app will close as expected.

Additional explanation regarding Android: HybridApp -> under the new logic added above we will keep the Android: HybridApp logic unchanged since in case the newly added condition doesn't pass it means we want to keep the existing NativeModules.HybridAppModule.exitApp() logic because without this I assume the app will not close as the Android: Native app does by default.

cc @adamgrzybowski @Kicu for a take on the proposed solution as they worked on setupCustomAndroidBackHandler

Result video (before / after)

Android: Native

Before	After
before.mp4	after.mp4

[linhvovan29546]

Proposal

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

Android - WS Switcher - App is closed when switching workspace and using device back button.

What is the root cause of that problem?

• The issue originates from this commit in the PR: Migrate E/App to use PlatformStackNavigation.
  • Prior to this change, the problem did not exist.
  • The root cause is the use of the Replace navigation method instead of Push.
  • When switching to another workspace and pressing the back button, the app closes because the stack contains only one entry, which triggers this condition.

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

• When switching workspaces from WorkspaceSwitcherPage, we should use the Push navigation method instead of Replace. To implement we need:
• Update the getActionForBottomTabNavigator function
  • Add an optional prop shouldReplaceHome with a default value of true here
    

    App/src/libs/Navigation/switchPolicyID.ts

    Line 29 in 7925c9b

    function getActionForBottomTabNavigator(action: StackNavigationAction, state: NavigationState<RootStackParamList>, policyID?: string): Writable<NavigationAction> | undefined {

  • And here
    

    App/src/libs/Navigation/switchPolicyID.ts

    Line 76 in 7925c9b

    export default function switchPolicyID(navigation: NavigationContainerRef<RootStackParamList> | null, {policyID, route}: SwitchPolicyIDParams) {

    
    

    App/src/libs/Navigation/switchPolicyID.ts

    Line 102 in 7925c9b

    const actionForBottomTabNavigator = getActionForBottomTabNavigator(action, rootState, policyID);

    App/src/libs/Navigation/switchPolicyID.ts

    Line 29 in 7925c9b

    function getActionForBottomTabNavigator(action: StackNavigationAction, state: NavigationState<RootStackParamList>, policyID?: string): Writable<NavigationAction> | undefined {

• Update the navigateWithSwitchPolicyID function in WorkspaceSwitcherPage:
  • Pass shouldReplaceHome: false .
  • This ensures the navigation method is Push instead of Replace when called from this screen.
  • (Optional) Add a condition to ensure this change only applies to Android.

Results

### Demo Videos

Before	After
back-button-before.mp4	back-button-after.mp4

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

Because this issue come from this, we need to check other related cases.

What alternative solutions did you explore? (Optional)

N/A

[saifelance]

Proposal

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

App closes when switching workspaces and pressing the back button, instead of showing Inbox.

What is the root cause of that problem?

Navigation stack isn't updated correctly, causing the app to exit on back press.

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

The first block updates the stack to push SCREENS.HOME or SCREENS.INBOX after workspace switch, directing the user to the Inbox or Home screen. The second block handles back button presses, ensuring the app navigates to Inbox instead of exiting. The third block updates the central pane and handles the navigation stack based on the layout.

1. Handle Back Button Navigation Correctly

if (checkIfActionPayloadNameIsEqual(actionForBottomTabNavigator, SCREENS.HOME)) {
    delete params.reportID;

    // Ensure the navigation stack is updated to include SCREENS.HOME or SCREENS.INBOX
    root.dispatch({
        type: CONST.NAVIGATION.ACTION_TYPE.PUSH,
        payload: {
            name: SCREENS.HOME,  // Or SCREENS.INBOX if needed
            params,
        },
    });
}

2. Prevent App Exit on Back Press

// Handle back press fallback to prevent app exit
root.dispatch({
    type: CONST.NAVIGATION.ACTION_TYPE.PUSH,
    payload: {
        name: SCREENS.INBOX,  // Redirect to Inbox instead of closing the app
        params: {},
    },
});

3. Central Pane Updates Correctly

if (shouldAddToCentralPane) {
    root.dispatch({
        type: CONST.NAVIGATION.ACTION_TYPE.PUSH,
        payload: {
            name: topmostCentralPaneRoute?.name || SCREENS.HOME,  // Default to HOME if undefined
            params,
        },
    });
} else {
    // If the layout is narrow, pop the stack to show the bottom tab navigator
    root.dispatch({
        type: 'POP_TO_TOP',
        target: rootState.key,
    });
}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

• Test workspace switching and back button behavior.
• Verify stack integrity after multiple switches.
• Test error scenarios (e.g., offline, app backgrounding).

What alternative solutions did you explore? (Optional)

• Frontend fallback to redirect to Inbox.
• Backend consistency improvements.
• Store previous navigation stack.

[melvin-bot]

@RachCHopkins, @eh2077 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[eh2077]

Reviewing proposals

[adamgrzybowski]

I am unsure why we decided to use setParams for switching the policyID. It replaces an entry in history instead of pushing a new screen with a new policy which is an additive action. This change seems to be the root cause of this problem.

But if we assume that switching policy with setParams is the way, then your @ikevin127 proposal makes sense to me.

One thing worth mentioning is that if we go
global > policyX > policyY and then back, the policy I expect would be "policyX" and the outcome is the policy will be set to global.

However, I'm not sure this is that important, considering that with the changes to setParams, history no longer works for policies anyway.

Alternatively, the second proposal by @linhvovan29546 suggests going back to the old method with PUSH instead of REPLACE. That makes even more sense to me as it should fix the history but we would need to understand which problem was supposed to be solved by using setParams

cc: @mountiny you seem to have some context about native stacks @WojtekBoman as policy police

[mountiny]

// If there already is a 'Home' route, we want to change the params rather than pushing a new 'Home' route,
// so that the screen does not get re-mounted. This would cause an empty screen/white flash when navigating back from the workspace switcher.

I think that @chrispader @kirillzyusko added this to fix a bug they encountered, but I agree that if this ruins the history its not the best solution and we might want to rethink it.

But we need to make sure there is no regression after switching back to that.

[ikevin127]

But if we assume that switching policy with setParams is the way

I think that's the way as it was done on purpose in this PR:

• update: Update workspace switcher to look and act more like a filter. #53102

This is the reason why I mentioned using replace in my proposal because based on that recent PR I assumed we don't want to keep history of switching in-between workspaces.

But if we do, my proposal can be flexible and details like replacing the navigation method with push instead of my proposal's suggested replace can surely be handled during PR.

I don't consider other proposals mentioning push instead of replace a significant enough difference from my proposal to constitute a new proposal given our contributor guidelines mentioning:

Note: Before submitting a proposal on an issue, be sure to read any other existing proposals. ALL NEW PROPOSALS MUST BE DIFFERENT FROM EXISTING PROPOSALS. The difference should be important, meaningful or considerable.

Another reason would be that the Expected result clearly states:

After switching workspaces and using device back button, all the chats should be displayed again on inbox.

and there were no details mentioning the expected behaviour when switching between multiple workspaces then attempting the hardware go back button.

[Kicu]

I can throw my 2 cents here, I was responsible for making sure workspace switcher behaves correctly in search pages.
I think at this point in the app (imo) we are sitting on the fence. Some places we treat WS switcher like a global switch/filter (so replace) but in others we treat it much more like a part of search history and (important) we keep it in URL.

When I was working on search I made the policyID work correctly with the data from URL, so that these steps work:

1. go to Inbox
2. choose a policy (id: 1) via workspace switcher
3. go to Search (via bottom bar)
4. in search policy is the same
5. choose a different policy (id: 2) (it changes in the URL)
6. press back button
7. you land on Inbox with policy 1 because that was in the history

So as you can see at least in ☝️ flow, policyID behaves more like stacked history.

I don't have strong opinions which is better, but if we move into the direction of replace then we should check other places in the app and make them behave like that as well.

[mountiny]

I agree with @Kicu I think that the policyId should be kept in history so you can go back to the previously chosen policy

This was just an oversight on our part as its quite niche flow @chrispader @kirillzyusko Could you please weigh in when you get a moment?

[linhvovan29546]

I think that @chrispader @kirillzyusko added this to fix a bug they encountered, but I agree that if this ruins the history its not the best solution and we might want to rethink it.

But we need to make sure there is no regression after switching back to that.

@mountiny I think the code changes to fix this issue. For the blank screen issue, it seems to be related to native-stack. However I can fix it by running Navigation.navigateWithSwitchPolicyID inside requestAnimationFrame in the JS.

[kirillzyusko]

@mountiny I think the problem comes from the changes made by @chrispader? If so I would wait when @chrispader returns from its vacation and can shed more light on the issue (because I'm not aware what @chrispader was attempting to solve)

For me it looks like there was a Home screen that was re-mounting somehow and @chrispader fixed it via replace calls, but it produced other issues.

After reading this conversation I think that the fix that @chrispader made could produce #53359 as well (or at the problem is very highly related to each other).

Also what @Kicu explains makes sense to me. So maybe we just need to go back with push approach (assuming it will fix this problem) and check that a reported issue not reproducible anymore? In this case this:

I think the code changes to fix #49937 (comment). For the blank screen issue, it seems to be related to native-stack. However I can fix it by running Navigation.navigateWithSwitchPolicyID inside requestAnimationFrame in the JS.

Makes more sense for me/sounds like a better approach 👍

[chrispader]

Yes, so the problem i was trying to solve was that we would see a blank HomeScreen for a brief moment the when we would change the workspace, because the screen is re-mounted and therefore re-renders.

Under the previous stack navigation, this was not causing issues, but with @react-navigation/native-stack we can see that the mounting and rendering of the screen content is still ongoing while the screen is already displayed.

I was not aware, that we want to be able to keep a "workspace history" that we can go back to. So the fix with setParams is not going to work in this case.

This is the original i and @kirillzyusko are referring to, reported by @ishpaul777: #49937 (comment)

Migrate.E.App.PlatformStackNavigation.mov

[chrispader]

@mountiny I think the code changes to fix this issue. For the blank screen issue, it seems to be related to native-stack. However I can fix it by running Navigation.navigateWithSwitchPolicyID inside requestAnimationFrame in the JS.

Putting Navigation.navigateWithSwitchPolicyID in requestAnimationFrame() doesn't fix the problem for me. This is what i get:

ScreenRecording_12-11-2024.15-16-44_1.MP4

[chrispader]

A fix that removes the blank screen but basically just delays the route push and the content re-rendering is putting the Navigation.navigateWithSwitchPolicyID in an InteractionManager.runAfterInteractions(...). This is not as beautiful/sexy UI-wise as it was before NativeStack, but it doesn't look as weird as a blank screen:

ScreenRecording_12-11-2024.15-24-18_1.MP4

wdyt? @mountiny @kirillzyusko

[chrispader]

@kirillzyusko i can confirm that the changes discussed are also causing #53359. This change also fixes the issue on the search page:

ScreenRecording_12-11-2024.16.MP4

[eh2077]

Not overdue, valuable discussions continue!

[chrispader]

@mountiny @eh2077 is somebody already assigned/working on this? Lmk if i should prepare a PR, otherwise here are the changes needed to revert my previous changes and apply the InteractionManager.runAfterInteractions() fix:

#54030

[eh2077]

@chrispader Cool! The InteractionManager.runAfterInteractions() solution also makes sense to me. I think we can move this forward with @chrispader 's fix here #54030

Edited: We can revisit this issue after reworking the original PR.

@mountiny All yours!

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @Beamanator, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[mountiny]

Sounds good, lets make sure this is working fine on web too