Merge pull request #22 from software-mansion-labs/@kosmydel/ideal-nav…

…-address-review Addressed ideal nav CR comments: Expensify#33280 (comment) Expensify#33280 (comment) Expensify#33280 (comment) Expensify#33280 (comment)
software-mansion-labs · Jan 8, 2024 · 0fb334a · 0fb334a
2 parents 7845e2a + 93d634b
commit 0fb334a
Show file tree

Hide file tree

Showing 52 changed files with 222 additions and 241 deletions.
diff --git a/src/CONST.ts b/src/CONST.ts
@@ -3064,6 +3064,11 @@ const CONST = {
         EMAIL: 'EMAIL',
         REPORT: 'REPORT',
     },
+
+    WORKSPACE_SWITCHER: {
+        NAME: 'Expensify',
+        SUBSCRIPT_ICON_SIZE: 8,
+    },
 } as const;
 
 export default CONST;
diff --git a/src/components/HeaderPageLayout.js b/src/components/HeaderPageLayout.js
@@ -39,6 +39,8 @@ const propTypes = {
     /** Style to apply to the children container */
     // eslint-disable-next-line react/forbid-prop-types
     childrenContainerStyles: PropTypes.arrayOf(PropTypes.object),
+
+    shouldShowOfflineIndicator: PropTypes.bool,
 };
 
 const defaultProps = {
@@ -48,9 +50,21 @@ const defaultProps = {
     scrollViewContainerStyles: [],
     childrenContainerStyles: [],
     footer: null,
+    shouldShowOfflineIndicator: false,
 };
 
-function HeaderPageLayout({backgroundColor, children, footer, headerContainerStyles, scrollViewContainerStyles, childrenContainerStyles, style, headerContent, ...propsToPassToHeader}) {
+function HeaderPageLayout({
+    backgroundColor,
+    children,
+    footer,
+    headerContainerStyles,
+    scrollViewContainerStyles,
+    childrenContainerStyles,
+    style,
+    headerContent,
+    shouldShowOfflineIndicator,
+    ...propsToPassToHeader
+}) {
     const theme = useTheme();
     const styles = useThemeStyles();
     const StyleUtils = useStyleUtils();
@@ -72,6 +86,7 @@ function HeaderPageLayout({backgroundColor, children, footer, headerContainerSty
             includeSafeAreaPaddingBottom={false}
             offlineIndicatorStyle={[appBGColor]}
             testID={HeaderPageLayout.displayName}
+            shouldShowOfflineIndicator={shouldShowOfflineIndicator}
         >
             {({safeAreaPaddingBottomStyle}) => (
                 <>

diff --git a/src/components/IllustratedHeaderPageLayout.js b/src/components/IllustratedHeaderPageLayout.js
@@ -23,37 +23,41 @@ const propTypes = {
 
     /** Overlay content to display on top of animation */
     overlayContent: PropTypes.func,
+
+    shouldShowOfflineIndicator: PropTypes.bool,
 };
 
 const defaultProps = {
     backgroundColor: undefined,
     footer: null,
     overlayContent: null,
+    shouldShowOfflineIndicator: false,
 };
 
-function IllustratedHeaderPageLayout({backgroundColor, children, illustration, footer, overlayContent, ...propsToPassToHeader}) {
+function IllustratedHeaderPageLayout({backgroundColor, children, illustration, footer, overlayContent, shouldShowOfflineIndicator, ...propsToPassToHeader}) {
     const theme = useTheme();
     const styles = useThemeStyles();
-    const shouldUseMaxHeight = !propsToPassToHeader.shouldShowBackButton;
+    const shouldLimitHeight = !propsToPassToHeader.shouldShowBackButton;
 
     return (
         <HeaderPageLayout
             backgroundColor={backgroundColor || theme.appBG}
             title={propsToPassToHeader.title}
+            shouldShowOfflineIndicator={shouldShowOfflineIndicator}
             headerContent={
                 <>
                     <Lottie
                         source={illustration}
                         style={styles.w100}
-                        webStyle={shouldUseMaxHeight ? styles.h100 : styles.w100}
+                        webStyle={shouldLimitHeight ? styles.h100 : styles.w100}
                         autoPlay
                         loop
                     />
                     {overlayContent && overlayContent()}
                 </>
             }
             // TODO: move to variables
-            headerContainerStyles={[styles.justifyContentCenter, styles.w100, shouldUseMaxHeight && styles.centralPaneAnimation]}
+            headerContainerStyles={[styles.justifyContentCenter, styles.w100, shouldLimitHeight && styles.centralPaneAnimation]}
             footer={footer}
             // eslint-disable-next-line react/jsx-props-no-spreading
             {...propsToPassToHeader}

diff --git a/src/components/ReimbursementAccountLoadingIndicator.js b/src/components/ReimbursementAccountLoadingIndicator.js
@@ -24,7 +24,7 @@ function ReimbursementAccountLoadingIndicator(props) {
     const {translate} = useLocalize();
     return (
         <ScreenWrapper
-            shouldShowOfflineIndicator={false}
+            shouldShowOfflineIndicatorSmallWidth={false}
             style={[StyleSheet.absoluteFillObject, styles.reimbursementAccountFullScreenLoading]}
             testID={ReimbursementAccountLoadingIndicator.displayName}
         >

diff --git a/src/components/ScreenWrapper.tsx b/src/components/ScreenWrapper.tsx
@@ -77,7 +77,7 @@ type ScreenWrapperProps = {
     shouldEnableMinHeight?: boolean;
 
     /** Whether to show offline indicator */
-    shouldShowOfflineIndicator?: boolean;
+    shouldShowOfflineIndicatorSmallWidth?: boolean;
 
     /**
      * The navigation prop is passed by the navigator. It is used to trigger the onEntryTransitionEnd callback
@@ -86,6 +86,9 @@ type ScreenWrapperProps = {
      * This is required because transitionEnd event doesn't trigger in the testing environment.
      */
     navigation?: StackNavigationProp<RootStackParamList>;
+
+    /** Is central pane */
+    shouldShowOfflineIndicator?: boolean;
 };
 
 function ScreenWrapper(
@@ -99,13 +102,14 @@ function ScreenWrapper(
         shouldEnablePickerAvoiding = true,
         headerGapStyles,
         children,
-        shouldShowOfflineIndicator = true,
+        shouldShowOfflineIndicatorSmallWidth = true,
         offlineIndicatorStyle,
         style,
         shouldDismissKeyboardBeforeClose = true,
         onEntryTransitionEnd,
         testID,
         navigation: navigationProp,
+        shouldShowOfflineIndicator = false,
     }: ScreenWrapperProps,
     ref: ForwardedRef<View>,
 ) {
@@ -198,7 +202,7 @@ function ScreenWrapper(
                 }
 
                 // We always need the safe area padding bottom if we're showing the offline indicator since it is bottom-docked.
-                if (includeSafeAreaPaddingBottom || (isOffline && shouldShowOfflineIndicator)) {
+                if (includeSafeAreaPaddingBottom || (isOffline && shouldShowOfflineIndicatorSmallWidth)) {
                     paddingStyle.paddingBottom = paddingBottom;
                 }
 
@@ -237,7 +241,13 @@ function ScreenWrapper(
                                               })
                                             : children
                                     }
-                                    {isSmallScreenWidth && shouldShowOfflineIndicator && <OfflineIndicator style={offlineIndicatorStyle} />}
+                                    {isSmallScreenWidth && shouldShowOfflineIndicatorSmallWidth && <OfflineIndicator style={offlineIndicatorStyle} />}
+                                    {!isSmallScreenWidth && shouldShowOfflineIndicator && (
+                                        <OfflineIndicator
+                                            containerStyles={[]}
+                                            style={[styles.pl5, styles.offlineIndicatorRow, offlineIndicatorStyle]}
+                                        />
+                                    )}
                                 </PickerAvoidingView>
                             </KeyboardAvoidingView>
                         </View>

diff --git a/src/components/Section/index.js b/src/components/Section/index.js
@@ -78,11 +78,11 @@ const defaultProps = {
 
 function Section({children, childrenStyles, containerStyles, icon, IconComponent, cardLayout, iconContainerStyles, menuItems, subtitle, subtitleStyles, subtitleMuted, title, titleStyles}) {
     const styles = useThemeStyles();
-    const {isMobileScreenWidth} = useWindowDimensions();
+    const {isSmallScreenWidth} = useWindowDimensions();
 
     return (
         <>
-            <View style={[styles.pageWrapper, styles.cardSection, ...containerStyles, isMobileScreenWidth ? styles.p5 : styles.p8]}>
+            <View style={[styles.pageWrapper, styles.cardSection, ...containerStyles, isSmallScreenWidth ? styles.p5 : styles.p8]}>
                 {cardLayout === CARD_LAYOUT.ICON_ON_TOP && (
                     <IconSection
                         icon={icon}

diff --git a/src/components/SubscriptAvatar.tsx b/src/components/SubscriptAvatar.tsx
@@ -53,7 +53,7 @@ type SubscriptAvatarProps = {
     /** Subscript avatar URL or icon */
     secondaryAvatar?: SubAvatar;
 
-    /** Subscript icon type */
+    /** Subscript icon */
     subscriptIcon?: SubIcon;
 
     /** Removes margin from around the avatar, used for the chat view */

diff --git a/src/components/WorkspaceSwitcherButton.tsx b/src/components/WorkspaceSwitcherButton.tsx
@@ -16,8 +16,8 @@ function WorkspaceSwitcherButton() {
             onPress={() => {}}
         >
             <SubscriptAvatar
-                mainAvatar={{source: Expensicons.ExpensifyAppIcon, name: 'Expensify', type: CONST.ICON_TYPE_AVATAR}}
-                subscriptIcon={{source: Expensicons.DownArrow, width: 8, height: 8}}
+                mainAvatar={{source: Expensicons.ExpensifyAppIcon, name: CONST.WORKSPACE_SWITCHER.NAME, type: CONST.ICON_TYPE_AVATAR}}
+                subscriptIcon={{source: Expensicons.DownArrow, width: CONST.WORKSPACE_SWITCHER.SUBSCRIPT_ICON_SIZE, height: CONST.WORKSPACE_SWITCHER.SUBSCRIPT_ICON_SIZE}}
                 showTooltip={false}
                 noMargin
             />

diff --git a/src/hooks/useWindowDimensions/index.native.ts b/src/hooks/useWindowDimensions/index.native.ts
@@ -12,7 +12,6 @@ export default function (): WindowDimensions {
     const isSmallScreenWidth = true;
     const isMediumScreenWidth = false;
     const isLargeScreenWidth = false;
-    const isMobileScreenWidth = true;
 
     return {
         windowWidth,
@@ -21,6 +20,5 @@ export default function (): WindowDimensions {
         isSmallScreenWidth,
         isMediumScreenWidth,
         isLargeScreenWidth,
-        isMobileScreenWidth,
     };
 }
diff --git a/src/hooks/useWindowDimensions/index.ts b/src/hooks/useWindowDimensions/index.ts
@@ -14,7 +14,6 @@ export default function (): WindowDimensions {
     const isSmallScreenWidth = windowWidth <= variables.mobileResponsiveWidthBreakpoint;
     const isMediumScreenWidth = windowWidth > variables.mobileResponsiveWidthBreakpoint && windowWidth <= variables.tabletResponsiveWidthBreakpoint;
     const isLargeScreenWidth = windowWidth > variables.tabletResponsiveWidthBreakpoint;
-    const isMobileScreenWidth = isExtraSmallScreenHeight || isSmallScreenWidth;
 
     return {
         windowWidth,
@@ -23,6 +22,5 @@ export default function (): WindowDimensions {
         isSmallScreenWidth,
         isMediumScreenWidth,
         isLargeScreenWidth,
-        isMobileScreenWidth,
     };
 }
diff --git a/src/hooks/useWindowDimensions/types.ts b/src/hooks/useWindowDimensions/types.ts
@@ -5,7 +5,6 @@ type WindowDimensions = {
     isSmallScreenWidth: boolean;
     isMediumScreenWidth: boolean;
     isLargeScreenWidth: boolean;
-    isMobileScreenWidth: boolean;
 };
 
 export default WindowDimensions;
diff --git a/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx b/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx
@@ -25,9 +25,7 @@ function BottomTabBar() {
     // Parent navigator of the bottom tab bar is the root navigator.
     const currentTabName = useNavigationState<RootStackParamList, string | undefined>((state) => {
         const topmostBottomTabRoute = getTopmostBottomTabRoute(state);
-        if (topmostBottomTabRoute) {
-            return topmostBottomTabRoute.name;
-        }
+        return topmostBottomTabRoute?.name;
     });
 
     return (

diff --git a/.../createCustomBottomTabNavigator/TopBar.js → ...createCustomBottomTabNavigator/TopBar.tsx b/.../createCustomBottomTabNavigator/TopBar.js → ...createCustomBottomTabNavigator/TopBar.tsx
@@ -1,20 +1,19 @@
-import PropTypes from 'prop-types';
 import React, {useCallback} from 'react';
 import {View} from 'react-native';
-import * as Expensicons from '@components/Icon/Expensicons';
-import PressableWithFeedback from '@components/Pressable/PressableWithFeedback';
 import Search from '@components/Search';
-import SubscriptAvatar from '@components/SubscriptAvatar';
+import WorkspaceSwitcherButton from '@components/WorkspaceSwitcherButton';
 import useLocalize from '@hooks/useLocalize';
 import useThemeStyles from '@hooks/useThemeStyles';
 import Navigation from '@libs/Navigation/Navigation';
 import SignInOrAvatarWithOptionalStatus from '@pages/home/sidebar/SignInOrAvatarWithOptionalStatus';
 import * as Session from '@userActions/Session';
-import CONST from '@src/CONST';
 import ROUTES from '@src/ROUTES';
 
-// TODO-IDEAL: isCreateMenuOpen wasn't used before
-function TopBar({isCreateMenuOpen = false}) {
+type Props = {
+    isCreateMenuOpen?: boolean;
+};
+
+function TopBar({isCreateMenuOpen = false}: Props) {
     const styles = useThemeStyles();
     const {translate} = useLocalize();
 
@@ -32,30 +31,20 @@ function TopBar({isCreateMenuOpen = false}) {
             style={[styles.gap4, styles.flexRow, styles.ph5, styles.pv5, styles.justifyContentBetween, styles.alignItemsCenter]}
             dataSet={{dragArea: true}}
         >
-            <PressableWithFeedback role={CONST.ROLE.BUTTON}>
-                <SubscriptAvatar
-                    mainAvatar={{source: Expensicons.ExpensifyAppIcon, name: 'Expensify', type: CONST.ICON_TYPE_AVATAR}}
-                    subscriptIcon={{source: Expensicons.DownArrow, width: 8, height: 8}}
-                    showTooltip={false}
-                    noMargin
-                />
-            </PressableWithFeedback>
+            <WorkspaceSwitcherButton />
             <Search
                 placeholder={translate('sidebarScreen.buttonSearch')}
                 onPress={Session.checkIfActionIsAllowed(showSearchPage)}
                 containerStyle={styles.flexGrow1}
             />
-            <SignInOrAvatarWithOptionalStatus isCreateMenuOpen={isCreateMenuOpen} />
+            <SignInOrAvatarWithOptionalStatus
+                // @ts-expect-error TODO: Remove this once Sidebar page (https://github.com/Expensify/App/issues/25220) is migrated to TypeScript.
+                isCreateMenuOpen={isCreateMenuOpen}
+            />
         </View>
     );
 }
 
 TopBar.displayName = 'TopBar';
-TopBar.propTypes = {
-    isCreateMenuOpen: PropTypes.bool,
-};
-TopBar.defaultProps = {
-    isCreateMenuOpen: false,
-};
 
 export default TopBar;
diff --git a/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/index.tsx b/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/index.tsx
@@ -6,6 +6,7 @@ import PropTypes from 'prop-types';
 import React from 'react';
 import {View} from 'react-native';
 import ScreenWrapper from '@components/ScreenWrapper';
+import useThemeStyles from '@hooks/useThemeStyles';
 import type {NavigationStateRoute} from '@libs/Navigation/types';
 import SCREENS from '@src/SCREENS';
 import BottomTabBar from './BottomTabBar';
@@ -58,11 +59,15 @@ function CustomBottomTabNavigator({initialRouteName, children, screenOptions, ..
         initialRouteName,
     });
 
+    const styles = useThemeStyles();
     const stateToRender = getStateToRender(state);
 
     return (
-        <ScreenWrapper testID={CustomBottomTabNavigator.displayName}>
-            <View style={{flex: 1}}>
+        <ScreenWrapper
+            testID={CustomBottomTabNavigator.displayName}
+            shouldShowOfflineIndicatorSmallWidth={false}
+        >
+            <View style={styles.flex1}>
                 <TopBar />
                 <NavigationContent>
                     <StackView

diff --git a/src/libs/Navigation/AppNavigator/createCustomFullScreenNavigator/CustomFullScreenRouter.tsx b/src/libs/Navigation/AppNavigator/createCustomFullScreenNavigator/CustomFullScreenRouter.tsx
@@ -2,7 +2,7 @@ import type {NavigationState, ParamListBase, PartialState, RouterConfigOptions,
 import {StackRouter} from '@react-navigation/native';
 import getIsSmallScreenWidth from '@libs/getIsSmallScreenWidth';
 import SCREENS from '@src/SCREENS';
-import type {ResponsiveStackNavigatorRouterOptions} from './types';
+import type {FullScreenNavigatorRouterOptions} from './types';
 
 // TODO: export states to separate file
 type State = NavigationState | PartialState<NavigationState>;
@@ -29,7 +29,7 @@ const addCentralPaneNavigatorRoute = (state: State) => {
     (state.index as number) = state.routes.length - 1;
 };
 
-function CustomFullScreenRouter(options: ResponsiveStackNavigatorRouterOptions) {
+function CustomFullScreenRouter(options: FullScreenNavigatorRouterOptions) {
     const stackRouter = StackRouter(options);
 
     return {
@@ -44,7 +44,6 @@ function CustomFullScreenRouter(options: ResponsiveStackNavigatorRouterOptions)
         },
         getRehydratedState(partialState: StackNavigationState<ParamListBase>, {routeNames, routeParamList, routeGetIdList}: RouterConfigOptions): StackNavigationState<ParamListBase> {
             const isSmallScreenWidth = getIsSmallScreenWidth();
-            // Make sure that there is at least one CentralPaneNavigator (ReportScreen by default) in the state if this is a wide layout
             if (!isAtLeastOneInState(partialState, SCREENS.SETTINGS_CENTRAL_PANE) && !isSmallScreenWidth) {
                 // If we added a route we need to make sure that the state.stale is true to generate new key for this route
 

diff --git a/src/libs/Navigation/AppNavigator/createCustomFullScreenNavigator/index.native.tsx b/src/libs/Navigation/AppNavigator/createCustomFullScreenNavigator/index.native.tsx
@@ -2,21 +2,13 @@ import type {ParamListBase, StackActionHelpers, StackNavigationState} from '@rea
 import {createNavigatorFactory, useNavigationBuilder} from '@react-navigation/native';
 import type {StackNavigationEventMap, StackNavigationOptions} from '@react-navigation/stack';
 import {StackView} from '@react-navigation/stack';
-import React, {useRef} from 'react';
-import useWindowDimensions from '@hooks/useWindowDimensions';
 import CustomFullScreenRouter from './CustomFullScreenRouter';
-import type {ResponsiveStackNavigatorProps, ResponsiveStackNavigatorRouterOptions} from './types';
-
-function ResponsiveStackNavigator(props: ResponsiveStackNavigatorProps) {
-    const {isSmallScreenWidth} = useWindowDimensions();
-
-    const isSmallScreenWidthRef = useRef(isSmallScreenWidth);
-
-    isSmallScreenWidthRef.current = isSmallScreenWidth;
+import type {FullScreenNavigatorProps, FullScreenNavigatorRouterOptions} from './types';
 
+function FullScreenNavigator(props: FullScreenNavigatorProps) {
     const {navigation, state, descriptors, NavigationContent} = useNavigationBuilder<
         StackNavigationState<ParamListBase>,
-        ResponsiveStackNavigatorRouterOptions,
+        FullScreenNavigatorRouterOptions,
         StackActionHelpers<ParamListBase>,
         StackNavigationOptions,
         StackNavigationEventMap
@@ -39,6 +31,6 @@ function ResponsiveStackNavigator(props: ResponsiveStackNavigatorProps) {
     );
 }
 
-ResponsiveStackNavigator.displayName = 'ResponsiveStackNavigator';
+FullScreenNavigator.displayName = 'FullScreenNavigator';
 
-export default createNavigatorFactory<StackNavigationState<ParamListBase>, StackNavigationOptions, StackNavigationEventMap, typeof ResponsiveStackNavigator>(ResponsiveStackNavigator);
+export default createNavigatorFactory<StackNavigationState<ParamListBase>, StackNavigationOptions, StackNavigationEventMap, typeof FullScreenNavigator>(FullScreenNavigator);