Skip to content
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

@swm/global nav menu v1 #28277

Merged
merged 32 commits into from
Oct 7, 2023
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
aacb4a6
feat: first look at adding global nav menu
WoLewicki Sep 11, 2023
fe7851f
feat: remove unused onLayout and move globalNavigation higher
WoLewicki Sep 12, 2023
5bc0dec
feat: prepare app for different menus
WoLewicki Sep 12, 2023
b1af947
add first global navigation option
adamgrzybowski Sep 26, 2023
2344abc
style changes
adamgrzybowski Sep 26, 2023
a966c89
style changes v2
adamgrzybowski Sep 28, 2023
76ef562
Merge branch 'main' into @swm/global-nav-menu-v1
adamgrzybowski Sep 28, 2023
f8784a0
fix initial global navigation option on small screen
adamgrzybowski Sep 29, 2023
3c0295e
adjustments
adamgrzybowski Sep 29, 2023
c83bad0
change OD to OLDDOT
adamgrzybowski Sep 29, 2023
1c8c9db
set includePaddingTop to false
adamgrzybowski Oct 2, 2023
5f10a5b
Merge branch 'main' into @swm/global-nav-menu-v1
adamgrzybowski Oct 2, 2023
96197e3
Update src/ROUTES.ts
adamgrzybowski Oct 2, 2023
a2c65db
Update src/pages/home/sidebar/GlobalNavigation/GlobalNavigationMenuIt…
adamgrzybowski Oct 2, 2023
ce4f907
adjustments v2
adamgrzybowski Oct 2, 2023
d962df5
Merge branch 'main' into @swm/global-nav-menu-v1
adamgrzybowski Oct 3, 2023
6001c23
Merge branch 'main' into @swm/global-nav-menu-v1
adamgrzybowski Oct 3, 2023
0f29a39
style adjustments
adamgrzybowski Oct 3, 2023
95f47b3
Update src/SCREENS.ts
adamgrzybowski Oct 4, 2023
dfd2e82
Update src/libs/Navigation/getTopMostCentralPaneRouteName.js
adamgrzybowski Oct 4, 2023
50117d7
adjustments v3
adamgrzybowski Oct 4, 2023
36e6c61
adjustments v4
adamgrzybowski Oct 4, 2023
6a361b2
Merge branch 'main' into @swm/global-nav-menu-v1
adamgrzybowski Oct 4, 2023
c35523c
remove nested ScreenWrapper
adamgrzybowski Oct 4, 2023
a5ae067
Update src/components/LHNOptionsList/OptionRowLHN.js
adamgrzybowski Oct 4, 2023
1f6da9f
Revert "Update src/components/LHNOptionsList/OptionRowLHN.js"
adamgrzybowski Oct 4, 2023
ca99028
fix styles for OptionRowLHN
adamgrzybowski Oct 4, 2023
7250a2f
fix GLOBAL_NAVIGATION_MAPPING
adamgrzybowski Oct 4, 2023
8bb5406
adjustments v5
adamgrzybowski Oct 5, 2023
4c056fa
fix color for focused item in global navigation
adamgrzybowski Oct 5, 2023
a6ad209
fix top elements aligment
adamgrzybowski Oct 5, 2023
9e87953
change ref for GlobalNavigationMenuItem
adamgrzybowski Oct 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {ReportAttachmentsProvider} from './pages/home/report/ReportAttachmentsCo
import * as Session from './libs/actions/Session';
import useDefaultDragAndDrop from './hooks/useDefaultDragAndDrop';
import OnyxUpdateManager from './libs/actions/OnyxUpdateManager';
import {SidebarNavigationContextProvider} from './pages/home/sidebar/SidebarNavigationContext';

// For easier debugging and development, when we are in web we expose Onyx to the window, so you can more easily set data into Onyx
if (window && Environment.isDevelopment()) {
Expand Down Expand Up @@ -64,6 +65,7 @@ function App() {
EnvironmentProvider,
ThemeProvider,
ThemeStylesProvider,
SidebarNavigationContextProvider,
]}
>
<CustomStatusBar />
Expand Down
10 changes: 10 additions & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2671,19 +2671,29 @@ const CONST = {
DEFAULT_COORDINATE: [-122.4021, 37.7911],
STYLE_URL: 'mapbox://styles/expensify/cllcoiqds00cs01r80kp34tmq',
},

ONYX_UPDATE_TYPES: {
HTTPS: 'https',
PUSHER: 'pusher',
},

EVENTS: {
SCROLLING: 'scrolling',
},

HORIZONTAL_SPACER: {
DEFAULT_BORDER_BOTTOM_WIDTH: 1,
DEFAULT_MARGIN_VERTICAL: 8,
HIDDEN_MARGIN_VERTICAL: 0,
HIDDEN_BORDER_BOTTOM_WIDTH: 0,
},

GLOBAL_NAVIGATION_OPTION: {
HOME: 'home',
CHATS: 'chats',
SPEND: 'spend',
WORKSPACES: 'workspaces',
},
} as const;

export default CONST;
9 changes: 9 additions & 0 deletions src/GLOBAL_NAVIGATION_MAPPING.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import CONST from './CONST';
import SCREENS from './SCREENS';

export default {
[CONST.GLOBAL_NAVIGATION_OPTION.HOME]: [SCREENS.HOME_OLDDOT],
[CONST.GLOBAL_NAVIGATION_OPTION.CHATS]: [SCREENS.REPORT],
[CONST.GLOBAL_NAVIGATION_OPTION.SPEND]: [SCREENS.EXPENSES_OLDDOT, SCREENS.REPORTS_OLDDOT, SCREENS.INSIGHTS_OLDDOT],
[CONST.GLOBAL_NAVIGATION_OPTION.WORKSPACES]: [SCREENS.INDIVIDUAL_WORKSPACES_OLDDOT, SCREENS.GROUPS_WORKSPACES_OLDDOT, SCREENS.CARDS_AND_DOMAINS_OLDDOT],
} as const;
13 changes: 13 additions & 0 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,4 +318,17 @@ export default {
// These are some on-off routes that will be removed once they're no longer needed (see GH issues for details)
SAASTR: 'saastr',
SBE: 'sbe',

// Iframe screens from olddot
HOME_OLDDOT: 'home',

// Spend tab
EXPENSES_OLDDOT: 'expenses',
REPORTS_OLDDOT: 'reports',
INSIGHTS_OLDDOT: 'insights',

// Workspaces tab
INDIVIDUALS_OLDDOT: 'individual_workspaces',
GROUPS_OLDDOT: 'group_workspaces',
CARDS_AND_DOMAINS_OLDDOT: 'cards-and-domains',
hayata-suenaga marked this conversation as resolved.
Show resolved Hide resolved
} as const;
13 changes: 13 additions & 0 deletions src/SCREENS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,17 @@ export default {
SIGN_IN_WITH_APPLE_DESKTOP: 'AppleSignInDesktop',
SIGN_IN_WITH_GOOGLE_DESKTOP: 'GoogleSignInDesktop',
DESKTOP_SIGN_IN_REDIRECT: 'DesktopSignInRedirect',

// Iframe screens from olddot
HOME_OLDDOT: 'Home_OLDDOT',

// Spend tab
EXPENSES_OLDDOT: 'Expenses_OLDDOT',
REPORTS_OLDDOT: 'Reports_OLDDOT',
INSIGHTS_OLDDOT: 'Insights_OLDDOT',

// Workspaces tab
INDIVIDUAL_WORKSPACES_OLDDOT: 'IndividualWorkspaces_OLDDOT',
GROUPS_WORKSPACES_OLDDOT: 'GroupWorkspaces_OLDDOT',
CARDS_AND_DOMAINS_OLDDOT: 'CardsAndDomains_OLDDOT',
} as const;
2 changes: 1 addition & 1 deletion src/components/EnvironmentBadge.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function EnvironmentBadge() {
success={environment === CONST.ENVIRONMENT.STAGING || environment === CONST.ENVIRONMENT.ADHOC}
error={environment !== CONST.ENVIRONMENT.STAGING && environment !== CONST.ENVIRONMENT.ADHOC}
text={text}
badgeStyles={[styles.alignSelfEnd, styles.headerEnvBadge]}
badgeStyles={[styles.alignSelfEnd, styles.headerEnvBadge, styles.ml1]}
textStyles={[styles.headerEnvBadgeText]}
environment={environment}
/>
Expand Down
3 changes: 3 additions & 0 deletions src/components/FloatingActionButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import themeColors from '../styles/themes/default';
import Tooltip from './Tooltip';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import PressableWithFeedback from './Pressable/PressableWithFeedback';
import variables from '../styles/variables';

const AnimatedIcon = Animated.createAnimatedComponent(Icon);
AnimatedIcon.displayName = 'AnimatedIcon';
Expand Down Expand Up @@ -100,6 +101,8 @@ class FloatingActionButton extends PureComponent {
style={[styles.floatingActionButton, StyleUtils.getAnimatedFABStyle(rotate, backgroundColor)]}
>
<AnimatedIcon
width={variables.iconSizeSmall}
height={variables.iconSizeSmall}
src={Expensicons.Plus}
fill={fill}
/>
Expand Down
8 changes: 4 additions & 4 deletions src/components/LHNOptionsList/OptionRowLHN.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const propTypes = {
};

const defaultProps = {
hoverStyle: styles.sidebarLinkHover,
hoverStyle: styles.sidebarLinkHoverLHN,
viewMode: 'default',
onSelectRow: () => {},
style: null,
Expand Down Expand Up @@ -97,7 +97,7 @@ function OptionRowLHN(props) {
: [styles.chatLinkRowPressable, styles.flexGrow1, styles.optionItemAvatarNameWrapper, styles.optionRow, styles.justifyContentCenter],
);
const hoveredBackgroundColor = props.hoverStyle && props.hoverStyle.backgroundColor ? props.hoverStyle.backgroundColor : themeColors.sidebar;
const focusedBackgroundColor = styles.sidebarLinkActive.backgroundColor;
const focusedBackgroundColor = styles.sidebarLinkActiveLHN.backgroundColor;

const hasBrickError = optionItem.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
const defaultSubscriptSize = optionItem.isExpenseRequest ? CONST.AVATAR_SIZE.SMALL_NORMAL : CONST.AVATAR_SIZE.DEFAULT;
Expand Down Expand Up @@ -174,8 +174,8 @@ function OptionRowLHN(props) {
styles.flexRow,
styles.alignItemsCenter,
styles.justifyContentBetween,
styles.sidebarLink,
styles.sidebarLinkInner,
styles.sidebarLinkLHN,
styles.sidebarLinkInnerLHN,
StyleUtils.getBackgroundColorStyle(themeColors.sidebar),
props.isFocused ? styles.sidebarLinkActive : null,
(hovered || isContextMenuActive) && !props.isFocused ? props.hoverStyle : null,
Expand Down
3 changes: 3 additions & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1793,4 +1793,7 @@ export default {
selectSuggestedAddress: 'Please select a suggested address',
},
},
globalNavigationOptions: {
chats: 'Chats',
},
} satisfies TranslationBase;
3 changes: 3 additions & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2277,4 +2277,7 @@ export default {
selectSuggestedAddress: 'Por favor, selecciona una dirección sugerida',
},
},
globalNavigationOptions: {
chats: 'Chats',
stitesExpensify marked this conversation as resolved.
Show resolved Hide resolved
},
} satisfies EnglishTranslation;
5 changes: 5 additions & 0 deletions src/libs/Navigation/Navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import linkingConfig from './linkingConfig';
import navigationRef from './navigationRef';
import NAVIGATORS from '../../NAVIGATORS';
import originalGetTopmostReportId from './getTopmostReportId';
import originalGetTopMostCentralPaneRouteName from './getTopMostCentralPaneRouteName';
import originalGetTopmostReportActionId from './getTopmostReportActionID';
import getStateFromPath from './getStateFromPath';
import SCREENS from '../../SCREENS';
Expand Down Expand Up @@ -47,6 +48,9 @@ function canNavigate(methodName, params = {}) {
// Re-exporting the getTopmostReportId here to fill in default value for state. The getTopmostReportId isn't defined in this file to avoid cyclic dependencies.
const getTopmostReportId = (state = navigationRef.getState()) => originalGetTopmostReportId(state);

// Re-exporting the getTopMostCentralPaneRouteName here to fill in default value for state. The getTopMostCentralPaneRouteName isn't defined in this file to avoid cyclic dependencies.
const getTopMostCentralPaneRouteName = (state = navigationRef.getState()) => originalGetTopMostCentralPaneRouteName(state);
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved

// Re-exporting the getTopmostReportActionID here to fill in default value for state. The getTopmostReportActionID isn't defined in this file to avoid cyclic dependencies.
const getTopmostReportActionId = (state = navigationRef.getState()) => originalGetTopmostReportActionId(state);

Expand Down Expand Up @@ -272,6 +276,7 @@ export default {
setIsNavigationReady,
getTopmostReportId,
getRouteNameFromStateEvent,
getTopMostCentralPaneRouteName,
getTopmostReportActionId,
};

Expand Down
7 changes: 6 additions & 1 deletion src/libs/Navigation/NavigationRoot.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useRef, useEffect} from 'react';
import React, {useRef, useEffect, useContext} from 'react';
import PropTypes from 'prop-types';
import {NavigationContainer, DefaultTheme, getPathFromState} from '@react-navigation/native';
import {useSharedValue, useAnimatedReaction, interpolateColor, withTiming, withDelay, Easing, runOnJS} from 'react-native-reanimated';
Expand All @@ -11,6 +11,7 @@ import Log from '../Log';
import StatusBar from '../StatusBar';
import useCurrentReportID from '../../hooks/useCurrentReportID';
import useWindowDimensions from '../../hooks/useWindowDimensions';
import {SidebarNavigationContext} from '../../pages/home/sidebar/SidebarNavigationContext';

// https://reactnavigation.org/docs/themes
const navigationTheme = {
Expand Down Expand Up @@ -53,6 +54,7 @@ function parseAndLogRoute(state) {
function NavigationRoot(props) {
useFlipper(navigationRef);
const firstRenderRef = useRef(true);
const globalNavigation = useContext(SidebarNavigationContext);

const {updateCurrentReportID} = useCurrentReportID();
const {isSmallScreenWidth} = useWindowDimensions();
Expand Down Expand Up @@ -128,6 +130,9 @@ function NavigationRoot(props) {
}, 0);
parseAndLogRoute(state);
animateStatusBarBackgroundColor();

// Update the global navigation to show the correct selected menu items.
globalNavigation.updateFromNavigationState(state);
};

return (
Expand Down
32 changes: 32 additions & 0 deletions src/libs/Navigation/getTopMostCentralPaneRouteName.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import lodashFindLast from 'lodash/findLast';

/**
* Find the name of top most central pane route.
hayata-suenaga marked this conversation as resolved.
Show resolved Hide resolved
*
* @param {Object} state - The react-navigation state
* @returns {String | undefined} - It's possible that there is no central pane in the state.
*/
function getTopMostCentralPaneRouteName(state) {
if (!state) {
return undefined;
}
const topmostCentralPane = lodashFindLast(state.routes, (route) => route.name === 'CentralPaneNavigator');

if (!topmostCentralPane) {
return undefined;
}

if (topmostCentralPane.state && topmostCentralPane.state.routes) {
// State may don't have index in some cases. But in this case there will be only one route in state.
hayata-suenaga marked this conversation as resolved.
Show resolved Hide resolved
return topmostCentralPane.state.routes[topmostCentralPane.state.index || 0].name;
}

if (topmostCentralPane.params) {
// State may don't have inner state in some cases (e.g generating actions from path). But in this case there will be params available.
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved
return topmostCentralPane.params.screen;
}

return undefined;
}

export default getTopMostCentralPaneRouteName;
10 changes: 7 additions & 3 deletions src/libs/Navigation/linkTo.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import linkingConfig from './linkingConfig';
import getTopmostReportId from './getTopmostReportId';
import getStateFromPath from './getStateFromPath';
import CONST from '../../CONST';
import getTopMostCentralPaneRouteName from './getTopMostCentralPaneRouteName';

/**
* Motivation for this function is described in NAVIGATION.md
Expand Down Expand Up @@ -61,12 +62,15 @@ export default function linkTo(navigation, path, type) {

// If action type is different than NAVIGATE we can't change it to the PUSH safely
if (action.type === CONST.NAVIGATION.ACTION_TYPE.NAVIGATE) {
// Make sure that we are pushing a screen that is not currently on top of the stack.
const shouldPushIfCentralPane =
action.payload.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR &&
(getTopMostCentralPaneRouteName(root.getState()) !== getTopMostCentralPaneRouteName(state) || getTopmostReportId(root.getState()) !== getTopmostReportId(state));

// In case if type is 'FORCED_UP' we replace current screen with the provided. This means the current screen no longer exists in the stack
if (type === CONST.NAVIGATION.TYPE.FORCED_UP) {
action.type = CONST.NAVIGATION.ACTION_TYPE.REPLACE;

// If this action is navigating to the report screen and the top most navigator is different from the one we want to navigate - PUSH the new screen to the top of the stack
} else if (action.payload.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR && getTopmostReportId(root.getState()) !== getTopmostReportId(state)) {
} else if (shouldPushIfCentralPane) {
action.type = CONST.NAVIGATION.ACTION_TYPE.PUSH;

// If the type is UP, we deeplinked into one of the RHP flows and we want to replace the current screen with the previous one in the flow
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import React from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import Text from '../../../../components/Text';
import styles from '../../../../styles/styles';
import * as StyleUtils from '../../../../styles/StyleUtils';
import Icon from '../../../../components/Icon';
import CONST from '../../../../CONST';
import variables from '../../../../styles/variables';
import PressableWithFeedback from '../../../../components/Pressable/PressableWithFeedback';
import refPropTypes from '../../../../components/refPropTypes';

const propTypes = {
/** Icon to display */
icon: PropTypes.elementType,

/** Text to display for the item */
title: PropTypes.string,

/** Function to fire when component is pressed */
onPress: PropTypes.func,

/** A ref to forward to PressableWithFeedback */
forwardedRef: refPropTypes,

/** Whether item is focused or active */
isFocused: PropTypes.bool,
};

const defaultProps = {
icon: undefined,
isFocused: false,
onPress: () => {},
title: '',
forwardedRef: null,
};

function GlobalNavigationMenuItem({icon, title, isFocused, onPress, forwardedRef}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need forwardedRef prop. We should use the second parameter ref and in React.forwardRef we call the function directly without creating another layer.

export default React.forwardRef(GlobalNavigationMenuItem);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got this error/warning with this approach. Is there a way to fix it without creating another layer?

react.development.js:209 Warning: forwardRef render functions do not support propTypes or defaultProps. Did you accidentally pass a React component?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@kacper-mikolajczak kacper-mikolajczak Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi folks @hayata-suenaga @adamgrzybowski , thank you for pointing this out!

Following the error, we cannot attach propTypes to render function inside forwardRef, so in case where we need propTypes, I would simply fall back to declaring it inline (still omitting the indirect forwardedRef prop):

const GlobalNavigationMenuItem = React.forwardRef((props, ref) => {
/* Component Declaration */
})

GlobalNavigationMenuItem.propTypes = propTypes
...

export default GlobalNavigationMenuItem

Let me know if that works in this case. If you find that useful, I will update the docs accordingly, thanks!

P.S.
Ideally we would use TS for constraining the props shape, so that would not be the issue and we will declare component directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamgrzybowski were you able to solve the issue?

return (
<PressableWithFeedback
onPress={() => !isFocused && onPress()}
style={styles.globalNavigationItemContainer}
ref={forwardedRef}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.MENUITEM}
accessibilityLabel={title}
>
{({pressed}) => (
<View style={[styles.alignItemsCenter, styles.flexRow, styles.flex1]}>
<View style={styles.globalNavigationSelectionIndicator(isFocused)} />
<View style={[styles.flexColumn, styles.flex1, styles.alignItemsCenter, styles.mr1]}>
<Icon
additionalStyles={[styles.popoverMenuIcon]}
pressed={pressed}
src={icon}
fill={isFocused ? StyleUtils.getIconFillColor(CONST.BUTTON_STATES.DEFAULT, true) : StyleUtils.getIconFillColor()}
/>
<View style={[styles.mt1, styles.alignItemsCenter]}>
<Text style={[StyleUtils.getFontSizeStyle(variables.fontSizeExtraSmall), styles.globalNavigationMenuItem(isFocused)]}>{title}</Text>
</View>
</View>
</View>
)}
</PressableWithFeedback>
);
}

GlobalNavigationMenuItem.propTypes = propTypes;
GlobalNavigationMenuItem.defaultProps = defaultProps;
GlobalNavigationMenuItem.displayName = 'GlobalNavigationMenuItem';

export default React.forwardRef((props, ref) => (
<GlobalNavigationMenuItem
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
forwardedRef={ref}
/>
));
Loading
Loading