From a4a2483daafbc3a171a318033222c9d15244cc13 Mon Sep 17 00:00:00 2001 From: tienifr Date: Wed, 2 Aug 2023 05:36:26 +0700 Subject: [PATCH 01/10] fix: popover reaction list does not update dynamically --- .../ReportActionItemEmojiReactions.js | 2 +- .../BasePopoverReactionList.js} | 84 +++++++++++++------ .../ReactionList/PopoverReactionList/index.js | 33 ++++++++ 3 files changed, 94 insertions(+), 25 deletions(-) rename src/pages/home/report/ReactionList/{PopoverReactionList.js => PopoverReactionList/BasePopoverReactionList.js} (71%) create mode 100644 src/pages/home/report/ReactionList/PopoverReactionList/index.js diff --git a/src/components/Reactions/ReportActionItemEmojiReactions.js b/src/components/Reactions/ReportActionItemEmojiReactions.js index 7102f9982f52..806e87b4301d 100644 --- a/src/components/Reactions/ReportActionItemEmojiReactions.js +++ b/src/components/Reactions/ReportActionItemEmojiReactions.js @@ -91,7 +91,7 @@ function ReportActionItemEmojiReactions(props) { }; const onReactionListOpen = (event) => { - reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reaction); + reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reactionEmojiName, props.reportActionID); }; return { diff --git a/src/pages/home/report/ReactionList/PopoverReactionList.js b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js similarity index 71% rename from src/pages/home/report/ReactionList/PopoverReactionList.js rename to src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js index 8a9b32b5fd9a..677a120518aa 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js @@ -2,17 +2,30 @@ import React from 'react'; import {Dimensions} from 'react-native'; import lodashGet from 'lodash/get'; 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 withCurrentUserPersonalDetails from '../../../../components/withCurrentUserPersonalDetails'; -import * as PersonalDetailsUtils from '../../../../libs/PersonalDetailsUtils'; -import * as EmojiUtils from '../../../../libs/EmojiUtils'; -import CONST from '../../../../CONST'; - -class PopoverReactionList extends React.Component { +import {withOnyx} from "react-native-onyx"; +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 withCurrentUserPersonalDetails from '../../../../../components/withCurrentUserPersonalDetails'; +import * as PersonalDetailsUtils from '../../../../../libs/PersonalDetailsUtils'; +import * as EmojiUtils from '../../../../../libs/EmojiUtils'; +import CONST from '../../../../../CONST'; +import ONYXKEYS from "../../../../../ONYXKEYS"; +import EmojiReactionsPropTypes from "../../../../../components/Reactions/EmojiReactionsPropTypes"; + +const propTypes = { + emojiReactions: EmojiReactionsPropTypes, + + ...withLocalizePropTypes, +} + +const defaultProps = { + emojiReactions: {}, +}; + +class BasePopoverReactionList extends React.Component { constructor(props) { super(props); @@ -55,13 +68,29 @@ class PopoverReactionList extends React.Component { 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.props.reportActionID !== nextProps.reportActionID || + !_.isEqual(this.props.emojiReactions, nextProps.emojiReactions) || + !_.isEqual(this.state, nextState) || + previousLocale !== nextLocale ); } + componentDidUpdate() { + // Hide the list when all reactions are removed + if (this.state.isPopoverVisible && !_.some(lodashGet(this.props.emojiReactions, [this.state.emojiName, 'users']), (user) => !_.isNull(user))) { + this.hideReactionList(); + } + + const selectedReaction = lodashGet(this.props.emojiReactions, [this.state.emojiName]); + const {emojiCount, emojiCodes, hasUserReacted, users} = this.getReactionInformation(selectedReaction); + this.setState({ + users, + emojiCodes, + emojiCount, + hasUserReacted, + }); + } + componentWillUnmount() { if (!this.dimensionsEventListener) { return; @@ -70,7 +99,7 @@ class PopoverReactionList extends React.Component { } /** - * Get the PopoverReactionList anchor position + * Get the BasePopoverReactionList anchor position * We calculate the achor coordinates from measureInWindow async method * * @returns {Promise} @@ -120,14 +149,12 @@ class PopoverReactionList extends React.Component { * * @param {Object} [event] - A press event. * @param {Element} reactionListAnchor - reactionListAnchor - * @param {Object} emojiReaction * @param {String} emojiName - Name of emoji */ - showReactionList(event, reactionListAnchor, emojiReaction) { + showReactionList(event, reactionListAnchor, emojiName) { const nativeEvent = event.nativeEvent || {}; this.reactionListAnchor = reactionListAnchor; - const selectedReaction = emojiReaction; - const {emojiName} = emojiReaction; + const selectedReaction = lodashGet(this.props.emojiReactions, [emojiName]); const {emojiCount, emojiCodes, hasUserReacted, users} = this.getReactionInformation(selectedReaction); this.getReactionListMeasuredLocation().then(({x, y}) => { this.setState({ @@ -150,7 +177,7 @@ class PopoverReactionList extends React.Component { } /** - * This gets called on Dimensions change to find the anchor coordinates for the action PopoverReactionList. + * This gets called on Dimensions change to find the anchor coordinates for the action BasePopoverReactionList. */ measureReactionListPosition() { if (!this.state.isPopoverVisible) { @@ -207,6 +234,15 @@ class PopoverReactionList extends React.Component { } } -PopoverReactionList.propTypes = withLocalizePropTypes; - -export default compose(withLocalize, withCurrentUserPersonalDetails)(PopoverReactionList); +BasePopoverReactionList.propTypes = propTypes; +BasePopoverReactionList.defaultProps = defaultProps; + +export default compose( + withLocalize, + withCurrentUserPersonalDetails, + withOnyx({ + emojiReactions: { + key: ({reportActionID}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportActionID}`, + }, + }), +)(BasePopoverReactionList); diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/index.js b/src/pages/home/report/ReactionList/PopoverReactionList/index.js new file mode 100644 index 000000000000..13ff09ec4b87 --- /dev/null +++ b/src/pages/home/report/ReactionList/PopoverReactionList/index.js @@ -0,0 +1,33 @@ +import React, {forwardRef, useImperativeHandle, useRef, useState} from "react"; +import BasePopoverReactionList from "./BasePopoverReactionList"; + +const PopoverReactionList = forwardRef((props, ref) => { + const innerReactionListRef = useRef(); + const [reactionListReportActionID, setReactionListReportActionID] = useState(""); + + /** + * Show the ReactionList modal popover. + * + * @param {Object} [event] - A press event. + * @param {Element} reactionListAnchor - reactionListAnchor + * @param {String} emojiName - Name of emoji + * @param {String} reportActionID + */ + const showReactionList = (event, reactionListAnchor, emojiName, reportActionID) => { + setReactionListReportActionID(reportActionID); + innerReactionListRef.current.showReactionList(event, reactionListAnchor, emojiName); + }; + + useImperativeHandle(ref, () => ({showReactionList, setReactionListReportActionID}), []); + + return ( + + ); +}); + +PopoverReactionList.displayName = 'PopoverReactionList'; + +export default PopoverReactionList; From ca3ee00c95d2f6261f300ea0a77b28e054de9379 Mon Sep 17 00:00:00 2001 From: tienifr Date: Wed, 2 Aug 2023 05:44:49 +0700 Subject: [PATCH 02/10] fix lint --- .../PopoverReactionList/BasePopoverReactionList.js | 8 ++++---- .../home/report/ReactionList/PopoverReactionList/index.js | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js index 677a120518aa..2f6f112d5523 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js @@ -2,7 +2,7 @@ import React from 'react'; import {Dimensions} from 'react-native'; import lodashGet from 'lodash/get'; import _ from 'underscore'; -import {withOnyx} from "react-native-onyx"; +import {withOnyx} from 'react-native-onyx'; import * as Report from '../../../../../libs/actions/Report'; import withLocalize, {withLocalizePropTypes} from '../../../../../components/withLocalize'; import PopoverWithMeasuredContent from '../../../../../components/PopoverWithMeasuredContent'; @@ -12,14 +12,14 @@ import withCurrentUserPersonalDetails from '../../../../../components/withCurren import * as PersonalDetailsUtils from '../../../../../libs/PersonalDetailsUtils'; import * as EmojiUtils from '../../../../../libs/EmojiUtils'; import CONST from '../../../../../CONST'; -import ONYXKEYS from "../../../../../ONYXKEYS"; -import EmojiReactionsPropTypes from "../../../../../components/Reactions/EmojiReactionsPropTypes"; +import ONYXKEYS from '../../../../../ONYXKEYS'; +import EmojiReactionsPropTypes from '../../../../../components/Reactions/EmojiReactionsPropTypes'; const propTypes = { emojiReactions: EmojiReactionsPropTypes, ...withLocalizePropTypes, -} +}; const defaultProps = { emojiReactions: {}, diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/index.js b/src/pages/home/report/ReactionList/PopoverReactionList/index.js index 13ff09ec4b87..3b7e041ab61e 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/index.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/index.js @@ -1,9 +1,9 @@ -import React, {forwardRef, useImperativeHandle, useRef, useState} from "react"; -import BasePopoverReactionList from "./BasePopoverReactionList"; +import React, {forwardRef, useImperativeHandle, useRef, useState} from 'react'; +import BasePopoverReactionList from './BasePopoverReactionList'; const PopoverReactionList = forwardRef((props, ref) => { const innerReactionListRef = useRef(); - const [reactionListReportActionID, setReactionListReportActionID] = useState(""); + const [reactionListReportActionID, setReactionListReportActionID] = useState(''); /** * Show the ReactionList modal popover. From c282fa19576d490b6a6c4fa036146087d5e9dd56 Mon Sep 17 00:00:00 2001 From: tienifr Date: Thu, 3 Aug 2023 14:38:40 +0700 Subject: [PATCH 03/10] add propTypes --- .../ReportActionItemEmojiReactions.js | 3 +- .../ReactionList/PopoverReactionList/index.js | 29 ++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/components/Reactions/ReportActionItemEmojiReactions.js b/src/components/Reactions/ReportActionItemEmojiReactions.js index 806e87b4301d..43353c71d194 100644 --- a/src/components/Reactions/ReportActionItemEmojiReactions.js +++ b/src/components/Reactions/ReportActionItemEmojiReactions.js @@ -91,7 +91,8 @@ function ReportActionItemEmojiReactions(props) { }; const onReactionListOpen = (event) => { - reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reactionEmojiName, props.reportActionID); + reactionListRef.current.setReactionListReportActionID(props.reportActionID); + reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reactionEmojiName); }; return { diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/index.js b/src/pages/home/report/ReactionList/PopoverReactionList/index.js index 3b7e041ab61e..0f7e7fcc2acf 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/index.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/index.js @@ -1,7 +1,16 @@ import React, {forwardRef, useImperativeHandle, useRef, useState} from 'react'; +import PropTypes from "prop-types"; import BasePopoverReactionList from './BasePopoverReactionList'; -const PopoverReactionList = forwardRef((props, ref) => { +const propTypes = { + ref: PropTypes.oneOfType([PropTypes.func, PropTypes.object]), +} + +const defaultProps = { + ref: () => {}, +} + +function PopoverReactionList(props) { const innerReactionListRef = useRef(); const [reactionListReportActionID, setReactionListReportActionID] = useState(''); @@ -11,14 +20,12 @@ const PopoverReactionList = forwardRef((props, ref) => { * @param {Object} [event] - A press event. * @param {Element} reactionListAnchor - reactionListAnchor * @param {String} emojiName - Name of emoji - * @param {String} reportActionID */ - const showReactionList = (event, reactionListAnchor, emojiName, reportActionID) => { - setReactionListReportActionID(reportActionID); + const showReactionList = (event, reactionListAnchor, emojiName) => { innerReactionListRef.current.showReactionList(event, reactionListAnchor, emojiName); }; - useImperativeHandle(ref, () => ({showReactionList, setReactionListReportActionID}), []); + useImperativeHandle(props.ref, () => ({showReactionList, setReactionListReportActionID}), []); return ( { reportActionID={reactionListReportActionID} /> ); -}); +} +PopoverReactionList.propTypes = propTypes; +PopoverReactionList.defaultProps = defaultProps; PopoverReactionList.displayName = 'PopoverReactionList'; -export default PopoverReactionList; +export default forwardRef((props, ref) => ( + +)); From 7c57255da0ab9671f3937cfb3b4e3ff47e24d477 Mon Sep 17 00:00:00 2001 From: tienifr Date: Thu, 3 Aug 2023 15:27:31 +0700 Subject: [PATCH 04/10] fix lint --- .../home/report/ReactionList/PopoverReactionList/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/index.js b/src/pages/home/report/ReactionList/PopoverReactionList/index.js index 0f7e7fcc2acf..268c03045083 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/index.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/index.js @@ -1,14 +1,14 @@ import React, {forwardRef, useImperativeHandle, useRef, useState} from 'react'; -import PropTypes from "prop-types"; +import PropTypes from 'prop-types'; import BasePopoverReactionList from './BasePopoverReactionList'; const propTypes = { ref: PropTypes.oneOfType([PropTypes.func, PropTypes.object]), -} +}; const defaultProps = { ref: () => {}, -} +}; function PopoverReactionList(props) { const innerReactionListRef = useRef(); From c15b7552e0022595300c5b64f44c3fa6d1f290e9 Mon Sep 17 00:00:00 2001 From: tienifr Date: Thu, 3 Aug 2023 18:39:20 +0700 Subject: [PATCH 05/10] rename ref to innerRef --- .../home/report/ReactionList/PopoverReactionList/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/index.js b/src/pages/home/report/ReactionList/PopoverReactionList/index.js index 268c03045083..64d22df0d16c 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/index.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/index.js @@ -3,11 +3,11 @@ import PropTypes from 'prop-types'; import BasePopoverReactionList from './BasePopoverReactionList'; const propTypes = { - ref: PropTypes.oneOfType([PropTypes.func, PropTypes.object]), + innerRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]), }; const defaultProps = { - ref: () => {}, + innerRef: () => {}, }; function PopoverReactionList(props) { @@ -25,7 +25,7 @@ function PopoverReactionList(props) { innerReactionListRef.current.showReactionList(event, reactionListAnchor, emojiName); }; - useImperativeHandle(props.ref, () => ({showReactionList, setReactionListReportActionID}), []); + useImperativeHandle(props.innerRef, () => ({showReactionList, setReactionListReportActionID}), []); return ( ( )); From 7afde51cbeb1137655f49a07cdcd57fb8ab2bd8a Mon Sep 17 00:00:00 2001 From: tienifr Date: Fri, 4 Aug 2023 04:20:49 +0700 Subject: [PATCH 06/10] showReactionList as callback after setState --- .../ReportActionItemEmojiReactions.js | 3 +-- .../ReactionList/PopoverReactionList/index.js | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/components/Reactions/ReportActionItemEmojiReactions.js b/src/components/Reactions/ReportActionItemEmojiReactions.js index 43353c71d194..806e87b4301d 100644 --- a/src/components/Reactions/ReportActionItemEmojiReactions.js +++ b/src/components/Reactions/ReportActionItemEmojiReactions.js @@ -91,8 +91,7 @@ function ReportActionItemEmojiReactions(props) { }; const onReactionListOpen = (event) => { - reactionListRef.current.setReactionListReportActionID(props.reportActionID); - reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reactionEmojiName); + reactionListRef.current.showReactionList(event, popoverReactionListAnchor.current, reactionEmojiName, props.reportActionID); }; return { diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/index.js b/src/pages/home/report/ReactionList/PopoverReactionList/index.js index 64d22df0d16c..000a13070f0a 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/index.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/index.js @@ -1,4 +1,4 @@ -import React, {forwardRef, useImperativeHandle, useRef, useState} from 'react'; +import React, {forwardRef, useEffect, useImperativeHandle, useRef, useState} from 'react'; import PropTypes from 'prop-types'; import BasePopoverReactionList from './BasePopoverReactionList'; @@ -12,20 +12,32 @@ const defaultProps = { function PopoverReactionList(props) { const innerReactionListRef = useRef(); + const callbackRef = useRef(() => {}); const [reactionListReportActionID, setReactionListReportActionID] = useState(''); + // Avoid race condition since setState is asynchronous + useEffect(() => { + if (!reactionListReportActionID) { + return; + } + + callbackRef.current(); + }, [reactionListReportActionID]); + /** * Show the ReactionList modal popover. * * @param {Object} [event] - A press event. * @param {Element} reactionListAnchor - reactionListAnchor * @param {String} emojiName - Name of emoji + * @param {String} reportActionID - ID of the report action */ - const showReactionList = (event, reactionListAnchor, emojiName) => { - innerReactionListRef.current.showReactionList(event, reactionListAnchor, emojiName); + const showReactionList = (event, reactionListAnchor, emojiName, reportActionID) => { + callbackRef.current = () => innerReactionListRef.current.showReactionList(event, reactionListAnchor, emojiName); + setReactionListReportActionID(reportActionID); }; - useImperativeHandle(props.innerRef, () => ({showReactionList, setReactionListReportActionID}), []); + useImperativeHandle(props.innerRef, () => ({showReactionList}), []); return ( Date: Fri, 4 Aug 2023 07:05:45 +0700 Subject: [PATCH 07/10] revert setState approach --- .../ReactionList/PopoverReactionList/index.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/index.js b/src/pages/home/report/ReactionList/PopoverReactionList/index.js index 000a13070f0a..85c87505a460 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/index.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/index.js @@ -1,4 +1,4 @@ -import React, {forwardRef, useEffect, useImperativeHandle, useRef, useState} from 'react'; +import React, {forwardRef, useImperativeHandle, useRef, useState} from 'react'; import PropTypes from 'prop-types'; import BasePopoverReactionList from './BasePopoverReactionList'; @@ -12,18 +12,8 @@ const defaultProps = { function PopoverReactionList(props) { const innerReactionListRef = useRef(); - const callbackRef = useRef(() => {}); const [reactionListReportActionID, setReactionListReportActionID] = useState(''); - // Avoid race condition since setState is asynchronous - useEffect(() => { - if (!reactionListReportActionID) { - return; - } - - callbackRef.current(); - }, [reactionListReportActionID]); - /** * Show the ReactionList modal popover. * @@ -33,8 +23,8 @@ function PopoverReactionList(props) { * @param {String} reportActionID - ID of the report action */ const showReactionList = (event, reactionListAnchor, emojiName, reportActionID) => { - callbackRef.current = () => innerReactionListRef.current.showReactionList(event, reactionListAnchor, emojiName); setReactionListReportActionID(reportActionID); + innerReactionListRef.current.showReactionList(event, reactionListAnchor, emojiName); }; useImperativeHandle(props.innerRef, () => ({showReactionList}), []); From 2d8d0604324cb7dd0980678c092b6db35512b194 Mon Sep 17 00:00:00 2001 From: tienifr Date: Sat, 5 Aug 2023 01:28:59 +0700 Subject: [PATCH 08/10] optimize re-render --- .../BasePopoverReactionList.js | 103 +++++++++--------- .../ReactionList/PopoverReactionList/index.js | 19 ++-- 2 files changed, 63 insertions(+), 59 deletions(-) diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js index 2f6f112d5523..52de6b5ce950 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js @@ -3,6 +3,7 @@ import {Dimensions} from 'react-native'; import lodashGet from 'lodash/get'; import _ from 'underscore'; import {withOnyx} from 'react-native-onyx'; +import PropTypes from 'prop-types'; import * as Report from '../../../../../libs/actions/Report'; import withLocalize, {withLocalizePropTypes} from '../../../../../components/withLocalize'; import PopoverWithMeasuredContent from '../../../../../components/PopoverWithMeasuredContent'; @@ -16,12 +17,16 @@ import ONYXKEYS from '../../../../../ONYXKEYS'; import EmojiReactionsPropTypes from '../../../../../components/Reactions/EmojiReactionsPropTypes'; const propTypes = { + reportActionID: PropTypes.string, + emojiName: PropTypes.string, emojiReactions: EmojiReactionsPropTypes, ...withLocalizePropTypes, }; const defaultProps = { + reportActionID: '', + emojiName: '', emojiReactions: {}, }; @@ -41,14 +46,8 @@ class BasePopoverReactionList extends React.Component { horizontal: 0, vertical: 0, }, - users: [], - emojiCodes: [], - emojiName: '', - emojiCount: 0, - hasUserReacted: false, }; - this.onPopoverHideActionCallback = () => {}; this.reactionListAnchor = undefined; this.showReactionList = this.showReactionList.bind(this); this.hideReactionList = this.hideReactionList.bind(this); @@ -56,7 +55,6 @@ class BasePopoverReactionList extends React.Component { this.getReactionListMeasuredLocation = this.getReactionListMeasuredLocation.bind(this); this.getReactionInformation = this.getReactionInformation.bind(this); this.dimensionsEventListener = null; - this.contentRef = React.createRef(); } componentDidMount() { @@ -64,31 +62,36 @@ class BasePopoverReactionList extends React.Component { } shouldComponentUpdate(nextProps, nextState) { + if (!this.state.isPopoverVisible && !nextState.isPopoverVisible) { + return false; + } + const previousLocale = lodashGet(this.props, 'preferredLocale', CONST.LOCALES.DEFAULT); const nextLocale = lodashGet(nextProps, 'preferredLocale', CONST.LOCALES.DEFAULT); + const prevReaction = lodashGet(this.props.emojiReactions, this.props.emojiName); + const nextReaction = lodashGet(nextProps.emojiReactions, this.props.emojiName); return ( this.props.reportActionID !== nextProps.reportActionID || - !_.isEqual(this.props.emojiReactions, nextProps.emojiReactions) || + this.props.emojiName !== nextProps.emojiName || + !_.isEqual(prevReaction, nextReaction) || !_.isEqual(this.state, nextState) || previousLocale !== nextLocale ); } componentDidUpdate() { + if (!this.state.isPopoverVisible) { + return; + } + // Hide the list when all reactions are removed - if (this.state.isPopoverVisible && !_.some(lodashGet(this.props.emojiReactions, [this.state.emojiName, 'users']), (user) => !_.isNull(user))) { - this.hideReactionList(); + const isEmptyList = !_.some(lodashGet(this.props.emojiReactions, [this.props.emojiName, 'users'])); + if (!isEmptyList) { + return; } - const selectedReaction = lodashGet(this.props.emojiReactions, [this.state.emojiName]); - const {emojiCount, emojiCodes, hasUserReacted, users} = this.getReactionInformation(selectedReaction); - this.setState({ - users, - emojiCodes, - emojiCount, - hasUserReacted, - }); + this.hideReactionList(); } componentWillUnmount() { @@ -123,20 +126,23 @@ class BasePopoverReactionList extends React.Component { getReactionInformation(selectedReaction) { if (!selectedReaction) { return { - users: [], - emojiCodes: [], emojiName: '', emojiCount: 0, + emojiCodes: [], + hasUserReacted: false, + users: [], }; } 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 emojiName = selectedReaction.emojiName; + const emoji = EmojiUtils.findEmojiByName(emojiName); const emojiCodes = EmojiUtils.getUniqueEmojiCodes(emoji, selectedReaction.users); const hasUserReacted = Report.hasAccountIDEmojiReacted(this.props.currentUserPersonalDetails.accountID, reactionUsers); const users = PersonalDetailsUtils.getPersonalDetailsByIDs(userAccountIDs, this.props.currentUserPersonalDetails.accountID, true); return { + emojiName, emojiCount, emojiCodes, hasUserReacted, @@ -149,13 +155,10 @@ class BasePopoverReactionList extends React.Component { * * @param {Object} [event] - A press event. * @param {Element} reactionListAnchor - reactionListAnchor - * @param {String} emojiName - Name of emoji */ - showReactionList(event, reactionListAnchor, emojiName) { + showReactionList(event, reactionListAnchor) { const nativeEvent = event.nativeEvent || {}; this.reactionListAnchor = reactionListAnchor; - const selectedReaction = lodashGet(this.props.emojiReactions, [emojiName]); - const {emojiCount, emojiCodes, hasUserReacted, users} = this.getReactionInformation(selectedReaction); this.getReactionListMeasuredLocation().then(({x, y}) => { this.setState({ cursorRelativePosition: { @@ -166,12 +169,7 @@ class BasePopoverReactionList extends React.Component { horizontal: nativeEvent.pageX, vertical: nativeEvent.pageY, }, - users, - emojiName, - emojiCodes, - emojiCount, isPopoverVisible: true, - hasUserReacted, }); }); } @@ -206,30 +204,31 @@ class BasePopoverReactionList extends React.Component { } render() { + const selectedReaction = this.state.isPopoverVisible ? lodashGet(this.props.emojiReactions, [this.props.emojiName]) : null; + const {emojiName, emojiCount, emojiCodes, hasUserReacted, users} = this.getReactionInformation(selectedReaction); + return ( - <> - + - - - + hasUserReacted={hasUserReacted} + /> + ); } } diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/index.js b/src/pages/home/report/ReactionList/PopoverReactionList/index.js index 85c87505a460..6d6641877c36 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/index.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/index.js @@ -13,6 +13,7 @@ const defaultProps = { function PopoverReactionList(props) { const innerReactionListRef = useRef(); const [reactionListReportActionID, setReactionListReportActionID] = useState(''); + const [reactionListEmojiName, setReactionListEmojiName] = useState(''); /** * Show the ReactionList modal popover. @@ -24,6 +25,7 @@ function PopoverReactionList(props) { */ const showReactionList = (event, reactionListAnchor, emojiName, reportActionID) => { setReactionListReportActionID(reportActionID); + setReactionListEmojiName(emojiName); innerReactionListRef.current.showReactionList(event, reactionListAnchor, emojiName); }; @@ -33,6 +35,7 @@ function PopoverReactionList(props) { ); } @@ -41,10 +44,12 @@ PopoverReactionList.propTypes = propTypes; PopoverReactionList.defaultProps = defaultProps; PopoverReactionList.displayName = 'PopoverReactionList'; -export default forwardRef((props, ref) => ( - -)); +export default React.memo( + forwardRef((props, ref) => ( + + )), +); From f4578d43e8e3041f8eeeacd847e7f8100beece03 Mon Sep 17 00:00:00 2001 From: tienifr Date: Sat, 5 Aug 2023 02:37:54 +0700 Subject: [PATCH 09/10] assign ref --- .../ReactionList/PopoverReactionList/BasePopoverReactionList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js index 25d1b258aa81..e1aef07ba60f 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js @@ -158,7 +158,7 @@ class BasePopoverReactionList extends React.Component { */ showReactionList(event, reactionListAnchor) { const nativeEvent = event.nativeEvent || {}; - this.reactionListAnchor = reactionListAnchor; + this.reactionListAnchor.current = reactionListAnchor; this.getReactionListMeasuredLocation().then(({x, y}) => { this.setState({ cursorRelativePosition: { From a7295d51b6d067db7e91762aa1d67532891d8ca0 Mon Sep 17 00:00:00 2001 From: tienifr Date: Mon, 7 Aug 2023 01:26:04 +0700 Subject: [PATCH 10/10] remove redundant param --- .../ReactionList/PopoverReactionList/BasePopoverReactionList.js | 2 +- src/pages/home/report/ReactionList/PopoverReactionList/index.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js index e1aef07ba60f..9303d7a5bc39 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/BasePopoverReactionList.js @@ -69,7 +69,7 @@ class BasePopoverReactionList extends React.Component { const previousLocale = lodashGet(this.props, 'preferredLocale', CONST.LOCALES.DEFAULT); const nextLocale = lodashGet(nextProps, 'preferredLocale', CONST.LOCALES.DEFAULT); const prevReaction = lodashGet(this.props.emojiReactions, this.props.emojiName); - const nextReaction = lodashGet(nextProps.emojiReactions, this.props.emojiName); + const nextReaction = lodashGet(nextProps.emojiReactions, nextProps.emojiName); return ( this.props.reportActionID !== nextProps.reportActionID || diff --git a/src/pages/home/report/ReactionList/PopoverReactionList/index.js b/src/pages/home/report/ReactionList/PopoverReactionList/index.js index 6d6641877c36..c39eeddb7fd0 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList/index.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList/index.js @@ -26,7 +26,7 @@ function PopoverReactionList(props) { const showReactionList = (event, reactionListAnchor, emojiName, reportActionID) => { setReactionListReportActionID(reportActionID); setReactionListEmojiName(emojiName); - innerReactionListRef.current.showReactionList(event, reactionListAnchor, emojiName); + innerReactionListRef.current.showReactionList(event, reactionListAnchor); }; useImperativeHandle(props.innerRef, () => ({showReactionList}), []);