From c07e5d4992c35382a2e502cb305c28a6d8300820 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 14 Dec 2016 15:31:35 +0000 Subject: [PATCH] Improve the performance of MemberEventListSummary - The MessagePanel now uses the same key for the MELS instances rendered so that entirely new instances are not created, they are simply passed new props (namely when new events arrive). - MELS itself now uses `shouldComponentUpdate` so that it only updates if it is given a different number of events to previous or if it is toggled to expand. --- src/components/structures/MessagePanel.js | 8 +- .../views/elements/MemberEventListSummary.js | 158 ++++++++++-------- 2 files changed, 95 insertions(+), 71 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 53bf560673..a2e6c1bc7e 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -296,6 +296,12 @@ module.exports = React.createClass({ // Wrap consecutive member events in a ListSummary if (isMembershipChange(mxEv)) { let ts1 = mxEv.getTs(); + // Ensure that the key of the MemberEventListSummary does not change with new + // member events. This will prevent it from being re-created necessarily, and + // instead will allow new props to be provided. In turn, the shouldComponentUpdate + // method on MELS can be used to prevent unnecessary renderings. + // `prevEvent` at this point is a non-member event or null. + const key = "memberlistsummary-" + (prevEvent ? prevEvent.getId() : ""); if (this._wantsDateSeparator(prevEvent, ts1)) { let dateSeparator =
    • ; @@ -332,7 +338,7 @@ module.exports = React.createClass({ ret.push( {eventTiles} diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 69bcd7c203..50fd93f01c 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -42,7 +42,7 @@ module.exports = React.createClass({ return { summaryLength: 3, threshold: 3, - avatarsMaxLength: 5 + avatarsMaxLength: 5, }; }, @@ -144,88 +144,106 @@ module.exports = React.createClass({ ); }, - render: function() { - let summary = null; - - // Reorder events so that joins come before leaves - let eventsToRender = this.props.events; - - // Create an array of events that are not "cancelled-out" by another - // A join of sender S is cancelled out by a leave of sender S etc. - let filteredEvents = []; - let senders = new Set(eventsToRender.map((e) => e.getSender())); - senders.forEach( - (userId) => { - let userEvents = eventsToRender.filter((e) => { - return e.getSender() === userId; - }); - - // NB: These may be the same event, in which case the lastEvent is used - // because prev_content should != content - let firstEvent = userEvents[0]; - let lastEvent = userEvents[userEvents.length - 1]; - - // Membership BEFORE eventsToRender - let previousMembership = firstEvent.getPrevContent().membership || "leave"; - - // Otherwise, if the last membership event differs from previousMembership, - // use that. - if (previousMembership !== lastEvent.getContent().membership) { - filteredEvents.push(lastEvent); - } - } + shouldComponentUpdate: function(nextProps, nextState) { + return ( + nextProps.events.length !== this.props.events.length || + nextState.expanded !== this.state.expanded ); + }, - let joinAndLeft = (eventsToRender.length - filteredEvents.length) / 2; - if (joinAndLeft <= 0 || joinAndLeft % 1 !== 0) { - joinAndLeft = null; - } - - let joinEvents = filteredEvents.filter((ev) => { - return ev.event.content.membership === 'join'; - }); - - let leaveEvents = filteredEvents.filter((ev) => { - return ev.event.content.membership === 'leave'; - }); - + render: function() { + let eventsToRender = this.props.events; let fewEvents = eventsToRender.length < this.props.threshold; let expanded = this.state.expanded || fewEvents; - let expandedEvents = null; + let expandedEvents = null; if (expanded) { expandedEvents = this.props.children; } - let avatars = this._renderAvatars(joinEvents.concat(leaveEvents)); - - let toggleButton = null; - let summaryContainer = null; - if (!fewEvents) { - summary = this._renderSummary(joinEvents, leaveEvents); - toggleButton = ( - - {expanded ? 'collapse' : 'expand'} - - ); - let plural = (joinEvents.length + leaveEvents.length > 0) ? 'others' : 'users'; - let noun = (joinAndLeft === 1 ? 'user' : plural); - - summaryContainer = ( -
    -
    - - {avatars} - - - {summary}{joinAndLeft ? joinAndLeft + ' ' + noun + ' joined and left' : ''} -   - {toggleButton} -
    + if (fewEvents) { + return ( +
    + {expandedEvents}
    ); } + // Map user IDs to the first and last member events in eventsToRender for each user + let userEvents = { + // $userId : {first : e0, last : e1} + }; + + eventsToRender.forEach((e) => { + const userId = e.getSender(); + // Initialise a user's events + if (!userEvents[userId]) { + userEvents[userId] = {first: null, last: null}; + } + if (!userEvents[userId].first) { + userEvents[userId].first = e; + } else { + userEvents[userId].last = e; + } + }); + + // Populate the join/leave event arrays with events that represent what happened + // overall to a user's membership. If no events are added to either array for a + // particular user, they will be considered a user that "joined and left". + let joinEvents = []; + let leaveEvents = []; + let joinedAndLeft = 0; + let senders = Object.keys(userEvents); + senders.forEach( + (userId) => { + let firstEvent = userEvents[userId].first; + let lastEvent = userEvents[userId].last; + // Only one membership event was recorded for this userId + if (!lastEvent) { + lastEvent = firstEvent; + } + + // Membership BEFORE eventsToRender + let previousMembership = firstEvent.getPrevContent().membership || "leave"; + + // If the last membership event differs from previousMembership, use that. + if (previousMembership !== lastEvent.getContent().membership) { + if (lastEvent.event.content.membership === 'join') { + joinEvents.push(lastEvent); + } else if (lastEvent.event.content.membership === 'leave') { + leaveEvents.push(lastEvent); + } + } else { + // Increment the number of users whose membership change was nil overall + joinedAndLeft++; + } + } + ); + + let avatars = this._renderAvatars(joinEvents.concat(leaveEvents)); + let summary = this._renderSummary(joinEvents, leaveEvents); + let toggleButton = ( + + {expanded ? 'collapse' : 'expand'} + + ); + let plural = (joinEvents.length + leaveEvents.length > 0) ? 'others' : 'users'; + let noun = (joinedAndLeft === 1 ? 'user' : plural); + + let summaryContainer = ( +
    +
    + + {avatars} + + + {summary}{joinedAndLeft ? joinedAndLeft + ' ' + noun + ' joined and left' : ''} +   + {toggleButton} +
    +
    + ); + return (
    {summaryContainer}