-
Notifications
You must be signed in to change notification settings - Fork 3k
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 all 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,27 @@ | ||
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. |
||
/** | ||
* This HOC is dependent on the platform. On native platforms, screens that aren't already displayed in the navigation stack should be frozen to prevent unnecessary rendering. | ||
* It's handled this way only on mobile platforms because on the web, more than one screen is displayed in a wide layout, so these screens shouldn't be frozen. | ||
*/ | ||
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,9 @@ | ||
import type {ComponentType} from 'react'; | ||
|
||
/** | ||
* This HOC is dependent on the platform. On native platforms, screens that aren't already displayed in the navigation stack should be frozen to prevent unnecessary rendering. | ||
* It's handled this way only on mobile platforms because on the web, more than one screen is displayed in a wide layout, so these screens shouldn't be frozen. | ||
*/ | ||
export default function withPrepareCentralPaneScreen(WrappedComponent: ComponentType) { | ||
return WrappedComponent; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import type {CentralPaneName} from '@libs/Navigation/types'; | ||
import withPrepareCentralPaneScreen from '@src/components/withPrepareCentralPaneScreen'; | ||
import SCREENS from '@src/SCREENS'; | ||
import type ReactComponentModule from '@src/types/utils/ReactComponentModule'; | ||
|
||
type Screens = Partial<Record<CentralPaneName, () => React.ComponentType>>; | ||
|
||
const CENTRAL_PANE_SCREENS = { | ||
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. BugZero Checklist These changes caused this issue |
||
[SCREENS.SETTINGS.WORKSPACES]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/workspace/WorkspacesListPage').default), | ||
[SCREENS.SETTINGS.PREFERENCES.ROOT]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/settings/Preferences/PreferencesPage').default), | ||
[SCREENS.SETTINGS.SECURITY]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/settings/Security/SecuritySettingsPage').default), | ||
[SCREENS.SETTINGS.PROFILE.ROOT]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/settings/Profile/ProfilePage').default), | ||
[SCREENS.SETTINGS.WALLET.ROOT]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/settings/Wallet/WalletPage').default), | ||
[SCREENS.SETTINGS.ABOUT]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/settings/AboutPage/AboutPage').default), | ||
[SCREENS.SETTINGS.TROUBLESHOOT]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/settings/Troubleshoot/TroubleshootPage').default), | ||
[SCREENS.SETTINGS.SAVE_THE_WORLD]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/TeachersUnite/SaveTheWorldPage').default), | ||
[SCREENS.SETTINGS.SUBSCRIPTION.ROOT]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/settings/Subscription/SubscriptionSettingsPage').default), | ||
[SCREENS.SEARCH.CENTRAL_PANE]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('../../../pages/Search/SearchPage').default), | ||
[SCREENS.REPORT]: () => withPrepareCentralPaneScreen(require<ReactComponentModule>('./ReportScreenWrapper').default), | ||
} satisfies Screens; | ||
|
||
export default CENTRAL_PANE_SCREENS; |
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