From af8faa8a2cb1ee12dcf6974988b8f172f633bb5b Mon Sep 17 00:00:00 2001 From: Saturnino Abril Date: Tue, 18 Dec 2018 23:07:42 +0800 Subject: [PATCH] [MM-13433] Fix unnecessary rendering for reactions component and reaction list screen (#2464) * fix unnecessary rendering for reactions component and reaction list screen * update redux commit --- app/components/reactions/index.js | 21 ++------------ app/components/reactions/reactions.js | 28 ++++++++++++++----- app/utils/reaction.js | 8 +++--- app/utils/reaction.test.js | 40 +++++++++++++-------------- package-lock.json | 11 ++------ package.json | 2 +- 6 files changed, 52 insertions(+), 58 deletions(-) diff --git a/app/components/reactions/index.js b/app/components/reactions/index.js index d29458654..e778343b6 100644 --- a/app/components/reactions/index.js +++ b/app/components/reactions/index.js @@ -45,26 +45,11 @@ function makeMapStateToProps() { } const currentUserId = getCurrentUserId(state); - const reactionsForPost = getReactionsForPostSelector(state, ownProps.postId); - - const highlightedReactions = []; - const reactionsByName = reactionsForPost.reduce((reactions, reaction) => { - if (reactions.has(reaction.emoji_name)) { - reactions.get(reaction.emoji_name).push(reaction); - } else { - reactions.set(reaction.emoji_name, [reaction]); - } - - if (reaction.user_id === currentUserId) { - highlightedReactions.push(reaction.emoji_name); - } - - return reactions; - }, new Map()); + const reactions = getReactionsForPostSelector(state, ownProps.postId); return { - highlightedReactions, - reactions: reactionsByName, + currentUserId, + reactions, theme: getTheme(state), canAddReaction, canRemoveReaction, diff --git a/app/components/reactions/reactions.js b/app/components/reactions/reactions.js index aaaa87bae..78f2e52b3 100644 --- a/app/components/reactions/reactions.js +++ b/app/components/reactions/reactions.js @@ -24,11 +24,11 @@ export default class Reactions extends PureComponent { getReactionsForPost: PropTypes.func.isRequired, removeReaction: PropTypes.func.isRequired, }).isRequired, - highlightedReactions: PropTypes.array.isRequired, + currentUserId: PropTypes.string.isRequired, navigator: PropTypes.object.isRequired, position: PropTypes.oneOf(['right', 'left']), postId: PropTypes.string.isRequired, - reactions: PropTypes.object.isRequired, + reactions: PropTypes.object, theme: PropTypes.object.isRequired, canAddReaction: PropTypes.bool, canRemoveReaction: PropTypes.bool.isRequired, @@ -44,7 +44,7 @@ export default class Reactions extends PureComponent { componentDidMount() { const {actions, postId, reactions} = this.props; - if (!reactions?.size) { + if (reactions) { actions.getReactionsForPost(postId); } } @@ -108,13 +108,27 @@ export default class Reactions extends PureComponent { } renderReactions = () => { - const {highlightedReactions, navigator, reactions, theme, postId} = this.props; + const {currentUserId, navigator, reactions, theme, postId} = this.props; + const highlightedReactions = []; + const reactionsByName = Object.values(reactions).reduce((acc, reaction) => { + if (acc.has(reaction.emoji_name)) { + acc.get(reaction.emoji_name).push(reaction); + } else { + acc.set(reaction.emoji_name, [reaction]); + } - return Array.from(reactions.keys()).map((r) => { + if (reaction.user_id === currentUserId) { + highlightedReactions.push(reaction.emoji_name); + } + + return acc; + }, new Map()); + + return Array.from(reactionsByName.keys()).map((r) => { return ( { +export function getReactionsByName(reactions = {}) { + return Object.values(reactions).reduce((acc, reaction) => { const byName = acc[reaction.emoji_name] || []; acc[reaction.emoji_name] = [...byName, reaction]; @@ -62,6 +62,6 @@ export function getSortedReactionsForHeader(reactionsByName = {}) { return [{name: ALL_EMOJIS, count: totalCount}, ...sortedReactionsForHeader]; } -export function getUniqueUserIds(reactions = []) { - return reactions.map((reaction) => reaction.user_id).filter((id, index, arr) => arr.indexOf(id) === index); +export function getUniqueUserIds(reactions = {}) { + return Object.values(reactions).map((reaction) => reaction.user_id).filter((id, index, arr) => arr.indexOf(id) === index); } diff --git a/app/utils/reaction.test.js b/app/utils/reaction.test.js index 1c0d1d392..05876925e 100644 --- a/app/utils/reaction.test.js +++ b/app/utils/reaction.test.js @@ -49,14 +49,14 @@ describe('getReactionsByName', () => { output: {}, }, { name: 'Should match reactions by name, single', - reactions: [{name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}], + reactions: {'user_id_1-+1': {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}}, output: {'+1': [{emoji_name: '+1', name: 'thumbs_up', post_id: 'post_id_1', user_id: 'user_id_1'}]}, }, { name: 'Should match reactions by name, many', - reactions: [ - {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}, - {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_2'}, - ], + reactions: { + 'user_id_1-+1': {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}, + 'user_id_2-+1': {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_2'}, + }, output: { '+1': [ {emoji_name: '+1', name: 'thumbs_up', post_id: 'post_id_1', user_id: 'user_id_1'}, @@ -65,11 +65,11 @@ describe('getReactionsByName', () => { }, }, { name: 'Should match reactions by name, many names', - reactions: [ - {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}, - {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_2'}, - {name: 'thumbs_down', emoji_name: '-1', post_id: 'post_id_1', user_id: 'user_id_1'}, - ], + reactions: { + 'user_id_1-+1': {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}, + 'user_id_2-+1': {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_2'}, + 'user_id_1--1': {name: 'thumbs_down', emoji_name: '-1', post_id: 'post_id_1', user_id: 'user_id_1'}, + }, output: { '+1': [ {emoji_name: '+1', name: 'thumbs_up', post_id: 'post_id_1', user_id: 'user_id_1'}, @@ -149,22 +149,22 @@ describe('getUniqueUserIds', () => { output: [], }, { name: 'Should match unique user ID', - reactions: [{name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}], + reactions: {'user_id_1-+1': {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}}, output: ['user_id_1'], }, { name: 'Should match unique user IDs', - reactions: [ - {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}, - {name: 'thumbs_down', emoji_name: '-1', post_id: 'post_id_1', user_id: 'user_id_1'}, - ], + reactions: { + 'user_id_1-+1': {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}, + 'user_id_1--1': {name: 'thumbs_down', emoji_name: '-1', post_id: 'post_id_1', user_id: 'user_id_1'}, + }, output: ['user_id_1'], }, { name: 'Should match unique user IDs', - reactions: [ - {name: 'smile', emoji_name: 'smile', post_id: 'post_id_1', user_id: 'user_id_2'}, - {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}, - {name: 'thumbs_down', emoji_name: '-1', post_id: 'post_id_1', user_id: 'user_id_1'}, - ], + reactions: { + 'user_id_2-smile': {name: 'smile', emoji_name: 'smile', post_id: 'post_id_1', user_id: 'user_id_2'}, + 'user_id_1-+1': {name: 'thumbs_up', emoji_name: '+1', post_id: 'post_id_1', user_id: 'user_id_1'}, + 'user_id_1--1': {name: 'thumbs_down', emoji_name: '-1', post_id: 'post_id_1', user_id: 'user_id_1'}, + }, output: ['user_id_2', 'user_id_1'], }]; diff --git a/package-lock.json b/package-lock.json index 114969f61..be35dcdb6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8376,8 +8376,8 @@ "integrity": "sha1-izqsWIuKZuSXXjzepn97sylgH6w=" }, "mattermost-redux": { - "version": "github:mattermost/mattermost-redux#64e8fdb056e19455b2bbe6297e76b77c80149d01", - "from": "github:mattermost/mattermost-redux#64e8fdb056e19455b2bbe6297e76b77c80149d01", + "version": "github:mattermost/mattermost-redux#c8364a767889a8759969b8c1591b0c6ae5e1b77c", + "from": "github:mattermost/mattermost-redux#c8364a767889a8759969b8c1591b0c6ae5e1b77c", "requires": { "deep-equal": "1.0.1", "eslint-plugin-header": "2.0.0", @@ -8393,7 +8393,7 @@ "redux-persist": "4.9.1", "redux-thunk": "2.3.0", "reselect": "4.0.0", - "serialize-error": "3.0.0", + "serialize-error": "2.1.0", "shallow-equals": "1.0.0" }, "dependencies": { @@ -8416,11 +8416,6 @@ "lodash": "^4.17.4", "lodash-es": "^4.17.4" } - }, - "serialize-error": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/serialize-error/-/serialize-error-3.0.0.tgz", - "integrity": "sha512-+y3nkkG/go1Vdw+2f/+XUXM1DXX1XcxTl99FfiD/OEPUNw4uo0i6FKABfTAN5ZcgGtjTRZcEbxcE/jtXbEY19A==" } } }, diff --git a/package.json b/package.json index a93e93ec1..48e0134bb 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "intl": "1.2.5", "jail-monkey": "1.0.0", "jsc-android": "236355.1.0", - "mattermost-redux": "github:mattermost/mattermost-redux#64e8fdb056e19455b2bbe6297e76b77c80149d01", + "mattermost-redux": "github:mattermost/mattermost-redux#c8364a767889a8759969b8c1591b0c6ae5e1b77c", "mime-db": "1.37.0", "moment-timezone": "0.5.23", "prop-types": "15.6.2",