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

Refactor the EmojiPicker to a functional component #20187

Merged
merged 20 commits into from
Jun 29, 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
267 changes: 117 additions & 150 deletions src/components/EmojiPicker/EmojiPicker.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, {useState, useEffect, useRef, forwardRef, useImperativeHandle} from 'react';
import {Dimensions, Keyboard} from 'react-native';
import _ from 'underscore';
import EmojiPickerMenu from './EmojiPickerMenu';
Expand All @@ -9,6 +9,7 @@ import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDime
import withViewportOffsetTop, {viewportOffsetTopPropTypes} from '../withViewportOffsetTop';
import compose from '../../libs/compose';
import * as StyleUtils from '../../styles/StyleUtils';
import calculateAnchorPosition from '../../libs/calculateAnchorPosition';

const DEFAULT_ANCHOR_ORIGIN = {
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.RIGHT,
Expand All @@ -20,184 +21,150 @@ const propTypes = {
...viewportOffsetTopPropTypes,
};

class EmojiPicker extends React.Component {
constructor(props) {
super(props);

this.hideEmojiPicker = this.hideEmojiPicker.bind(this);
this.showEmojiPicker = this.showEmojiPicker.bind(this);
this.selectEmoji = this.selectEmoji.bind(this);
this.measureEmojiPopoverAnchorPosition = this.measureEmojiPopoverAnchorPosition.bind(this);
this.measureEmojiPopoverAnchorPositionAndUpdateState = this.measureEmojiPopoverAnchorPositionAndUpdateState.bind(this);
this.focusEmojiSearchInput = this.focusEmojiSearchInput.bind(this);
this.onModalHide = () => {};
this.onEmojiSelected = () => {};

this.state = {
reportAction: {},
isEmojiPickerVisible: false,

// The horizontal and vertical position (relative to the window) where the emoji popover will display.
emojiPopoverAnchorPosition: {
horizontal: 0,
vertical: 0,
},

emojiPopoverAnchorOrigin: DEFAULT_ANCHOR_ORIGIN,
const EmojiPicker = forwardRef((props, ref) => {
Copy link
Member

Choose a reason for hiding this comment

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

Please use function keyword here. A new rule is coming for this.#20724

Copy link
Member

Choose a reason for hiding this comment

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

Clarifying this #20724 (comment)

const [isEmojiPickerVisible, setIsEmojiPickerVisible] = useState(false);
const [emojiPopoverAnchorPosition, setEmojiPopoverAnchorPosition] = useState({
horizontal: 0,
vertical: 0,
});
const [reportAction, setReportAction] = useState({});
const emojiPopoverAnchorOrigin = useRef(DEFAULT_ANCHOR_ORIGIN);
Copy link
Member

@parasharrajat parasharrajat Jun 16, 2023

Choose a reason for hiding this comment

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

This was previously on the state. Not sure about the impact but should we move it to the state? useRef might not cause any issues asemojiPopoverAnchorOrigin is only updated when emojiPopoverAnchorPosition is updated. Thus changes will be reflected properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, hence why we don't need it in the state anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

This has caused a small regression in #23751
We did indeed need emojiPopoverAnchorOrigin to also be stored in state, it was causing emoji popover position to be wrong if you open emoji popover repeatedly (from a different menu, hence the different position)

1. Click on the emoji picker icon inside the context menu and observe its position
2. Click anywhere on the screen to close the modal and hover over the last posted comment again
3. Click on the emoji picker icon from the hovered context menu
4. Click anywhere on the screen to close the modal
5. Right click on the posted comment
6. Click on the emoji picker icon and observe its position

const emojiPopoverAnchor = useRef(null);
const onModalHide = useRef(() => {});
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]);

componentDidMount() {
this.emojiPopoverDimensionListener = Dimensions.addEventListener('change', this.measureEmojiPopoverAnchorPositionAndUpdateState);
}
/**
* 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 {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
*/
const showEmojiPicker = (onModalHideValue, onEmojiSelectedValue, emojiPopoverAnchorValue, anchorOrigin, onWillShow = () => {}, reportActionValue) => {
onModalHide.current = onModalHideValue;
onEmojiSelected.current = onEmojiSelectedValue;
emojiPopoverAnchor.current = emojiPopoverAnchorValue;

componentDidUpdate(prevProps, prevState) {
if (prevState.isEmojiPickerVisible === this.state.isEmojiPickerVisible || !this.state.isEmojiPickerVisible) {
return;
if (emojiPopoverAnchor.current) {
// Drop focus to avoid blue focus ring.
emojiPopoverAnchor.current.blur();
}

// Dismiss the keyboard to provide a focus for the emoji picker to avoid selection issues.
Keyboard.dismiss();
}
calculateAnchorPosition(emojiPopoverAnchor.current).then((value) => {
onWillShow();
setIsEmojiPickerVisible(true);
setEmojiPopoverAnchorPosition(value);
emojiPopoverAnchorOrigin.current = anchorOrigin || DEFAULT_ANCHOR_ORIGIN;
setReportAction(reportActionValue);
});
};

/**
* Hide the emoji picker menu.
*
* @param {Boolean} isNavigating
*/
const hideEmojiPicker = (isNavigating) => {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should use useCallback here. And same for all methods which were bound to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we're now using all refs in these functions - I don't believe useCallback is necessary, right?

Copy link
Member

Choose a reason for hiding this comment

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

The only difference is that now on each rerender these methods are recreated. When they were bound to this, they weren't recreated even when passed direct to props in JSX. If we use useCallback it will be same as before and function won't be recreated.

if (isNavigating) {
onModalHide.current = () => {};
}
emojiPopoverAnchor.current = null;
setIsEmojiPickerVisible(false);
};

componentWillUnmount() {
if (!this.emojiPopoverDimensionListener) {
/**
* Focus the search input in the emoji picker.
*/
const focusEmojiSearchInput = () => {
if (!emojiSearchInput.current) {
return;
}
this.emojiPopoverDimensionListener.remove();
}
emojiSearchInput.current.focus();
};

/**
* Callback for the emoji picker to add whatever emoji is chosen into the main input
*
* @param {String} emoji
* @param {Object} emojiObject
*/
selectEmoji(emoji, emojiObject) {
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 prop to will be set to null (synchronously)
// 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 (!this.emojiPopoverAnchor) {
if (!emojiPopoverAnchor.current) {
return;
}

this.hideEmojiPicker();
if (_.isFunction(this.onEmojiSelected)) {
this.onEmojiSelected(emoji, emojiObject);
hideEmojiPicker(false);
if (_.isFunction(onEmojiSelected.current)) {
onEmojiSelected.current(emoji, emojiObject);
}
}

/**
* Hide the emoji picker menu.
*
* @param {Boolean} isNavigating
*/
hideEmojiPicker(isNavigating) {
if (isNavigating) {
this.onModalHide = () => {};
}
this.emojiPopoverAnchor = null;
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.
*
* @param {Function} [onModalHide=() => {}] - Run a callback when Modal hides.
* @param {Function} [onEmojiSelected=() => {}] - Run a callback when Emoji selected.
* @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 = () => {}, reportAction) {
this.onModalHide = onModalHide;
this.onEmojiSelected = onEmojiSelected;
this.emojiPopoverAnchor = emojiPopoverAnchor;

if (this.emojiPopoverAnchor) {
// Drop focus to avoid blue focus ring.
emojiPopoverAnchor.blur();
}

this.measureEmojiPopoverAnchorPosition().then((emojiPopoverAnchorPosition) => {
onWillShow();
this.setState({reportAction, isEmojiPickerVisible: true, emojiPopoverAnchorPosition, emojiPopoverAnchorOrigin: anchorOrigin || DEFAULT_ANCHOR_ORIGIN});
});
}

measureEmojiPopoverAnchorPosition() {
return new Promise((resolve) => {
if (!this.emojiPopoverAnchor) {
return resolve({horizontal: 0, vertical: 0});
}
this.emojiPopoverAnchor.measureInWindow((x, y, width) => resolve({horizontal: x + width, vertical: y}));
});
}

measureEmojiPopoverAnchorPositionAndUpdateState() {
this.measureEmojiPopoverAnchorPosition().then((emojiPopoverAnchorPosition) => {
this.setState({emojiPopoverAnchorPosition});
});
}

/**
* Focus the search input in the emoji picker.
*/
focusEmojiSearchInput() {
// we won't focus the input if it's mobile device
if (!this.emojiSearchInput || this.props.isSmallScreenWidth) {
return;
}
this.emojiSearchInput.focus();
}

render() {
// 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
return (
<PopoverWithMeasuredContent
isVisible={this.state.isEmojiPickerVisible}
onClose={this.hideEmojiPicker}
onModalShow={this.focusEmojiSearchInput}
onModalHide={this.onModalHide}
hideModalContentWhileAnimating
shouldSetModalVisibility={false}
animationInTiming={1}
animationOutTiming={1}
anchorPosition={{
vertical: this.state.emojiPopoverAnchorPosition.vertical,
horizontal: this.state.emojiPopoverAnchorPosition.horizontal,
}}
popoverDimensions={{
width: CONST.EMOJI_PICKER_SIZE.WIDTH,
height: CONST.EMOJI_PICKER_SIZE.HEIGHT,
}}
outerStyle={StyleUtils.getOuterModalStyle(this.props.windowHeight, this.props.viewportOffsetTop)}
anchorAlignment={this.state.emojiPopoverAnchorOrigin}
innerContainerStyle={styles.popoverInnerContainer}
avoidKeyboard
>
<EmojiPickerMenu
onEmojiSelected={this.selectEmoji}
ref={(el) => (this.emojiSearchInput = el)}
/>
</PopoverWithMeasuredContent>
);
}
}
const isActiveReportAction = (actionID) => Boolean(actionID) && reportAction.reportActionID === actionID;

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

// 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
return (
<PopoverWithMeasuredContent
isVisible={isEmojiPickerVisible}
onClose={hideEmojiPicker}
onModalShow={focusEmojiSearchInput}
onModalHide={onModalHide.current}
hideModalContentWhileAnimating
shouldSetModalVisibility={false}
animationInTiming={1}
animationOutTiming={1}
anchorPosition={{
vertical: emojiPopoverAnchorPosition.vertical,
horizontal: emojiPopoverAnchorPosition.horizontal,
}}
popoverDimensions={{
width: CONST.EMOJI_PICKER_SIZE.WIDTH,
height: CONST.EMOJI_PICKER_SIZE.HEIGHT,
}}
anchorAlignment={emojiPopoverAnchorOrigin.current}
outerStyle={StyleUtils.getOuterModalStyle(props.windowHeight, props.viewportOffsetTop)}
innerContainerStyle={styles.popoverInnerContainer}
avoidKeyboard
>
<EmojiPickerMenu
onEmojiSelected={selectEmoji}
ref={(el) => (emojiSearchInput.current = el)}
/>
</PopoverWithMeasuredContent>
);
});

EmojiPicker.propTypes = propTypes;

EmojiPicker.displayName = 'EmojiPicker';
export default compose(withViewportOffsetTop, withWindowDimensions)(EmojiPicker);
3 changes: 2 additions & 1 deletion src/components/withViewportOffsetTop.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ export default function (WrappedComponent) {

WithViewportOffsetTop.displayName = `WithViewportOffsetTop(${getComponentDisplayName(WrappedComponent)})`;
WithViewportOffsetTop.propTypes = {
forwardedRef: PropTypes.oneOfType([PropTypes.func, PropTypes.shape({current: PropTypes.instanceOf(React.Component)})]),
// eslint-disable-next-line react/forbid-prop-types
forwardedRef: PropTypes.oneOfType([PropTypes.func, PropTypes.shape({current: PropTypes.any})]),
};
WithViewportOffsetTop.defaultProps = {
forwardedRef: undefined,
Expand Down
14 changes: 14 additions & 0 deletions src/libs/calculateAnchorPosition.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Gets the x,y position of the passed in component for the purpose of anchoring another component to it.
*
* @param {Element} anchorComponent
* @return {Promise<unknown>}
*/
export default function calculateAnchorPosition(anchorComponent) {
return new Promise((resolve) => {
if (!anchorComponent) {
return resolve({horizontal: 0, vertical: 0});
}
anchorComponent.measureInWindow((x, y, width) => resolve({horizontal: x + width, vertical: y}));
});
}