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

Converted SidebarLinks to fn component and improve use of window props #27855

Merged
merged 11 commits into from
Oct 4, 2023
181 changes: 84 additions & 97 deletions src/pages/home/sidebar/SidebarLinks.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
/* eslint-disable rulesdir/onyx-props-must-have-default */
import React from 'react';
import React, {useEffect, useRef} from 'react';
import {View} from 'react-native';
import _ from 'underscore';
import PropTypes from 'prop-types';
import styles from '../../../styles/styles';
import * as StyleUtils from '../../../styles/StyleUtils';
import ONYXKEYS from '../../../ONYXKEYS';
import safeAreaInsetPropTypes from '../../safeAreaInsetPropTypes';
import compose from '../../../libs/compose';
import Navigation from '../../../libs/Navigation/Navigation';
import ROUTES from '../../../ROUTES';
import Icon from '../../../components/Icon';
import * as Expensicons from '../../../components/Icon/Expensicons';
import Tooltip from '../../../components/Tooltip';
import CONST from '../../../CONST';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import * as App from '../../../libs/actions/App';
import withWindowDimensions from '../../../components/withWindowDimensions';
import LHNOptionsList from '../../../components/LHNOptionsList/LHNOptionsList';
import SidebarUtils from '../../../libs/SidebarUtils';
import Header from '../../../components/Header';
Expand All @@ -30,16 +27,15 @@ import KeyboardShortcut from '../../../libs/KeyboardShortcut';
import onyxSubscribe from '../../../libs/onyxSubscribe';
import * as ReportActionContextMenu from '../report/ContextMenu/ReportActionContextMenu';
import SignInOrAvatarWithOptionalStatus from './SignInOrAvatarWithOptionalStatus';
import useLocalize from '../../../hooks/useLocalize';
import useWindowDimensions from '../../../hooks/useWindowDimensions';

const basePropTypes = {
/** Toggles the navigation menu open and closed */
onLinkClick: PropTypes.func.isRequired,

/** Safe area insets required for mobile devices margins */
insets: safeAreaInsetPropTypes.isRequired,

/** Whether we are viewing below the responsive breakpoint */
isSmallScreenWidth: PropTypes.bool.isRequired,
};

const propTypes = {
Expand All @@ -49,47 +45,43 @@ const propTypes = {

isLoading: PropTypes.bool.isRequired,

// eslint-disable-next-line react/require-default-props
shubham1206agra marked this conversation as resolved.
Show resolved Hide resolved
priorityMode: PropTypes.oneOf(_.values(CONST.PRIORITY_MODE)),
shubham1206agra marked this conversation as resolved.
Show resolved Hide resolved

isActiveReport: PropTypes.func.isRequired,

...withLocalizePropTypes,
};

const defaultProps = {
priorityMode: CONST.PRIORITY_MODE.DEFAULT,
};

class SidebarLinks extends React.PureComponent {
constructor(props) {
super(props);
function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priorityMode = CONST.PRIORITY_MODE.DEFAULT, isActiveReport, isCreateMenuOpen}) {
const modal = useRef({});
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually pass a null as the initialiser.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually depends on the case to case basis.

const {translate} = useLocalize();
const {isSmallScreenWidth} = useWindowDimensions();

this.showSearchPage = this.showSearchPage.bind(this);
this.showReportPage = this.showReportPage.bind(this);

if (this.props.isSmallScreenWidth) {
App.confirmReadyToOpenApp();
useEffect(() => {
if (!isSmallScreenWidth) {
return;
}
}
App.confirmReadyToOpenApp();
}, [isSmallScreenWidth]);

componentDidMount() {
useEffect(() => {
App.setSidebarLoaded();
SidebarUtils.setIsSidebarLoadedReady();
this.isSidebarLoaded = true;

let modal = {};
this.unsubscribeOnyxModal = onyxSubscribe({
const unsubscribeOnyxModal = onyxSubscribe({
key: ONYXKEYS.MODAL,
callback: (modalArg) => {
modal = modalArg;
if (_.isNull(modalArg)) {
return;
}
Comment on lines +79 to +81
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To prevent

if (modal.current.willAlertModalBecomeVisible) {
         return;
}

from erroring out when the modal is null in onyx initially (Safeguarding the modal ref)

modal.current = modalArg;
},
});

const shortcutConfig = CONST.KEYBOARD_SHORTCUTS.ESCAPE;
this.unsubscribeEscapeKey = KeyboardShortcut.subscribe(
const unsubscribeEscapeKey = KeyboardShortcut.subscribe(
shortcutConfig.shortcutKey,
() => {
if (modal.willAlertModalBecomeVisible) {
if (modal.current.willAlertModalBecomeVisible) {
return;
}

Expand All @@ -102,97 +94,92 @@ class SidebarLinks extends React.PureComponent {
);

ReportActionContextMenu.hideContextMenu(false);
}

componentWillUnmount() {
SidebarUtils.resetIsSidebarLoadedReadyPromise();
if (this.unsubscribeEscapeKey) {
this.unsubscribeEscapeKey();
}
if (this.unsubscribeOnyxModal) {
this.unsubscribeOnyxModal();
}
}

showSearchPage() {
if (this.props.isCreateMenuOpen) {
return () => {
SidebarUtils.resetIsSidebarLoadedReadyPromise();
if (unsubscribeEscapeKey) {
unsubscribeEscapeKey();
}
if (unsubscribeOnyxModal) {
unsubscribeOnyxModal();
}
};
}, []);

const showSearchPage = () => {
if (isCreateMenuOpen) {
// Prevent opening Search page when click Search icon quickly after clicking FAB icon
return;
}

Navigation.navigate(ROUTES.SEARCH);
}
};
shubham1206agra marked this conversation as resolved.
Show resolved Hide resolved

/**
* Show Report page with selected report id
*
* @param {Object} option
* @param {String} option.reportID
*/
showReportPage(option) {
const showReportPage = (option) => {
// Prevent opening Report page when clicking LHN row quickly after clicking FAB icon
// or when clicking the active LHN row on large screens
// or when continuously clicking different LHNs, only apply to small screen
// since getTopmostReportId always returns on other devices
if (
this.props.isCreateMenuOpen ||
(!this.props.isSmallScreenWidth && this.props.isActiveReport(option.reportID)) ||
(this.props.isSmallScreenWidth && Navigation.getTopmostReportId())
) {
if (isCreateMenuOpen || (!isSmallScreenWidth && isActiveReport(option.reportID)) || (isSmallScreenWidth && Navigation.getTopmostReportId())) {
return;
}
Navigation.navigate(ROUTES.getReportRoute(option.reportID));
this.props.onLinkClick();
}

render() {
const viewMode = this.props.priorityMode === CONST.PRIORITY_MODE.GSD ? CONST.OPTION_MODE.COMPACT : CONST.OPTION_MODE.DEFAULT;

return (
<View style={[styles.flex1, styles.h100]}>
<View
style={[styles.flexRow, styles.ph5, styles.pv3, styles.justifyContentBetween, styles.alignItemsCenter]}
dataSet={{dragArea: true}}
>
<Header
title={
<LogoComponent
fill={defaultTheme.text}
width={variables.lhnLogoWidth}
height={variables.lhnLogoHeight}
/>
}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.TEXT}
shouldShowEnvironmentBadge
/>
<Tooltip text={this.props.translate('common.search')}>
<PressableWithoutFeedback
accessibilityLabel={this.props.translate('sidebarScreen.buttonSearch')}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
style={[styles.flexRow, styles.ph5]}
onPress={Session.checkIfActionIsAllowed(this.showSearchPage)}
>
<Icon src={Expensicons.MagnifyingGlass} />
</PressableWithoutFeedback>
</Tooltip>
<SignInOrAvatarWithOptionalStatus isCreateMenuOpen={this.props.isCreateMenuOpen} />
</View>

<LHNOptionsList
style={[this.props.isLoading ? styles.flexShrink1 : styles.flex1]}
contentContainerStyles={[styles.sidebarListContainer, {paddingBottom: StyleUtils.getSafeAreaMargins(this.props.insets).marginBottom}]}
data={this.props.optionListItems}
onSelectRow={this.showReportPage}
shouldDisableFocusOptions={this.props.isSmallScreenWidth}
optionMode={viewMode}
onLinkClick();
};
shubham1206agra marked this conversation as resolved.
Show resolved Hide resolved

const viewMode = priorityMode === CONST.PRIORITY_MODE.GSD ? CONST.OPTION_MODE.COMPACT : CONST.OPTION_MODE.DEFAULT;

return (
<View style={[styles.flex1, styles.h100]}>
<View
style={[styles.flexRow, styles.ph5, styles.pv3, styles.justifyContentBetween, styles.alignItemsCenter]}
dataSet={{dragArea: true}}
>
<Header
title={
<LogoComponent
fill={defaultTheme.textLight}
shubham1206agra marked this conversation as resolved.
Show resolved Hide resolved
width={variables.lhnLogoWidth}
height={variables.lhnLogoHeight}
/>
}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.TEXT}
shouldShowEnvironmentBadge
/>
{this.props.isLoading && <OptionsListSkeletonView shouldAnimate />}
<Tooltip text={translate('common.search')}>
<PressableWithoutFeedback
accessibilityLabel={translate('sidebarScreen.buttonSearch')}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
style={[styles.flexRow, styles.ph5]}
onPress={Session.checkIfActionIsAllowed(showSearchPage)}
>
<Icon src={Expensicons.MagnifyingGlass} />
</PressableWithoutFeedback>
</Tooltip>
<SignInOrAvatarWithOptionalStatus isCreateMenuOpen={isCreateMenuOpen} />
</View>
);
}

<LHNOptionsList
style={[isLoading ? styles.flexShrink1 : styles.flex1]}
contentContainerStyles={[styles.sidebarListContainer, {paddingBottom: StyleUtils.getSafeAreaMargins(insets).marginBottom}]}
data={optionListItems}
onSelectRow={showReportPage}
shouldDisableFocusOptions={isSmallScreenWidth}
optionMode={viewMode}
/>
{isLoading && <OptionsListSkeletonView shouldAnimate />}
</View>
);
}

SidebarLinks.propTypes = propTypes;
SidebarLinks.defaultProps = defaultProps;
export default compose(withLocalize, withWindowDimensions)(SidebarLinks);
SidebarLinks.displayName = 'SidebarLinks';

export default SidebarLinks;
export {basePropTypes};
3 changes: 1 addition & 2 deletions src/pages/home/sidebar/SidebarLinksData.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const defaultProps = {
policies: [],
};

function SidebarLinksData({isFocused, allReportActions, betas, chatReports, currentReportID, insets, isLoadingReportData, isSmallScreenWidth, onLinkClick, policies, priorityMode}) {
function SidebarLinksData({isFocused, allReportActions, betas, chatReports, currentReportID, insets, isLoadingReportData, onLinkClick, policies, priorityMode}) {
const {translate} = useLocalize();

const reportIDsRef = useRef(null);
Expand Down Expand Up @@ -107,7 +107,6 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr
// Forwarded props:
onLinkClick={onLinkClick}
insets={insets}
isSmallScreenWidth={isSmallScreenWidth}
priorityMode={priorityMode}
// Data props:
isActiveReport={isActiveReport}
Expand Down
11 changes: 2 additions & 9 deletions src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,9 @@ import ScreenWrapper from '../../../../components/ScreenWrapper';
import Timing from '../../../../libs/actions/Timing';
import CONST from '../../../../CONST';
import Performance from '../../../../libs/Performance';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../../components/withWindowDimensions';
import sidebarPropTypes from './sidebarPropTypes';
import * as Browser from '../../../../libs/Browser';

const propTypes = {
...sidebarPropTypes,
...windowDimensionsPropTypes,
};

/**
* Function called when a pinned chat is selected.
*/
Expand Down Expand Up @@ -42,7 +36,6 @@ function BaseSidebarScreen(props) {
<SidebarLinksData
onLinkClick={startTimer}
insets={insets}
isSmallScreenWidth={props.isSmallScreenWidth}
onLayout={props.onLayout}
/>
</View>
Expand All @@ -53,7 +46,7 @@ function BaseSidebarScreen(props) {
);
}

BaseSidebarScreen.propTypes = propTypes;
BaseSidebarScreen.propTypes = sidebarPropTypes;
BaseSidebarScreen.displayName = 'BaseSidebarScreen';

export default withWindowDimensions(BaseSidebarScreen);
export default BaseSidebarScreen;
7 changes: 4 additions & 3 deletions src/pages/home/sidebar/SidebarScreen/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import sidebarPropTypes from './sidebarPropTypes';
import BaseSidebarScreen from './BaseSidebarScreen';
import FloatingActionButtonAndPopover from './FloatingActionButtonAndPopover';
import FreezeWrapper from '../../../../libs/Navigation/FreezeWrapper';
import withWindowDimensions from '../../../../components/withWindowDimensions';
import useWindowDimensions from '../../../../hooks/useWindowDimensions';
import StatusBar from '../../../../libs/StatusBar';
import themeColors from '../../../../styles/themes/default';

function SidebarScreen(props) {
const popoverModal = useRef(null);
const {isSmallScreenWidth} = useWindowDimensions();

useFocusEffect(
useCallback(() => {
Expand Down Expand Up @@ -48,7 +49,7 @@ function SidebarScreen(props) {
};

return (
<FreezeWrapper keepVisible={!props.isSmallScreenWidth}>
<FreezeWrapper keepVisible={!isSmallScreenWidth}>
<BaseSidebarScreen
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
Expand All @@ -66,4 +67,4 @@ function SidebarScreen(props) {
SidebarScreen.propTypes = sidebarPropTypes;
SidebarScreen.displayName = 'SidebarScreen';

export default withWindowDimensions(SidebarScreen);
export default SidebarScreen;
7 changes: 4 additions & 3 deletions src/pages/home/sidebar/SidebarScreen/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ import sidebarPropTypes from './sidebarPropTypes';
import BaseSidebarScreen from './BaseSidebarScreen';
import FloatingActionButtonAndPopover from './FloatingActionButtonAndPopover';
import FreezeWrapper from '../../../../libs/Navigation/FreezeWrapper';
import withWindowDimensions from '../../../../components/withWindowDimensions';
import useWindowDimensions from '../../../../hooks/useWindowDimensions';

function SidebarScreen(props) {
const {isSmallScreenWidth} = useWindowDimensions();
return (
<FreezeWrapper keepVisible={!props.isSmallScreenWidth}>
<FreezeWrapper keepVisible={!isSmallScreenWidth}>
<BaseSidebarScreen
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
Expand All @@ -21,4 +22,4 @@ function SidebarScreen(props) {
SidebarScreen.propTypes = sidebarPropTypes;
SidebarScreen.displayName = 'SidebarScreen';

export default withWindowDimensions(SidebarScreen);
export default SidebarScreen;
2 changes: 0 additions & 2 deletions src/pages/home/sidebar/SidebarScreen/sidebarPropTypes.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import PropTypes from 'prop-types';
import {windowDimensionsPropTypes} from '../../../../components/withWindowDimensions';

const sidebarPropTypes = {
/** Callback when onLayout of sidebar is called */
onLayout: PropTypes.func,
...windowDimensionsPropTypes,
};
export default sidebarPropTypes;
Loading