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

@@ -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;
     }
 

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<TagID, ListLayout>;
 }
 
 const TAG_ORDER: TagID[] = [
@@ -151,7 +149,6 @@ export default class RoomList2 extends React.Component<IProps, IState> {
 
         this.state = {
             sublists: {},
-            layouts: new Map<TagID, ListLayout>(),
         };
 
         this.dispatcherRef = defaultDispatcher.register(this.onAction);
@@ -227,12 +224,7 @@ export default class RoomList2 extends React.Component<IProps, IState> {
         const newLists = RoomListStore.instance.orderedLists;
         console.log("new lists", newLists);
 
-        const layoutMap = new Map<TagID, ListLayout>();
+        this.setState({sublists: newLists}, () => {
-        for (const tagId of Object.keys(newLists)) {
-            layoutMap.set(tagId, new ListLayout(tagId));
-        }
-
-        this.setState({sublists: newLists, layouts: layoutMap}, () => {
             this.props.onResize();
         });
     };
@@ -302,7 +294,6 @@ export default class RoomList2 extends React.Component<IProps, IState> {
                     onAddRoom={onAddRoomFn}
                     addRoomLabel={aesthetics.addRoomLabel}
                     isInvite={aesthetics.isInvite}
-                    layout={this.state.layouts.get(orderedTagId)}
                     isMinimized={this.props.isMinimized}
                     onResize={this.props.onResize}
                     extraBadTilesThatShouldntExist={extraTiles}

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<IProps, IState> {
     private headerButton = createRef<HTMLDivElement>();
     private sublistRef = createRef<HTMLDivElement>();
     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<IProps, IState> {
     }
 
     private get numVisibleTiles(): number {
-        if (!this.props.layout) return 0;
+        const nVisible = Math.floor(this.layout.visibleTiles);
-        const nVisible = Math.floor(this.props.layout.visibleTiles);
         return Math.min(nVisible, this.numTiles);
     }
 
@@ -135,7 +137,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
             // 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<IProps, IState> {
                 }
                 // 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<IProps, IState> {
         // 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 heightBefore = this.layout.visibleTiles;
-        const heightInTiles = this.props.layout.pixelsToTiles(refToElement.clientHeight);
+        const heightInTiles = this.layout.pixelsToTiles(refToElement.clientHeight);
-        this.props.layout.setVisibleTilesWithin(heightInTiles, this.numTiles);
+        this.layout.setVisibleTilesWithin(heightInTiles, this.numTiles);
-        if (heightBefore === this.props.layout.visibleTiles) return; // no-op
+        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<IProps, IState> {
 
     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<IProps, IState> {
     };
 
     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<IProps, IState> {
     };
 
     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<IProps, IState> {
     };
 
     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<IProps, IState> {
                     <RoomTile2
                         room={room}
                         key={`room-${room.roomId}`}
-                        showMessagePreview={this.props.layout.showPreviews}
+                        showMessagePreview={this.layout.showPreviews}
                         isMinimized={this.props.isMinimized}
                         tag={this.props.tagId}
                     />
@@ -404,7 +406,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
                             <StyledMenuItemCheckbox
                                 onClose={this.onCloseMenu}
                                 onChange={this.onMessagePreviewChanged}
-                                checked={this.props.layout.showPreviews}
+                                checked={this.layout.showPreviews}
                             >
                                 {_t("Message preview")}
                             </StyledMenuItemCheckbox>
@@ -496,7 +498,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
 
                     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<IProps, IState> {
                                     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<IProps, IState> {
 
         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<IProps, IState> {
                         {showMoreText}
                     </RovingAccessibleButton>
                 );
-            } 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 = (
                     <span className='mx_RoomSublist2_showNButtonText'>

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<TagID, ListLayout>();
+
+    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;

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<ActionPayload> {
     private algorithm = new Algorithm();
     private filterConditions: IFilterCondition[] = [];
     private tagWatcher = new TagWatcher(this);
+    private layoutMap: Map<TagID, ListLayout> = new Map<TagID, ListLayout>();
 
     private readonly watchedSettings = [
         'feature_custom_tags',
@@ -416,6 +418,8 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
         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<ActionPayload> {
         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);