From dd16ec070c7446361a0d42d0f91076b13c92723e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 21 Jul 2020 20:24:16 -0600 Subject: [PATCH] Replace countRoomsWithNotif with a dedicated NotificationState Fixes https://github.com/vector-im/riot-web/issues/14694 Instead of spending 10-1000ms in a function iterating over a whole lot of room events, we can use our cached state from the Notification State Store. This commit sets up a structure that could be applied to communities in the TagPanel too, as that could probably use a similar optimization. This reduces the updateStatusIndicator() time to just 4ms on average. --- src/RoomNotifs.js | 21 ------- src/components/structures/MatrixChat.tsx | 13 ++-- .../RoomNotificationStateStore.ts | 19 ++++++ .../SummarizedNotificationState.ts | 62 +++++++++++++++++++ 4 files changed, 87 insertions(+), 28 deletions(-) create mode 100644 src/stores/notifications/SummarizedNotificationState.ts diff --git a/src/RoomNotifs.js b/src/RoomNotifs.js index 4614bef378..a86c521ac4 100644 --- a/src/RoomNotifs.js +++ b/src/RoomNotifs.js @@ -34,27 +34,6 @@ export function shouldShowMentionBadge(roomNotifState) { return MENTION_BADGE_STATES.includes(roomNotifState); } -export function countRoomsWithNotif(rooms) { - return rooms.reduce((result, room, index) => { - const roomNotifState = getRoomNotifsState(room.roomId); - const highlight = room.getUnreadNotificationCount('highlight') > 0; - const notificationCount = room.getUnreadNotificationCount(); - - const notifBadges = notificationCount > 0 && shouldShowNotifBadge(roomNotifState); - const mentionBadges = highlight && shouldShowMentionBadge(roomNotifState); - const isInvite = room.hasMembershipState(MatrixClientPeg.get().credentials.userId, 'invite'); - const badges = notifBadges || mentionBadges || isInvite; - - if (badges) { - result.count++; - if (highlight) { - result.highlight = true; - } - } - return result; - }, {count: 0, highlight: false}); -} - export function aggregateNotificationCount(rooms) { return rooms.reduce((result, room) => { const roomNotifState = getRoomNotifsState(room.roomId); diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 920b7e4dec..e68e1c53ae 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -58,7 +58,6 @@ import { messageForSyncError } from '../../utils/ErrorUtils'; import ResizeNotifier from "../../utils/ResizeNotifier"; import AutoDiscoveryUtils, { ValidatedServerConfig } from "../../utils/AutoDiscoveryUtils"; import DMRoomMap from '../../utils/DMRoomMap'; -import { countRoomsWithNotif } from '../../RoomNotifs'; import ThemeWatcher from "../../settings/watchers/ThemeWatcher"; import { FontWatcher } from '../../settings/watchers/FontWatcher'; import { storeRoomAliasInCache } from '../../RoomAliasCache'; @@ -75,6 +74,7 @@ import { import {showToast as showNotificationsToast} from "../../toasts/DesktopNotificationsToast"; import { OpenToTabPayload } from "../../dispatcher/payloads/OpenToTabPayload"; import ErrorDialog from "../views/dialogs/ErrorDialog"; +import { RoomNotificationStateStore } from "../../stores/notifications/RoomNotificationStateStore"; /** constants for MatrixChat.state.view */ export enum Views { @@ -1844,21 +1844,20 @@ export default class MatrixChat extends React.PureComponent { } updateStatusIndicator(state: string, prevState: string) { - // only count visible rooms to not torment the user with notification counts in rooms they can't see - // it will include highlights from the previous version of the room internally - const notifCount = countRoomsWithNotif(MatrixClientPeg.get().getVisibleRooms()).count; + const notificationState = RoomNotificationStateStore.instance.globalState; + const numUnreadRooms = notificationState.numUnreadStates; // we know that states === rooms here if (PlatformPeg.get()) { PlatformPeg.get().setErrorStatus(state === 'ERROR'); - PlatformPeg.get().setNotificationCount(notifCount); + PlatformPeg.get().setNotificationCount(numUnreadRooms); } this.subTitleStatus = ''; if (state === "ERROR") { this.subTitleStatus += `[${_t("Offline")}] `; } - if (notifCount > 0) { - this.subTitleStatus += `[${notifCount}]`; + if (numUnreadRooms > 0) { + this.subTitleStatus += `[${numUnreadRooms}]`; } this.setPageSubtitle(); diff --git a/src/stores/notifications/RoomNotificationStateStore.ts b/src/stores/notifications/RoomNotificationStateStore.ts index 91147b0cf4..7c6ecb7f7a 100644 --- a/src/stores/notifications/RoomNotificationStateStore.ts +++ b/src/stores/notifications/RoomNotificationStateStore.ts @@ -21,6 +21,8 @@ import { DefaultTagID, TagID } from "../room-list/models"; import { FetchRoomFn, ListNotificationState } from "./ListNotificationState"; import { Room } from "matrix-js-sdk/src/models/room"; import { RoomNotificationState } from "./RoomNotificationState"; +import { NotificationState } from "./NotificationState"; +import { SummarizedNotificationState } from "./SummarizedNotificationState"; interface IState {} @@ -33,6 +35,23 @@ export class RoomNotificationStateStore extends AsyncStoreWithClient { super(defaultDispatcher, {}); } + /** + * Gets a snapshot of notification state for all visible rooms. The number of states recorded + * on the SummarizedNotificationState is equivalent to rooms. + */ + public get globalState(): SummarizedNotificationState { + // If we're not ready yet, just return an empty state + if (!this.matrixClient) return new SummarizedNotificationState(); + + // Only count visible rooms to not torment the user with notification counts in rooms they can't see. + // This will include highlights from the previous version of the room internally + const globalState = new SummarizedNotificationState(); + for (const room of this.matrixClient.getVisibleRooms()) { + globalState.add(this.getRoomState(room)); + } + return globalState; + } + /** * 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. diff --git a/src/stores/notifications/SummarizedNotificationState.ts b/src/stores/notifications/SummarizedNotificationState.ts new file mode 100644 index 0000000000..9d5773b97e --- /dev/null +++ b/src/stores/notifications/SummarizedNotificationState.ts @@ -0,0 +1,62 @@ +/* +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 { NotificationColor } from "./NotificationColor"; +import { NotificationState } from "./NotificationState"; + +/** + * Summarizes a number of states into a unique snapshot. To populate, call + * the add() function with the notification states to be included. + * + * Useful for community notification counts, global notification counts, etc. + */ +export class SummarizedNotificationState extends NotificationState { + private _totalStatesWithUnread = 0; + + constructor() { + super(); + this._symbol = null; + this._count = 0; + this._color = NotificationColor.None; + } + + public get numUnreadStates(): number { + return this._totalStatesWithUnread; + } + + /** + * Append a notification state to this snapshot, taking the loudest NotificationColor + * of the two. By default this will not adopt the symbol of the other notification + * state to prevent the count from being lost in typical usage. + * @param other The other notification state to append. + * @param includeSymbol If true, the notification state's symbol will be taken if one + * is present. + */ + public add(other: NotificationState, includeSymbol = false) { + if (other.symbol && includeSymbol) { + this._symbol = other.symbol; + } + if (other.count) { + this._count += other.count; + } + if (other.color > this.color) { + this._color = other.color; + } + if (other.hasUnreadCount) { + this._totalStatesWithUnread++; + } + } +}