From 8737fb4a153b6a57c756e20607caa3a3eeede178 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Tue, 8 Jun 2021 20:43:46 +0530 Subject: [PATCH 1/7] set the focus on the Edit ox on edit action --- src/components/Modal/BaseModal.js | 8 ++++++-- src/components/Modal/ModalPropTypes.js | 4 ++++ src/pages/home/report/ReportActionContextMenu.js | 9 +++++++-- src/pages/home/report/ReportActionItem.js | 9 ++++++++- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/components/Modal/BaseModal.js b/src/components/Modal/BaseModal.js index f0b5fb6a1a7a..201002f995c4 100644 --- a/src/components/Modal/BaseModal.js +++ b/src/components/Modal/BaseModal.js @@ -28,7 +28,9 @@ class BaseModal extends PureComponent { */ hideModalAndRemoveEventListeners() { this.unsubscribeFromKeyEvents(); - setModalVisibility(false); + if (this.props.setModalVisibility) { + setModalVisibility(false); + } this.props.onModalHide(); } @@ -79,7 +81,9 @@ class BaseModal extends PureComponent { onBackButtonPress={this.props.onClose} onModalShow={() => { this.subscribeToKeyEvents(); - setModalVisibility(true); + if (this.props.setModalVisibility) { + setModalVisibility(true); + } this.props.onModalShow(); }} onModalHide={this.hideModalAndRemoveEventListeners} diff --git a/src/components/Modal/ModalPropTypes.js b/src/components/Modal/ModalPropTypes.js index 6e6f7dd850d7..cadac0b02d1a 100644 --- a/src/components/Modal/ModalPropTypes.js +++ b/src/components/Modal/ModalPropTypes.js @@ -4,6 +4,9 @@ import CONST from '../../CONST'; import {windowDimensionsPropTypes} from '../withWindowDimensions'; const propTypes = { + /** Should be announce the Modal visibility changes? */ + setModalVisibility: PropTypes.bool, + /** Callback method fired when the user requests to close the modal */ onClose: PropTypes.func.isRequired, @@ -49,6 +52,7 @@ const propTypes = { }; const defaultProps = { + setModalVisibility: true, onSubmit: null, type: '', onModalHide: () => {}, diff --git a/src/pages/home/report/ReportActionContextMenu.js b/src/pages/home/report/ReportActionContextMenu.js index 8f4079dc0d79..badeca9cac64 100755 --- a/src/pages/home/report/ReportActionContextMenu.js +++ b/src/pages/home/report/ReportActionContextMenu.js @@ -112,12 +112,17 @@ class ReportActionContextMenu extends React.Component { icon: Pencil, shouldShow: () => canEditReportAction(this.props.reportAction), onPress: () => { - this.props.hidePopover(); - saveReportActionDraft( + const editAction = () => saveReportActionDraft( this.props.reportID, this.props.reportAction.reportActionID, _.isEmpty(this.props.draftMessage) ? this.getActionText() : '', ); + this.props.hidePopover(editAction); + + // When menu is mini, there is no popover. Thus we need to call the editAction manually + if (this.props.isMini) { + editAction(); + } }, }, { diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index f71726ec1cae..eb9e6d06d834 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -63,6 +63,7 @@ class ReportActionItem extends Component { this.state = { isPopoverVisible: false, + onPopoverHide: () => {}, cursorPosition: { horizontal: 0, vertical: 0, @@ -173,9 +174,13 @@ class ReportActionItem extends Component { /** * Hide the ReportActionContextMenu modal popover. + * @param {Function} onHideCallback Callback to be called after popover is completely hidden */ - hidePopover() { + hidePopover(onHideCallback) { this.setState({isPopoverVisible: false}); + if (onHideCallback) { + this.setState({onPopoverHide: onHideCallback}); + } } /** @@ -268,10 +273,12 @@ class ReportActionItem extends Component { Date: Tue, 8 Jun 2021 23:01:21 +0530 Subject: [PATCH 2/7] fix: callback needs to reset for each action --- src/pages/home/report/ReportActionContextMenu.js | 8 +++----- src/pages/home/report/ReportActionItem.js | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/pages/home/report/ReportActionContextMenu.js b/src/pages/home/report/ReportActionContextMenu.js index 62581bb3b62b..dfad85843346 100755 --- a/src/pages/home/report/ReportActionContextMenu.js +++ b/src/pages/home/report/ReportActionContextMenu.js @@ -132,9 +132,7 @@ class ReportActionContextMenu extends React.Component { text: this.props.translate('reportActionContextMenu.deleteComment'), icon: Trashcan, shouldShow: () => canEditReportAction(this.props.reportAction), - onPress: () => { - this.setState({isDeleteCommentConfirmModalVisible: true}); - }, + onPress: () => this.setState({isDeleteCommentConfirmModalVisible: true}), }, ]; @@ -170,10 +168,10 @@ class ReportActionContextMenu extends React.Component { * Hides the popover menu with an optional delay * * @param {Boolean} shouldDelay whether the menu should close after a delay - * @param {Boolean} onHideCallback Callback to be callled after Popover Menu is hidden + * @param {Function} [onHideCallback=() => {}] Callback to be called after Popover Menu is hidden * @memberof ReportActionContextMenu */ - hidePopover(shouldDelay, onHideCallback) { + hidePopover(shouldDelay, onHideCallback = () => {}) { if (!shouldDelay) { this.props.hidePopover(onHideCallback); return; diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index eb9e6d06d834..e3c0404af90b 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -61,9 +61,9 @@ class ReportActionItem extends Component { constructor(props) { super(props); + this.onPopoverHide = () => {}; this.state = { isPopoverVisible: false, - onPopoverHide: () => {}, cursorPosition: { horizontal: 0, vertical: 0, @@ -179,7 +179,7 @@ class ReportActionItem extends Component { hidePopover(onHideCallback) { this.setState({isPopoverVisible: false}); if (onHideCallback) { - this.setState({onPopoverHide: onHideCallback}); + this.onPopoverHide = onHideCallback; } } @@ -273,7 +273,7 @@ class ReportActionItem extends Component { Date: Wed, 9 Jun 2021 19:50:57 +0530 Subject: [PATCH 3/7] added a new composer focus strategy --- src/libs/ReportActionComposeFocusManager.js | 35 +++++++++++++++++++ src/pages/home/report/ReportActionCompose.js | 5 ++- .../home/report/ReportActionContextMenu.js | 5 +-- src/pages/home/report/ReportActionItem.js | 2 +- .../report/ReportActionItemMessageEdit.js | 2 ++ 5 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 src/libs/ReportActionComposeFocusManager.js diff --git a/src/libs/ReportActionComposeFocusManager.js b/src/libs/ReportActionComposeFocusManager.js new file mode 100644 index 000000000000..082cb62e3f4d --- /dev/null +++ b/src/libs/ReportActionComposeFocusManager.js @@ -0,0 +1,35 @@ +let focusCallback = null; + +/** + * Register a callback to be called when focus is requested. + * Typical uses of this would be call the focus on the ReportActionComposer. + * + * @param {Function} callback callback to register + */ +function onComposerFocus(callback) { + focusCallback = callback; +} + +/** + * Request focus on the ReportActionComposer + * + */ +function focus() { + if (focusCallback) { + focusCallback(); + } +} + +/** + * Clear the registered focus callback + * + */ +function clear() { + focusCallback = null; +} + +export default { + onComposerFocus, + focus, + clear, +}; diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index af29d8387657..8130c85ed813 100755 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -51,6 +51,7 @@ import Navigation from '../../../libs/Navigation/Navigation'; import ROUTES from '../../../ROUTES'; import ReportActionPropTypes from './ReportActionPropTypes'; import {canEditReportAction} from '../../../libs/reportUtils'; +import ReportActionComposeFocusManager from '../../../libs/ReportActionComposeFocusManager'; const propTypes = { /** Beta features list */ @@ -150,6 +151,7 @@ class ReportActionCompose extends React.Component { } componentDidMount() { + ReportActionComposeFocusManager.onComposerFocus(this.focus); Dimensions.addEventListener('change', this.measureEmojiPopoverAnchorPosition); } @@ -165,6 +167,7 @@ class ReportActionCompose extends React.Component { } componentWillUnmount() { + ReportActionComposeFocusManager.clear(); Dimensions.removeEventListener('change', this.measureEmojiPopoverAnchorPosition); } @@ -203,7 +206,7 @@ class ReportActionCompose extends React.Component { * Focus the composer text input */ focus() { - if (this.textInput) { + if (this.shouldFocusInputOnScreenFocus && this.props.isFocused && this.textInput) { // There could be other animations running while we trigger manual focus. // This prevents focus from making those animations janky. InteractionManager.runAfterInteractions(() => { diff --git a/src/pages/home/report/ReportActionContextMenu.js b/src/pages/home/report/ReportActionContextMenu.js index dfad85843346..0ecb518938c6 100755 --- a/src/pages/home/report/ReportActionContextMenu.js +++ b/src/pages/home/report/ReportActionContextMenu.js @@ -18,6 +18,7 @@ import compose from '../../../libs/compose'; import {isReportMessageAttachment, canEditReportAction} from '../../../libs/reportUtils'; import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize'; import ConfirmModal from '../../../components/ConfirmModal'; +import ReportActionComposeFocusManager from '../../../libs/ReportActionComposeFocusManager'; const propTypes = { /** The ID of the report this report action is attached to. */ @@ -87,7 +88,7 @@ class ReportActionContextMenu extends React.Component { } else { Clipboard.setString(html); } - this.hidePopover(true); + this.hidePopover(true, ReportActionComposeFocusManager.focus); }, }, @@ -106,7 +107,7 @@ class ReportActionContextMenu extends React.Component { onPress: () => { updateLastReadActionID(this.props.reportID, this.props.reportAction.sequenceNumber); setNewMarkerPosition(this.props.reportID, this.props.reportAction.sequenceNumber); - this.hidePopover(true); + this.hidePopover(true, ReportActionComposeFocusManager.focus); }, }, diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index e3c0404af90b..e0ef49c4e3ab 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -177,10 +177,10 @@ class ReportActionItem extends Component { * @param {Function} onHideCallback Callback to be called after popover is completely hidden */ hidePopover(onHideCallback) { - this.setState({isPopoverVisible: false}); if (onHideCallback) { this.onPopoverHide = onHideCallback; } + this.setState({isPopoverVisible: false}); } /** diff --git a/src/pages/home/report/ReportActionItemMessageEdit.js b/src/pages/home/report/ReportActionItemMessageEdit.js index 798fc3ad5d0c..bf6413754cd8 100644 --- a/src/pages/home/report/ReportActionItemMessageEdit.js +++ b/src/pages/home/report/ReportActionItemMessageEdit.js @@ -10,6 +10,7 @@ import {scrollToIndex} from '../../../libs/ReportScrollManager'; import toggleReportActionComposeView from '../../../libs/toggleReportActionComposeView'; import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions'; import Button from '../../../components/Button'; +import ReportActionComposeFocusManager from '../../../libs/ReportActionComposeFocusManager'; const propTypes = { /** All the data of the action */ @@ -73,6 +74,7 @@ class ReportActionItemMessageEdit extends React.Component { deleteDraft() { saveReportActionDraft(this.props.reportID, this.props.action.reportActionID, ''); toggleReportActionComposeView(true, this.props.isSmallScreenWidth); + ReportActionComposeFocusManager.focus(); } /** From 713dfb096deca92a1619b5634f6e5744b9ea4018 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Wed, 9 Jun 2021 20:04:52 +0530 Subject: [PATCH 4/7] fix: bug transItionEnd does not fire on IOS --- src/libs/Navigation/AppNavigator/AuthScreens.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index a032193f97f8..9814be32abf0 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -72,11 +72,11 @@ Onyx.connect({ const RootStack = createCustomModalStackNavigator(); // We want to delay the re-rendering for components(e.g. ReportActionCompose) -// that depends on modal visibility until Modal is completely closed or its transition has ended -// When modal screen is focused and animation transition is ended, update modal visibility in Onyx +// that depends on modal visibility until Modal is completely closed and its focused +// When modal screen is focused, update modal visibility in Onyx // https://reactnavigation.org/docs/navigation-events/ const modalScreenListeners = { - transitionEnd: () => { + focus: () => { setModalVisibility(true); }, beforeRemove: () => { From 43bf806751d7643f8f9a005f19d0c77b73aba0fc Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Wed, 9 Jun 2021 23:56:14 +0530 Subject: [PATCH 5/7] tweaks --- src/components/Modal/BaseModal.js | 4 ++-- src/components/Modal/ModalPropTypes.js | 4 ++-- src/libs/ReportActionComposeFocusManager.js | 4 +++- src/pages/home/report/ReportActionContextMenu.js | 6 ++++-- src/pages/home/report/ReportActionItem.js | 2 +- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/components/Modal/BaseModal.js b/src/components/Modal/BaseModal.js index 201002f995c4..56604e48e119 100644 --- a/src/components/Modal/BaseModal.js +++ b/src/components/Modal/BaseModal.js @@ -28,7 +28,7 @@ class BaseModal extends PureComponent { */ hideModalAndRemoveEventListeners() { this.unsubscribeFromKeyEvents(); - if (this.props.setModalVisibility) { + if (this.props.shouldSetModalVisibility) { setModalVisibility(false); } this.props.onModalHide(); @@ -81,7 +81,7 @@ class BaseModal extends PureComponent { onBackButtonPress={this.props.onClose} onModalShow={() => { this.subscribeToKeyEvents(); - if (this.props.setModalVisibility) { + if (this.props.shouldSetModalVisibility) { setModalVisibility(true); } this.props.onModalShow(); diff --git a/src/components/Modal/ModalPropTypes.js b/src/components/Modal/ModalPropTypes.js index cadac0b02d1a..ce95a73eedf2 100644 --- a/src/components/Modal/ModalPropTypes.js +++ b/src/components/Modal/ModalPropTypes.js @@ -5,7 +5,7 @@ import {windowDimensionsPropTypes} from '../withWindowDimensions'; const propTypes = { /** Should be announce the Modal visibility changes? */ - setModalVisibility: PropTypes.bool, + shouldSetModalVisibility: PropTypes.bool, /** Callback method fired when the user requests to close the modal */ onClose: PropTypes.func.isRequired, @@ -52,7 +52,7 @@ const propTypes = { }; const defaultProps = { - setModalVisibility: true, + shouldSetModalVisibility: true, onSubmit: null, type: '', onModalHide: () => {}, diff --git a/src/libs/ReportActionComposeFocusManager.js b/src/libs/ReportActionComposeFocusManager.js index 082cb62e3f4d..9edd46c798fe 100644 --- a/src/libs/ReportActionComposeFocusManager.js +++ b/src/libs/ReportActionComposeFocusManager.js @@ -1,3 +1,5 @@ +import _ from 'underscore'; + let focusCallback = null; /** @@ -15,7 +17,7 @@ function onComposerFocus(callback) { * */ function focus() { - if (focusCallback) { + if (_.isFunction(focusCallback)) { focusCallback(); } } diff --git a/src/pages/home/report/ReportActionContextMenu.js b/src/pages/home/report/ReportActionContextMenu.js index 0ecb518938c6..c38b8d7db847 100755 --- a/src/pages/home/report/ReportActionContextMenu.js +++ b/src/pages/home/report/ReportActionContextMenu.js @@ -121,11 +121,13 @@ class ReportActionContextMenu extends React.Component { this.props.reportAction.reportActionID, _.isEmpty(this.props.draftMessage) ? this.getActionText() : '', ); - this.hidePopover(false, editAction); - // When menu is mini, there is no popover. Thus we need to call the editAction manually if (this.props.isMini) { + // No popover to hide, call editAction immediately editAction(); + } else { + // Hide popover, then call editAction + this.hidePopover(false, editAction); } }, }, diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index e0ef49c4e3ab..41d0a110800f 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -177,7 +177,7 @@ class ReportActionItem extends Component { * @param {Function} onHideCallback Callback to be called after popover is completely hidden */ hidePopover(onHideCallback) { - if (onHideCallback) { + if (_.isFunction(onHideCallback)) { this.onPopoverHide = onHideCallback; } this.setState({isPopoverVisible: false}); From fda1967e0d461265c37a9b21fb37368402a51f76 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Wed, 9 Jun 2021 23:58:02 +0530 Subject: [PATCH 6/7] typo --- src/components/Modal/ModalPropTypes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Modal/ModalPropTypes.js b/src/components/Modal/ModalPropTypes.js index ce95a73eedf2..8da6403308a3 100644 --- a/src/components/Modal/ModalPropTypes.js +++ b/src/components/Modal/ModalPropTypes.js @@ -4,7 +4,7 @@ import CONST from '../../CONST'; import {windowDimensionsPropTypes} from '../withWindowDimensions'; const propTypes = { - /** Should be announce the Modal visibility changes? */ + /** Should we announce the Modal visibility changes? */ shouldSetModalVisibility: PropTypes.bool, /** Callback method fired when the user requests to close the modal */ From 65662dd7b0c3100743670698ab1cb5e9b583ff4f Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 10 Jun 2021 23:47:17 +0530 Subject: [PATCH 7/7] fix: prop name --- src/pages/home/report/ReportActionItem.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 41d0a110800f..9b8b7d6d0947 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -278,7 +278,7 @@ class ReportActionItem extends Component { animationIn="fadeIn" animationOutTiming={1} measureContent={this.measureContent} - setModalVisibility={false} + shouldSetModalVisibility={false} >