From 1ddcbbea7110987d2e2e441dbe7897b668fea1b4 Mon Sep 17 00:00:00 2001 From: aimane-chnaif Date: Fri, 11 Nov 2022 06:46:23 +1000 Subject: [PATCH 1/5] prevent FAB menu and other page showing simultaneously --- src/pages/home/sidebar/SidebarLinks.js | 20 +++++-- .../SidebarScreen/BaseSidebarScreen.js | 52 ++++++++++++++++++- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index eec8c56f0dcd..88e97e5bac2e 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -84,9 +84,22 @@ const defaultProps = { class SidebarLinks extends React.Component { showSearchPage() { + if (this.props.isMenuOpen) { + // Prevent opening Search page when click Search icon quickly after clicking FAB icon + return; + } Navigation.navigate(ROUTES.SEARCH); } + showReportPage(reportID) { + if (this.props.isMenuOpen) { + // Prevent opening Report page when click LHN row quickly after clicking FAB icon + return; + } + Navigation.navigate(ROUTES.getReportRoute(reportID)); + this.props.onLinkClick(); + } + render() { // Wait until the personalDetails are actually loaded before displaying the LHN if (_.isEmpty(this.props.personalDetails)) { @@ -121,7 +134,7 @@ class SidebarLinks extends React.Component { accessibilityLabel={this.props.translate('sidebarScreen.buttonSearch')} accessibilityRole="button" style={[styles.flexRow, styles.ph5]} - onPress={this.showSearchPage} + onPress={() => this.showSearchPage()} > @@ -147,10 +160,7 @@ class SidebarLinks extends React.Component { focusedIndex={_.findIndex(optionListItems, ( option => option.toString() === this.props.reportIDFromRoute ))} - onSelectRow={(option) => { - Navigation.navigate(ROUTES.getReportRoute(option.reportID)); - this.props.onLinkClick(); - }} + onSelectRow={option => this.showReportPage(option.reportID)} shouldDisableFocusOptions={this.props.isSmallScreenWidth} optionMode={this.props.priorityMode === CONST.PRIORITY_MODE.GSD ? 'compact' : 'default'} onLayout={App.setSidebarLoaded} diff --git a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js index c1118a3f37e5..0f157b5ed8bf 100644 --- a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js +++ b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js @@ -19,6 +19,7 @@ import Performance from '../../../../libs/Performance'; import * as Welcome from '../../../../libs/actions/Welcome'; import {sidebarPropTypes, sidebarDefaultProps} from './sidebarPropTypes'; import withDrawerState from '../../../../components/withDrawerState'; +import withNavigationFocus from '../../../../components/withNavigationFocus'; import KeyboardShortcutsModal from '../../../../components/KeyboardShortcutsModal'; const propTypes = { @@ -62,10 +63,51 @@ class BaseSidebarScreen extends Component { Welcome.show({routes, showCreateMenu: this.showCreateMenu}); } + componentDidUpdate(prevProps) { + if (!this.didBecomeInactive(prevProps)) { + return; + } + + // Hide menu manually when other pages are opened using shortcut key + this.hideCreateMenu(); + } + + didBecomeInactive(prevProps) { + // When Report page is just opened + if (!this.props.isDrawerOpen && prevProps.isDrawerOpen) { + return true; + } + + // When other page is just opened + if (!this.props.isFocused && prevProps.isFocused) { + return true; + } + + return false; + } + + isInactive() { + // When drawer is closed and Report page is open + if (this.props.isSmallScreenWidth && !this.props.isDrawerOpen) { + return true; + } + + // When other page is open + if (!this.props.isFocused) { + return true; + } + + return false; + } + /** * Method called when we click the floating action button */ showCreateMenu() { + if (this.isInactive()) { + // Prevent showing menu when click FAB icon quickly after opening other pages + return; + } this.setState({ isCreateMenuActive: true, }); @@ -76,6 +118,10 @@ class BaseSidebarScreen extends Component { * Method called when avatar is clicked */ navigateToSettings() { + if (this.state.isCreateMenuActive) { + // Prevent opening Settings page when click profile avatar quickly after clicking FAB icon + return; + } Navigation.navigate(ROUTES.SETTINGS); } @@ -85,6 +131,9 @@ class BaseSidebarScreen extends Component { * Selecting an item on CreateMenu or closing it by clicking outside of the modal component */ hideCreateMenu() { + if (!this.state.isCreateMenuActive) { + return; + } this.props.onHideCreateMenu(); this.setState({ isCreateMenuActive: false, @@ -116,6 +165,7 @@ class BaseSidebarScreen extends Component { onAvatarClick={this.navigateToSettings} isSmallScreenWidth={this.props.isSmallScreenWidth} isDrawerOpen={this.props.isDrawerOpen} + isMenuOpen={this.state.isCreateMenuActive} reportIDFromRoute={this.props.reportIDFromRoute} /> Date: Wed, 16 Nov 2022 05:37:37 +1000 Subject: [PATCH 2/5] small refactor on SidebarLinks, BaseSidebarScreen --- src/pages/home/sidebar/SidebarLinks.js | 39 +++++++++++----- .../SidebarScreen/BaseSidebarScreen.js | 44 ++++++++++--------- tests/utils/LHNTestUtils.js | 1 - 3 files changed, 52 insertions(+), 32 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 88e97e5bac2e..cc4f4f161a33 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -32,9 +32,6 @@ const propTypes = { /** Toggles the navigation menu open and closed */ onLinkClick: PropTypes.func.isRequired, - /** Navigates to settings and hides sidebar */ - onAvatarClick: PropTypes.func.isRequired, - /** Safe area insets required for mobile devices margins */ insets: safeAreaInsetPropTypes.isRequired, @@ -83,20 +80,42 @@ const defaultProps = { }; class SidebarLinks extends React.Component { + constructor(props) { + super(props); + + this.showSearchPage = this.showSearchPage.bind(this); + this.showSettingsPage = this.showSettingsPage.bind(this); + this.showReportPage = this.showReportPage.bind(this); + } + showSearchPage() { - if (this.props.isMenuOpen) { + if (this.props.isCreateMenuOpen) { // Prevent opening Search page when click Search icon quickly after clicking FAB icon return; } Navigation.navigate(ROUTES.SEARCH); } - showReportPage(reportID) { - if (this.props.isMenuOpen) { + showSettingsPage() { + if (this.state.isCreateMenuActive) { + // Prevent opening Settings page when click profile avatar quickly after clicking FAB icon + return; + } + Navigation.navigate(ROUTES.SETTINGS); + } + + /** + * Show Report page with selected report id + * + * @param {Object} option + * @param {String} option.reportID + */ + showReportPage(option) { + if (this.props.isCreateMenuOpen) { // Prevent opening Report page when click LHN row quickly after clicking FAB icon return; } - Navigation.navigate(ROUTES.getReportRoute(reportID)); + Navigation.navigate(ROUTES.getReportRoute(option.reportID)); this.props.onLinkClick(); } @@ -134,7 +153,7 @@ class SidebarLinks extends React.Component { accessibilityLabel={this.props.translate('sidebarScreen.buttonSearch')} accessibilityRole="button" style={[styles.flexRow, styles.ph5]} - onPress={() => this.showSearchPage()} + onPress={this.showSearchPage} > @@ -142,7 +161,7 @@ class SidebarLinks extends React.Component { option.toString() === this.props.reportIDFromRoute ))} - onSelectRow={option => this.showReportPage(option.reportID)} + onSelectRow={this.showReportPage} shouldDisableFocusOptions={this.props.isSmallScreenWidth} optionMode={this.props.priorityMode === CONST.PRIORITY_MODE.GSD ? 'compact' : 'default'} onLayout={App.setSidebarLoaded} diff --git a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js index 0f157b5ed8bf..7ecf96531279 100644 --- a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js +++ b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js @@ -8,6 +8,7 @@ import SidebarLinks from '../SidebarLinks'; import PopoverMenu from '../../../../components/PopoverMenu'; import FloatingActionButton from '../../../../components/FloatingActionButton'; import ScreenWrapper from '../../../../components/ScreenWrapper'; +import compose from '../../../../libs/compose'; import Navigation from '../../../../libs/Navigation/Navigation'; import ROUTES from '../../../../ROUTES'; import Timing from '../../../../libs/actions/Timing'; @@ -47,7 +48,6 @@ class BaseSidebarScreen extends Component { this.hideCreateMenu = this.hideCreateMenu.bind(this); this.startTimer = this.startTimer.bind(this); - this.navigateToSettings = this.navigateToSettings.bind(this); this.showCreateMenu = this.showCreateMenu.bind(this); this.state = { @@ -64,7 +64,7 @@ class BaseSidebarScreen extends Component { } componentDidUpdate(prevProps) { - if (!this.didBecomeInactive(prevProps)) { + if (!this.didScreenBecomeInactive(prevProps)) { return; } @@ -72,13 +72,19 @@ class BaseSidebarScreen extends Component { this.hideCreateMenu(); } - didBecomeInactive(prevProps) { - // When Report page is just opened + /** + * Check if LHN became inactive from active status + * + * @param {Object} prevProps + * @return {boolean} + */ + didScreenBecomeInactive(prevProps) { + // When the Drawer gets closed and ReportScreen is shown if (!this.props.isDrawerOpen && prevProps.isDrawerOpen) { return true; } - // When other page is just opened + // When other page is opened over LHN if (!this.props.isFocused && prevProps.isFocused) { return true; } @@ -86,7 +92,12 @@ class BaseSidebarScreen extends Component { return false; } - isInactive() { + /** + * Check if LHN is inactive + * + * @return {boolean} + */ + isScreenInactive() { // When drawer is closed and Report page is open if (this.props.isSmallScreenWidth && !this.props.isDrawerOpen) { return true; @@ -104,7 +115,7 @@ class BaseSidebarScreen extends Component { * Method called when we click the floating action button */ showCreateMenu() { - if (this.isInactive()) { + if (this.isScreenInactive()) { // Prevent showing menu when click FAB icon quickly after opening other pages return; } @@ -114,17 +125,6 @@ class BaseSidebarScreen extends Component { this.props.onShowCreateMenu(); } - /** - * Method called when avatar is clicked - */ - navigateToSettings() { - if (this.state.isCreateMenuActive) { - // Prevent opening Settings page when click profile avatar quickly after clicking FAB icon - return; - } - Navigation.navigate(ROUTES.SETTINGS); - } - /** * Method called either when: * Pressing the floating action button to open the CreateMenu modal @@ -162,10 +162,9 @@ class BaseSidebarScreen extends Component { {}} isSmallScreenWidth={false} reportIDFromRoute={reportIDFromRoute} /> From fc227dd5a7bfd186a9e9351107a7617d532b89ba Mon Sep 17 00:00:00 2001 From: aimane-chnaif Date: Wed, 16 Nov 2022 06:12:28 +1000 Subject: [PATCH 3/5] fix typo --- src/pages/home/sidebar/SidebarLinks.js | 2 +- src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index cc4f4f161a33..be43de83e3cd 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -97,7 +97,7 @@ class SidebarLinks extends React.Component { } showSettingsPage() { - if (this.state.isCreateMenuActive) { + if (this.props.isCreateMenuOpen) { // Prevent opening Settings page when click profile avatar quickly after clicking FAB icon return; } diff --git a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js index 7ecf96531279..1b7f6499611d 100644 --- a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js +++ b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js @@ -84,7 +84,7 @@ class BaseSidebarScreen extends Component { return true; } - // When other page is opened over LHN + // When any other page is opened over LHN if (!this.props.isFocused && prevProps.isFocused) { return true; } @@ -103,7 +103,7 @@ class BaseSidebarScreen extends Component { return true; } - // When other page is open + // When any other page is open if (!this.props.isFocused) { return true; } From b8f6762475e68b3718c70bf132c05e8dda65d27d Mon Sep 17 00:00:00 2001 From: aimane-chnaif Date: Wed, 16 Nov 2022 06:19:27 +1000 Subject: [PATCH 4/5] fix typo --- src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js index 1b7f6499611d..26c91da18fe3 100644 --- a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js +++ b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js @@ -73,10 +73,10 @@ class BaseSidebarScreen extends Component { } /** - * Check if LHN became inactive from active status + * Check if LHN status changed from active to inactive * * @param {Object} prevProps - * @return {boolean} + * @return {Boolean} */ didScreenBecomeInactive(prevProps) { // When the Drawer gets closed and ReportScreen is shown @@ -95,7 +95,7 @@ class BaseSidebarScreen extends Component { /** * Check if LHN is inactive * - * @return {boolean} + * @return {Boolean} */ isScreenInactive() { // When drawer is closed and Report page is open From f20e5aabaeff965c796ac7352c52f62ccd411746 Mon Sep 17 00:00:00 2001 From: aimane-chnaif Date: Wed, 16 Nov 2022 07:00:21 +1000 Subject: [PATCH 5/5] add comments for didScreenBecomeInactive, isScreenInactive functions --- src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js index 26c91da18fe3..9f200ec9356d 100644 --- a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js +++ b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js @@ -73,7 +73,8 @@ class BaseSidebarScreen extends Component { } /** - * Check if LHN status changed from active to inactive + * Check if LHN status changed from active to inactive. + * Used to close already opened FAB menu when open any other pages (i.e. Press Command + K on web). * * @param {Object} prevProps * @return {Boolean} @@ -93,7 +94,8 @@ class BaseSidebarScreen extends Component { } /** - * Check if LHN is inactive + * Check if LHN is inactive. + * Used to prevent FAB menu showing after opening any other pages. * * @return {Boolean} */