From 1b276e3355e5b5f780793c5c23a78a4abaf4b52e Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 12 Aug 2023 13:08:49 +0800 Subject: [PATCH 1/5] allow emoji picker to accept id instead of report action when showing --- src/components/EmojiPicker/EmojiPicker.js | 16 ++++++++-------- .../EmojiPicker/EmojiPickerButton.js | 12 ++++-------- src/components/Reactions/AddReactionBubble.js | 2 +- .../Reactions/MiniQuickEmojiReactions.js | 2 +- src/libs/actions/EmojiPickerAction.js | 18 +++++++++--------- src/pages/home/report/ReportActionCompose.js | 1 + src/pages/home/report/ReportActionItem.js | 2 +- .../home/report/ReportActionItemMessageEdit.js | 4 ++-- 8 files changed, 27 insertions(+), 30 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index 2a3a7ba296f2..e3ad9d91b651 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -27,8 +27,8 @@ const EmojiPicker = forwardRef((props, ref) => { horizontal: 0, vertical: 0, }); - const [reportAction, setReportAction] = useState({}); const [emojiPopoverAnchorOrigin, setEmojiPopoverAnchorOrigin] = useState(DEFAULT_ANCHOR_ORIGIN); + const [activeID, setActiveID] = useState(); const emojiPopoverAnchor = useRef(null); const onModalHide = useRef(() => {}); const onEmojiSelected = useRef(() => {}); @@ -42,9 +42,9 @@ const EmojiPicker = forwardRef((props, ref) => { * @param {Element} emojiPopoverAnchorValue - 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} reportActionValue - ReportAction for EmojiPicker + * @param {Object} id - Unique id for EmojiPicker */ - const showEmojiPicker = (onModalHideValue, onEmojiSelectedValue, emojiPopoverAnchorValue, anchorOrigin, onWillShow = () => {}, reportActionValue) => { + const showEmojiPicker = (onModalHideValue, onEmojiSelectedValue, emojiPopoverAnchorValue, anchorOrigin, onWillShow = () => {}, id) => { onModalHide.current = onModalHideValue; onEmojiSelected.current = onEmojiSelectedValue; emojiPopoverAnchor.current = emojiPopoverAnchorValue; @@ -60,7 +60,7 @@ const EmojiPicker = forwardRef((props, ref) => { setIsEmojiPickerVisible(true); setEmojiPopoverAnchorPosition(value); setEmojiPopoverAnchorOrigin(anchorOriginValue); - setReportAction(reportActionValue); + setActiveID(id); }); }; @@ -107,16 +107,16 @@ const EmojiPicker = forwardRef((props, ref) => { }; /** - * Whether Context Menu is active for the Report Action. + * Whether emoji picker is active for the given id. * - * @param {Number|String} actionID + * @param {Number|String} id * @return {Boolean} */ - const isActiveReportAction = (actionID) => Boolean(actionID) && reportAction.reportActionID === actionID; + const isActive = (id) => Boolean(id) && id === activeID; const resetEmojiPopoverAnchor = () => (emojiPopoverAnchor.current = null); - useImperativeHandle(ref, () => ({showEmojiPicker, isActiveReportAction, hideEmojiPicker, isEmojiPickerVisible, resetEmojiPopoverAnchor})); + useImperativeHandle(ref, () => ({showEmojiPicker, isActive, hideEmojiPicker, isEmojiPickerVisible, resetEmojiPopoverAnchor})); useEffect(() => { if (isEmojiPickerVisible) { diff --git a/src/components/EmojiPicker/EmojiPickerButton.js b/src/components/EmojiPicker/EmojiPickerButton.js index c78e9fdd285a..d86f741d7681 100644 --- a/src/components/EmojiPicker/EmojiPickerButton.js +++ b/src/components/EmojiPicker/EmojiPickerButton.js @@ -17,12 +17,8 @@ const propTypes = { /** Id to use for the emoji picker button */ nativeID: PropTypes.string, - /** - * ReportAction for EmojiPicker. - */ - reportAction: PropTypes.shape({ - reportActionID: PropTypes.string, - }), + /** Unique id for emoji picker */ + emojiPickerID: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), ...withLocalizePropTypes, }; @@ -30,7 +26,7 @@ const propTypes = { const defaultProps = { isDisabled: false, nativeID: '', - reportAction: {}, + emojiPickerID: '', }; function EmojiPickerButton(props) { @@ -46,7 +42,7 @@ function EmojiPickerButton(props) { disabled={props.isDisabled} onPress={() => { if (!EmojiPickerAction.emojiPickerRef.current.isEmojiPickerVisible) { - EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor.current, undefined, () => {}, props.reportAction); + EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor.current, undefined, () => {}, props.emojiPickerID); } else { EmojiPickerAction.emojiPickerRef.current.hideEmojiPicker(); } diff --git a/src/components/Reactions/AddReactionBubble.js b/src/components/Reactions/AddReactionBubble.js index de34ebf95242..922be96084d8 100644 --- a/src/components/Reactions/AddReactionBubble.js +++ b/src/components/Reactions/AddReactionBubble.js @@ -67,7 +67,7 @@ function AddReactionBubble(props) { refParam || ref.current, anchorOrigin, props.onWillShowPicker, - props.reportAction, + props.reportAction.reportActionID, ); }; diff --git a/src/components/Reactions/MiniQuickEmojiReactions.js b/src/components/Reactions/MiniQuickEmojiReactions.js index 1ebb8a971827..82f83cb1e961 100644 --- a/src/components/Reactions/MiniQuickEmojiReactions.js +++ b/src/components/Reactions/MiniQuickEmojiReactions.js @@ -67,7 +67,7 @@ function MiniQuickEmojiReactions(props) { ref.current, undefined, () => {}, - props.reportAction, + props.reportAction.reportActionID, ); }; diff --git a/src/libs/actions/EmojiPickerAction.js b/src/libs/actions/EmojiPickerAction.js index de462ac283f3..f8322508731e 100644 --- a/src/libs/actions/EmojiPickerAction.js +++ b/src/libs/actions/EmojiPickerAction.js @@ -3,21 +3,21 @@ import React from 'react'; const emojiPickerRef = React.createRef(); /** - * Show the ReportActionContextMenu modal popover. + * Show the EmojiPicker modal popover. * * @param {Function} [onModalHide=() => {}] - Run a callback when Modal hides. * @param {Function} [onEmojiSelected=() => {}] - Run a callback when Emoji selected. * @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 + * @param {Object} id - Unique id for EmojiPicker */ -function showEmojiPicker(onModalHide = () => {}, onEmojiSelected = () => {}, emojiPopoverAnchor, anchorOrigin = undefined, onWillShow = () => {}, reportAction = {}) { +function showEmojiPicker(onModalHide = () => {}, onEmojiSelected = () => {}, emojiPopoverAnchor, anchorOrigin = undefined, onWillShow = () => {}, id) { if (!emojiPickerRef.current) { return; } - emojiPickerRef.current.showEmojiPicker(onModalHide, onEmojiSelected, emojiPopoverAnchor, anchorOrigin, onWillShow, reportAction); + emojiPickerRef.current.showEmojiPicker(onModalHide, onEmojiSelected, emojiPopoverAnchor, anchorOrigin, onWillShow, id); } /** @@ -33,16 +33,16 @@ function hideEmojiPicker(isNavigating) { } /** - * Whether Emoji Picker is active for the Report Action. + * Whether Emoji Picker is active for the given id. * - * @param {Number|String} actionID + * @param {Number|String} id * @return {Boolean} */ -function isActiveReportAction(actionID) { +function isActive(id) { if (!emojiPickerRef.current) { return; } - return emojiPickerRef.current.isActiveReportAction(actionID); + return emojiPickerRef.current.isActive(id); } function isEmojiPickerVisible() { @@ -59,4 +59,4 @@ function resetEmojiPopoverAnchor() { return emojiPickerRef.current.resetEmojiPopoverAnchor(); } -export {emojiPickerRef, showEmojiPicker, hideEmojiPicker, isActiveReportAction, isEmojiPickerVisible, resetEmojiPopoverAnchor}; +export {emojiPickerRef, showEmojiPicker, hideEmojiPicker, isActive, isEmojiPickerVisible, resetEmojiPopoverAnchor}; diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index ef783a208ef8..d2bdaaccc953 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -1197,6 +1197,7 @@ class ReportActionCompose extends React.Component { this.focus(true); }} onEmojiSelected={this.replaceSelectionWithText} + emojiPickerID={this.props.report.reportID} /> )} { // Skip if this is not the focused message so the other edit composer stays focused. // In small screen devices, when EmojiPicker is shown, the current edit message will lose focus, we need to check this case as well. - if (!isFocusedRef.current && !EmojiPickerAction.isActiveReportAction(props.action.reportActionID)) { + if (!isFocusedRef.current && !EmojiPickerAction.isActive(props.action.reportActionID)) { return; } @@ -358,7 +358,7 @@ function ReportActionItemMessageEdit(props) { }} onEmojiSelected={addEmojiToTextBox} nativeID={emojiButtonID} - reportAction={props.action} + emojiPickerID={props.action.reportActionID} /> From 675becaf4beca39d6e6177d339d87fd18ad14487 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 12 Aug 2023 13:09:21 +0800 Subject: [PATCH 2/5] only hide the emoji picker when it's the active one --- src/pages/home/ReportScreen.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index c859bc6b8f05..a4ed7a476380 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -156,7 +156,7 @@ class ReportScreen extends React.Component { componentDidUpdate(prevProps) { // If composer should be hidden, hide emoji picker as well - if (ReportUtils.shouldHideComposer(this.props.report)) { + if (ReportUtils.shouldHideComposer(this.props.report) && EmojiPickerAction.isActive(this.props.report.reportID)) { EmojiPickerAction.hideEmojiPicker(true); } From ac8e3b34b2b78b0c9c9bab6d505bc9d2406c31bd Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 22 Aug 2023 12:22:25 +0800 Subject: [PATCH 3/5] hide the emoji picker when composer is unmounted --- src/pages/home/ReportScreen.js | 5 ----- src/pages/home/report/ReportActionCompose.js | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index c6ef6a805257..5815e23ed75e 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -155,11 +155,6 @@ class ReportScreen extends React.Component { } componentDidUpdate(prevProps) { - // If composer should be hidden, hide emoji picker as well - if (ReportUtils.shouldDisableWriteActions(this.props.report) && EmojiPickerAction.isActive(this.props.report.reportID)) { - EmojiPickerAction.hideEmojiPicker(true); - } - // If you already have a report open and are deeplinking to a new report on native, // the ReportScreen never actually unmounts and the reportID in the route also doesn't change. // Therefore, we need to compare if the existing reportID is the same as the one in the route diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 65db537a5777..cb847a306304 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -54,6 +54,7 @@ import containerComposeStyles from '../../../styles/containerComposeStyles'; import * as Task from '../../../libs/actions/Task'; import * as Browser from '../../../libs/Browser'; import * as IOU from '../../../libs/actions/IOU'; +import * as EmojiPickerAction from '../../../libs/actions/EmojiPickerAction'; import useArrowKeyFocusManager from '../../../hooks/useArrowKeyFocusManager'; import PressableWithFeedback from '../../../components/Pressable/PressableWithFeedback'; import usePrevious from '../../../hooks/usePrevious'; @@ -975,6 +976,10 @@ function ReportActionCompose({ KeyDownListener.removeKeyDownPressListner(focusComposerOnKeyPress); unsubscribeNavigationBlur(); unsubscribeNavigationFocus(); + + if (EmojiPickerAction.isActive(report.reportID)) { + EmojiPickerAction.hideEmojiPicker(); + } }; // eslint-disable-next-line react-hooks/exhaustive-deps }, []); From c16a75b0be36edb5c21d2ed4d2b3392fd186fadc Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 22 Aug 2023 14:04:33 +0800 Subject: [PATCH 4/5] fix lint --- src/pages/home/ReportScreen.js | 1 - src/pages/home/report/ReportActionCompose.js | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 5815e23ed75e..83f0e4a6d506 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -30,7 +30,6 @@ import withViewportOffsetTop, {viewportOffsetTopPropTypes} from '../../component import * as ReportActionsUtils from '../../libs/ReportActionsUtils'; import personalDetailsPropType from '../personalDetailsPropType'; import getIsReportFullyVisible from '../../libs/getIsReportFullyVisible'; -import * as EmojiPickerAction from '../../libs/actions/EmojiPickerAction'; import MoneyRequestHeader from '../../components/MoneyRequestHeader'; import MoneyReportHeader from '../../components/MoneyReportHeader'; import * as ComposerActions from '../../libs/actions/Composer'; diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index cb847a306304..4b3384e465ff 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -54,7 +54,6 @@ import containerComposeStyles from '../../../styles/containerComposeStyles'; import * as Task from '../../../libs/actions/Task'; import * as Browser from '../../../libs/Browser'; import * as IOU from '../../../libs/actions/IOU'; -import * as EmojiPickerAction from '../../../libs/actions/EmojiPickerAction'; import useArrowKeyFocusManager from '../../../hooks/useArrowKeyFocusManager'; import PressableWithFeedback from '../../../components/Pressable/PressableWithFeedback'; import usePrevious from '../../../hooks/usePrevious'; @@ -977,8 +976,8 @@ function ReportActionCompose({ unsubscribeNavigationBlur(); unsubscribeNavigationFocus(); - if (EmojiPickerAction.isActive(report.reportID)) { - EmojiPickerAction.hideEmojiPicker(); + if (EmojiPickerActions.isActive(report.reportID)) { + EmojiPickerActions.hideEmojiPicker(); } }; // eslint-disable-next-line react-hooks/exhaustive-deps From a7c53edc968e3998e50a0566b8a7b84097569dff Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 23 Aug 2023 13:29:40 +0800 Subject: [PATCH 5/5] change emoji picker id type to string --- src/components/EmojiPicker/EmojiPicker.js | 4 ++-- src/components/EmojiPicker/EmojiPickerButton.js | 2 +- src/libs/actions/EmojiPickerAction.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index 4adb97a1fea9..40d91ff03267 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -42,7 +42,7 @@ const EmojiPicker = forwardRef((props, ref) => { * @param {Element} emojiPopoverAnchorValue - 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} id - Unique id for EmojiPicker + * @param {String} id - Unique id for EmojiPicker */ const showEmojiPicker = (onModalHideValue, onEmojiSelectedValue, emojiPopoverAnchorValue, anchorOrigin, onWillShow = () => {}, id) => { onModalHide.current = onModalHideValue; @@ -109,7 +109,7 @@ const EmojiPicker = forwardRef((props, ref) => { /** * Whether emoji picker is active for the given id. * - * @param {Number|String} id + * @param {String} id * @return {Boolean} */ const isActive = (id) => Boolean(id) && id === activeID; diff --git a/src/components/EmojiPicker/EmojiPickerButton.js b/src/components/EmojiPicker/EmojiPickerButton.js index d86f741d7681..cbfc3517117c 100644 --- a/src/components/EmojiPicker/EmojiPickerButton.js +++ b/src/components/EmojiPicker/EmojiPickerButton.js @@ -18,7 +18,7 @@ const propTypes = { nativeID: PropTypes.string, /** Unique id for emoji picker */ - emojiPickerID: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), + emojiPickerID: PropTypes.string, ...withLocalizePropTypes, }; diff --git a/src/libs/actions/EmojiPickerAction.js b/src/libs/actions/EmojiPickerAction.js index f8322508731e..84621af3a5b4 100644 --- a/src/libs/actions/EmojiPickerAction.js +++ b/src/libs/actions/EmojiPickerAction.js @@ -10,7 +10,7 @@ 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} id - Unique id for EmojiPicker + * @param {String} id - Unique id for EmojiPicker */ function showEmojiPicker(onModalHide = () => {}, onEmojiSelected = () => {}, emojiPopoverAnchor, anchorOrigin = undefined, onWillShow = () => {}, id) { if (!emojiPickerRef.current) { @@ -35,7 +35,7 @@ function hideEmojiPicker(isNavigating) { /** * Whether Emoji Picker is active for the given id. * - * @param {Number|String} id + * @param {String} id * @return {Boolean} */ function isActive(id) {