From 16398fbfc269b398d83ed2047a15e98894858568 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 22 Sep 2017 13:15:02 +0100 Subject: [PATCH] Allow TruncatedList to get children via a callback And update MemberList to use it as such. This means that the parent only needs to make react elements for the elements that will actually be rendered, rather than all of them. In practive this doesn't make a huge difference as making React elements is fairly fast, but experimentally (with all profiling turned on), MemberList went from 25ms in the constructor and 81ms in render to 38ms in constructor but sub 1ms render for Matrix HQ. --- .../views/elements/TruncatedList.js | 61 +++++++++++---- src/components/views/rooms/MemberList.js | 78 ++++++++++++------- 2 files changed, 95 insertions(+), 44 deletions(-) diff --git a/src/components/views/elements/TruncatedList.js b/src/components/views/elements/TruncatedList.js index e35e12e039..cc94b4ff9b 100644 --- a/src/components/views/elements/TruncatedList.js +++ b/src/components/views/elements/TruncatedList.js @@ -27,6 +27,16 @@ module.exports = React.createClass({ truncateAt: PropTypes.number, // The className to apply to the wrapping div className: PropTypes.string, + // A function that returns the children to be rendered into the element. + // Takes two integers which define the range of child indices to return. + // The start element is included, the end is not (as in `slice`). + // Returns an array. + // If omitted, the React child elements will be used. This parameter can be used + // to avoid creating unnecessary React elements. + getChildren: PropTypes.func, + // A function that should return the total number of child element available. + // Required if getChildren is supplied. + getChildCount: PropTypes.func, // A function which will be invoked when an overflow element is required. // This will be inserted after the children. createOverflowElement: PropTypes.func, @@ -43,33 +53,50 @@ module.exports = React.createClass({ }; }, + _getChildren: function(min, max) { + if (this.props.getChildren && this.props.getChildCount) { + return this.props.getChildren(min, max); + } else { + // XXX: I'm not sure why anything would pass null into this, it seems + // like a bizzare case to handle, but I'm preserving the behaviour. + // (see commit 38d5c7d5c5d5a34dc16ef5d46278315f5c57f542) + return React.Children.toArray(this.props.children).filter((c) => { + return c != null; + }).slice(min, max); + } + }, + + _getChildCount: function() { + if (this.props.getChildren && this.props.getChildCount) { + return this.props.getChildCount(); + } else { + return React.Children.toArray(this.props.children).filter((c) => { + return c != null; + }).length; + } + }, + render: function() { - let childsJsx = this.props.children; - let overflowJsx; - const childArray = React.Children.toArray(this.props.children).filter((c) => { - return c != null; - }); - - const childCount = childArray.length; + let childNodes; + let overflowNode = null; + const totalChildren = this._getChildCount(); + let upperBound = totalChildren; if (this.props.truncateAt >= 0) { - const overflowCount = childCount - this.props.truncateAt; - + const overflowCount = totalChildren - this.props.truncateAt; if (overflowCount > 1) { - overflowJsx = this.props.createOverflowElement( - overflowCount, childCount, + overflowNode = this.props.createOverflowElement( + overflowCount, totalChildren, ); - - // cut out the overflow elements - childArray.splice(childCount - overflowCount, overflowCount); - childsJsx = childArray; // use what is left + upperBound = this.props.truncateAt; } } + childNodes = this._getChildren(0, upperBound); return (
- {childsJsx} - {overflowJsx} + {childNodes} + {overflowNode}
); }, diff --git a/src/components/views/rooms/MemberList.js b/src/components/views/rooms/MemberList.js index 33d05235bf..9c75e2d052 100644 --- a/src/components/views/rooms/MemberList.js +++ b/src/components/views/rooms/MemberList.js @@ -1,6 +1,7 @@ /* Copyright 2015, 2016 OpenMarket Ltd Copyright 2017 Vector Creations Ltd +Copyright 2017 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,10 +15,11 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -var React = require('react'); + +import React from 'react'; import { _t } from '../../../languageHandler'; -var classNames = require('classnames'); -var Matrix = require("matrix-js-sdk"); +import classNames from 'classnames'; +import Matrix from 'matrix-js-sdk'; import Promise from 'bluebird'; var MatrixClientPeg = require("../../../MatrixClientPeg"); var Modal = require("../../../Modal"); @@ -27,13 +29,13 @@ var GeminiScrollbar = require('react-gemini-scrollbar'); var rate_limited_func = require('../../../ratelimitedfunc'); var CallHandler = require("../../../CallHandler"); -var INITIAL_LOAD_NUM_MEMBERS = 30; +const INITIAL_LOAD_NUM_MEMBERS = 30; module.exports = React.createClass({ displayName: 'MemberList', getInitialState: function() { - var state = { + const state = { members: [], // ideally we'd size this to the page height, but // in practice I find that a little constraining @@ -48,6 +50,8 @@ module.exports = React.createClass({ this.memberDict = this.getMemberDict(); state.members = this.roomMembers(); + state.filteredJoinedMembers = this._filterMembers(state.members, 'join'); + state.filteredInvitedMembers = this._filterMembers(state.members, 'invite'); return state; }, @@ -146,10 +150,12 @@ module.exports = React.createClass({ // console.log("Updating memberlist"); this.memberDict = this.getMemberDict(); - var self = this; - this.setState({ - members: self.roomMembers() - }); + const newState = { + members: this.roomMembers(), + }; + newState.filteredJoinedMembers = this._filterMembers(newState.members, 'join'); + newState.filteredInvitedMembers = this._filterMembers(newState.members, 'invite'); + this.setState(newState); }, 500), getMemberDict: function() { @@ -279,17 +285,17 @@ module.exports = React.createClass({ }, onSearchQueryChanged: function(ev) { - this.setState({ searchQuery: ev.target.value }); + const q = ev.target.value; + this.setState({ + searchQuery: q, + filteredJoinedMembers: this._filterMembers(this.state.members, 'join', q), + filteredInvitedMembers: this._filterMembers(this.state.members, 'invite', q), + }); }, - makeMemberTiles: function(membership, query) { - var MemberTile = sdk.getComponent("rooms.MemberTile"); - query = (query || "").toLowerCase(); - - var self = this; - - var memberList = self.state.members.filter(function(userId) { - var m = self.memberDict[userId]; + _filterMembers: function(members, membership, query) { + return members.filter((userId) => { + const m = this.memberDict[userId]; if (query) { const matchesName = m.name.toLowerCase().indexOf(query) !== -1; @@ -301,14 +307,23 @@ module.exports = React.createClass({ } return m.membership == membership; - }).map(function(userId) { - var m = self.memberDict[userId]; + }); + }, + + _makeMemberTiles: function(members, membership) { + const MemberTile = sdk.getComponent("rooms.MemberTile"); + + const memberList = members.map((userId) => { + const m = this.memberDict[userId]; return ( ); }); // XXX: surely this is not the right home for this logic. + // Double XXX: Now it's really, really not the right home for this logic: + // we shouldn't even be passing in the 'membership' param to this function. + // Ew, ew, and ew. if (membership === "invite") { // include 3pid invites (m.room.third_party_invite) state events. // The HS may have already converted these into m.room.member invites so @@ -341,9 +356,17 @@ module.exports = React.createClass({ return memberList; }, + _getChildrenJoined: function(min, max) { + return this._makeMemberTiles(this.state.filteredJoinedMembers.slice(min, max)); + }, + + _getChildCountJoined: function() { + return this.state.filteredJoinedMembers.length; + }, + render: function() { - var invitedSection = null; - var invitedMemberTiles = this.makeMemberTiles('invite', this.state.searchQuery); + let invitedSection = null; + const invitedMemberTiles = this._makeMemberTiles(this.state.filteredInvitedMembers, 'invite'); if (invitedMemberTiles.length > 0) { invitedSection = (
@@ -355,7 +378,7 @@ module.exports = React.createClass({ ); } - var inputBox = ( + const inputBox = (
); - var TruncatedList = sdk.getComponent("elements.TruncatedList"); + const TruncatedList = sdk.getComponent("elements.TruncatedList"); return (
{ inputBox } - {this.makeMemberTiles('join', this.state.searchQuery)} - + createOverflowElement={this._createOverflowTile} + getChildren={this._getChildrenJoined} + getChildCount={this._getChildCountJoined} + /> {invitedSection}