From f4811ff5fac8f752ce4d05042ad0d1d48688a9fa Mon Sep 17 00:00:00 2001 From: hublot Date: Fri, 16 Jun 2023 15:15:09 +0800 Subject: [PATCH 1/7] Hide ReportActionContextMenu when ReportActionItem is destroyed --- src/pages/home/report/ReportActionItem.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 802d0d9aa164..827b857df9c4 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -110,6 +110,19 @@ function ReportActionItem(props) { const popoverAnchorRef = useRef(); const downloadedPreviews = useRef([]); + useEffect(() => { + return () => { + // ReportActionContextMenu is a global component, + // we use ReportActionContextMenu.showContextMenu to show them, + // so we should also hide them when the current component is destroyed + if (!ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) { + return; + } + ReportActionContextMenu.hideContextMenu(); + ReportActionContextMenu.hideDeleteModal(); + } + }, []); + const isDraftEmpty = !props.draftMessage; useEffect(() => { if (isDraftEmpty) { From f8f78dd1092995c9724a4ae0445b2a5aee7a6957 Mon Sep 17 00:00:00 2001 From: hublot Date: Fri, 16 Jun 2023 15:31:16 +0800 Subject: [PATCH 2/7] Fix Lint --- src/pages/home/report/ReportActionItem.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 827b857df9c4..cb588343cb6b 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -110,18 +110,16 @@ function ReportActionItem(props) { const popoverAnchorRef = useRef(); const downloadedPreviews = useRef([]); - useEffect(() => { - return () => { - // ReportActionContextMenu is a global component, - // we use ReportActionContextMenu.showContextMenu to show them, - // so we should also hide them when the current component is destroyed - if (!ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) { - return; - } - ReportActionContextMenu.hideContextMenu(); - ReportActionContextMenu.hideDeleteModal(); + useEffect(() => () => { + // ReportActionContextMenu is a global component, + // we use ReportActionContextMenu.showContextMenu to show them, + // so we should also hide them when the current component is destroyed + if (!ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) { + return; } - }, []); + ReportActionContextMenu.hideContextMenu(); + ReportActionContextMenu.hideDeleteModal(); + }, [props.action.reportActionID]); const isDraftEmpty = !props.draftMessage; useEffect(() => { From bbc656abe74fc17424c44132e3acec391c5271c7 Mon Sep 17 00:00:00 2001 From: hublot Date: Fri, 16 Jun 2023 15:38:36 +0800 Subject: [PATCH 3/7] Run Prettier --- src/pages/home/report/ReportActionItem.js | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index cb588343cb6b..da22c0a4445b 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -110,16 +110,19 @@ function ReportActionItem(props) { const popoverAnchorRef = useRef(); const downloadedPreviews = useRef([]); - useEffect(() => () => { - // ReportActionContextMenu is a global component, - // we use ReportActionContextMenu.showContextMenu to show them, - // so we should also hide them when the current component is destroyed - if (!ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) { - return; - } - ReportActionContextMenu.hideContextMenu(); - ReportActionContextMenu.hideDeleteModal(); - }, [props.action.reportActionID]); + useEffect( + () => () => { + // ReportActionContextMenu is a global component, + // we use ReportActionContextMenu.showContextMenu to show them, + // so we should also hide them when the current component is destroyed + if (!ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) { + return; + } + ReportActionContextMenu.hideContextMenu(); + ReportActionContextMenu.hideDeleteModal(); + }, + [props.action.reportActionID], + ); const isDraftEmpty = !props.draftMessage; useEffect(() => { From 4bae63917e1153dd5cef28c3848795546b22076f Mon Sep 17 00:00:00 2001 From: hublot Date: Fri, 16 Jun 2023 17:23:58 +0800 Subject: [PATCH 4/7] Hide the EmojiPicker when the corresponding reportAction is deleted --- src/components/EmojiPicker/EmojiPicker.js | 16 ++++++++-- src/components/Reactions/AddReactionBubble.js | 6 ++++ .../Reactions/MiniQuickEmojiReactions.js | 8 +++++ .../BaseQuickEmojiReactions.js | 6 ++++ .../Reactions/ReportActionItemReactions.js | 2 +- src/libs/actions/EmojiPickerAction.js | 32 +++++++++++++++++-- .../report/ContextMenu/ContextMenuActions.js | 2 ++ src/pages/home/report/ReportActionItem.js | 15 +++++---- 8 files changed, 75 insertions(+), 12 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index ed1a349f4ea2..f87d08f3c02b 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -34,6 +34,7 @@ class EmojiPicker extends React.Component { this.onEmojiSelected = () => {}; this.state = { + reportAction: {}, isEmojiPickerVisible: false, // The horizontal and vertical position (relative to the window) where the emoji popover will display. @@ -100,6 +101,16 @@ class EmojiPicker extends React.Component { this.setState({isEmojiPickerVisible: false}); } + /** + * Whether Context Menu is active for the Report Action. + * + * @param {Number|String} actionID + * @return {Boolean} + */ + isActiveReportAction(actionID) { + return Boolean(actionID) && this.state.reportAction.reportActionID === actionID; + } + /** * Show the emoji picker menu. * @@ -108,8 +119,9 @@ class EmojiPicker extends React.Component { * @param {Element} emojiPopoverAnchor - Element to which Popover is anchored * @param {Object} [anchorOrigin=DEFAULT_ANCHOR_ORIGIN] - Anchor origin for Popover * @param {Function} [onWillShow=() => {}] - Run a callback when Popover will show + * @param {Object} reportAction - ReportAction for EmojiPicker */ - showEmojiPicker(onModalHide, onEmojiSelected, emojiPopoverAnchor, anchorOrigin, onWillShow = () => {}) { + showEmojiPicker(onModalHide, onEmojiSelected, emojiPopoverAnchor, anchorOrigin, onWillShow = () => {}, reportAction) { this.onModalHide = onModalHide; this.onEmojiSelected = onEmojiSelected; this.emojiPopoverAnchor = emojiPopoverAnchor; @@ -121,7 +133,7 @@ class EmojiPicker extends React.Component { this.measureEmojiPopoverAnchorPosition().then((emojiPopoverAnchorPosition) => { onWillShow(); - this.setState({isEmojiPickerVisible: true, emojiPopoverAnchorPosition, emojiPopoverAnchorOrigin: anchorOrigin || DEFAULT_ANCHOR_ORIGIN}); + this.setState({reportAction, isEmojiPickerVisible: true, emojiPopoverAnchorPosition, emojiPopoverAnchorOrigin: anchorOrigin || DEFAULT_ANCHOR_ORIGIN}); }); } diff --git a/src/components/Reactions/AddReactionBubble.js b/src/components/Reactions/AddReactionBubble.js index 7048bc9735cf..85886071b921 100644 --- a/src/components/Reactions/AddReactionBubble.js +++ b/src/components/Reactions/AddReactionBubble.js @@ -35,6 +35,11 @@ const propTypes = { */ onSelectEmoji: PropTypes.func.isRequired, + /** + * ReportAction for EmojiPicker. + */ + reportAction: PropTypes.object, + ...withLocalizePropTypes, }; @@ -57,6 +62,7 @@ function AddReactionBubble(props) { refParam || ref.current, anchorOrigin, props.onWillShowPicker, + props.reportAction ); }; diff --git a/src/components/Reactions/MiniQuickEmojiReactions.js b/src/components/Reactions/MiniQuickEmojiReactions.js index 91fa8817172c..75a9a6e1396f 100644 --- a/src/components/Reactions/MiniQuickEmojiReactions.js +++ b/src/components/Reactions/MiniQuickEmojiReactions.js @@ -28,6 +28,11 @@ const propTypes = { */ onEmojiPickerClosed: PropTypes.func, + /** + * ReportAction for EmojiPicker. + */ + reportAction: PropTypes.object, + ...withLocalizePropTypes, preferredSkinTone: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), }; @@ -56,6 +61,9 @@ function MiniQuickEmojiReactions(props) { props.onEmojiSelected(emojiObject); }, ref.current, + undefined, + () => {}, + props.reportAction ); }; diff --git a/src/components/Reactions/QuickEmojiReactions/BaseQuickEmojiReactions.js b/src/components/Reactions/QuickEmojiReactions/BaseQuickEmojiReactions.js index 7a0eb42a0ebd..38d2fb9f5c5a 100644 --- a/src/components/Reactions/QuickEmojiReactions/BaseQuickEmojiReactions.js +++ b/src/components/Reactions/QuickEmojiReactions/BaseQuickEmojiReactions.js @@ -29,6 +29,11 @@ const baseQuickEmojiReactionsPropTypes = { * to actually open the emoji picker. */ onPressOpenPicker: PropTypes.func, + + /** + * ReportAction for EmojiPicker. + */ + reportAction: PropTypes.object, }; const baseQuickEmojiReactionsDefaultProps = { @@ -68,6 +73,7 @@ function BaseQuickEmojiReactions(props) { onPressOpenPicker={props.onPressOpenPicker} onWillShowPicker={props.onWillShowPicker} onSelectEmoji={props.onEmojiSelected} + reportAction={props.reportAction} /> ); diff --git a/src/components/Reactions/ReportActionItemReactions.js b/src/components/Reactions/ReportActionItemReactions.js index a770032d3049..b19369fb859b 100644 --- a/src/components/Reactions/ReportActionItemReactions.js +++ b/src/components/Reactions/ReportActionItemReactions.js @@ -98,7 +98,7 @@ function ReportActionItemReactions(props) { ); })} - {reactionsWithCount.length > 0 && } + {reactionsWithCount.length > 0 && } ); } diff --git a/src/libs/actions/EmojiPickerAction.js b/src/libs/actions/EmojiPickerAction.js index 6aba8e43bd23..c5b6b4daccad 100644 --- a/src/libs/actions/EmojiPickerAction.js +++ b/src/libs/actions/EmojiPickerAction.js @@ -10,13 +10,39 @@ const emojiPickerRef = React.createRef(); * @param {Element} emojiPopoverAnchor - Element on which EmojiPicker is anchored * @param {Object} [anchorOrigin] - Anchor origin for Popover * @param {Function} [onWillShow=() => {}] - Run a callback when Popover will show + * @param {Object} reportAction - ReportAction for EmojiPicker */ -function showEmojiPicker(onModalHide = () => {}, onEmojiSelected = () => {}, emojiPopoverAnchor, anchorOrigin = undefined, onWillShow = () => {}) { +function showEmojiPicker(onModalHide = () => {}, onEmojiSelected = () => {}, emojiPopoverAnchor, anchorOrigin = undefined, onWillShow = () => {}, reportAction = {}) { if (!emojiPickerRef.current) { return; } - emojiPickerRef.current.showEmojiPicker(onModalHide, onEmojiSelected, emojiPopoverAnchor, anchorOrigin, onWillShow); + emojiPickerRef.current.showEmojiPicker(onModalHide, onEmojiSelected, emojiPopoverAnchor, anchorOrigin, onWillShow, reportAction); } -export {emojiPickerRef, showEmojiPicker}; +/** + * Hide the Emoji Picker modal. + * + * @param {Boolean} isNavigating + */ +function hideEmojiPicker(isNavigating) { + if (!emojiPickerRef.current) { + return; + } + return emojiPickerRef.current.hideEmojiPicker(isNavigating); +} + +/** + * Whether Emoji Picker is active for the Report Action. + * + * @param {Number|String} actionID + * @return {Boolean} + */ +function isActiveReportAction(actionID) { + if (!emojiPickerRef.current) { + return; + } + return emojiPickerRef.current.isActiveReportAction(actionID); +} + +export {emojiPickerRef, showEmojiPicker, hideEmojiPicker, isActiveReportAction}; diff --git a/src/pages/home/report/ContextMenu/ContextMenuActions.js b/src/pages/home/report/ContextMenu/ContextMenuActions.js index 32ce2c244c78..d4b35ffd3a47 100644 --- a/src/pages/home/report/ContextMenu/ContextMenuActions.js +++ b/src/pages/home/report/ContextMenu/ContextMenuActions.js @@ -69,6 +69,7 @@ export default [ onEmojiSelected={onEmojiSelected} onPressOpenPicker={openContextMenu} onEmojiPickerClosed={closeContextMenu} + reportAction={reportAction} /> ); } @@ -78,6 +79,7 @@ export default [ key="BaseQuickEmojiReactions" closeContextMenu={closeContextMenu} onEmojiSelected={onEmojiSelected} + reportAction={reportAction} /> ); }, diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index da22c0a4445b..8db8a19dd84c 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -27,6 +27,7 @@ import * as DeviceCapabilities from '../../../libs/DeviceCapabilities'; import MiniReportActionContextMenu from './ContextMenu/MiniReportActionContextMenu'; import * as ReportActionContextMenu from './ContextMenu/ReportActionContextMenu'; import * as ContextMenuActions from './ContextMenu/ContextMenuActions'; +import * as EmojiPickerAction from '../../../libs/actions/EmojiPickerAction'; import {withBlockedFromConcierge, withNetwork, withPersonalDetails, withReportActionsDrafts} from '../../../components/OnyxProvider'; import RenameAction from '../../../components/ReportActionItem/RenameAction'; import InlineSystemMessage from '../../../components/InlineSystemMessage'; @@ -112,14 +113,16 @@ function ReportActionItem(props) { useEffect( () => () => { - // ReportActionContextMenu is a global component, - // we use ReportActionContextMenu.showContextMenu to show them, + // ReportActionContextMenu and EmojiPicker are global component, + // we use showContextMenu and showEmojiPicker to show them, // so we should also hide them when the current component is destroyed - if (!ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) { - return; + if (ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) { + ReportActionContextMenu.hideContextMenu(); + ReportActionContextMenu.hideDeleteModal(); + } + if (EmojiPickerAction.isActiveReportAction(props.action.reportActionID)) { + EmojiPickerAction.hideEmojiPicker(true); } - ReportActionContextMenu.hideContextMenu(); - ReportActionContextMenu.hideDeleteModal(); }, [props.action.reportActionID], ); From f4cce813785515b54d813a3fa5307c8530c056b5 Mon Sep 17 00:00:00 2001 From: hublot Date: Fri, 16 Jun 2023 17:31:40 +0800 Subject: [PATCH 5/7] Fix Lint --- src/components/Reactions/AddReactionBubble.js | 1 + src/components/Reactions/MiniQuickEmojiReactions.js | 1 + .../Reactions/QuickEmojiReactions/BaseQuickEmojiReactions.js | 1 + src/libs/actions/EmojiPickerAction.js | 2 +- 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/Reactions/AddReactionBubble.js b/src/components/Reactions/AddReactionBubble.js index 85886071b921..a36ce9824e37 100644 --- a/src/components/Reactions/AddReactionBubble.js +++ b/src/components/Reactions/AddReactionBubble.js @@ -47,6 +47,7 @@ const defaultProps = { isContextMenu: false, onWillShowPicker: () => {}, onPressOpenPicker: undefined, + reportAction: {}, }; function AddReactionBubble(props) { diff --git a/src/components/Reactions/MiniQuickEmojiReactions.js b/src/components/Reactions/MiniQuickEmojiReactions.js index 75a9a6e1396f..975cfdcda6fc 100644 --- a/src/components/Reactions/MiniQuickEmojiReactions.js +++ b/src/components/Reactions/MiniQuickEmojiReactions.js @@ -40,6 +40,7 @@ const propTypes = { const defaultProps = { onEmojiPickerClosed: () => {}, preferredSkinTone: CONST.EMOJI_DEFAULT_SKIN_TONE, + reportAction: {}, }; /** diff --git a/src/components/Reactions/QuickEmojiReactions/BaseQuickEmojiReactions.js b/src/components/Reactions/QuickEmojiReactions/BaseQuickEmojiReactions.js index 38d2fb9f5c5a..e237083232d4 100644 --- a/src/components/Reactions/QuickEmojiReactions/BaseQuickEmojiReactions.js +++ b/src/components/Reactions/QuickEmojiReactions/BaseQuickEmojiReactions.js @@ -39,6 +39,7 @@ const baseQuickEmojiReactionsPropTypes = { const baseQuickEmojiReactionsDefaultProps = { onWillShowPicker: () => {}, onPressOpenPicker: () => {}, + reportAction: {}, }; const propTypes = { diff --git a/src/libs/actions/EmojiPickerAction.js b/src/libs/actions/EmojiPickerAction.js index c5b6b4daccad..9d4ef0b2e98c 100644 --- a/src/libs/actions/EmojiPickerAction.js +++ b/src/libs/actions/EmojiPickerAction.js @@ -29,7 +29,7 @@ function hideEmojiPicker(isNavigating) { if (!emojiPickerRef.current) { return; } - return emojiPickerRef.current.hideEmojiPicker(isNavigating); + emojiPickerRef.current.hideEmojiPicker(isNavigating); } /** From cb87880d66f00063f40f4d8791d8ebb5dd5d0a15 Mon Sep 17 00:00:00 2001 From: hublot Date: Fri, 16 Jun 2023 17:40:12 +0800 Subject: [PATCH 6/7] Replace PropTypes.object to PropTypes.shape --- src/components/Reactions/AddReactionBubble.js | 4 +++- src/components/Reactions/MiniQuickEmojiReactions.js | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/Reactions/AddReactionBubble.js b/src/components/Reactions/AddReactionBubble.js index a36ce9824e37..e4a89d866f27 100644 --- a/src/components/Reactions/AddReactionBubble.js +++ b/src/components/Reactions/AddReactionBubble.js @@ -38,7 +38,9 @@ const propTypes = { /** * ReportAction for EmojiPicker. */ - reportAction: PropTypes.object, + reportAction: PropTypes.shape({ + reportActionID: PropTypes.string.isRequired, + }), ...withLocalizePropTypes, }; diff --git a/src/components/Reactions/MiniQuickEmojiReactions.js b/src/components/Reactions/MiniQuickEmojiReactions.js index 975cfdcda6fc..7ca5deb8a9d1 100644 --- a/src/components/Reactions/MiniQuickEmojiReactions.js +++ b/src/components/Reactions/MiniQuickEmojiReactions.js @@ -31,7 +31,9 @@ const propTypes = { /** * ReportAction for EmojiPicker. */ - reportAction: PropTypes.object, + reportAction: PropTypes.shape({ + reportActionID: PropTypes.string.isRequired, + }), ...withLocalizePropTypes, preferredSkinTone: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), From bd957fd26289a314b5b0eb38d19816eb98509c51 Mon Sep 17 00:00:00 2001 From: hublot Date: Fri, 16 Jun 2023 17:57:10 +0800 Subject: [PATCH 7/7] Run Prettier --- src/components/Reactions/AddReactionBubble.js | 2 +- src/components/Reactions/MiniQuickEmojiReactions.js | 2 +- src/components/Reactions/ReportActionItemReactions.js | 7 ++++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/components/Reactions/AddReactionBubble.js b/src/components/Reactions/AddReactionBubble.js index e4a89d866f27..113968e8b9ad 100644 --- a/src/components/Reactions/AddReactionBubble.js +++ b/src/components/Reactions/AddReactionBubble.js @@ -65,7 +65,7 @@ function AddReactionBubble(props) { refParam || ref.current, anchorOrigin, props.onWillShowPicker, - props.reportAction + props.reportAction, ); }; diff --git a/src/components/Reactions/MiniQuickEmojiReactions.js b/src/components/Reactions/MiniQuickEmojiReactions.js index 7ca5deb8a9d1..5c77726d8070 100644 --- a/src/components/Reactions/MiniQuickEmojiReactions.js +++ b/src/components/Reactions/MiniQuickEmojiReactions.js @@ -66,7 +66,7 @@ function MiniQuickEmojiReactions(props) { ref.current, undefined, () => {}, - props.reportAction + props.reportAction, ); }; diff --git a/src/components/Reactions/ReportActionItemReactions.js b/src/components/Reactions/ReportActionItemReactions.js index b19369fb859b..8a818e1c7bff 100644 --- a/src/components/Reactions/ReportActionItemReactions.js +++ b/src/components/Reactions/ReportActionItemReactions.js @@ -98,7 +98,12 @@ function ReportActionItemReactions(props) { ); })} - {reactionsWithCount.length > 0 && } + {reactionsWithCount.length > 0 && ( + + )} ); }