Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix emoji picker hide itself when there is an archived chat in the navigation stack #24479

Merged
merged 10 commits into from
Aug 24, 2023
16 changes: 8 additions & 8 deletions src/components/EmojiPicker/EmojiPicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {});
Expand All @@ -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 {String} 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;
Expand All @@ -60,7 +60,7 @@ const EmojiPicker = forwardRef((props, ref) => {
setIsEmojiPickerVisible(true);
setEmojiPopoverAnchorPosition(value);
setEmojiPopoverAnchorOrigin(anchorOriginValue);
setReportAction(reportActionValue);
setActiveID(id);
});
};

Expand Down Expand Up @@ -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 {String} id
* @return {Boolean}
*/
const isActiveReportAction = (actionID) => Boolean(actionID) && reportAction.reportActionID === actionID;
const isActive = (id) => Boolean(id) && id === activeID;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If type of id and activeID is different (one is number, another is string), === will not work.
Let's make them consistent as string type


const resetEmojiPopoverAnchor = () => (emojiPopoverAnchor.current = null);

useImperativeHandle(ref, () => ({showEmojiPicker, isActiveReportAction, hideEmojiPicker, isEmojiPickerVisible, resetEmojiPopoverAnchor}));
useImperativeHandle(ref, () => ({showEmojiPicker, isActive, hideEmojiPicker, isEmojiPickerVisible, resetEmojiPopoverAnchor}));

useEffect(() => {
const emojiPopoverDimensionListener = Dimensions.addEventListener('change', () => {
Expand Down
12 changes: 4 additions & 8 deletions src/components/EmojiPicker/EmojiPickerButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,16 @@ 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.string,

...withLocalizePropTypes,
};

const defaultProps = {
isDisabled: false,
nativeID: '',
reportAction: {},
emojiPickerID: '',
};

function EmojiPickerButton(props) {
Expand All @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/Reactions/AddReactionBubble.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function AddReactionBubble(props) {
refParam || ref.current,
anchorOrigin,
props.onWillShowPicker,
props.reportAction,
props.reportAction.reportActionID,
);
};

Expand Down
2 changes: 1 addition & 1 deletion src/components/Reactions/MiniQuickEmojiReactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function MiniQuickEmojiReactions(props) {
ref.current,
undefined,
() => {},
props.reportAction,
props.reportAction.reportActionID,
);
};

Expand Down
18 changes: 9 additions & 9 deletions src/libs/actions/EmojiPickerAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {String} 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);
}

/**
Expand All @@ -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 {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() {
Expand All @@ -59,4 +59,4 @@ function resetEmojiPopoverAnchor() {
return emojiPickerRef.current.resetEmojiPopoverAnchor();
}

export {emojiPickerRef, showEmojiPicker, hideEmojiPicker, isActiveReportAction, isEmojiPickerVisible, resetEmojiPopoverAnchor};
export {emojiPickerRef, showEmojiPicker, hideEmojiPicker, isActive, isEmojiPickerVisible, resetEmojiPopoverAnchor};
5 changes: 0 additions & 5 deletions src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -155,10 +154,6 @@ class ReportScreen extends React.Component {
}

componentDidUpdate(prevProps) {
if (ReportUtils.shouldDisableWriteActions(this.props.report)) {
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
Expand Down
5 changes: 5 additions & 0 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -975,6 +975,10 @@ function ReportActionCompose({
KeyDownListener.removeKeyDownPressListner(focusComposerOnKeyPress);
unsubscribeNavigationBlur();
unsubscribeNavigationFocus();

if (EmojiPickerActions.isActive(report.reportID)) {
EmojiPickerActions.hideEmojiPicker();
}
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand Down Expand Up @@ -1234,6 +1238,7 @@ function ReportActionCompose({
isDisabled={isBlockedFromConcierge || disabled}
onModalHide={() => focus(true)}
onEmojiSelected={replaceSelectionWithText}
emojiPickerID={report.reportID}
/>
)}
<View
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function ReportActionItem(props) {
ReportActionContextMenu.hideContextMenu();
ReportActionContextMenu.hideDeleteModal();
}
if (EmojiPickerAction.isActiveReportAction(props.action.reportActionID)) {
if (EmojiPickerAction.isActive(props.action.reportActionID)) {
EmojiPickerAction.hideEmojiPicker(true);
}
if (reactionListRef.current && reactionListRef.current.isActiveReportAction(props.action.reportActionID)) {
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/report/ReportActionItemMessageEdit.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ function ReportActionItemMessageEdit(props) {
return () => {
// 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;
}

Expand Down Expand Up @@ -382,7 +382,7 @@ function ReportActionItemMessageEdit(props) {
}}
onEmojiSelected={addEmojiToTextBox}
nativeID={emojiButtonID}
reportAction={props.action}
emojiPickerID={props.action.reportActionID}
/>
</View>

Expand Down