Skip to content

Commit

Permalink
Merge pull request #23371 from jfquevedol2198/fix/23307-0-undefined-r…
Browse files Browse the repository at this point in the history 
…eaction-emoji-tooltip

fixed: dev: '0:undefined:' displayed in reaction emoji tooltip
  • Loading branch information
@stitesExpensify
stitesExpensify authored Jul 26, 2023
2 parents 2b67fa3 + 9578fbb commit e32ff5e
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 110 deletions.
2 changes: 1 addition & 1 deletion src/components/Reactions/ReportActionItemEmojiReactions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function ReportActionItemEmojiReactions(props) {
};

const onReactionListOpen = (event) => {
reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reaction.emoji, props.reportActionID);
reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reaction);
};

return (
Expand Down
119 changes: 14 additions & 105 deletions src/pages/home/report/ReactionList/PopoverReactionList.js
View file
Original file line number Diff line number Diff line change
@@ -1,38 +1,17 @@
import React from 'react';
import {Dimensions} from 'react-native';
import _ from 'underscore';
import lodashGet from 'lodash/get';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
import _ from 'underscore';
import * as Report from '../../../../libs/actions/Report';
import withLocalize, {withLocalizePropTypes} from '../../../../components/withLocalize';
import PopoverWithMeasuredContent from '../../../../components/PopoverWithMeasuredContent';
import BaseReactionList from './BaseReactionList';
import compose from '../../../../libs/compose';
import reportPropTypes from '../../../reportPropTypes';
import reportActionPropTypes from '../reportActionPropTypes';
import ONYXKEYS from '../../../../ONYXKEYS';
import withCurrentUserPersonalDetails from '../../../../components/withCurrentUserPersonalDetails';
import * as PersonalDetailsUtils from '../../../../libs/PersonalDetailsUtils';
import * as EmojiUtils from '../../../../libs/EmojiUtils';
import * as ReportActionsUtils from '../../../../libs/ReportActionsUtils';
import * as ReportUtils from '../../../../libs/ReportUtils';
import CONST from '../../../../CONST';

const propTypes = {
/** The report currently being looked at */
report: reportPropTypes.isRequired,

/** Actions from the ChatReport */
reportActions: PropTypes.shape(reportActionPropTypes),

...withLocalizePropTypes,
};

const defaultProps = {
reportActions: {},
};

class PopoverReactionList extends React.Component {
constructor(props) {
super(props);
Expand All @@ -54,7 +33,6 @@ class PopoverReactionList extends React.Component {
emojiName: '',
emojiCount: 0,
hasUserReacted: false,
reportActionID: '',
};

this.onPopoverHideActionCallback = () => {};
Expand All @@ -63,7 +41,6 @@ class PopoverReactionList extends React.Component {
this.hideReactionList = this.hideReactionList.bind(this);
this.measureReactionListPosition = this.measureReactionListPosition.bind(this);
this.getReactionListMeasuredLocation = this.getReactionListMeasuredLocation.bind(this);
this.getSelectedReaction = this.getSelectedReaction.bind(this);
this.getReactionInformation = this.getReactionInformation.bind(this);
this.dimensionsEventListener = null;
this.contentRef = React.createRef();
Expand All @@ -74,46 +51,17 @@ class PopoverReactionList extends React.Component {
}

shouldComponentUpdate(nextProps, nextState) {
const selectedReaction = this.getSelectedReaction(nextProps.reportActions, nextState.reportActionID, nextState.emojiName);
const {emojiCount, emojiCodes, hasUserReacted, users} = this.getReactionInformation(selectedReaction);
const previousLocale = lodashGet(this.props, 'preferredLocale', CONST.LOCALES.DEFAULT);
const nextLocale = lodashGet(nextProps, 'preferredLocale', CONST.LOCALES.DEFAULT);

return (
this.state.isPopoverVisible !== nextState.isPopoverVisible ||
this.state.popoverAnchorPosition !== nextState.popoverAnchorPosition ||
previousLocale !== nextLocale ||
(this.state.isPopoverVisible &&
(this.state.reportActionID !== nextState.reportActionID ||
this.state.emojiName !== nextState.emojiName ||
this.state.emojiCount !== emojiCount ||
this.state.hasUserReacted !== hasUserReacted ||
!_.isEqual(this.state.emojiCodes, emojiCodes) ||
!_.isEqual(this.state.users, users)))
(this.state.isPopoverVisible && (this.state.reportActionID !== nextState.reportActionID || this.state.emojiName !== nextState.emojiName))
);
}

componentDidUpdate() {
if (!this.state.emojiName) {
return;
}

const selectedReaction = this.getSelectedReaction(this.props.reportActions, this.state.reportActionID, this.state.emojiName);
if (!selectedReaction) {
this.setState({
isPopoverVisible: false,
});
} else {
const {emojiCount, emojiCodes, hasUserReacted, users} = this.getReactionInformation(selectedReaction);
this.setState({
users,
emojiCodes,
emojiCount,
hasUserReacted,
});
}
}

componentWillUnmount() {
if (!this.dimensionsEventListener) {
return;
Expand All @@ -137,36 +85,6 @@ class PopoverReactionList extends React.Component {
});
}

/**
* Get the selected reaction from report action.
*
* @param {Object} reportAction
* @param {String} emojiName - Name of emoji
* @returns {Object}
*/
getSelectedReactionFromAction(reportAction, emojiName) {
const reactions = lodashGet(reportAction, ['message', 0, 'reactions'], []);
const reactionsWithCount = _.filter(reactions, (reaction) => reaction.users.length > 0);
return _.find(reactionsWithCount, (reaction) => reaction.emoji === emojiName);
}

/**
* Get the selected reaction.
*
* @param {Array<Object>} reportActions
* @param {String} reportActionID
* @param {String} emojiName - Name of emoji
* @returns {Object}
*/
getSelectedReaction(reportActions, reportActionID, emojiName) {
const reportAction = _.find(reportActions, (action) => action.reportActionID === reportActionID);
if (!reportAction || ReportUtils.isThreadFirstChat(reportAction, this.props.report.reportID)) {
const parentReportAction = ReportActionsUtils.getParentReportAction(this.props.report);
return this.getSelectedReactionFromAction(parentReportAction, emojiName);
}
return this.getSelectedReactionFromAction(reportAction, emojiName);
}

/**
* Get the reaction information.
*
Expand All @@ -182,12 +100,13 @@ class PopoverReactionList extends React.Component {
emojiCount: 0,
};
}
const emojiCount = selectedReaction.users.length;
const reactionUsers = _.map(selectedReaction.users, (sender) => sender.accountID);
const emoji = EmojiUtils.findEmojiByName(selectedReaction.emoji);
const reactionUsers = _.pick(selectedReaction.users, _.identity);
const emojiCount = _.map(reactionUsers, (user) => user).length;
const userAccountIDs = _.map(reactionUsers, (user, accountID) => Number(accountID));
const emoji = EmojiUtils.findEmojiByName(selectedReaction.emojiName);
const emojiCodes = EmojiUtils.getUniqueEmojiCodes(emoji, selectedReaction.users);
const hasUserReacted = Report.hasAccountIDReacted(this.props.currentUserPersonalDetails.accountID, reactionUsers);
const users = PersonalDetailsUtils.getPersonalDetailsByIDs(reactionUsers, this.props.currentUserPersonalDetails.accountID, true);
const hasUserReacted = Report.hasAccountIDEmojiReacted(this.props.currentUserPersonalDetails.accountID, reactionUsers);
const users = PersonalDetailsUtils.getPersonalDetailsByIDs(userAccountIDs, this.props.currentUserPersonalDetails.accountID, true);
return {
emojiCount,
emojiCodes,
Expand All @@ -201,13 +120,14 @@ class PopoverReactionList extends React.Component {
*
* @param {Object} [event] - A press event.
* @param {Element} reactionListAnchor - reactionListAnchor
* @param {Object} emojiReaction
* @param {String} emojiName - Name of emoji
* @param {String} reportActionID
*/
showReactionList(event, reactionListAnchor, emojiName, reportActionID) {
showReactionList(event, reactionListAnchor, emojiReaction) {
const nativeEvent = event.nativeEvent || {};
this.reactionListAnchor = reactionListAnchor;
const selectedReaction = this.getSelectedReaction(this.props.reportActions, reportActionID, emojiName);
const selectedReaction = emojiReaction;
const {emojiName} = emojiReaction;
const {emojiCount, emojiCodes, hasUserReacted, users} = this.getReactionInformation(selectedReaction);
this.getReactionListMeasuredLocation().then(({x, y}) => {
this.setState({
Expand All @@ -225,7 +145,6 @@ class PopoverReactionList extends React.Component {
emojiCount,
isPopoverVisible: true,
hasUserReacted,
reportActionID,
});
});
}
Expand Down Expand Up @@ -288,16 +207,6 @@ class PopoverReactionList extends React.Component {
}
}

PopoverReactionList.propTypes = propTypes;
PopoverReactionList.defaultProps = defaultProps;
PopoverReactionList.propTypes = withLocalizePropTypes;

export default compose(
withLocalize,
withOnyx({
reportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`,
canEvict: false,
},
}),
withCurrentUserPersonalDetails,
)(PopoverReactionList);
export default compose(withLocalize, withCurrentUserPersonalDetails)(PopoverReactionList);
5 changes: 1 addition & 4 deletions src/pages/home/report/ReportActionsView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,7 @@ function ReportActionsView(props) {
newMarkerReportActionID={newMarkerReportActionID}
policy={props.policy}
/>
<PopoverReactionList
ref={context.reactionListRef}
report={props.report}
/>
<PopoverReactionList ref={context.reactionListRef} />
</>
);
}
Expand Down
10 changes: 10 additions & 0 deletions tests/actions/ReportTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,8 @@ describe('actions/Report', () => {
return waitForPromisesToResolve();
})
.then(() => {
reportAction = _.first(_.values(reportActions));

// Expect the reaction to exist in the reportActionsReactions collection
expect(reportActionsReactions).toHaveProperty(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`);

Expand All @@ -583,15 +585,21 @@ describe('actions/Report', () => {
expect(reportActionReaction[EMOJI.name].users[TEST_USER_ACCOUNT_ID]).toBeNull();
})
.then(() => {
reportAction = _.first(_.values(reportActions));

// Add the same reaction to the same report action with a different skintone
Report.toggleEmojiReaction(REPORT_ID, reportAction, EMOJI);
return waitForPromisesToResolve()
.then(() => {
reportAction = _.first(_.values(reportActions));

const reportActionReaction = reportActionsReactions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`];
Report.toggleEmojiReaction(REPORT_ID, reportAction, EMOJI, reportActionReaction, EMOJI_SKIN_TONE);
return waitForPromisesToResolve();
})
.then(() => {
reportAction = _.first(_.values(reportActions));

// Expect the reaction to exist in the reportActionsReactions collection
expect(reportActionsReactions).toHaveProperty(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`);

Expand Down Expand Up @@ -670,6 +678,8 @@ describe('actions/Report', () => {
return waitForPromisesToResolve();
})
.then(() => {
resultAction = _.first(_.values(reportActions));

// Now we toggle the reaction while the skin tone has changed.
// As the emoji doesn't support skin tones, the emoji
// should get removed instead of added again.
Expand Down

0 comments on commit e32ff5e

Please sign in to comment.