-
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 21 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 platform. On native platforms screens that aren't already dipslayed in the navigation stack should be freezed to prevent from unnecessary rendering. | ||
mountiny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* It's handled this way only on mobile platforms, because on web more than one screen is displayed in a wide layout, so these screens shouldn't be freezed. | ||
mountiny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
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 platform. On native platforms screens that aren't already dipslayed in the navigation stack should be freezed to prevent from unnecessary rendering. | ||
mountiny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* It's handled this way only on mobile platforms, because on web more than one screen is displayed in a wide layout, so these screens shouldn't be freezed. | ||
mountiny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
export default function withPrepareCentralPaneScreen(WrappedComponent: ComponentType) { | ||
return WrappedComponent; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import type {CentralPaneName} from '@libs/Navigation/types'; | ||
import SCREENS from '@src/SCREENS'; | ||
|
||
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]: () => 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; | ||
|
||
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