From 3ce2309ae2d92331f4b3d1794e226e89376a014f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 13 Feb 2019 14:49:14 +0100 Subject: [PATCH 1/7] get scrolltop and scrollevent out of autohide/indicator scrollbar --- src/components/structures/AutoHideScrollbar.js | 5 +++++ src/components/structures/IndicatorScrollbar.js | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/components/structures/AutoHideScrollbar.js b/src/components/structures/AutoHideScrollbar.js index a385df0401..0f93f20407 100644 --- a/src/components/structures/AutoHideScrollbar.js +++ b/src/components/structures/AutoHideScrollbar.js @@ -114,10 +114,15 @@ export default class AutoHideScrollbar extends React.Component { } } + getScrollTop() { + return this.containerRef.scrollTop; + } + render() { return (
{ this.props.children } diff --git a/src/components/structures/IndicatorScrollbar.js b/src/components/structures/IndicatorScrollbar.js index c3e54ee900..e1516d1f64 100644 --- a/src/components/structures/IndicatorScrollbar.js +++ b/src/components/structures/IndicatorScrollbar.js @@ -59,6 +59,10 @@ export default class IndicatorScrollbar extends React.Component { } } + getScrollTop() { + return this._autoHideScrollbar.getScrollTop(); + } + componentWillUnmount() { if (this._scrollElement) { this._scrollElement.removeEventListener("scroll", this.checkOverflow); From 0de2161a0da8ef80b2d46f0a9071b5d317436fe6 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 13 Feb 2019 14:50:43 +0100 Subject: [PATCH 2/7] create LazyRenderList component to trim list to visible part --- .../views/elements/LazyRenderList.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 src/components/views/elements/LazyRenderList.js diff --git a/src/components/views/elements/LazyRenderList.js b/src/components/views/elements/LazyRenderList.js new file mode 100644 index 0000000000..69abc34128 --- /dev/null +++ b/src/components/views/elements/LazyRenderList.js @@ -0,0 +1,33 @@ +/* +Copyright 2019 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. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +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. +*/ + +const OVERFLOW_ITEMS = 2; + +export default function LazyRenderList(props) { + const {items, itemHeight, scrollTop, height, renderItem} = props; + + const firstIdx = Math.max(0, Math.floor(scrollTop / itemHeight) - OVERFLOW_ITEMS); + const itemsAfterFirst = items.length - firstIdx; + const amount = Math.min(Math.ceil(height / itemHeight) + OVERFLOW_ITEMS, itemsAfterFirst); + const beforeSpace = firstIdx * itemHeight; + const itemsAfter = itemsAfterFirst - amount; + const afterSpace = itemsAfter * itemHeight; + const renderedItems = items.slice(firstIdx, firstIdx + amount); + + return (
+ { renderedItems.map(renderItem) } +
); +} From 84163bed11149235bb22a5dfc15a53196a4ce053 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 13 Feb 2019 14:51:09 +0100 Subject: [PATCH 3/7] wrap roomtiles in LazyRenderList to improve perf for big accounts --- src/components/structures/RoomSubList.js | 70 ++++++++++++++++-------- 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/src/components/structures/RoomSubList.js b/src/components/structures/RoomSubList.js index ca2be85b35..ba61bb5ad1 100644 --- a/src/components/structures/RoomSubList.js +++ b/src/components/structures/RoomSubList.js @@ -27,7 +27,9 @@ import IndicatorScrollbar from './IndicatorScrollbar'; import { KeyCode } from '../../Keyboard'; import { Group } from 'matrix-js-sdk'; import PropTypes from 'prop-types'; - +import RoomTile from "../views/rooms/RoomTile"; +import LazyRenderList from "../views/elements/LazyRenderList"; +import * as _ from "lodash"; // turn this on for drop & drag console debugging galore const debug = false; @@ -58,8 +60,20 @@ const RoomSubList = React.createClass({ }, getInitialState: function() { + // throttle updates to LazyRenderList + this._onScroll = _.throttle( + this._onScroll, 100, + {leading: false, trailing: true}, + ); + this._updateLazyRenderHeight = _.throttle( + this._updateLazyRenderHeight, 100, + {leading: false, trailing: true}, + ); return { hidden: this.props.startAsHidden || false, + // some values to get LazyRenderList starting + scrollerHeight: 800, + scrollTop: 0, }; }, @@ -134,24 +148,21 @@ const RoomSubList = React.createClass({ this.setState(this.state); }, - makeRoomTiles: function() { - const RoomTile = sdk.getComponent("rooms.RoomTile"); - return this.props.list.map((room, index) => { - return 0 || this.props.isInvite} - notificationCount={room.getUnreadNotificationCount()} - isInvite={this.props.isInvite} - refreshSubList={this._updateSubListCount} - incomingCall={null} - onClick={this.onRoomTileClick} - />; - }); + makeRoomTile: function(room) { + return 0 || this.props.isInvite} + notificationCount={room.getUnreadNotificationCount()} + isInvite={this.props.isInvite} + refreshSubList={this._updateSubListCount} + incomingCall={null} + onClick={this.onRoomTileClick} + />; }, _onNotifBadgeClick: function(e) { @@ -270,6 +281,15 @@ const RoomSubList = React.createClass({ if (this.refs.subList) { this.refs.subList.style.height = `${height}px`; } + this._updateLazyRenderHeight(height); + }, + + _updateLazyRenderHeight: function(height) { + this.setState({scrollerHeight: height}); + }, + + _onScroll: function() { + this.setState({scrollTop: this.refs.scroller.getScrollTop()}); }, render: function() { @@ -287,12 +307,16 @@ const RoomSubList = React.createClass({ {this._getHeaderJsx(isCollapsed)}
; } else { - const tiles = this.makeRoomTiles(); - tiles.push(...this.props.extraTiles); + const items = this.props.list.concat(this.props.extraTiles); return
{this._getHeaderJsx(isCollapsed)} - - { tiles } + +
; } From 60d0ed4c0119b8bec97b1a3ecabb55095d5c9650 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 13 Feb 2019 17:16:20 +0100 Subject: [PATCH 4/7] make LazyRenderList stateful for better performance it only rerenders when visible range it would render based on the props gets OVERFLOW_MARGIN(5) items from the current renderRange --- src/components/structures/RoomSubList.js | 18 ++-- .../views/elements/LazyRenderList.js | 85 ++++++++++++++++--- 2 files changed, 81 insertions(+), 22 deletions(-) diff --git a/src/components/structures/RoomSubList.js b/src/components/structures/RoomSubList.js index ba61bb5ad1..e5a96d0923 100644 --- a/src/components/structures/RoomSubList.js +++ b/src/components/structures/RoomSubList.js @@ -61,14 +61,14 @@ const RoomSubList = React.createClass({ getInitialState: function() { // throttle updates to LazyRenderList - this._onScroll = _.throttle( - this._onScroll, 100, - {leading: false, trailing: true}, - ); - this._updateLazyRenderHeight = _.throttle( - this._updateLazyRenderHeight, 100, - {leading: false, trailing: true}, - ); + // this._onScroll = _.throttle( + // this._onScroll, 50, + // {leading: false, trailing: true}, + // ); + // this._updateLazyRenderHeight = _.throttle( + // this._updateLazyRenderHeight, 100, + // {leading: false, trailing: true}, + // ); return { hidden: this.props.startAsHidden || false, // some values to get LazyRenderList starting @@ -307,7 +307,7 @@ const RoomSubList = React.createClass({ {this._getHeaderJsx(isCollapsed)}
; } else { - const items = this.props.list.concat(this.props.extraTiles); + const items = this.props.list; //.concat(this.props.extraTiles); return
{this._getHeaderJsx(isCollapsed)} diff --git a/src/components/views/elements/LazyRenderList.js b/src/components/views/elements/LazyRenderList.js index 69abc34128..ec56b9a532 100644 --- a/src/components/views/elements/LazyRenderList.js +++ b/src/components/views/elements/LazyRenderList.js @@ -14,20 +14,79 @@ See the License for the specific language governing permissions and limitations under the License. */ -const OVERFLOW_ITEMS = 2; +import React from "react"; -export default function LazyRenderList(props) { - const {items, itemHeight, scrollTop, height, renderItem} = props; +const OVERFLOW_ITEMS = 20; +const OVERFLOW_MARGIN = 5; - const firstIdx = Math.max(0, Math.floor(scrollTop / itemHeight) - OVERFLOW_ITEMS); - const itemsAfterFirst = items.length - firstIdx; - const amount = Math.min(Math.ceil(height / itemHeight) + OVERFLOW_ITEMS, itemsAfterFirst); - const beforeSpace = firstIdx * itemHeight; - const itemsAfter = itemsAfterFirst - amount; - const afterSpace = itemsAfter * itemHeight; - const renderedItems = items.slice(firstIdx, firstIdx + amount); +class ItemRange { + constructor(topCount, renderCount, bottomCount) { + this.topCount = topCount; + this.renderCount = renderCount; + this.bottomCount = bottomCount; + } - return (
- { renderedItems.map(renderItem) } -
); + contains(range) { + return range.topCount >= this.topCount && + (range.topCount + range.renderCount) <= (this.topCount + this.renderCount); + } + + expand(amount) { + const topGrow = Math.min(amount, this.topCount); + const bottomGrow = Math.min(amount, this.bottomCount); + return new ItemRange( + this.topCount - topGrow, + this.renderCount + topGrow + bottomGrow, + this.bottomCount - bottomGrow, + ); + } +} + +export default class LazyRenderList extends React.Component { + constructor(props) { + super(props); + const renderRange = LazyRenderList.getVisibleRangeFromProps(props).expand(OVERFLOW_ITEMS); + this.state = {renderRange}; + } + + static getVisibleRangeFromProps(props) { + const {items, itemHeight, scrollTop, height} = props; + const length = items ? items.length : 0; + const topCount = Math.max(0, Math.floor(scrollTop / itemHeight)); + const itemsAfterTop = length - topCount; + const renderCount = Math.min(Math.ceil(height / itemHeight), itemsAfterTop); + const bottomCount = itemsAfterTop - renderCount; + return new ItemRange(topCount, renderCount, bottomCount); + } + + componentWillReceiveProps(props) { + const state = this.state; + const range = LazyRenderList.getVisibleRangeFromProps(props); + // only update state if the new range isn't contained by the old anymore + if (!state.renderRange || !state.renderRange.contains(range.expand(OVERFLOW_MARGIN))) { + this.setState({renderRange: range.expand(OVERFLOW_ITEMS)}); + } + } + + shouldComponentUpdate(nextProps, nextState) { + const itemsChanged = nextProps.items !== this.props.items; + const rangeChanged = nextState.renderRange !== this.state.renderRange; + return itemsChanged || rangeChanged; + } + + render() { + const {itemHeight, items, renderItem} = this.props; + + const {renderRange} = this.state; + const paddingTop = renderRange.topCount * itemHeight; + const paddingBottom = renderRange.bottomCount * itemHeight; + const renderedItems = items.slice( + renderRange.topCount, + renderRange.topCount + renderRange.renderCount, + ); + + return (
+ { renderedItems.map(renderItem) } +
); + } } From 39632428d0df7eefc6fddd0b64b9747bb91ffe52 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 13 Feb 2019 18:36:19 +0100 Subject: [PATCH 5/7] remove throttling after improving LazyRenderList perf --- src/components/structures/RoomSubList.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/components/structures/RoomSubList.js b/src/components/structures/RoomSubList.js index e5a96d0923..a66447a83d 100644 --- a/src/components/structures/RoomSubList.js +++ b/src/components/structures/RoomSubList.js @@ -29,7 +29,6 @@ import { Group } from 'matrix-js-sdk'; import PropTypes from 'prop-types'; import RoomTile from "../views/rooms/RoomTile"; import LazyRenderList from "../views/elements/LazyRenderList"; -import * as _ from "lodash"; // turn this on for drop & drag console debugging galore const debug = false; @@ -60,15 +59,6 @@ const RoomSubList = React.createClass({ }, getInitialState: function() { - // throttle updates to LazyRenderList - // this._onScroll = _.throttle( - // this._onScroll, 50, - // {leading: false, trailing: true}, - // ); - // this._updateLazyRenderHeight = _.throttle( - // this._updateLazyRenderHeight, 100, - // {leading: false, trailing: true}, - // ); return { hidden: this.props.startAsHidden || false, // some values to get LazyRenderList starting From 42409691b3035c75fc0476ddebddfd4598400931 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 13 Feb 2019 18:36:46 +0100 Subject: [PATCH 6/7] try to not trigger rerenders in LazyRenderList by not creating new arrays --- src/components/structures/RoomSubList.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/components/structures/RoomSubList.js b/src/components/structures/RoomSubList.js index a66447a83d..f7f74da728 100644 --- a/src/components/structures/RoomSubList.js +++ b/src/components/structures/RoomSubList.js @@ -282,6 +282,20 @@ const RoomSubList = React.createClass({ this.setState({scrollTop: this.refs.scroller.getScrollTop()}); }, + _getRenderItems: function() { + // try our best to not create a new array + // because LazyRenderList rerender when the items prop + // is not the same object as the previous value + const {list, extraTiles} = this.props; + if (!extraTiles || !extraTiles.length) { + return list; + } + if (!list || list.length) { + return extraTiles; + } + return list.concat(extraTiles); + }, + render: function() { const len = this.props.list.length + this.props.extraTiles.length; const isCollapsed = this.state.hidden && !this.props.forceExpand; @@ -297,7 +311,6 @@ const RoomSubList = React.createClass({ {this._getHeaderJsx(isCollapsed)}
; } else { - const items = this.props.list; //.concat(this.props.extraTiles); return
{this._getHeaderJsx(isCollapsed)} @@ -306,7 +319,7 @@ const RoomSubList = React.createClass({ height={ this.state.scrollerHeight } renderItem={ this.makeRoomTile } itemHeight={34} - items={items} /> + items={this._getRenderItems()} />
; } From e51f279f363792bf9609956db5cdb0b9fa3ee243 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 13 Feb 2019 18:49:09 +0100 Subject: [PATCH 7/7] dont assume there items is an array --- src/components/views/elements/LazyRenderList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/LazyRenderList.js b/src/components/views/elements/LazyRenderList.js index ec56b9a532..b7916510a4 100644 --- a/src/components/views/elements/LazyRenderList.js +++ b/src/components/views/elements/LazyRenderList.js @@ -80,7 +80,7 @@ export default class LazyRenderList extends React.Component { const {renderRange} = this.state; const paddingTop = renderRange.topCount * itemHeight; const paddingBottom = renderRange.bottomCount * itemHeight; - const renderedItems = items.slice( + const renderedItems = (items || []).slice( renderRange.topCount, renderRange.topCount + renderRange.renderCount, );