From 42498d32cc4554e2317df18cc719c1fa2d07253e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 14:36:06 -0600 Subject: [PATCH 01/10] Move community filtering into the sublist This is a step towards the room list not having to concern itself with the search parameters. --- src/components/views/rooms/RoomList.tsx | 5 +-- src/components/views/rooms/RoomSublist.tsx | 46 ++++++++++++++++++---- src/stores/room-list/RoomListStore.ts | 15 +++++++ 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index dd8b567c26c..0ff6d8d2223 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -266,12 +266,11 @@ export default class RoomList extends React.Component { } }; - private renderCommunityInvites(): React.ReactElement[] { + private renderCommunityInvites(): TemporaryTile[] { // TODO: Put community invites in a more sensible place (not in the room list) // See https://github.com/vector-im/riot-web/issues/14456 return MatrixClientPeg.get().getGroups().filter(g => { - if (g.myMembership !== 'invite') return false; - return !this.searchFilter || this.searchFilter.matches(g.name || ""); + return g.myMembership === 'invite'; }).map(g => { const avatar = ( { @@ -146,8 +148,18 @@ export default class RoomSublist extends React.Component { return padding; } + private get extraTiles(): TemporaryTile[] | null { + if (this.state.filteredExtraTiles) { + return this.state.filteredExtraTiles; + } + if (this.props.extraBadTilesThatShouldntExist) { + return this.props.extraBadTilesThatShouldntExist; + } + return null; + } + private get numTiles(): number { - return RoomSublist.calcNumTiles(this.state.rooms, this.props.extraBadTilesThatShouldntExist); + return RoomSublist.calcNumTiles(this.state.rooms, this.extraTiles); } private static calcNumTiles(rooms: Room[], extraTiles: any[]) { @@ -168,9 +180,10 @@ export default class RoomSublist extends React.Component { this.setState({isExpanded: !this.layout.isCollapsed}); } } + const prevExtraTiles = prevState.filteredExtraTiles || prevProps.extraBadTilesThatShouldntExist; // as the rooms can come in one by one we need to reevaluate // the amount of available rooms to cap the amount of requested visible rooms by the layout - if (RoomSublist.calcNumTiles(prevState.rooms, prevProps.extraBadTilesThatShouldntExist) !== this.numTiles) { + if (RoomSublist.calcNumTiles(prevState.rooms, prevExtraTiles) !== this.numTiles) { this.setState({height: this.calculateInitialHeight()}); } } @@ -191,14 +204,14 @@ export default class RoomSublist extends React.Component { // If we're supposed to handle extra tiles, take the performance hit and re-render all the // time so we don't have to consider them as part of the visible room optimization. const prevExtraTiles = this.props.extraBadTilesThatShouldntExist || []; - const nextExtraTiles = nextProps.extraBadTilesThatShouldntExist || []; + const nextExtraTiles = (nextState.filteredExtraTiles || nextProps.extraBadTilesThatShouldntExist) || []; if (prevExtraTiles.length > 0 || nextExtraTiles.length > 0) { return true; } // If we're about to update the height of the list, we don't really care about which rooms // are visible or not for no-op purposes, so ensure that the height calculation runs through. - if (RoomSublist.calcNumTiles(nextState.rooms, nextProps.extraBadTilesThatShouldntExist) !== this.numTiles) { + if (RoomSublist.calcNumTiles(nextState.rooms, nextExtraTiles) !== this.numTiles) { return true; } @@ -238,10 +251,26 @@ export default class RoomSublist extends React.Component { } private onListsUpdated = () => { + const stateUpdates: IState & any = {}; // &any is to avoid a cast on the initializer + + if (this.props.extraBadTilesThatShouldntExist) { + const nameCondition = RoomListStore.instance.getFirstNameFilterCondition(); + if (nameCondition) { + stateUpdates.filteredExtraTiles = this.props.extraBadTilesThatShouldntExist + .filter(t => nameCondition.matches(t.props.displayName || "")); + } else if (this.state.filteredExtraTiles) { + stateUpdates.filteredExtraTiles = null; + } + } + const currentRooms = this.state.rooms; const newRooms = RoomListStore.instance.orderedLists[this.props.tagId] || []; if (arrayHasOrderChange(currentRooms, newRooms)) { - this.setState({rooms: newRooms}); + stateUpdates.rooms = newRooms; + } + + if (Object.keys(stateUpdates).length > 0) { + this.setState(stateUpdates); } }; @@ -484,8 +513,9 @@ export default class RoomSublist extends React.Component { } } - if (this.props.extraBadTilesThatShouldntExist) { - tiles.push(...this.props.extraBadTilesThatShouldntExist); + if (this.extraTiles) { + // HACK: We break typing here, but this 'extra tiles' property shouldn't exist. + (tiles as any[]).push(...this.extraTiles); } // We only have to do this because of the extra tiles. We do it conditionally diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index 1f6c14ba2f8..7049381c753 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -32,6 +32,7 @@ import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; import RoomListLayoutStore from "./RoomListLayoutStore"; import { MarkedExecution } from "../../utils/MarkedExecution"; import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; +import { NameFilterCondition } from "./filters/NameFilterCondition"; interface IState { tagsEnabled?: boolean; @@ -588,6 +589,20 @@ export class RoomListStoreClass extends AsyncStoreWithClient { this.updateFn.trigger(); } + /** + * Gets the first (and ideally only) name filter condition. If one isn't present, + * this returns null. + * @returns The first name filter condition, or null if none. + */ + public getFirstNameFilterCondition(): NameFilterCondition | null { + for (const filter of this.filterConditions) { + if (filter instanceof NameFilterCondition) { + return filter; + } + } + return null; + } + /** * Gets the tags for a room identified by the store. The returned set * should never be empty, and will contain DefaultTagID.Untagged if From c6033b941065b89d1e6770ad980e036bb78b47cc Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 Jul 2020 14:42:53 -0600 Subject: [PATCH 02/10] Move search out of RoomList and LeftPanel, into RoomSearch This prevents the entire left panel from having to re-mount whenever the search query changes. --- src/components/structures/LeftPanel.tsx | 8 ------- src/components/structures/RoomSearch.tsx | 30 ++++++++++++++---------- src/components/views/rooms/RoomList.tsx | 22 ++--------------- 3 files changed, 20 insertions(+), 40 deletions(-) diff --git a/src/components/structures/LeftPanel.tsx b/src/components/structures/LeftPanel.tsx index 86136233d8e..bc17bbe23f3 100644 --- a/src/components/structures/LeftPanel.tsx +++ b/src/components/structures/LeftPanel.tsx @@ -44,7 +44,6 @@ interface IProps { } interface IState { - searchFilter: string; showBreadcrumbs: boolean; showTagPanel: boolean; } @@ -69,7 +68,6 @@ export default class LeftPanel extends React.Component { super(props); this.state = { - searchFilter: "", showBreadcrumbs: BreadcrumbsStore.instance.visible, showTagPanel: SettingsStore.getValue('TagPanel.enableTagPanel'), }; @@ -97,10 +95,6 @@ export default class LeftPanel extends React.Component { this.props.resizeNotifier.off("middlePanelResizedNoisy", this.onResize); } - private onSearch = (term: string): void => { - this.setState({searchFilter: term}); - }; - private onExplore = () => { dis.fire(Action.ViewRoomDirectory); }; @@ -366,7 +360,6 @@ export default class LeftPanel extends React.Component { onKeyDown={this.onKeyDown} > { onKeyDown={this.onKeyDown} resizeNotifier={null} collapsed={false} - searchFilter={this.state.searchFilter} onFocus={this.onFocus} onBlur={this.onBlur} isMinimized={this.props.isMinimized} diff --git a/src/components/structures/RoomSearch.tsx b/src/components/structures/RoomSearch.tsx index 1451630c976..69504e9ab89 100644 --- a/src/components/structures/RoomSearch.tsx +++ b/src/components/structures/RoomSearch.tsx @@ -24,9 +24,10 @@ import { throttle } from 'lodash'; import { Key } from "../../Keyboard"; import AccessibleButton from "../views/elements/AccessibleButton"; import { Action } from "../../dispatcher/actions"; +import RoomListStore from "../../stores/room-list/RoomListStore"; +import { NameFilterCondition } from "../../stores/room-list/filters/NameFilterCondition"; interface IProps { - onQueryUpdate: (newQuery: string) => void; isMinimized: boolean; onVerticalArrow(ev: React.KeyboardEvent): void; onEnter(ev: React.KeyboardEvent): boolean; @@ -40,6 +41,7 @@ interface IState { export default class RoomSearch extends React.PureComponent { private dispatcherRef: string; private inputRef: React.RefObject = createRef(); + private searchFilter: NameFilterCondition = new NameFilterCondition(); constructor(props: IProps) { super(props); @@ -52,6 +54,21 @@ export default class RoomSearch extends React.PureComponent { this.dispatcherRef = defaultDispatcher.register(this.onAction); } + public componentDidUpdate(prevProps: Readonly, prevState: Readonly): void { + if (prevState.query !== this.state.query) { + const hadSearch = !!this.searchFilter.search.trim(); + const haveSearch = !!this.state.query.trim(); + this.searchFilter.search = this.state.query; + if (!hadSearch && haveSearch) { + // started a new filter - add the condition + RoomListStore.instance.addFilter(this.searchFilter); + } else if (hadSearch && !haveSearch) { + // cleared a filter - remove the condition + RoomListStore.instance.removeFilter(this.searchFilter); + } // else the filter hasn't changed enough for us to care here + } + } + public componentWillUnmount() { defaultDispatcher.unregister(this.dispatcherRef); } @@ -78,19 +95,8 @@ export default class RoomSearch extends React.PureComponent { private onChange = () => { if (!this.inputRef.current) return; this.setState({query: this.inputRef.current.value}); - this.onSearchUpdated(); }; - // it wants this at the top of the file, but we know better - // tslint:disable-next-line - private onSearchUpdated = throttle( - () => { - // We can't use the state variable because it can lag behind the input. - // The lag is most obvious when deleting/clearing text with the keyboard. - this.props.onQueryUpdate(this.inputRef.current.value); - }, 200, {trailing: true, leading: true}, - ); - private onFocus = (ev: React.FocusEvent) => { this.setState({focused: true}); ev.target.select(); diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 0ff6d8d2223..762fe0a256b 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -31,7 +31,6 @@ import dis from "../../../dispatcher/dispatcher"; import defaultDispatcher from "../../../dispatcher/dispatcher"; import RoomSublist from "./RoomSublist"; import { ActionPayload } from "../../../dispatcher/payloads"; -import { NameFilterCondition } from "../../../stores/room-list/filters/NameFilterCondition"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; import GroupAvatar from "../avatars/GroupAvatar"; import TemporaryTile from "./TemporaryTile"; @@ -52,7 +51,6 @@ interface IProps { onResize: () => void; resizeNotifier: ResizeNotifier; collapsed: boolean; - searchFilter: string; isMinimized: boolean; } @@ -150,8 +148,7 @@ function customTagAesthetics(tagId: TagID): ITagAesthetics { }; } -export default class RoomList extends React.Component { - private searchFilter: NameFilterCondition = new NameFilterCondition(); +export default class RoomList extends React.PureComponent { private dispatcherRef; private customTagStoreRef; @@ -165,21 +162,6 @@ export default class RoomList extends React.Component { this.dispatcherRef = defaultDispatcher.register(this.onAction); } - public componentDidUpdate(prevProps: Readonly): void { - if (prevProps.searchFilter !== this.props.searchFilter) { - const hadSearch = !!this.searchFilter.search.trim(); - const haveSearch = !!this.props.searchFilter.trim(); - this.searchFilter.search = this.props.searchFilter; - if (!hadSearch && haveSearch) { - // started a new filter - add the condition - RoomListStore.instance.addFilter(this.searchFilter); - } else if (hadSearch && !haveSearch) { - // cleared a filter - remove the condition - RoomListStore.instance.removeFilter(this.searchFilter); - } // else the filter hasn't changed enough for us to care here - } - } - public componentDidMount(): void { RoomListStore.instance.on(LISTS_UPDATE_EVENT, this.updateLists); this.customTagStoreRef = CustomRoomTagStore.addListener(this.updateLists); @@ -339,7 +321,7 @@ export default class RoomList extends React.Component { isMinimized={this.props.isMinimized} onResize={this.props.onResize} extraBadTilesThatShouldntExist={extraTiles} - isFiltered={!!this.searchFilter.search} + isFiltered={!!RoomListStore.instance.getFirstNameFilterCondition()} /> ); } From 517c93a7d57130e9378f5eb1c260a945dbb05ba7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sun, 26 Jul 2020 14:00:01 -0600 Subject: [PATCH 03/10] Move expand-on-filter into the sublist It's a bit more complicated this way, but helps reduce some of the latency involved in remounting the entire room list. --- src/components/views/rooms/RoomList.tsx | 1 - src/components/views/rooms/RoomSublist.tsx | 22 +++++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 762fe0a256b..f4b9de93b17 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -321,7 +321,6 @@ export default class RoomList extends React.PureComponent { isMinimized={this.props.isMinimized} onResize={this.props.onResize} extraBadTilesThatShouldntExist={extraTiles} - isFiltered={!!RoomListStore.instance.getFirstNameFilterCondition()} /> ); } diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 63fb14de3ae..5d76cf7b05b 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -69,7 +69,6 @@ interface IProps { isMinimized: boolean; tagId: TagID; onResize: () => void; - isFiltered: boolean; // TODO: Don't use this. It's for community invites, and community invites shouldn't be here. // You should feel bad if you use this. @@ -102,17 +101,19 @@ export default class RoomSublist extends React.Component { private dispatcherRef: string; private layout: ListLayout; private heightAtStart: number; + private isBeingFiltered: boolean; constructor(props: IProps) { super(props); this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId); this.heightAtStart = 0; + this.isBeingFiltered = !!RoomListStore.instance.getFirstNameFilterCondition(); this.state = { notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId), contextMenuPosition: null, isResizing: false, - isExpanded: this.props.isFiltered ? this.props.isFiltered : !this.layout.isCollapsed, + isExpanded: this.isBeingFiltered ? this.isBeingFiltered : !this.layout.isCollapsed, height: 0, // to be fixed in a moment, we need `rooms` to calculate this. rooms: RoomListStore.instance.orderedLists[this.props.tagId] || [], }; @@ -173,13 +174,6 @@ export default class RoomSublist extends React.Component { public componentDidUpdate(prevProps: Readonly, prevState: Readonly) { this.state.notificationState.setRooms(this.state.rooms); - if (prevProps.isFiltered !== this.props.isFiltered) { - if (this.props.isFiltered) { - this.setState({isExpanded: true}); - } else { - this.setState({isExpanded: !this.layout.isCollapsed}); - } - } const prevExtraTiles = prevState.filteredExtraTiles || prevProps.extraBadTilesThatShouldntExist; // as the rooms can come in one by one we need to reevaluate // the amount of available rooms to cap the amount of requested visible rooms by the layout @@ -269,6 +263,16 @@ export default class RoomSublist extends React.Component { stateUpdates.rooms = newRooms; } + const isStillBeingFiltered = !!RoomListStore.instance.getFirstNameFilterCondition(); + if (isStillBeingFiltered !== this.isBeingFiltered) { + this.isBeingFiltered = isStillBeingFiltered; + if (isStillBeingFiltered) { + stateUpdates.isExpanded = true; + } else { + stateUpdates.isExpanded = !this.layout.isCollapsed; + } + } + if (Object.keys(stateUpdates).length > 0) { this.setState(stateUpdates); } From a15aae4daf304f1985701be98401a8e71d2b6d36 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sun, 26 Jul 2020 15:01:18 -0600 Subject: [PATCH 04/10] Apply a throttle to filter condition updates --- src/stores/room-list/filters/NameFilterCondition.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/stores/room-list/filters/NameFilterCondition.ts b/src/stores/room-list/filters/NameFilterCondition.ts index 6014a122f87..88edaecfb6c 100644 --- a/src/stores/room-list/filters/NameFilterCondition.ts +++ b/src/stores/room-list/filters/NameFilterCondition.ts @@ -18,6 +18,7 @@ import { Room } from "matrix-js-sdk/src/models/room"; import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "./IFilterCondition"; import { EventEmitter } from "events"; import { removeHiddenChars } from "matrix-js-sdk/src/utils"; +import { throttle } from "lodash"; /** * A filter condition for the room list which reveals rooms of a particular @@ -41,9 +42,13 @@ export class NameFilterCondition extends EventEmitter implements IFilterConditio public set search(val: string) { this._search = val; - this.emit(FILTER_CHANGED); + this.callUpdate(); } + private callUpdate = throttle(() => { + this.emit(FILTER_CHANGED); + }, 200, {trailing: true, leading: true}); + public isVisible(room: Room): boolean { const lcFilter = this.search.toLowerCase(); if (this.search[0] === '#') { From 51592ccfb69457d1aa8d5342710498b74afa067c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 27 Jul 2020 17:17:08 -0600 Subject: [PATCH 05/10] Remove watch notifications from SettingsStore It slows things down a bit, and in practice is more unlikely to be a problem than a savior. --- src/settings/SettingsStore.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/settings/SettingsStore.js b/src/settings/SettingsStore.js index dcdde46631a..488a15d003e 100644 --- a/src/settings/SettingsStore.js +++ b/src/settings/SettingsStore.js @@ -148,7 +148,6 @@ export default class SettingsStore { callbackFn(originalSettingName, changedInRoomId, atLevel, newValAtLevel, newValue); }; - console.log(`Starting watcher for ${settingName}@${roomId || ''} as ID ${watcherId}`); SettingsStore._watchers[watcherId] = localizedCallback; defaultWatchManager.watchSetting(settingName, roomId, localizedCallback); @@ -167,7 +166,6 @@ export default class SettingsStore { return; } - console.log(`Ending watcher ID ${watcherReference}`); defaultWatchManager.unwatchSetting(SettingsStore._watchers[watcherReference]); delete SettingsStore._watchers[watcherReference]; } From 5f034ee4edc645a385e6455df6bd9f065d3f7e8c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 27 Jul 2020 17:17:27 -0600 Subject: [PATCH 06/10] Ensure arrayHasDiff returns a boolean This is just maintenance noticed while debugging. --- src/utils/arrays.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/utils/arrays.ts b/src/utils/arrays.ts index 2a5b1b5f163..ea29ad182a5 100644 --- a/src/utils/arrays.ts +++ b/src/utils/arrays.ts @@ -53,6 +53,9 @@ export function arrayHasDiff(a: any[], b: any[]): boolean { // an element from the other. if (b.some(i => !a.includes(i))) return true; if (a.some(i => !b.includes(i))) return true; + + // if all the keys are common, say so + return false } else { return true; // different lengths means they are naturally diverged } From b91026fa895d4aef1627d48864fa9fd2807cc666 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 27 Jul 2020 17:18:01 -0600 Subject: [PATCH 07/10] Ensure CustomRoomTagStore doesn't fire useless updates This could in theory cause double rendering of the room list under some conditions. --- src/stores/CustomRoomTagStore.js | 8 ++++++-- src/utils/objects.ts | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/stores/CustomRoomTagStore.js b/src/stores/CustomRoomTagStore.js index 9967708c29e..1f24dc589af 100644 --- a/src/stores/CustomRoomTagStore.js +++ b/src/stores/CustomRoomTagStore.js @@ -22,6 +22,7 @@ import SettingsStore from "../settings/SettingsStore"; import RoomListStore, {LISTS_UPDATE_EVENT} from "./room-list/RoomListStore"; import {RoomNotificationStateStore} from "./notifications/RoomNotificationStateStore"; import {isCustomTag} from "./room-list/models"; +import {objectHasDiff} from "../utils/objects"; function commonPrefix(a, b) { const len = Math.min(a.length, b.length); @@ -107,7 +108,10 @@ class CustomRoomTagStore extends EventEmitter { } _onListsUpdated = () => { - this._setState({tags: this._getUpdatedTags()}); + const newTags = this._getUpdatedTags(); + if (!this._state.tags || objectHasDiff(this._state.tags, newTags)) { + this._setState({tags: newTags}); + } }; _onDispatch(payload) { @@ -134,7 +138,7 @@ class CustomRoomTagStore extends EventEmitter { _getUpdatedTags() { if (!SettingsStore.isFeatureEnabled("feature_custom_tags")) { - return; + return {}; // none } const newTagNames = Object.keys(RoomListStore.instance.orderedLists).filter(t => isCustomTag(t)).sort(); diff --git a/src/utils/objects.ts b/src/utils/objects.ts index db8248759d7..9dcc41ecd22 100644 --- a/src/utils/objects.ts +++ b/src/utils/objects.ts @@ -72,6 +72,23 @@ export function objectHasValueChange(a: any, b: any): boolean { return arrayHasDiff(aValues, bValues); } +/** + * Determines if any keys were added, removed, or changed between two objects. + * For changes, simple triple equal comparisons are done, not in-depth + * tree checking. + * @param a The first object. Must be defined. + * @param b The second object. Must be defined. + * @returns True if there's a difference between the objects, false otherwise + */ +export function objectHasDiff(a: any, b: any): boolean { + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + if (arrayHasDiff(aKeys, bKeys)) return true; + + const possibleChanges = arrayUnion(aKeys, bKeys); + return possibleChanges.some(k => a[k] !== b[k]); +} + /** * Determines the keys added, changed, and removed between two objects. * For changes, simple triple equal comparisons are done, not in-depth From 900c234434d6590c8ab25be46699cb5eb263081c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 27 Jul 2020 17:33:27 -0600 Subject: [PATCH 08/10] Internalize notification state handling for lists This reduces the update cost of rooms changing, and fixes a bug where when a sublist became filtered it would change the notification count of the sublist. This does change the expected usage of the state store to ensuring that only one place updates the rooms on the list states, which is currently the room list store. Ideally the state store could listen to the room list store to update itself, however due to a complicated require() loop it is not possible. --- src/components/views/rooms/RoomSublist.tsx | 15 ++++++--------- .../notifications/RoomNotificationStateStore.ts | 15 +++++++++------ src/stores/room-list/RoomListStore.ts | 13 ++++++++++++- src/stores/room-list/algorithms/Algorithm.ts | 4 ++++ 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 5d76cf7b05b..28b23eaa6db 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -38,7 +38,6 @@ import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import dis from "../../../dispatcher/dispatcher"; import defaultDispatcher from "../../../dispatcher/dispatcher"; import NotificationBadge from "./NotificationBadge"; -import { ListNotificationState } from "../../../stores/notifications/ListNotificationState"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import { Key } from "../../../Keyboard"; import { ActionPayload } from "../../../dispatcher/payloads"; @@ -50,6 +49,7 @@ import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; import { arrayHasOrderChange } from "../../../utils/arrays"; import { objectExcluding, objectHasValueChange } from "../../../utils/objects"; import TemporaryTile from "./TemporaryTile"; +import { NotificationState } from "../../../stores/notifications/NotificationState"; const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS @@ -86,7 +86,6 @@ interface ResizeDelta { type PartialDOMRect = Pick; interface IState { - notificationState: ListNotificationState; contextMenuPosition: PartialDOMRect; isResizing: boolean; isExpanded: boolean; // used for the for expand of the sublist when the room list is being filtered @@ -102,6 +101,7 @@ export default class RoomSublist extends React.Component { private layout: ListLayout; private heightAtStart: number; private isBeingFiltered: boolean; + private notificationState: NotificationState; constructor(props: IProps) { super(props); @@ -109,8 +109,8 @@ export default class RoomSublist extends React.Component { this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId); this.heightAtStart = 0; this.isBeingFiltered = !!RoomListStore.instance.getFirstNameFilterCondition(); + this.notificationState = RoomNotificationStateStore.instance.getListState(this.props.tagId); this.state = { - notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId), contextMenuPosition: null, isResizing: false, isExpanded: this.isBeingFiltered ? this.isBeingFiltered : !this.layout.isCollapsed, @@ -119,7 +119,6 @@ export default class RoomSublist extends React.Component { }; // Why Object.assign() and not this.state.height? Because TypeScript says no. this.state = Object.assign(this.state, {height: this.calculateInitialHeight()}); - this.state.notificationState.setRooms(this.state.rooms); this.dispatcherRef = defaultDispatcher.register(this.onAction); RoomListStore.instance.on(LISTS_UPDATE_EVENT, this.onListsUpdated); } @@ -173,7 +172,6 @@ export default class RoomSublist extends React.Component { } public componentDidUpdate(prevProps: Readonly, prevState: Readonly) { - this.state.notificationState.setRooms(this.state.rooms); const prevExtraTiles = prevState.filteredExtraTiles || prevProps.extraBadTilesThatShouldntExist; // as the rooms can come in one by one we need to reevaluate // the amount of available rooms to cap the amount of requested visible rooms by the layout @@ -239,7 +237,6 @@ export default class RoomSublist extends React.Component { } public componentWillUnmount() { - this.state.notificationState.destroy(); defaultDispatcher.unregister(this.dispatcherRef); RoomListStore.instance.off(LISTS_UPDATE_EVENT, this.onListsUpdated); } @@ -409,8 +406,8 @@ export default class RoomSublist extends React.Component { } else { // find the first room with a count of the same colour as the badge count room = this.state.rooms.find((r: Room) => { - const notifState = this.state.notificationState.getForRoom(r); - return notifState.count > 0 && notifState.color === this.state.notificationState.color; + const notifState = this.notificationState.getForRoom(r); + return notifState.count > 0 && notifState.color === this.notificationState.color; }); } @@ -626,7 +623,7 @@ export default class RoomSublist extends React.Component { const badge = ( { private static internalInstance = new RoomNotificationStateStore(); private roomMap = new Map(); + private listMap = new Map(); private constructor() { super(defaultDispatcher, {}); @@ -52,21 +53,23 @@ export class RoomNotificationStateStore extends AsyncStoreWithClient { } /** - * Creates a new list notification state. The consumer is expected to set the rooms - * on the notification state, and destroy the state when it no longer needs it. - * @param tagId The tag to create the notification state for. + * Gets an instance of the list state class for the given tag. + * @param tagId The tag to get the notification state for. * @returns The notification state for the tag. */ public getListState(tagId: TagID): ListNotificationState { - // Note: we don't cache these notification states as the consumer is expected to call - // .setRooms() on the returned object, which could confuse other consumers. + if (this.listMap.has(tagId)) { + return this.listMap.get(tagId); + } // TODO: Update if/when invites move out of the room list. const useTileCount = tagId === DefaultTagID.Invite; const getRoomFn: FetchRoomFn = (room: Room) => { return this.getRoomState(room); }; - return new ListNotificationState(useTileCount, tagId, getRoomFn); + const state = new ListNotificationState(useTileCount, tagId, getRoomFn); + this.listMap.set(tagId, state); + return state; } /** diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index 7049381c753..c3e7fd5c515 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -33,6 +33,7 @@ import RoomListLayoutStore from "./RoomListLayoutStore"; import { MarkedExecution } from "../../utils/MarkedExecution"; import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; import { NameFilterCondition } from "./filters/NameFilterCondition"; +import { RoomNotificationStateStore } from "../notifications/RoomNotificationStateStore"; interface IState { tagsEnabled?: boolean; @@ -55,7 +56,12 @@ export class RoomListStoreClass extends AsyncStoreWithClient { private algorithm = new Algorithm(); private filterConditions: IFilterCondition[] = []; private tagWatcher = new TagWatcher(this); - private updateFn = new MarkedExecution(() => this.emit(LISTS_UPDATE_EVENT)); + private updateFn = new MarkedExecution(() => { + for (const tagId of Object.keys(this.unfilteredLists)) { + RoomNotificationStateStore.instance.getListState(tagId).setRooms(this.unfilteredLists[tagId]); + } + this.emit(LISTS_UPDATE_EVENT); + }); private readonly watchedSettings = [ 'feature_custom_tags', @@ -72,6 +78,11 @@ export class RoomListStoreClass extends AsyncStoreWithClient { this.algorithm.on(FILTER_CHANGED, this.onAlgorithmFilterUpdated); } + public get unfilteredLists(): ITagMap { + if (!this.algorithm) return {}; // No tags yet. + return this.algorithm.getUnfilteredRooms(); + } + public get orderedLists(): ITagMap { if (!this.algorithm) return {}; // No tags yet. return this.algorithm.getOrderedRooms(); diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 667084d6533..9b2779d9002 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -465,6 +465,10 @@ export class Algorithm extends EventEmitter { return this.filteredRooms; } + public getUnfilteredRooms(): ITagMap { + return this._cachedStickyRooms || this.cachedRooms; + } + /** * This returns the same as getOrderedRooms(), but without the sticky room * map as it causes issues for sticky room handling (see sticky room handling From 2a8881f7539828f53d60037a82ada80930d6eeac Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 27 Jul 2020 17:41:23 -0600 Subject: [PATCH 09/10] Fix typing error --- src/components/views/rooms/RoomSublist.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 28b23eaa6db..54766c58dfa 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -49,7 +49,7 @@ import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; import { arrayHasOrderChange } from "../../../utils/arrays"; import { objectExcluding, objectHasValueChange } from "../../../utils/objects"; import TemporaryTile from "./TemporaryTile"; -import { NotificationState } from "../../../stores/notifications/NotificationState"; +import { ListNotificationState } from "../../../stores/notifications/ListNotificationState"; const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS @@ -101,7 +101,7 @@ export default class RoomSublist extends React.Component { private layout: ListLayout; private heightAtStart: number; private isBeingFiltered: boolean; - private notificationState: NotificationState; + private notificationState: ListNotificationState; constructor(props: IProps) { super(props); From 1573c88b47b79e0bc1d7579f4be84bf4d94effb8 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 28 Jul 2020 06:37:19 -0600 Subject: [PATCH 10/10] Update src/utils/arrays.ts Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- src/utils/arrays.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/arrays.ts b/src/utils/arrays.ts index ea29ad182a5..fa5515878f8 100644 --- a/src/utils/arrays.ts +++ b/src/utils/arrays.ts @@ -55,7 +55,7 @@ export function arrayHasDiff(a: any[], b: any[]): boolean { if (a.some(i => !b.includes(i))) return true; // if all the keys are common, say so - return false + return false; } else { return true; // different lengths means they are naturally diverged }