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's position changes on decreasing the screen size #22792

Merged
merged 7 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 26 additions & 19 deletions src/components/EmojiPicker/EmojiPicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,6 @@ const EmojiPicker = forwardRef((props, ref) => {
const onEmojiSelected = useRef(() => {});
const emojiSearchInput = useRef();

useEffect(() => {
if (isEmojiPickerVisible) {
Keyboard.dismiss();
}

const emojiPopoverDimensionListener = Dimensions.addEventListener('change', () => {
calculateAnchorPosition(emojiPopoverAnchor.current).then((value) => {
setEmojiPopoverAnchorPosition(value);
});
});
return () => {
emojiPopoverDimensionListener.remove();
};
}, [isEmojiPickerVisible]);

/**
* Show the emoji picker menu.
*
Expand Down Expand Up @@ -110,9 +95,7 @@ const EmojiPicker = forwardRef((props, ref) => {
const selectEmoji = (emoji, emojiObject) => {
// Prevent fast click / multiple emoji selection;
// The first click will hide the emoji picker by calling the hideEmojiPicker() function
// and in that function the emojiPopoverAnchor ref to will be set to null (synchronously)
// thus we rely on that prop to prevent fast click / multiple emoji selection
if (!emojiPopoverAnchor.current) {
if (!isEmojiPickerVisible) {
return;
}

Expand All @@ -130,7 +113,31 @@ const EmojiPicker = forwardRef((props, ref) => {
*/
const isActiveReportAction = (actionID) => Boolean(actionID) && reportAction.reportActionID === actionID;

useImperativeHandle(ref, () => ({showEmojiPicker, isActiveReportAction, hideEmojiPicker, isEmojiPickerVisible}));
const resetEmojiPopoverAnchor = () => (emojiPopoverAnchor.current = null);

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

useEffect(() => {
if (isEmojiPickerVisible) {
Keyboard.dismiss();
}

const emojiPopoverDimensionListener = Dimensions.addEventListener('change', () => {
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 && !props.isSmallScreenWidth) {
hideEmojiPicker();
}
return;
}
calculateAnchorPosition(emojiPopoverAnchor.current).then((value) => {
setEmojiPopoverAnchorPosition(value);
});
});
return () => {
emojiPopoverDimensionListener.remove();
};
}, [isEmojiPickerVisible, props.isSmallScreenWidth]);

// 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
Expand Down
3 changes: 2 additions & 1 deletion src/components/EmojiPicker/EmojiPickerButton.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, {useEffect} from 'react';
import PropTypes from 'prop-types';
import styles from '../../styles/styles';
import * as StyleUtils from '../../styles/StyleUtils';
Expand Down Expand Up @@ -35,6 +35,7 @@ const defaultProps = {

function EmojiPickerButton(props) {
let emojiPopoverAnchor = null;
useEffect(() => EmojiPickerAction.resetEmojiPopoverAnchor, []);
return (
<Tooltip text={props.translate('reportActionCompose.emoji')}>
<PressableWithoutFeedback
Expand Down
3 changes: 2 additions & 1 deletion src/components/Reactions/AddReactionBubble.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useRef} from 'react';
import React, {useRef, useEffect} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import Tooltip from '../Tooltip';
Expand Down Expand Up @@ -55,6 +55,7 @@ const defaultProps = {

function AddReactionBubble(props) {
const ref = useRef();
useEffect(() => EmojiPickerAction.resetEmojiPopoverAnchor, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed? Didn't we decide to not worry about this component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought we agreed to fix this issue as well #21982

#17603 (comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean originally, we would like to fix #17603 and #21982 in this PR, won't we?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep. We do. Apologies for the bump. I lost the context.


const onPress = () => {
const openPicker = (refParam, anchorOrigin) => {
Expand Down
9 changes: 8 additions & 1 deletion src/libs/actions/EmojiPickerAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,11 @@ function isEmojiPickerVisible() {
return emojiPickerRef.current.isEmojiPickerVisible;
}

export {emojiPickerRef, showEmojiPicker, hideEmojiPicker, isActiveReportAction, isEmojiPickerVisible};
function resetEmojiPopoverAnchor() {
if (!emojiPickerRef.current) {
return;
}
return emojiPickerRef.current.resetEmojiPopoverAnchor();
}

export {emojiPickerRef, showEmojiPicker, hideEmojiPicker, isActiveReportAction, isEmojiPickerVisible, resetEmojiPopoverAnchor};