From 05484384165f7c02e7f8f5c9bb38f8d2b1356cbd Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 4 Jun 2024 14:29:07 -0700 Subject: [PATCH 1/6] Search bottom-up to check if a component is inside a modal navigator --- src/hooks/useResponsiveLayout.ts | 17 ++++++++++++----- .../Navigators/LeftModalNavigator.tsx | 7 +++++-- .../Navigators/RightModalNavigator.tsx | 7 +++++-- src/libs/Navigation/Navigation.ts | 1 + 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/hooks/useResponsiveLayout.ts b/src/hooks/useResponsiveLayout.ts index 18b29653479c..d417f276fb6e 100644 --- a/src/hooks/useResponsiveLayout.ts +++ b/src/hooks/useResponsiveLayout.ts @@ -1,8 +1,8 @@ +import {NavigationContainerRefContext, NavigationContext} from '@react-navigation/native'; import {useContext} from 'react'; import ModalContext from '@components/Modal/ModalContext'; -import Navigation from '@libs/Navigation/Navigation'; import CONST from '@src/CONST'; -import useRootNavigationState from './useRootNavigationState'; +import NAVIGATORS from '@src/NAVIGATORS'; import useWindowDimensions from './useWindowDimensions'; type ResponsiveLayoutResult = { @@ -38,8 +38,15 @@ export default function useResponsiveLayout(): ResponsiveLayoutResult { // This means it will only be defined if the component calling this hook is a child of a modal component. See BaseModal for the provider. const {activeModalType} = useContext(ModalContext); - // This refers to the state of the root navigator, and is true if and only if the topmost navigator is the "left modal navigator" or the "right modal navigator" - const isDisplayedInModalNavigator = !!useRootNavigationState(Navigation.isModalNavigatorActive); + // We are using these contexts directly instead of useNavigation/useNavigationState, because those will throw an error if used outside a navigator. + // This hook can be used within or outside a navigator, so using useNavigationState does not work. + // Furthermore, wrapping useNavigationState in a try/catch does not work either, because that breaks the rules of hooks. + const navigationContainerRef = useContext(NavigationContainerRefContext); + const navigator = useContext(NavigationContext); + const currentNavigator = navigator ?? navigationContainerRef; + + const isDisplayedInNarrowModalNavigator = + !!currentNavigator?.getParent(NAVIGATORS.RIGHT_MODAL_NAVIGATOR as unknown as undefined) || !!currentNavigator?.getParent(NAVIGATORS.LEFT_MODAL_NAVIGATOR as unknown as undefined); // The component calling this hook is in a "narrow pane modal" if: const isInNarrowPaneModal = @@ -47,7 +54,7 @@ export default function useResponsiveLayout(): ResponsiveLayoutResult { activeModalType === CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED || // or there's a "right modal navigator" or "left modal navigator" on the top of the root navigation stack // and the component calling this hook is not the child of another modal type, such as a confirm modal - (isDisplayedInModalNavigator && !activeModalType); + (isDisplayedInNarrowModalNavigator && !activeModalType); const shouldUseNarrowLayout = isSmallScreenWidth || isInNarrowPaneModal; diff --git a/src/libs/Navigation/AppNavigator/Navigators/LeftModalNavigator.tsx b/src/libs/Navigation/AppNavigator/Navigators/LeftModalNavigator.tsx index 43241a431c32..b1cfb8e5d9a1 100644 --- a/src/libs/Navigation/AppNavigator/Navigators/LeftModalNavigator.tsx +++ b/src/libs/Navigation/AppNavigator/Navigators/LeftModalNavigator.tsx @@ -7,7 +7,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import ModalNavigatorScreenOptions from '@libs/Navigation/AppNavigator/ModalNavigatorScreenOptions'; import type {AuthScreensParamList, LeftModalNavigatorParamList} from '@libs/Navigation/types'; -import type NAVIGATORS from '@src/NAVIGATORS'; +import NAVIGATORS from '@src/NAVIGATORS'; import SCREENS from '@src/SCREENS'; import Overlay from './Overlay'; @@ -32,7 +32,10 @@ function LeftModalNavigator({navigation}: LeftModalNavigatorProps) { /> )} - + )} - + Date: Tue, 4 Jun 2024 14:30:20 -0700 Subject: [PATCH 2/6] Remove unused code --- src/hooks/useRootNavigationState.ts | 30 ----------------------------- src/libs/Navigation/Navigation.ts | 12 ------------ 2 files changed, 42 deletions(-) delete mode 100644 src/hooks/useRootNavigationState.ts diff --git a/src/hooks/useRootNavigationState.ts b/src/hooks/useRootNavigationState.ts deleted file mode 100644 index 3f03e1ceb253..000000000000 --- a/src/hooks/useRootNavigationState.ts +++ /dev/null @@ -1,30 +0,0 @@ -import type {EventListenerCallback, NavigationContainerEventMap} from '@react-navigation/native'; -import type {NavigationState} from '@react-navigation/routers'; -import {useCallback, useSyncExternalStore} from 'react'; -import {navigationRef} from '@libs/Navigation/Navigation'; - -/** - * This hook is a replacement for `useNavigationState` for nested navigators. - * If `useNavigationState` is used within a nested navigator then the state that's returned is the state of the nearest parent navigator, - * not the root navigator state representing the whole app's navigation tree. - * - * Use with caution, because re-rendering any component every time the root navigation state changes can be very costly for performance. - * That's why the selector is mandatory. - */ -function useRootNavigationState(selector: (state: NavigationState) => T): T | undefined { - const getSnapshot = useCallback(() => { - if (!navigationRef?.current) { - return; - } - return selector(navigationRef.current.getRootState()); - }, [selector]); - - const subscribeToRootState = useCallback((callback: EventListenerCallback) => { - const unsubscribe = navigationRef?.current?.addListener('state', callback); - return () => unsubscribe?.(); - }, []); - - return useSyncExternalStore(subscribeToRootState, getSnapshot); -} - -export default useRootNavigationState; diff --git a/src/libs/Navigation/Navigation.ts b/src/libs/Navigation/Navigation.ts index 14cd8567c0e6..8a083c28a60f 100644 --- a/src/libs/Navigation/Navigation.ts +++ b/src/libs/Navigation/Navigation.ts @@ -357,17 +357,6 @@ function navigateWithSwitchPolicyID(params: SwitchPolicyIDParams) { return switchPolicyID(navigationRef.current, params); } -/** - * Check if the modal is being displayed. - * - * @param state - MUST be the state of the root navigator for this to work. Do not use a child navigator state. - */ -function isModalNavigatorActive(state: NavigationState) { - const lastRoute = state?.routes?.at(-1); - const lastRouteName = lastRoute?.name; - return lastRouteName === NAVIGATORS.LEFT_MODAL_NAVIGATOR || lastRouteName === NAVIGATORS.RIGHT_MODAL_NAVIGATOR; -} - export default { setShouldPopAllStateOnUP, navigate, @@ -387,7 +376,6 @@ export default { parseHybridAppUrl, navigateWithSwitchPolicyID, resetToHome, - isModalNavigatorActive, closeRHPFlow, setNavigationActionToMicrotaskQueue, }; From 1527973a84971bf4a892bfb9dd812a14eddeabe4 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 4 Jun 2024 14:32:43 -0700 Subject: [PATCH 3/6] Remove unused imports --- src/libs/Navigation/Navigation.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libs/Navigation/Navigation.ts b/src/libs/Navigation/Navigation.ts index 8a083c28a60f..34e093f2b74b 100644 --- a/src/libs/Navigation/Navigation.ts +++ b/src/libs/Navigation/Navigation.ts @@ -1,7 +1,6 @@ import {findFocusedRoute} from '@react-navigation/core'; -import type {EventArg, NavigationContainerEventMap, NavigationState} from '@react-navigation/native'; +import type {EventArg, NavigationContainerEventMap} from '@react-navigation/native'; import {CommonActions, getPathFromState, StackActions} from '@react-navigation/native'; -import type {ValueOf} from 'type-fest'; import Log from '@libs/Log'; import * as ReportUtils from '@libs/ReportUtils'; import {getReport} from '@libs/ReportUtils'; From 382d4be9d958224e9cede49ecdd93b264aa54031 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 4 Jun 2024 14:41:52 -0700 Subject: [PATCH 4/6] Improve comment --- src/hooks/useResponsiveLayout.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hooks/useResponsiveLayout.ts b/src/hooks/useResponsiveLayout.ts index d417f276fb6e..e530c13c729c 100644 --- a/src/hooks/useResponsiveLayout.ts +++ b/src/hooks/useResponsiveLayout.ts @@ -41,6 +41,7 @@ export default function useResponsiveLayout(): ResponsiveLayoutResult { // We are using these contexts directly instead of useNavigation/useNavigationState, because those will throw an error if used outside a navigator. // This hook can be used within or outside a navigator, so using useNavigationState does not work. // Furthermore, wrapping useNavigationState in a try/catch does not work either, because that breaks the rules of hooks. + // Note that these three lines are copied closely from the internal implementation of useNavigation: https://github.com/react-navigation/react-navigation/blob/52a3234b7aaf4d4fcc9c0155f44f3ea2233f0f40/packages/core/src/useNavigation.tsx#L18-L28 const navigationContainerRef = useContext(NavigationContainerRefContext); const navigator = useContext(NavigationContext); const currentNavigator = navigator ?? navigationContainerRef; From 56d4ff2563dfcd3b6c52c92edb16ef45a93dd11d Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 4 Jun 2024 14:54:40 -0700 Subject: [PATCH 5/6] Safely call getParent --- src/hooks/useResponsiveLayout.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useResponsiveLayout.ts b/src/hooks/useResponsiveLayout.ts index e530c13c729c..b17948ba02ff 100644 --- a/src/hooks/useResponsiveLayout.ts +++ b/src/hooks/useResponsiveLayout.ts @@ -47,7 +47,7 @@ export default function useResponsiveLayout(): ResponsiveLayoutResult { const currentNavigator = navigator ?? navigationContainerRef; const isDisplayedInNarrowModalNavigator = - !!currentNavigator?.getParent(NAVIGATORS.RIGHT_MODAL_NAVIGATOR as unknown as undefined) || !!currentNavigator?.getParent(NAVIGATORS.LEFT_MODAL_NAVIGATOR as unknown as undefined); + !!currentNavigator?.getParent?.(NAVIGATORS.RIGHT_MODAL_NAVIGATOR as unknown as undefined) || !!currentNavigator?.getParent?.(NAVIGATORS.LEFT_MODAL_NAVIGATOR as unknown as undefined); // The component calling this hook is in a "narrow pane modal" if: const isInNarrowPaneModal = From 3d37f2673489822eb380e30f5c483e7a0450c123 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 4 Jun 2024 16:50:34 -0700 Subject: [PATCH 6/6] Wrap isDisplayedInNarrowModalNavigator in useMemo --- src/hooks/useResponsiveLayout.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/hooks/useResponsiveLayout.ts b/src/hooks/useResponsiveLayout.ts index b17948ba02ff..a26d50bc56b9 100644 --- a/src/hooks/useResponsiveLayout.ts +++ b/src/hooks/useResponsiveLayout.ts @@ -1,5 +1,5 @@ import {NavigationContainerRefContext, NavigationContext} from '@react-navigation/native'; -import {useContext} from 'react'; +import {useContext, useMemo} from 'react'; import ModalContext from '@components/Modal/ModalContext'; import CONST from '@src/CONST'; import NAVIGATORS from '@src/NAVIGATORS'; @@ -46,8 +46,12 @@ export default function useResponsiveLayout(): ResponsiveLayoutResult { const navigator = useContext(NavigationContext); const currentNavigator = navigator ?? navigationContainerRef; - const isDisplayedInNarrowModalNavigator = - !!currentNavigator?.getParent?.(NAVIGATORS.RIGHT_MODAL_NAVIGATOR as unknown as undefined) || !!currentNavigator?.getParent?.(NAVIGATORS.LEFT_MODAL_NAVIGATOR as unknown as undefined); + const isDisplayedInNarrowModalNavigator = useMemo( + () => + !!currentNavigator?.getParent?.(NAVIGATORS.RIGHT_MODAL_NAVIGATOR as unknown as undefined) || + !!currentNavigator?.getParent?.(NAVIGATORS.LEFT_MODAL_NAVIGATOR as unknown as undefined), + [currentNavigator], + ); // The component calling this hook is in a "narrow pane modal" if: const isInNarrowPaneModal =