From c5812d6c277beb442b5f4765f2d2b8b928a17998 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 18:27:50 -0600 Subject: [PATCH] Move list layout management to its own store This is more general maintenance than performance as the RoomList doesn't need to be generating layouts for the sublists, and it certainly doesn't need to be creating a bunch of extra ones. The sublists are perfectly capable of getting their own layout instance and using it, and we are perfectly able to limit the number of these things we create through the session's lifespan. --- src/@types/global.d.ts | 2 + src/components/views/rooms/RoomList2.tsx | 11 +---- src/components/views/rooms/RoomSublist2.tsx | 44 +++++++++-------- src/stores/room-list/RoomListLayoutStore.ts | 54 +++++++++++++++++++++ src/stores/room-list/RoomListStore2.ts | 13 ++--- 5 files changed, 84 insertions(+), 40 deletions(-) create mode 100644 src/stores/room-list/RoomListLayoutStore.ts diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index 2c2fec759c..fc52296d8b 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -21,6 +21,7 @@ import ToastStore from "../stores/ToastStore"; import DeviceListener from "../DeviceListener"; import { RoomListStore2 } from "../stores/room-list/RoomListStore2"; import { PlatformPeg } from "../PlatformPeg"; +import RoomListLayoutStore from "../stores/room-list/RoomListLayoutStore"; declare global { interface Window { @@ -34,6 +35,7 @@ declare global { mx_ToastStore: ToastStore; mx_DeviceListener: DeviceListener; mx_RoomListStore2: RoomListStore2; + mx_RoomListLayoutStore: RoomListLayoutStore; mxPlatformPeg: PlatformPeg; } diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index fb0136fb29..348c424927 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -32,7 +32,6 @@ import defaultDispatcher from "../../../dispatcher/dispatcher"; import RoomSublist2 from "./RoomSublist2"; import { ActionPayload } from "../../../dispatcher/payloads"; import { NameFilterCondition } from "../../../stores/room-list/filters/NameFilterCondition"; -import { ListLayout } from "../../../stores/room-list/ListLayout"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; import GroupAvatar from "../avatars/GroupAvatar"; import TemporaryTile from "./TemporaryTile"; @@ -66,7 +65,6 @@ interface IProps { interface IState { sublists: ITagMap; - layouts: Map; } const TAG_ORDER: TagID[] = [ @@ -151,7 +149,6 @@ export default class RoomList2 extends React.Component { this.state = { sublists: {}, - layouts: new Map(), }; this.dispatcherRef = defaultDispatcher.register(this.onAction); @@ -227,12 +224,7 @@ export default class RoomList2 extends React.Component { const newLists = RoomListStore.instance.orderedLists; console.log("new lists", newLists); - const layoutMap = new Map(); - for (const tagId of Object.keys(newLists)) { - layoutMap.set(tagId, new ListLayout(tagId)); - } - - this.setState({sublists: newLists, layouts: layoutMap}, () => { + this.setState({sublists: newLists}, () => { this.props.onResize(); }); }; @@ -302,7 +294,6 @@ export default class RoomList2 extends React.Component { onAddRoom={onAddRoomFn} addRoomLabel={aesthetics.addRoomLabel} isInvite={aesthetics.isInvite} - layout={this.state.layouts.get(orderedTagId)} isMinimized={this.props.isMinimized} onResize={this.props.onResize} extraBadTilesThatShouldntExist={extraTiles} diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 732585d3ac..3eadc9a313 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -45,6 +45,7 @@ import {ActionPayload} from "../../../dispatcher/payloads"; import { Enable, Resizable } from "re-resizable"; import { Direction } from "re-resizable/lib/resizer"; import { polyfillTouchEvent } from "../../../@types/polyfill"; +import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -74,7 +75,6 @@ interface IProps { onAddRoom?: () => void; addRoomLabel: string; isInvite: boolean; - layout?: ListLayout; isMinimized: boolean; tagId: TagID; onResize: () => void; @@ -98,10 +98,13 @@ export default class RoomSublist2 extends React.Component { private headerButton = createRef(); private sublistRef = createRef(); private dispatcherRef: string; + private layout: ListLayout; constructor(props: IProps) { super(props); + this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId); + this.state = { notificationState: new ListNotificationState(this.props.isInvite, this.props.tagId), contextMenuPosition: null, @@ -116,8 +119,7 @@ export default class RoomSublist2 extends React.Component { } private get numVisibleTiles(): number { - if (!this.props.layout) return 0; - const nVisible = Math.floor(this.props.layout.visibleTiles); + const nVisible = Math.floor(this.layout.visibleTiles); return Math.min(nVisible, this.numTiles); } @@ -135,7 +137,7 @@ export default class RoomSublist2 extends React.Component { // XXX: we have to do this a tick later because we have incorrect intermediate props during a room change // where we lose the room we are changing from temporarily and then it comes back in an update right after. setImmediate(() => { - const isCollapsed = this.props.layout.isCollapsed; + const isCollapsed = this.layout.isCollapsed; const roomIndex = this.props.rooms.findIndex((r) => r.roomId === payload.room_id); if (isCollapsed && roomIndex > -1) { @@ -143,7 +145,7 @@ export default class RoomSublist2 extends React.Component { } // extend the visible section to include the room if it is entirely invisible if (roomIndex >= this.numVisibleTiles) { - this.props.layout.visibleTiles = this.props.layout.tilesWithPadding(roomIndex + 1, MAX_PADDING_HEIGHT); + this.layout.visibleTiles = this.layout.tilesWithPadding(roomIndex + 1, MAX_PADDING_HEIGHT); this.forceUpdate(); // because the layout doesn't trigger a re-render } }); @@ -170,10 +172,10 @@ export default class RoomSublist2 extends React.Component { // resizing started*, meaning it is fairly useless for us. This is why we just use // the client height and run with it. - const heightBefore = this.props.layout.visibleTiles; - const heightInTiles = this.props.layout.pixelsToTiles(refToElement.clientHeight); - this.props.layout.setVisibleTilesWithin(heightInTiles, this.numTiles); - if (heightBefore === this.props.layout.visibleTiles) return; // no-op + const heightBefore = this.layout.visibleTiles; + const heightInTiles = this.layout.pixelsToTiles(refToElement.clientHeight); + this.layout.setVisibleTilesWithin(heightInTiles, this.numTiles); + if (heightBefore === this.layout.visibleTiles) return; // no-op this.forceUpdate(); // because the layout doesn't trigger a re-render }; @@ -187,13 +189,13 @@ export default class RoomSublist2 extends React.Component { private onShowAllClick = () => { const numVisibleTiles = this.numVisibleTiles; - this.props.layout.visibleTiles = this.props.layout.tilesWithPadding(this.numTiles, MAX_PADDING_HEIGHT); + this.layout.visibleTiles = this.layout.tilesWithPadding(this.numTiles, MAX_PADDING_HEIGHT); this.forceUpdate(); // because the layout doesn't trigger a re-render setImmediate(this.focusRoomTile, numVisibleTiles); // focus the tile after the current bottom one }; private onShowLessClick = () => { - this.props.layout.visibleTiles = this.props.layout.defaultVisibleTiles; + this.layout.visibleTiles = this.layout.defaultVisibleTiles; this.forceUpdate(); // because the layout doesn't trigger a re-render // focus will flow to the show more button here }; @@ -241,7 +243,7 @@ export default class RoomSublist2 extends React.Component { }; private onMessagePreviewChanged = () => { - this.props.layout.showPreviews = !this.props.layout.showPreviews; + this.layout.showPreviews = !this.layout.showPreviews; this.forceUpdate(); // because the layout doesn't trigger a re-render }; @@ -293,13 +295,13 @@ export default class RoomSublist2 extends React.Component { }; private toggleCollapsed = () => { - this.props.layout.isCollapsed = !this.props.layout.isCollapsed; + this.layout.isCollapsed = !this.layout.isCollapsed; this.forceUpdate(); // because the layout doesn't trigger an update setImmediate(() => this.props.onResize()); // needs to happen when the DOM is updated }; private onHeaderKeyDown = (ev: React.KeyboardEvent) => { - const isCollapsed = this.props.layout && this.props.layout.isCollapsed; + const isCollapsed = this.layout && this.layout.isCollapsed; switch (ev.key) { case Key.ARROW_LEFT: ev.stopPropagation(); @@ -339,7 +341,7 @@ export default class RoomSublist2 extends React.Component { }; private renderVisibleTiles(): React.ReactElement[] { - if (this.props.layout && this.props.layout.isCollapsed) { + if (this.layout && this.layout.isCollapsed) { // don't waste time on rendering return []; } @@ -353,7 +355,7 @@ export default class RoomSublist2 extends React.Component { @@ -404,7 +406,7 @@ export default class RoomSublist2 extends React.Component { {_t("Message preview")} @@ -496,7 +498,7 @@ export default class RoomSublist2 extends React.Component { const collapseClasses = classNames({ 'mx_RoomSublist2_collapseBtn': true, - 'mx_RoomSublist2_collapseBtn_collapsed': this.props.layout && this.props.layout.isCollapsed, + 'mx_RoomSublist2_collapseBtn_collapsed': this.layout && this.layout.isCollapsed, }); const classes = classNames({ @@ -524,7 +526,7 @@ export default class RoomSublist2 extends React.Component { tabIndex={tabIndex} className="mx_RoomSublist2_headerText" role="treeitem" - aria-expanded={!this.props.layout || !this.props.layout.isCollapsed} + aria-expanded={!this.layout.isCollapsed} aria-level={1} onClick={this.onHeaderClick} onContextMenu={this.onContextMenu} @@ -558,7 +560,7 @@ export default class RoomSublist2 extends React.Component { let content = null; if (visibleTiles.length > 0) { - const layout = this.props.layout; // to shorten calls + const layout = this.layout; // to shorten calls const maxTilesFactored = layout.tilesWithResizerBoxFactor(this.numTiles); const showMoreBtnClasses = classNames({ @@ -587,7 +589,7 @@ export default class RoomSublist2 extends React.Component { {showMoreText} ); - } else if (this.numTiles <= visibleTiles.length && this.numTiles > this.props.layout.defaultVisibleTiles) { + } else if (this.numTiles <= visibleTiles.length && this.numTiles > this.layout.defaultVisibleTiles) { // we have all tiles visible - add a button to show less let showLessText = ( diff --git a/src/stores/room-list/RoomListLayoutStore.ts b/src/stores/room-list/RoomListLayoutStore.ts new file mode 100644 index 0000000000..dd4364f5fc --- /dev/null +++ b/src/stores/room-list/RoomListLayoutStore.ts @@ -0,0 +1,54 @@ +/* +Copyright 2020 The Matrix.org Foundation C.I.C. + +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. +*/ + +import { TagID } from "./models"; +import { ListLayout } from "./ListLayout"; + +export default class RoomListLayoutStore { + private static internalInstance: RoomListLayoutStore; + + private readonly layoutMap = new Map(); + + public ensureLayoutExists(tagId: TagID) { + if (!this.layoutMap.has(tagId)) { + this.layoutMap.set(tagId, new ListLayout(tagId)); + } + } + + public getLayoutFor(tagId: TagID): ListLayout { + if (!this.layoutMap.has(tagId)) { + this.layoutMap.set(tagId, new ListLayout(tagId)); + } + return this.layoutMap.get(tagId); + } + + // Note: this primarily exists for debugging, and isn't really intended to be used by anything. + public async resetLayouts() { + console.warn("Resetting layouts for room list"); + for (const layout of this.layoutMap.values()) { + layout.reset(); + } + } + + public static get instance(): RoomListLayoutStore { + if (!RoomListLayoutStore.internalInstance) { + RoomListLayoutStore.internalInstance = new RoomListLayoutStore(); + } + return RoomListLayoutStore.internalInstance; + } +} + +window.mx_RoomListLayoutStore = RoomListLayoutStore.instance; diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 82c419d79d..6020e46a12 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -32,6 +32,7 @@ import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm"; import { EffectiveMembership, getEffectiveMembership } from "./membership"; import { ListLayout } from "./ListLayout"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; +import RoomListLayoutStore from "./RoomListLayoutStore"; interface IState { tagsEnabled?: boolean; @@ -50,6 +51,7 @@ export class RoomListStore2 extends AsyncStore { private algorithm = new Algorithm(); private filterConditions: IFilterCondition[] = []; private tagWatcher = new TagWatcher(this); + private layoutMap: Map = new Map(); private readonly watchedSettings = [ 'feature_custom_tags', @@ -416,6 +418,8 @@ export class RoomListStore2 extends AsyncStore { for (const tagId of OrderedDefaultTagIDs) { sorts[tagId] = this.calculateTagSorting(tagId); orders[tagId] = this.calculateListOrder(tagId); + + RoomListLayoutStore.instance.ensureLayoutExists(tagId); } if (this.state.tagsEnabled) { @@ -434,15 +438,6 @@ export class RoomListStore2 extends AsyncStore { this.emit(LISTS_UPDATE_EVENT, this); } - // Note: this primarily exists for debugging, and isn't really intended to be used by anything. - public async resetLayouts() { - console.warn("Resetting layouts for room list"); - for (const tagId of Object.keys(this.orderedLists)) { - new ListLayout(tagId).reset(); - } - await this.regenerateAllLists(); - } - public addFilter(filter: IFilterCondition): void { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log("Adding filter condition:", filter);