From b3b4dafcf6611206e83b7244ddb026dca03e4468 Mon Sep 17 00:00:00 2001 From: Rocio Perez-Cano Date: Tue, 3 Jan 2023 16:12:40 +0100 Subject: [PATCH 01/10] Move KeyboardShortcut modal to report view --- src/pages/home/ReportScreen.js | 2 ++ src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 5b665c525b38..082c1f827f3d 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -33,6 +33,7 @@ import withLocalize from '../../components/withLocalize'; import reportPropTypes from '../reportPropTypes'; import FullPageNotFoundView from '../../components/BlockingViews/FullPageNotFoundView'; import ReportHeaderSkeletonView from '../../components/ReportHeaderSkeletonView'; +import KeyboardShortcutsModal from "../../components/KeyboardShortcutsModal"; const propTypes = { /** Navigation route context info provided by react navigation */ @@ -323,6 +324,7 @@ class ReportScreen extends React.Component { + ); diff --git a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js index 14ca7bc531e5..1f8258350524 100644 --- a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js +++ b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js @@ -9,7 +9,6 @@ import Timing from '../../../../libs/actions/Timing'; import CONST from '../../../../CONST'; import Performance from '../../../../libs/Performance'; import withDrawerState from '../../../../components/withDrawerState'; -import KeyboardShortcutsModal from '../../../../components/KeyboardShortcutsModal'; import withWindowDimensions, {windowDimensionsPropTypes} from '../../../../components/withWindowDimensions'; import compose from '../../../../libs/compose'; import sidebarPropTypes from './sidebarPropTypes'; @@ -65,7 +64,6 @@ class BaseSidebarScreen extends Component { reportIDFromRoute={this.props.reportIDFromRoute} /> - {this.props.children} )} From 561e354cbb87796e50f4faf2b1079e664129ac55 Mon Sep 17 00:00:00 2001 From: Rocio Perez-Cano Date: Tue, 7 Feb 2023 22:51:16 -0500 Subject: [PATCH 02/10] Listen for modal opening --- src/Expensify.js | 2 ++ src/components/Popover/index.js | 12 ++++++++++-- src/components/Popover/popoverPropTypes.js | 2 +- src/pages/home/ReportScreen.js | 2 -- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Expensify.js b/src/Expensify.js index 1a48abdc57a9..b4a4f341194e 100644 --- a/src/Expensify.js +++ b/src/Expensify.js @@ -27,6 +27,7 @@ import Navigation from './libs/Navigation/Navigation'; import DeeplinkWrapper from './components/DeeplinkWrapper'; import PopoverReportActionContextMenu from './pages/home/report/ContextMenu/PopoverReportActionContextMenu'; import * as ReportActionContextMenu from './pages/home/report/ContextMenu/ReportActionContextMenu'; +import KeyboardShortcutsModal from './components/KeyboardShortcutsModal'; // This lib needs to be imported, but it has nothing to export since all it contains is an Onyx connection // eslint-disable-next-line no-unused-vars @@ -197,6 +198,7 @@ class Expensify extends PureComponent { {!this.state.isSplashShown && ( <> + { if (!props.fullscreen && !props.isSmallScreenWidth) { return createPortal( { Popover.propTypes = propTypes; Popover.defaultProps = defaultProps; -Popover.displayName = 'Popover'; -export default withWindowDimensions(Popover); +export default compose( + withWindowDimensions, + withOnyx({ + isShortcutsModalOpen: {key: ONYXKEYS.IS_SHORTCUTS_MODAL_OPEN}, + }), +)(Popover); diff --git a/src/components/Popover/popoverPropTypes.js b/src/components/Popover/popoverPropTypes.js index 839ab68204d0..d93e97a0297f 100644 --- a/src/components/Popover/popoverPropTypes.js +++ b/src/components/Popover/popoverPropTypes.js @@ -4,7 +4,7 @@ import {propTypes as modalPropTypes, defaultProps as defaultModalProps} from '.. import CONST from '../../CONST'; const propTypes = { - ...(_.omit(modalPropTypes, ['type', 'popoverAnchorPosition'])), + ...(_.omit(modalPropTypes, ['type', 'popoverAnchorPosition', 'isVisible'])), /** The anchor position of the popover */ anchorPosition: PropTypes.shape({ diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index e68ad6a215fe..8b5d746e1f47 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -33,7 +33,6 @@ import withLocalize from '../../components/withLocalize'; import reportPropTypes from '../reportPropTypes'; import FullPageNotFoundView from '../../components/BlockingViews/FullPageNotFoundView'; import ReportHeaderSkeletonView from '../../components/ReportHeaderSkeletonView'; -import KeyboardShortcutsModal from "../../components/KeyboardShortcutsModal"; const propTypes = { /** Navigation route context info provided by react navigation */ @@ -315,7 +314,6 @@ class ReportScreen extends React.Component { - ); From c13ad940bd546962d476a339167a5b421a6d6795 Mon Sep 17 00:00:00 2001 From: Rocio Perez-Cano Date: Tue, 7 Feb 2023 23:08:36 -0500 Subject: [PATCH 03/10] Use props properly --- src/components/Popover/index.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/components/Popover/index.js b/src/components/Popover/index.js index 01bf66df27a0..e9b0d9d1c652 100644 --- a/src/components/Popover/index.js +++ b/src/components/Popover/index.js @@ -1,13 +1,24 @@ import React from 'react'; +import PropTypes from 'prop-types'; import {createPortal} from 'react-dom'; import {withOnyx} from 'react-native-onyx'; -import {propTypes, defaultProps} from './popoverPropTypes'; +import {propTypes as popoverPropTypes, defaultProps as popoverDefaultProps} from './popoverPropTypes'; import CONST from '../../CONST'; import Modal from '../Modal'; import withWindowDimensions from '../withWindowDimensions'; import compose from '../../libs/compose'; import ONYXKEYS from '../../ONYXKEYS'; +const propTypes = { + isShortcutsModalOpen: PropTypes.bool, + ...popoverPropTypes +}; + +const defaultProps = { + isShortcutsModalOpen: false, + ...popoverDefaultProps +}; + /* * This is a convenience wrapper around the Modal component for a responsive Popover. * On small screen widths, it uses BottomDocked modal type, and a Popover type on wide screen widths. @@ -16,7 +27,7 @@ const Popover = (props) => { if (!props.fullscreen && !props.isSmallScreenWidth) { return createPortal( { } return ( Date: Wed, 8 Feb 2023 14:34:06 -0500 Subject: [PATCH 04/10] Don't let props get overwritten --- src/components/Popover/index.js | 10 +++++----- src/components/Popover/popoverPropTypes.js | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/Popover/index.js b/src/components/Popover/index.js index e9b0d9d1c652..9b990bf27439 100644 --- a/src/components/Popover/index.js +++ b/src/components/Popover/index.js @@ -27,11 +27,11 @@ const Popover = (props) => { if (!props.fullscreen && !props.isSmallScreenWidth) { return createPortal( { } return ( Date: Wed, 8 Feb 2023 15:35:15 -0500 Subject: [PATCH 05/10] Close popover when opening shortcuts --- src/components/Popover/index.js | 67 ++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/src/components/Popover/index.js b/src/components/Popover/index.js index 9b990bf27439..31c4b7ca2a47 100644 --- a/src/components/Popover/index.js +++ b/src/components/Popover/index.js @@ -23,35 +23,50 @@ const defaultProps = { * This is a convenience wrapper around the Modal component for a responsive Popover. * On small screen widths, it uses BottomDocked modal type, and a Popover type on wide screen widths. */ -const Popover = (props) => { - if (!props.fullscreen && !props.isSmallScreenWidth) { - return createPortal( + +class Popover extends React.Component { + constructor(props) { + super(props); + } + + componentDidUpdate(prevProps) { + // There are modals that can show up on top of these pop-overs, for example, the keyboard shortcut menu, + // if that happens, close the pop-over as if we were clicking outside. + if (this.props.isShortcutsModalOpen && this.props.isVisible) { + this.props.onClose(); + } + } + + render () { + if (!this.props.fullscreen && !this.props.isSmallScreenWidth) { + return createPortal( + , + document.body, + ); + } + return ( , - document.body, + {...this.props} + isVisible={this.props.isVisible && !this.props.isShortcutsModalOpen} + type={this.props.isSmallScreenWidth ? CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED : CONST.MODAL.MODAL_TYPE.POPOVER} + popoverAnchorPosition={this.props.isSmallScreenWidth ? undefined : this.props.anchorPosition} + fullscreen={this.props.isSmallScreenWidth ? true : this.props.fullscreen} + animationInTiming={this.props.disableAnimation && !this.props.isSmallScreenWidth ? 1 : this.props.animationInTiming} + animationOutTiming={this.props.disableAnimation && !this.props.isSmallScreenWidth ? 1 : this.props.animationOutTiming} + /> ); - } - return ( - - ); -}; + }; +} Popover.propTypes = propTypes; Popover.defaultProps = defaultProps; From 3e8776e1b2171da08750e08501b95c680cbfa228 Mon Sep 17 00:00:00 2001 From: Rocio Perez-Cano Date: Wed, 8 Feb 2023 15:47:14 -0500 Subject: [PATCH 06/10] Linter --- src/components/Popover/index.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/components/Popover/index.js b/src/components/Popover/index.js index 31c4b7ca2a47..b640814c537e 100644 --- a/src/components/Popover/index.js +++ b/src/components/Popover/index.js @@ -11,12 +11,12 @@ import ONYXKEYS from '../../ONYXKEYS'; const propTypes = { isShortcutsModalOpen: PropTypes.bool, - ...popoverPropTypes + ...popoverPropTypes, }; const defaultProps = { isShortcutsModalOpen: false, - ...popoverDefaultProps + ...popoverDefaultProps, }; /* @@ -25,19 +25,17 @@ const defaultProps = { */ class Popover extends React.Component { - constructor(props) { - super(props); - } - componentDidUpdate(prevProps) { + if (!this.props.isShortcutsModalOpen || !this.props.isVisible) { + return; + } + // There are modals that can show up on top of these pop-overs, for example, the keyboard shortcut menu, // if that happens, close the pop-over as if we were clicking outside. - if (this.props.isShortcutsModalOpen && this.props.isVisible) { - this.props.onClose(); - } + this.props.onClose(); } - render () { + render() { if (!this.props.fullscreen && !this.props.isSmallScreenWidth) { return createPortal( ); - }; + } } Popover.propTypes = propTypes; From ca736f4e15416a4dcde1f3b006aaf31780c91c16 Mon Sep 17 00:00:00 2001 From: Rocio Perez-Cano Date: Wed, 8 Feb 2023 15:50:54 -0500 Subject: [PATCH 07/10] I forgot to remove unused parameter --- src/components/Popover/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Popover/index.js b/src/components/Popover/index.js index b640814c537e..bb712a37262f 100644 --- a/src/components/Popover/index.js +++ b/src/components/Popover/index.js @@ -25,7 +25,7 @@ const defaultProps = { */ class Popover extends React.Component { - componentDidUpdate(prevProps) { + componentDidUpdate() { if (!this.props.isShortcutsModalOpen || !this.props.isVisible) { return; } From dcbbbfc54781669f449b5c33ab14f18978ffe620 Mon Sep 17 00:00:00 2001 From: Rocio Perez-Cano Date: Thu, 9 Feb 2023 17:08:04 -0500 Subject: [PATCH 08/10] Revert class --- src/components/Popover/index.js | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/components/Popover/index.js b/src/components/Popover/index.js index bb712a37262f..1b3f2e4fc600 100644 --- a/src/components/Popover/index.js +++ b/src/components/Popover/index.js @@ -23,19 +23,15 @@ const defaultProps = { * This is a convenience wrapper around the Modal component for a responsive Popover. * On small screen widths, it uses BottomDocked modal type, and a Popover type on wide screen widths. */ - -class Popover extends React.Component { - componentDidUpdate() { - if (!this.props.isShortcutsModalOpen || !this.props.isVisible) { - return; +const Popover = (props) => { + render() { + if (this.props.isShortcutsModalOpen && this.props.isVisiblee) { + // There are modals that can show up on top of these pop-overs, for example, the keyboard shortcut menu, + // if that happens, close the pop-over as if we were clicking outside. + this.props.onClose(); + return null; } - // There are modals that can show up on top of these pop-overs, for example, the keyboard shortcut menu, - // if that happens, close the pop-over as if we were clicking outside. - this.props.onClose(); - } - - render() { if (!this.props.fullscreen && !this.props.isSmallScreenWidth) { return createPortal( ); } -} +}; Popover.propTypes = propTypes; Popover.defaultProps = defaultProps; From 8122a869035dfa5cde6ed6f96e0ecc3aead9cad6 Mon Sep 17 00:00:00 2001 From: Rocio Perez-Cano Date: Thu, 9 Feb 2023 17:18:38 -0500 Subject: [PATCH 09/10] Remove render --- src/components/Popover/index.js | 60 ++++++++++++++++----------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/src/components/Popover/index.js b/src/components/Popover/index.js index 1b3f2e4fc600..638ea7b011a7 100644 --- a/src/components/Popover/index.js +++ b/src/components/Popover/index.js @@ -24,42 +24,40 @@ const defaultProps = { * On small screen widths, it uses BottomDocked modal type, and a Popover type on wide screen widths. */ const Popover = (props) => { - render() { - if (this.props.isShortcutsModalOpen && this.props.isVisiblee) { - // There are modals that can show up on top of these pop-overs, for example, the keyboard shortcut menu, - // if that happens, close the pop-over as if we were clicking outside. - this.props.onClose(); - return null; - } + if (props.isShortcutsModalOpen && props.isVisible) { + // There are modals that can show up on top of these pop-overs, for example, the keyboard shortcut menu, + // if that happens, close the pop-over as if we were clicking outside. + props.onClose(); + return null; + } - if (!this.props.fullscreen && !this.props.isSmallScreenWidth) { - return createPortal( - , - document.body, - ); - } - return ( + if (!props.fullscreen && !props.isSmallScreenWidth) { + return createPortal( + {...props} + isVisible={props.isVisible && !props.isShortcutsModalOpen} + type={CONST.MODAL.MODAL_TYPE.POPOVER} + popoverAnchorPosition={props.anchorPosition} + animationInTiming={props.disableAnimation ? 1 : props.animationInTiming} + animationOutTiming={props.disableAnimation ? 1 : props.animationOutTiming} + shouldCloseOnOutsideClick + />, + document.body, ); } + return ( + + ); }; Popover.propTypes = propTypes; From 40f53721ce146cc2d8c0f5f8767e437cfb443ef9 Mon Sep 17 00:00:00 2001 From: Rocio Perez-Cano Date: Thu, 9 Feb 2023 17:25:05 -0500 Subject: [PATCH 10/10] Bring back displayname --- src/components/Popover/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Popover/index.js b/src/components/Popover/index.js index 638ea7b011a7..54ca87239593 100644 --- a/src/components/Popover/index.js +++ b/src/components/Popover/index.js @@ -62,6 +62,7 @@ const Popover = (props) => { Popover.propTypes = propTypes; Popover.defaultProps = defaultProps; +Popover.displayName = 'Popover'; export default compose( withWindowDimensions,