From c9663a581553519b918c84873f1557c2b3383f19 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 23 Dec 2023 13:03:24 +0800 Subject: [PATCH 1/4] pass the ref object instead of the value --- src/components/EmojiPicker/EmojiPicker.js | 26 ++++++++++++++----- .../EmojiPicker/EmojiPickerButton.js | 2 +- .../EmojiPicker/EmojiPickerButtonDropdown.js | 2 +- src/components/Reactions/AddReactionBubble.js | 2 +- .../Reactions/MiniQuickEmojiReactions.js | 2 +- .../Reactions/QuickEmojiReactions/index.js | 2 +- .../QuickEmojiReactions/index.native.js | 2 +- 7 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index 6c05a4b6a5b5..99247edd6b44 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -1,5 +1,5 @@ import PropTypes from 'prop-types'; -import React, {forwardRef, useEffect, useImperativeHandle, useRef, useState} from 'react'; +import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useRef, useState} from 'react'; import {Dimensions} from 'react-native'; import _ from 'underscore'; import PopoverWithMeasuredContent from '@components/PopoverWithMeasuredContent'; @@ -30,18 +30,28 @@ const EmojiPicker = forwardRef((props, ref) => { }); const [emojiPopoverAnchorOrigin, setEmojiPopoverAnchorOrigin] = useState(DEFAULT_ANCHOR_ORIGIN); const [activeID, setActiveID] = useState(); - const emojiPopoverAnchor = useRef(null); + const emojiPopoverAnchorRef = useRef(null); const onModalHide = useRef(() => {}); const onEmojiSelected = useRef(() => {}); const emojiSearchInput = useRef(); const {isSmallScreenWidth, windowHeight} = useWindowDimensions(); + /** + * Get the popover anchor ref + * + * emojiPopoverAnchorRef contains either null or the ref object of the anchor element. + * { current: { current: anchorElement } } + * + * Don't directly get the ref from emojiPopoverAnchorRef, instead use getEmojiPopoverAnchor() + */ + const getEmojiPopoverAnchor = useCallback(() => emojiPopoverAnchorRef.current || emojiPopoverAnchorRef, []); + /** * Show the emoji picker menu. * * @param {Function} [onModalHideValue=() => {}] - Run a callback when Modal hides. * @param {Function} [onEmojiSelectedValue=() => {}] - Run a callback when Emoji selected. - * @param {Element} emojiPopoverAnchorValue - Element to which Popover is anchored + * @param {React.MutableRefObject} 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 {String} id - Unique id for EmojiPicker @@ -49,7 +59,8 @@ const EmojiPicker = forwardRef((props, ref) => { const showEmojiPicker = (onModalHideValue, onEmojiSelectedValue, emojiPopoverAnchorValue, anchorOrigin, onWillShow = () => {}, id) => { onModalHide.current = onModalHideValue; onEmojiSelected.current = onEmojiSelectedValue; - emojiPopoverAnchor.current = emojiPopoverAnchorValue; + emojiPopoverAnchorRef.current = emojiPopoverAnchorValue; + const emojiPopoverAnchor = getEmojiPopoverAnchor(); if (emojiPopoverAnchor.current && emojiPopoverAnchor.current.blur) { // Drop focus to avoid blue focus ring. emojiPopoverAnchor.current.blur(); @@ -75,7 +86,7 @@ const EmojiPicker = forwardRef((props, ref) => { if (isNavigating) { onModalHide.current = () => {}; } - emojiPopoverAnchor.current = null; + emojiPopoverAnchorRef.current = null; setIsEmojiPickerVisible(false); }; @@ -118,12 +129,13 @@ const EmojiPicker = forwardRef((props, ref) => { const clearActive = () => setActiveID(null); - const resetEmojiPopoverAnchor = () => (emojiPopoverAnchor.current = null); + const resetEmojiPopoverAnchor = () => (emojiPopoverAnchorRef.current = null); useImperativeHandle(ref, () => ({showEmojiPicker, isActive, clearActive, hideEmojiPicker, isEmojiPickerVisible, resetEmojiPopoverAnchor})); useEffect(() => { const emojiPopoverDimensionListener = Dimensions.addEventListener('change', () => { + const emojiPopoverAnchor = getEmojiPopoverAnchor(); if (!emojiPopoverAnchor.current) { // In small screen width, the window size change might be due to keyboard open/hide, we should avoid hide EmojiPicker in those cases if (isEmojiPickerVisible && !isSmallScreenWidth) { @@ -159,7 +171,7 @@ const EmojiPicker = forwardRef((props, ref) => { vertical: emojiPopoverAnchorPosition.vertical, horizontal: emojiPopoverAnchorPosition.horizontal, }} - anchorRef={emojiPopoverAnchor} + anchorRef={getEmojiPopoverAnchor()} withoutOverlay popoverDimensions={{ width: CONST.EMOJI_PICKER_SIZE.WIDTH, diff --git a/src/components/EmojiPicker/EmojiPickerButton.js b/src/components/EmojiPicker/EmojiPickerButton.js index 5888bf30b71a..e627119270dd 100644 --- a/src/components/EmojiPicker/EmojiPickerButton.js +++ b/src/components/EmojiPicker/EmojiPickerButton.js @@ -49,7 +49,7 @@ function EmojiPickerButton(props) { return; } if (!EmojiPickerAction.emojiPickerRef.current.isEmojiPickerVisible) { - EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor.current, undefined, () => {}, props.emojiPickerID); + EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor, undefined, () => {}, props.emojiPickerID); } else { EmojiPickerAction.emojiPickerRef.current.hideEmojiPicker(); } diff --git a/src/components/EmojiPicker/EmojiPickerButtonDropdown.js b/src/components/EmojiPicker/EmojiPickerButtonDropdown.js index 43687ec4b7a8..bfcb66aeefbb 100644 --- a/src/components/EmojiPicker/EmojiPickerButtonDropdown.js +++ b/src/components/EmojiPicker/EmojiPickerButtonDropdown.js @@ -36,7 +36,7 @@ function EmojiPickerButtonDropdown(props) { return; } - EmojiPickerAction.showEmojiPicker(props.onModalHide, (emoji) => props.onInputChange(emoji), emojiPopoverAnchor.current, { + EmojiPickerAction.showEmojiPicker(props.onModalHide, (emoji) => props.onInputChange(emoji), emojiPopoverAnchor, { horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT, vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP, shiftVertical: 4, diff --git a/src/components/Reactions/AddReactionBubble.js b/src/components/Reactions/AddReactionBubble.js index 61ad4ce76d64..a9bfdd367615 100644 --- a/src/components/Reactions/AddReactionBubble.js +++ b/src/components/Reactions/AddReactionBubble.js @@ -66,7 +66,7 @@ function AddReactionBubble(props) { (emojiCode, emojiObject) => { props.onSelectEmoji(emojiObject); }, - refParam || ref.current, + refParam || ref, anchorOrigin, props.onWillShowPicker, props.reportAction.reportActionID, diff --git a/src/components/Reactions/MiniQuickEmojiReactions.js b/src/components/Reactions/MiniQuickEmojiReactions.js index 34d336887031..376afcb9ade5 100644 --- a/src/components/Reactions/MiniQuickEmojiReactions.js +++ b/src/components/Reactions/MiniQuickEmojiReactions.js @@ -66,7 +66,7 @@ function MiniQuickEmojiReactions(props) { (emojiCode, emojiObject) => { props.onEmojiSelected(emojiObject, props.emojiReactions); }, - ref.current, + ref, undefined, () => {}, props.reportAction.reportActionID, diff --git a/src/components/Reactions/QuickEmojiReactions/index.js b/src/components/Reactions/QuickEmojiReactions/index.js index e4399b634136..0366071f9c81 100644 --- a/src/components/Reactions/QuickEmojiReactions/index.js +++ b/src/components/Reactions/QuickEmojiReactions/index.js @@ -17,7 +17,7 @@ const propTypes = { function QuickEmojiReactions(props) { const onPressOpenPicker = (openPicker) => { - openPicker(contextMenuRef.current.contentRef.current, { + openPicker(contextMenuRef.current.contentRef, { horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.RIGHT, vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP, }); diff --git a/src/components/Reactions/QuickEmojiReactions/index.native.js b/src/components/Reactions/QuickEmojiReactions/index.native.js index 239fd7b4c8a8..c29bd2665e4a 100644 --- a/src/components/Reactions/QuickEmojiReactions/index.native.js +++ b/src/components/Reactions/QuickEmojiReactions/index.native.js @@ -23,7 +23,7 @@ function QuickEmojiReactions(props) { // As the menu which includes the button to open the emoji picker // gets closed, before the picker actually opens, we pass the composer // ref as anchor for the emoji picker popover. - openPicker(ReportActionComposeFocusManager.composerRef.current); + openPicker(ReportActionComposeFocusManager.composerRef); }); }; From c8112a6d9f8bc8a13f50dbe8398f816326c37816 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 23 Dec 2023 14:22:05 +0800 Subject: [PATCH 2/4] fix lint --- src/components/EmojiPicker/EmojiPicker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index 99247edd6b44..01ea48f02804 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -153,7 +153,7 @@ const EmojiPicker = forwardRef((props, ref) => { } emojiPopoverDimensionListener.remove(); }; - }, [isEmojiPickerVisible, isSmallScreenWidth, emojiPopoverAnchorOrigin]); + }, [isEmojiPickerVisible, isSmallScreenWidth, emojiPopoverAnchorOrigin, getEmojiPopoverAnchor]); // There is no way to disable animations, and they are really laggy, because there are so many // emojis. The best alternative is to set it to 1ms so it just "pops" in and out From 193d71e3b988b90c9599696ea0fe6b5cfdcbe7fa Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 23 Dec 2023 14:40:28 +0800 Subject: [PATCH 3/4] prettier --- src/components/EmojiPicker/EmojiPicker.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index 01ea48f02804..532eb61a99a9 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -38,10 +38,10 @@ const EmojiPicker = forwardRef((props, ref) => { /** * Get the popover anchor ref - * + * * emojiPopoverAnchorRef contains either null or the ref object of the anchor element. * { current: { current: anchorElement } } - * + * * Don't directly get the ref from emojiPopoverAnchorRef, instead use getEmojiPopoverAnchor() */ const getEmojiPopoverAnchor = useCallback(() => emojiPopoverAnchorRef.current || emojiPopoverAnchorRef, []); From 5d57bccaf722ba9e671ba14db14b4a7b074a760c Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 26 Dec 2023 13:54:40 +0800 Subject: [PATCH 4/4] change type --- src/libs/actions/EmojiPickerAction.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/EmojiPickerAction.ts b/src/libs/actions/EmojiPickerAction.ts index 07b00a508dad..0f9dd4b727aa 100644 --- a/src/libs/actions/EmojiPickerAction.ts +++ b/src/libs/actions/EmojiPickerAction.ts @@ -10,7 +10,14 @@ type AnchorOrigin = { // TODO: Move this type to src/components/EmojiPicker/EmojiPicker.js once it is converted to TS type EmojiPickerRef = { - showEmojiPicker: (onModalHideValue?: () => void, onEmojiSelectedValue?: () => void, emojiPopoverAnchor?: View, anchorOrigin?: AnchorOrigin, onWillShow?: () => void, id?: string) => void; + showEmojiPicker: ( + onModalHideValue?: () => void, + onEmojiSelectedValue?: () => void, + emojiPopoverAnchor?: React.MutableRefObject, + anchorOrigin?: AnchorOrigin, + onWillShow?: () => void, + id?: string, + ) => void; isActive: (id: string) => boolean; clearActive: () => void; hideEmojiPicker: (isNavigating: boolean) => void;