-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Simplify the RootNavigator structure #42582
Changes from 13 commits
fb87e2e
1411be4
aa779db
87ccd3e
a381924
da6c4fb
43d4ca0
7eef033
5410620
eb2473d
118c049
e87c30a
ca681c7
c06c5dc
09ba271
28c7a3f
99d5523
a7a288a
64d29a7
19e05dd
3fc2ede
5a133a7
9bc1bec
be4d349
baaf214
e92622a
0698b3f
73721e3
9d137bd
70254e5
c905a4c
38c21a0
0efb9f0
becff51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import type {ComponentType, ForwardedRef, RefAttributes} from 'react'; | ||
import React from 'react'; | ||
import getComponentDisplayName from '@libs/getComponentDisplayName'; | ||
import FreezeWrapper from '@libs/Navigation/FreezeWrapper'; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably need some comments here and in the other file about what preparing means for native and web. |
||
export default function withPrepareCentralPaneScreen<TProps, TRef>( | ||
WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>, | ||
): (props: TProps & React.RefAttributes<TRef>) => React.ReactElement | null { | ||
function WithPrepareCentralPaneScreen(props: TProps, ref: ForwardedRef<TRef>) { | ||
return ( | ||
<FreezeWrapper> | ||
<WrappedComponent | ||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
{...props} | ||
ref={ref} | ||
/> | ||
</FreezeWrapper> | ||
); | ||
} | ||
|
||
WithPrepareCentralPaneScreen.displayName = `WithPrepareCentralPaneScreen(${getComponentDisplayName(WrappedComponent)})`; | ||
return React.forwardRef(WithPrepareCentralPaneScreen); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import type {ComponentType} from 'react'; | ||
|
||
export default function withPrepareCentralPaneScreen(WrappedComponent: ComponentType) { | ||
return WrappedComponent; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ import {View} from 'react-native'; | |
import type {OnyxEntry} from 'react-native-onyx'; | ||
import Onyx, {withOnyx} from 'react-native-onyx'; | ||
import OptionsListContextProvider from '@components/OptionListContextProvider'; | ||
import withPrepareCentralPaneScreen from '@components/withPrepareCentralPaneScreen'; | ||
import useOnboardingLayout from '@hooks/useOnboardingLayout'; | ||
import useStyleUtils from '@hooks/useStyleUtils'; | ||
import useThemeStyles from '@hooks/useThemeStyles'; | ||
|
@@ -40,11 +41,12 @@ import ROUTES from '@src/ROUTES'; | |
import SCREENS from '@src/SCREENS'; | ||
import type * as OnyxTypes from '@src/types/onyx'; | ||
import type {SelectedTimezone, Timezone} from '@src/types/onyx/PersonalDetails'; | ||
import type {CentralPaneName} from './CENTRAL_PANE_SCREENS'; | ||
import {CENTRAL_PANE_SCREENS, getCentralPaneScreenInitialParams} from './CENTRAL_PANE_SCREENS'; | ||
import createCustomStackNavigator from './createCustomStackNavigator'; | ||
import defaultScreenOptions from './defaultScreenOptions'; | ||
import getRootNavigatorScreenOptions from './getRootNavigatorScreenOptions'; | ||
import BottomTabNavigator from './Navigators/BottomTabNavigator'; | ||
import CentralPaneNavigator from './Navigators/CentralPaneNavigator'; | ||
import FeatureTrainingModalNavigator from './Navigators/FeatureTrainingModalNavigator'; | ||
import FullScreenNavigator from './Navigators/FullScreenNavigator'; | ||
import LeftModalNavigator from './Navigators/LeftModalNavigator'; | ||
|
@@ -284,20 +286,26 @@ function AuthScreens({session, lastOpenedPublicRoomID, initialLastUpdateIDApplie | |
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, []); | ||
|
||
const CentralPaneNameOptions = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am wondering if this name is good for this structure |
||
headerShown: false, | ||
title: 'New Expensify', | ||
|
||
// Prevent unnecessary scrolling | ||
cardStyle: styles.cardStyleNavigator, | ||
}; | ||
|
||
return ( | ||
<OptionsListContextProvider> | ||
<View style={styles.rootNavigatorContainerStyles(isSmallScreenWidth)}> | ||
<RootStack.Navigator isSmallScreenWidth={isSmallScreenWidth}> | ||
<RootStack.Navigator | ||
screenOptions={screenOptions.centralPaneNavigator} | ||
isSmallScreenWidth={isSmallScreenWidth} | ||
> | ||
<RootStack.Screen | ||
name={NAVIGATORS.BOTTOM_TAB_NAVIGATOR} | ||
options={screenOptions.bottomTab} | ||
component={BottomTabNavigator} | ||
/> | ||
<RootStack.Screen | ||
name={NAVIGATORS.CENTRAL_PANE_NAVIGATOR} | ||
options={screenOptions.centralPaneNavigator} | ||
component={CentralPaneNavigator} | ||
/> | ||
<RootStack.Screen | ||
name={SCREENS.VALIDATE_LOGIN} | ||
options={{ | ||
|
@@ -419,6 +427,18 @@ function AuthScreens({session, lastOpenedPublicRoomID, initialLastUpdateIDApplie | |
options={defaultScreenOptions} | ||
component={ConnectionCompletePage} | ||
/> | ||
{Object.entries(CENTRAL_PANE_SCREENS).map(([screenName, componentGetter]) => { | ||
const centralPaneName = screenName as CentralPaneName; | ||
return ( | ||
<RootStack.Screen | ||
key={centralPaneName} | ||
name={centralPaneName} | ||
initialParams={getCentralPaneScreenInitialParams(centralPaneName)} | ||
getComponent={() => withPrepareCentralPaneScreen(componentGetter())} | ||
options={CentralPaneNameOptions} | ||
/> | ||
); | ||
})} | ||
</RootStack.Navigator> | ||
</View> | ||
</OptionsListContextProvider> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import type {ValueOf} from 'type-fest'; | ||
import getCurrentUrl from '@libs/Navigation/currentUrl'; | ||
import type {CentralPaneScreensParamList} from '@libs/Navigation/types'; | ||
import CONST from '@src/CONST'; | ||
import SCREENS from '@src/SCREENS'; | ||
|
||
type CentralPaneName = keyof CentralPaneScreensParamList; | ||
|
||
type Screens = Partial<Record<CentralPaneName, () => React.ComponentType>>; | ||
|
||
const CENTRAL_PANE_SCREENS = { | ||
[SCREENS.SETTINGS.WORKSPACES]: () => require('../../../pages/workspace/WorkspacesListPage').default as React.ComponentType, | ||
[SCREENS.SETTINGS.PREFERENCES.ROOT]: () => require('../../../pages/settings/Preferences/PreferencesPage').default as React.ComponentType, | ||
[SCREENS.SETTINGS.SECURITY]: () => require('../../../pages/settings/Security/SecuritySettingsPage').default as React.ComponentType, | ||
[SCREENS.SETTINGS.PROFILE.ROOT]: () => require('../../../pages/settings/Profile/ProfilePage').default as React.ComponentType, | ||
[SCREENS.SETTINGS.WALLET.ROOT]: () => require('../../../pages/settings/Wallet/WalletPage').default as React.ComponentType, | ||
[SCREENS.SETTINGS.ABOUT]: () => require('../../../pages/settings/AboutPage/AboutPage').default as React.ComponentType, | ||
[SCREENS.SETTINGS.TROUBLESHOOT]: () => require('../../../pages/settings/Troubleshoot/TroubleshootPage').default as React.ComponentType, | ||
[SCREENS.SETTINGS.SAVE_THE_WORLD]: () => require('../../../pages/TeachersUnite/SaveTheWorldPage').default as React.ComponentType, | ||
[SCREENS.SETTINGS.SUBSCRIPTION.ROOT]: () => require('../../../pages/settings/Subscription/SubscriptionSettingsPage').default as React.ComponentType, | ||
[SCREENS.SEARCH.CENTRAL_PANE]: () => require('../../../pages/Search/SearchPage').default as React.ComponentType, | ||
[SCREENS.REPORT]: () => require('./ReportScreenWrapper').default as React.ComponentType, | ||
} satisfies Screens; | ||
|
||
const CENTRAL_PANE_SCREEN_NAMES = Object.keys(CENTRAL_PANE_SCREENS); | ||
|
||
function getCentralPaneScreenInitialParams(screenName: CentralPaneName): Partial<ValueOf<CentralPaneScreensParamList>> { | ||
const url = getCurrentUrl(); | ||
const openOnAdminRoom = url ? new URL(url).searchParams.get('openOnAdminRoom') : undefined; | ||
|
||
if (screenName === SCREENS.SEARCH.CENTRAL_PANE) { | ||
return {sortBy: CONST.SEARCH_TABLE_COLUMNS.DATE, sortOrder: CONST.SORT_ORDER.DESC}; | ||
} | ||
|
||
if (screenName === SCREENS.REPORT && openOnAdminRoom === 'true') { | ||
return {openOnAdminRoom: true}; | ||
} | ||
|
||
return undefined; | ||
} | ||
|
||
export type {CentralPaneName}; | ||
|
||
export {CENTRAL_PANE_SCREENS, CENTRAL_PANE_SCREEN_NAMES, getCentralPaneScreenInitialParams}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import React from 'react'; | ||
import type {CentralPaneNavigatorParamList, NavigationPartialRoute} from '@libs/Navigation/types'; | ||
import type {AuthScreensParamList, NavigationPartialRoute} from '@libs/Navigation/types'; | ||
|
||
const ActiveRouteContext = React.createContext<NavigationPartialRoute<keyof CentralPaneNavigatorParamList> | undefined>(undefined); | ||
const ActiveRouteContext = React.createContext<NavigationPartialRoute<keyof AuthScreensParamList> | undefined>(undefined); | ||
|
||
export default ActiveRouteContext; |
This file was deleted.
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it more correct if we use type
CentralPaneScreensParamList
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CentralPaneScreensParamList
is a subset ofAuthScreens
and it has been added to increase code readability and handle types correctly in thegetCentralPaneScreenInitialParams
function. For screens, I believe we should rely on the main set of screens.cc: @adamgrzybowski
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the navigation object should be typed with all the screens from AuthNavigator | RootStack