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.

Sign up for GitHub

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

Fix: WS switcher and LHN are not reset to Expensify after leaving #45285

Merged
merged 14 commits into from
Jul 31, 2024
Merged

Fix: WS switcher and LHN are not reset to Expensify after leaving #45285

Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
bc0168d
Fix: WS switcher and LHN are not reset to Expensify after leaving
truph01 Jul 11, 2024
f306ab2
Fix: Move effect to BottomTabNavigator
truph01 Jul 11, 2024
d554f68
Fix: Only subscribe active workspace ID
truph01 Jul 16, 2024
f490e28
merge main
truph01 Jul 18, 2024
2778283
Fix: Resolve conflict
truph01 Jul 23, 2024
ddd1819
merge main
truph01 Jul 24, 2024
6c2550f
Fix: Move useEffect to BaseSidebarScreen
truph01 Jul 24, 2024
6885046
Fix: Lint
truph01 Jul 24, 2024
9253a2c
Fix: Lint
truph01 Jul 24, 2024
92bb5aa
Fix: Lint
truph01 Jul 24, 2024
08f2deb
rename variable
truph01 Jul 25, 2024
6e76a9d
Feat: Merge latest main
truph01 Jul 30, 2024
baaca67
Feat: Remove didResetActiveWorkspaceIDRef
truph01 Jul 30, 2024
e1c650d
Feat: Move useOnyx before useEffect
truph01 Jul 30, 2024
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
22 changes: 21 additions & 1 deletion src/libs/Navigation/AppNavigator/Navigators/BottomTabNavigator.tsx
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Wasn't this changed supposed to be on the SidebarScreen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the SidebarScreen is always mounted again once we leave the workspace then return to LHN. Because it is mounted again, so the logic:

!selectedPolicy && !prevSelectedPolicy

that used in my solution is always false because both of them are undefined when we leave the workspace then return to LHN.

Copy link
Contributor

Choose a reason for hiding this comment

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

Move the logic back to the Sidebar and in the useEffect only check for the current selected policy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s77rt As I said above, we cannot move the logic to Sidebar. Can you help check again?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why we can't go with the suggested solution above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I explained above

Copy link
Contributor

Choose a reason for hiding this comment

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

both of them are undefined

You should check only the current selected policy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah you mean we should !selectedPolicy instead of !selectedPolicy && !prevSelectedPolicy, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes

Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
import {useNavigationState} from '@react-navigation/native';
import type {StackNavigationOptions} from '@react-navigation/stack';
import React from 'react';
import React, {useEffect} from 'react';
import {useOnyx} from 'react-native-onyx';
import useActiveWorkspace from '@hooks/useActiveWorkspace';
import usePrevious from '@hooks/usePrevious';
import {updateLastAccessedWorkspace} from '@libs/actions/Policy/Policy';
import createCustomBottomTabNavigator from '@libs/Navigation/AppNavigator/createCustomBottomTabNavigator';
import getTopmostBottomTabRoute from '@libs/Navigation/getTopmostBottomTabRoute';
import getTopmostCentralPaneRoute from '@libs/Navigation/getTopmostCentralPaneRoute';
import Navigation from '@libs/Navigation/Navigation';
import type {BottomTabNavigatorParamList, BottomTabScreensParamList, NavigationPartialRoute, RootStackParamList} from '@libs/Navigation/types';
import {isBottomTabName} from '@libs/NavigationUtils';
import SidebarScreen from '@pages/home/sidebar/SidebarScreen';
import SearchPageBottomTab from '@pages/Search/SearchPageBottomTab';
import ONYXKEYS from '@src/ONYXKEYS';
import SCREENS from '@src/SCREENS';
import type ReactComponentModule from '@src/types/utils/ReactComponentModule';
import ActiveBottomTabRouteContext from './ActiveBottomTabRouteContext';
Expand Down Expand Up @@ -35,6 +41,20 @@ function BottomTabNavigator() {

return route;
});

const {activeWorkspaceID} = useActiveWorkspace();
const [selectedPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${activeWorkspaceID ?? -1}`);
const prevSelectedPolicy = usePrevious(selectedPolicy);
useEffect(() => {
if (!selectedPolicy && !prevSelectedPolicy) {
return;
}
if (prevSelectedPolicy && !selectedPolicy && activeWorkspaceID !== undefined) {
Navigation.navigateWithSwitchPolicyID({policyID: undefined});
updateLastAccessedWorkspace(undefined);
}
}, [prevSelectedPolicy, selectedPolicy, activeWorkspaceID]);

return (
<ActiveBottomTabRouteContext.Provider value={activeRoute}>
<Tab.Navigator screenOptions={screenOptions}>
Expand Down
Loading