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

fix: WS switcher blank screen, keep workspace history #54030

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
12 changes: 3 additions & 9 deletions src/components/WorkspaceSwitcherButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,9 @@ type WorkspaceSwitcherButtonOnyxProps = {
policy: OnyxEntry<Policy>;
};

type WorkspaceSwitcherButtonProps = WorkspaceSwitcherButtonOnyxProps & {
/**
* Callback used to keep track of the workspace switching process in the BaseSidebarScreen.
*/
onSwitchWorkspace?: () => void;
};
type WorkspaceSwitcherButtonProps = WorkspaceSwitcherButtonOnyxProps;

function WorkspaceSwitcherButton({policy, onSwitchWorkspace}: WorkspaceSwitcherButtonProps) {
function WorkspaceSwitcherButton({policy}: WorkspaceSwitcherButtonProps) {
const {translate} = useLocalize();
const theme = useTheme();

Expand All @@ -41,7 +36,7 @@ function WorkspaceSwitcherButton({policy, onSwitchWorkspace}: WorkspaceSwitcherB
source: avatar,
name: policy?.name ?? '',
type: CONST.ICON_TYPE_WORKSPACE,
id: policy?.id ?? '-1',
id: policy?.id ?? CONST.DEFAULT_NUMBER_ID,
};
}, [policy]);

Expand All @@ -54,7 +49,6 @@ function WorkspaceSwitcherButton({policy, onSwitchWorkspace}: WorkspaceSwitcherB
accessible
testID="WorkspaceSwitcherButton"
onPress={() => {
onSwitchWorkspace?.();
pressableRef?.current?.blur();
interceptAnonymousUser(() => {
Navigation.navigate(ROUTES.WORKSPACE_SWITCHER);
Expand Down
7 changes: 4 additions & 3 deletions src/libs/Navigation/AppNavigator/AuthScreens.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import Log from '@libs/Log';
import NavBarManager from '@libs/NavBarManager';
import getCurrentUrl from '@libs/Navigation/currentUrl';
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import Animations from '@libs/Navigation/PlatformStackNavigation/navigationOptions/animation';
import Presentation from '@libs/Navigation/PlatformStackNavigation/navigationOptions/presentation';
import type {PlatformStackNavigationOptions} from '@libs/Navigation/PlatformStackNavigation/types';
import shouldOpenOnAdminRoom from '@libs/Navigation/shouldOpenOnAdminRoom';
Expand Down Expand Up @@ -147,7 +148,7 @@ Onyx.connect({
return;
}

currentAccountID = value.accountID ?? -1;
currentAccountID = value.accountID ?? CONST.DEFAULT_NUMBER_ID;

if (Navigation.isActiveRoute(ROUTES.SIGN_IN_MODAL)) {
// This means sign in in RHP was successful, so we can subscribe to user events
Expand Down Expand Up @@ -249,7 +250,7 @@ function AuthScreens({session, lastOpenedPublicRoomID, initialLastUpdateIDApplie
}

const initialReport = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, shouldOpenOnAdminRoom(), activeWorkspaceID);
return initialReport?.reportID ?? '';
return initialReport?.reportID;
});

useEffect(() => {
Expand Down Expand Up @@ -464,7 +465,7 @@ function AuthScreens({session, lastOpenedPublicRoomID, initialLastUpdateIDApplie
options={{
headerShown: false,
presentation: Presentation.TRANSPARENT_MODAL,
animation: 'none',
animation: Animations.NONE,
}}
getComponent={loadProfileAvatar}
listeners={modalScreenListeners}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,9 @@ type TopBarProps = {
activeWorkspaceID?: string;
shouldDisplaySearch?: boolean;
shouldDisplayCancelSearch?: boolean;

/**
* Callback used to keep track of the workspace switching process in the BaseSidebarScreen.
* Passed to the WorkspaceSwitcherButton component.
*/
onSwitchWorkspace?: () => void;
};

function TopBar({breadcrumbLabel, activeWorkspaceID, shouldDisplaySearch = true, shouldDisplayCancelSearch = false, onSwitchWorkspace}: TopBarProps) {
function TopBar({breadcrumbLabel, activeWorkspaceID, shouldDisplaySearch = true, shouldDisplayCancelSearch = false}: TopBarProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const policy = usePolicy(activeWorkspaceID);
Expand All @@ -53,10 +47,7 @@ function TopBar({breadcrumbLabel, activeWorkspaceID, shouldDisplaySearch = true,
dataSet={{dragArea: true}}
>
<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter, styles.ml2]}>
<WorkspaceSwitcherButton
policy={policy}
onSwitchWorkspace={onSwitchWorkspace}
/>
<WorkspaceSwitcherButton policy={policy} />

<View style={[styles.ml3, styles.flex1]}>
<Breadcrumbs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import type {ParamListBase} from '@react-navigation/native';
import {createNavigatorFactory} from '@react-navigation/native';
import React from 'react';
import createPlatformStackNavigatorComponent from '@libs/Navigation/PlatformStackNavigation/createPlatformStackNavigatorComponent';
import Animations from '@libs/Navigation/PlatformStackNavigation/navigationOptions/animation';
import type {ExtraContentProps, PlatformStackNavigationEventMap, PlatformStackNavigationOptions, PlatformStackNavigationState} from '@libs/Navigation/PlatformStackNavigation/types';
import BottomTabBar from './BottomTabBar';
import BottomTabNavigationContentWrapper from './BottomTabNavigationContentWrapper';
import useCustomState from './useCustomState';

const defaultScreenOptions: PlatformStackNavigationOptions = {
animation: 'none',
animation: Animations.NONE,
};

function ExtraContent({state}: ExtraContentProps) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const InternalPlatformAnimations = {
IOS_FROM_RIGHT: 'ios_from_right',
SIMPLE_PUSH: 'simple_push',
FADE: 'fade',
NONE: 'none',
} as const;

const Animations = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import type {NativeStackNavigationOptions} from '@react-navigation/native-stack';
import Animations from '..';
import {InternalPlatformAnimations} from '..';
import type NoneTransitionNavigationOptions from './types';

const none: NoneTransitionNavigationOptions = {animation: Animations.NONE, gestureEnabled: false} satisfies NativeStackNavigationOptions;
const none: NoneTransitionNavigationOptions = {
animation: InternalPlatformAnimations.NONE,
gestureEnabled: false,
} satisfies NativeStackNavigationOptions;

export default none;
13 changes: 1 addition & 12 deletions src/libs/Navigation/switchPolicyID.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {CommonActions, getActionFromState} from '@react-navigation/core';
import {getActionFromState} from '@react-navigation/core';
import type {NavigationAction, NavigationContainerRef, NavigationState, PartialState} from '@react-navigation/native';
import {getPathFromState} from '@react-navigation/native';
import type {Writable} from 'type-fest';
Expand Down Expand Up @@ -52,17 +52,6 @@ function getActionForBottomTabNavigator(action: StackNavigationAction, state: Na
params.policyID = policyID;
}

// If the last route in the BottomTabNavigator is already 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.
const homeRoute = bottomTabNavigatorRoute.state.routes.at(-1);
if (homeRoute && homeRoute.name === SCREENS.HOME) {
return {
...CommonActions.setParams(params),
source: homeRoute?.key,
};
}

// If there is no 'Home' route in the BottomTabNavigator or if we are updating a different navigator, we want to push a new route.
return {
Comment on lines -55 to -65
Copy link
Member

Choose a reason for hiding this comment

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

Simply removing this caused #54509

type: CONST.NAVIGATION.ACTION_TYPE.PUSH,
payload: {
Expand Down
7 changes: 5 additions & 2 deletions src/pages/WorkspaceSwitcherPage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import type {BrickRoad} from '@libs/WorkspacesSettingsUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import switchPolicyAfterInteractions from './switchPolicyAfterInteractions';
import WorkspaceCardCreateAWorkspace from './WorkspaceCardCreateAWorkspace';

type WorkspaceListItem = {
Expand Down Expand Up @@ -87,7 +88,9 @@ function WorkspaceSwitcherPage() {
setActiveWorkspaceID(newPolicyID);
Navigation.goBack();
if (newPolicyID !== activeWorkspaceID) {
Navigation.navigateWithSwitchPolicyID({policyID: newPolicyID});
// On native platforms, we will see a blank screen if we navigate to a new HomeScreen route while navigating back at the same time.
// Therefore we delay switching the workspace until after back navigation, using the InteractionManager.
switchPolicyAfterInteractions(newPolicyID);
}
},
[activeWorkspaceID, setActiveWorkspaceID, isFocused],
Expand All @@ -102,7 +105,7 @@ function WorkspaceSwitcherPage() {
.filter((policy) => PolicyUtils.shouldShowPolicy(policy, !!isOffline, currentUserLogin) && !policy?.isJoinRequestPending)
.map((policy) => ({
text: policy?.name ?? '',
policyID: policy?.id ?? '-1',
policyID: policy?.id,
brickRoadIndicator: getIndicatorTypeForPolicy(policy?.id),
icons: [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import {InteractionManager} from 'react-native';
import Navigation from '@libs/Navigation/Navigation';

function switchPolicyAfterInteractions(newPolicyID: string | undefined) {
InteractionManager.runAfterInteractions(() => {
Navigation.navigateWithSwitchPolicyID({policyID: newPolicyID});
});
}

export default switchPolicyAfterInteractions;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import Navigation from '@libs/Navigation/Navigation';

function switchPolicyAfterInteractions(newPolicyID: string | undefined) {
Navigation.navigateWithSwitchPolicyID({policyID: newPolicyID});
}

export default switchPolicyAfterInteractions;
21 changes: 3 additions & 18 deletions src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useEffect, useRef} from 'react';
import React, {useEffect} from 'react';
import {View} from 'react-native';
import {useOnyx} from 'react-native-onyx';
import ScreenWrapper from '@components/ScreenWrapper';
Expand All @@ -21,32 +21,18 @@ function BaseSidebarScreen() {
const activeWorkspaceID = useActiveWorkspaceFromNavigationState();
const {translate} = useLocalize();
const {shouldUseNarrowLayout} = useResponsiveLayout();
const [activeWorkspace] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${activeWorkspaceID ?? -1}`);
const [activeWorkspace] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${activeWorkspaceID ?? CONST.DEFAULT_NUMBER_ID}`);

useEffect(() => {
Performance.markStart(CONST.TIMING.SIDEBAR_LOADED);
Timing.start(CONST.TIMING.SIDEBAR_LOADED);
}, []);

const isSwitchingWorkspace = useRef(false);
useEffect(() => {
// Whether the active workspace or the "Everything" page is loaded
const isWorkspaceOrEverythingLoaded = !!activeWorkspace || activeWorkspaceID === undefined;

// If we are currently switching workspaces, we don't want to do anything until the target workspace is loaded
if (isSwitchingWorkspace.current) {
if (isWorkspaceOrEverythingLoaded) {
isSwitchingWorkspace.current = false;
}
return;
}

// Otherwise, if the workspace is already loaded, we don't need to do anything
if (isWorkspaceOrEverythingLoaded) {
if (!!activeWorkspace || activeWorkspaceID === undefined) {
return;
}

isSwitchingWorkspace.current = true;
Navigation.navigateWithSwitchPolicyID({policyID: undefined});
updateLastAccessedWorkspace(undefined);
}, [activeWorkspace, activeWorkspaceID]);
Expand All @@ -67,7 +53,6 @@ function BaseSidebarScreen() {
breadcrumbLabel={translate('common.inbox')}
activeWorkspaceID={activeWorkspaceID}
shouldDisplaySearch={shouldDisplaySearch}
onSwitchWorkspace={() => (isSwitchingWorkspace.current = true)}
/>
<View style={[styles.flex1]}>
<SidebarLinksData insets={insets} />
Expand Down
Loading