diff --git a/src/components/views/avatars/DecoratedRoomAvatar.tsx b/src/components/views/avatars/DecoratedRoomAvatar.tsx index e0ad3202b8..40ba15af33 100644 --- a/src/components/views/avatars/DecoratedRoomAvatar.tsx +++ b/src/components/views/avatars/DecoratedRoomAvatar.tsx @@ -21,8 +21,8 @@ import { TagID } from '../../../stores/room-list/models'; import RoomAvatar from "./RoomAvatar"; import RoomTileIcon from "../rooms/RoomTileIcon"; import NotificationBadge from '../rooms/NotificationBadge'; -import { INotificationState } from "../../../stores/notifications/INotificationState"; -import { TagSpecificNotificationState } from "../../../stores/notifications/TagSpecificNotificationState"; +import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; +import { NotificationState } from "../../../stores/notifications/NotificationState"; interface IProps { room: Room; @@ -33,7 +33,7 @@ interface IProps { } interface IState { - notificationState?: INotificationState; + notificationState?: NotificationState; } export default class DecoratedRoomAvatar extends React.PureComponent { @@ -42,7 +42,7 @@ export default class DecoratedRoomAvatar extends React.PureComponent= NotificationColor.Red; - const hasCount = notification.color >= NotificationColor.Grey; const hasAnySymbol = notification.symbol || notification.count > 0; - let isEmptyBadge = !hasAnySymbol || !hasCount; + let isEmptyBadge = !hasAnySymbol || !notification.hasUnreadCount; if (forceCount) { isEmptyBadge = false; - if (!hasCount) return null; // Can't render a badge + if (!notification.hasUnreadCount) return null; // Can't render a badge } let symbol = notification.symbol || formatMinimalBadgeCount(notification.count); @@ -117,8 +114,8 @@ export default class NotificationBadge extends React.PureComponent 0 && symbol.length < 3, 'mx_NotificationBadge_3char': symbol.length > 2, diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index 348c424927..3a3ae3707e 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -37,9 +37,9 @@ import GroupAvatar from "../avatars/GroupAvatar"; import TemporaryTile from "./TemporaryTile"; import { StaticNotificationState } from "../../../stores/notifications/StaticNotificationState"; import { NotificationColor } from "../../../stores/notifications/NotificationColor"; -import { TagSpecificNotificationState } from "../../../stores/notifications/TagSpecificNotificationState"; import { Action } from "../../../dispatcher/actions"; import { ViewRoomDeltaPayload } from "../../../dispatcher/payloads/ViewRoomDeltaPayload"; +import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; // 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 @@ -201,14 +201,11 @@ export default class RoomList2 extends React.Component { let listRooms = lists[t]; if (unread) { - // TODO Be smarter and not spin up a bunch of wasted listeners just to kill them 4 lines later - // https://github.com/vector-im/riot-web/issues/14035 - const notificationStates = rooms.map(r => new TagSpecificNotificationState(r, t)); // filter to only notification rooms (and our current active room so we can index properly) - listRooms = notificationStates.filter(state => { - return state.room.roomId === roomId || state.color >= NotificationColor.Bold; + listRooms = listRooms.filter(r => { + const state = RoomNotificationStateStore.instance.getRoomState(r, t); + return state.room.roomId === roomId || state.isUnread; }); - notificationStates.forEach(state => state.destroy()); } rooms.push(...listRooms); @@ -293,7 +290,6 @@ export default class RoomList2 extends React.Component { label={_t(aesthetics.sectionLabel)} onAddRoom={onAddRoomFn} addRoomLabel={aesthetics.addRoomLabel} - isInvite={aesthetics.isInvite} 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 3eadc9a313..73aa97b6e8 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 { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -74,7 +75,6 @@ interface IProps { label: string; onAddRoom?: () => void; addRoomLabel: string; - isInvite: boolean; isMinimized: boolean; tagId: TagID; onResize: () => void; @@ -106,7 +106,7 @@ export default class RoomSublist2 extends React.Component { this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId); this.state = { - notificationState: new ListNotificationState(this.props.isInvite, this.props.tagId), + notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId), contextMenuPosition: null, isResizing: false, }; diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 4ecd6bb1ff..db8084baa2 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -46,15 +46,14 @@ import { MUTE, } from "../../../RoomNotifs"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; -import { TagSpecificNotificationState } from "../../../stores/notifications/TagSpecificNotificationState"; -import { INotificationState } from "../../../stores/notifications/INotificationState"; import NotificationBadge from "./NotificationBadge"; -import { NotificationColor } from "../../../stores/notifications/NotificationColor"; import { Volume } from "../../../RoomNotifsTypes"; import RoomListStore from "../../../stores/room-list/RoomListStore2"; import RoomListActions from "../../../actions/RoomListActions"; import defaultDispatcher from "../../../dispatcher/dispatcher"; import {ActionPayload} from "../../../dispatcher/payloads"; +import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; +import { NotificationState } from "../../../stores/notifications/NotificationState"; // 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 @@ -80,7 +79,7 @@ type PartialDOMRect = Pick; interface IState { hover: boolean; - notificationState: INotificationState; + notificationState: NotificationState; selected: boolean; notificationsMenuPosition: PartialDOMRect; generalMenuPosition: PartialDOMRect; @@ -132,7 +131,7 @@ export default class RoomTile2 extends React.Component { this.state = { hover: false, - notificationState: new TagSpecificNotificationState(this.props.room, this.props.tag), + notificationState: RoomNotificationStateStore.instance.getRoomState(this.props.room, this.props.tag), selected: ActiveRoomObserver.activeRoomId === this.props.room.roomId, notificationsMenuPosition: null, generalMenuPosition: null, @@ -492,11 +491,10 @@ export default class RoomTile2 extends React.Component { } } - const notificationColor = this.state.notificationState.color; const nameClasses = classNames({ "mx_RoomTile2_name": true, "mx_RoomTile2_nameWithPreview": !!messagePreview, - "mx_RoomTile2_nameHasUnreadEvents": notificationColor >= NotificationColor.Bold, + "mx_RoomTile2_nameHasUnreadEvents": this.state.notificationState.isUnread, }); let nameContainer = ( @@ -513,15 +511,15 @@ export default class RoomTile2 extends React.Component { // The following labels are written in such a fashion to increase screen reader efficiency (speed). if (this.props.tag === DefaultTagID.Invite) { // append nothing - } else if (notificationColor >= NotificationColor.Red) { + } else if (this.state.notificationState.hasMentions) { ariaLabel += " " + _t("%(count)s unread messages including mentions.", { count: this.state.notificationState.count, }); - } else if (notificationColor >= NotificationColor.Grey) { + } else if (this.state.notificationState.hasUnreadCount) { ariaLabel += " " + _t("%(count)s unread messages.", { count: this.state.notificationState.count, }); - } else if (notificationColor >= NotificationColor.Bold) { + } else if (this.state.notificationState.isUnread) { ariaLabel += " " + _t("Unread messages."); } diff --git a/src/components/views/rooms/TemporaryTile.tsx b/src/components/views/rooms/TemporaryTile.tsx index b6c165ecda..a3ee7eb5bd 100644 --- a/src/components/views/rooms/TemporaryTile.tsx +++ b/src/components/views/rooms/TemporaryTile.tsx @@ -18,16 +18,15 @@ import React from "react"; import classNames from "classnames"; import { RovingTabIndexWrapper } from "../../../accessibility/RovingTabIndex"; import AccessibleButton from "../../views/elements/AccessibleButton"; -import { INotificationState } from "../../../stores/notifications/INotificationState"; import NotificationBadge from "./NotificationBadge"; -import { NotificationColor } from "../../../stores/notifications/NotificationColor"; +import { NotificationState } from "../../../stores/notifications/NotificationState"; interface IProps { isMinimized: boolean; isSelected: boolean; displayName: string; avatar: React.ReactElement; - notificationState: INotificationState; + notificationState: NotificationState; onClick: () => void; } @@ -74,7 +73,7 @@ export default class TemporaryTile extends React.Component { const nameClasses = classNames({ "mx_RoomTile2_name": true, - "mx_RoomTile2_nameHasUnreadEvents": this.props.notificationState.color >= NotificationColor.Bold, + "mx_RoomTile2_nameHasUnreadEvents": this.props.notificationState.isUnread, }); let nameContainer = ( diff --git a/src/stores/notifications/INotificationState.ts b/src/stores/notifications/INotificationState.ts deleted file mode 100644 index 65bd7b7957..0000000000 --- a/src/stores/notifications/INotificationState.ts +++ /dev/null @@ -1,26 +0,0 @@ -/* -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 { EventEmitter } from "events"; -import { NotificationColor } from "./NotificationColor"; - -export const NOTIFICATION_STATE_UPDATE = "update"; - -export interface INotificationState extends EventEmitter { - symbol?: string; - count: number; - color: NotificationColor; -} diff --git a/src/stores/notifications/ListNotificationState.ts b/src/stores/notifications/ListNotificationState.ts index 5773693b47..6c5f6fc6dd 100644 --- a/src/stores/notifications/ListNotificationState.ts +++ b/src/stores/notifications/ListNotificationState.ts @@ -14,23 +14,20 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventEmitter } from "events"; -import { INotificationState, NOTIFICATION_STATE_UPDATE } from "./INotificationState"; import { NotificationColor } from "./NotificationColor"; -import { IDestroyable } from "../../utils/IDestroyable"; import { TagID } from "../room-list/models"; import { Room } from "matrix-js-sdk/src/models/room"; import { arrayDiff } from "../../utils/arrays"; import { RoomNotificationState } from "./RoomNotificationState"; -import { TagSpecificNotificationState } from "./TagSpecificNotificationState"; +import { NOTIFICATION_STATE_UPDATE, NotificationState } from "./NotificationState"; -export class ListNotificationState extends EventEmitter implements IDestroyable, INotificationState { - private _count: number; - private _color: NotificationColor; +export type FetchRoomFn = (room: Room) => RoomNotificationState; + +export class ListNotificationState extends NotificationState { private rooms: Room[] = []; private states: { [roomId: string]: RoomNotificationState } = {}; - constructor(private byTileCount = false, private tagId: TagID) { + constructor(private byTileCount = false, private tagId: TagID, private getRoomFn: FetchRoomFn) { super(); } @@ -38,14 +35,6 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, return null; // This notification state doesn't support symbols } - public get count(): number { - return this._count; - } - - public get color(): NotificationColor { - return this._color; - } - public setRooms(rooms: Room[]) { // If we're only concerned about the tile count, don't bother setting up listeners. if (this.byTileCount) { @@ -62,10 +51,9 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, if (!state) continue; // We likely just didn't have a badge (race condition) delete this.states[oldRoom.roomId]; state.off(NOTIFICATION_STATE_UPDATE, this.onRoomNotificationStateUpdate); - state.destroy(); } for (const newRoom of diff.added) { - const state = new TagSpecificNotificationState(newRoom, this.tagId); + const state = this.getRoomFn(newRoom); state.on(NOTIFICATION_STATE_UPDATE, this.onRoomNotificationStateUpdate); if (this.states[newRoom.roomId]) { // "Should never happen" disclaimer. @@ -85,8 +73,9 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, } public destroy() { + super.destroy(); for (const state of Object.values(this.states)) { - state.destroy(); + state.off(NOTIFICATION_STATE_UPDATE, this.onRoomNotificationStateUpdate); } this.states = {}; } @@ -96,7 +85,7 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, }; private calculateTotalState() { - const before = {count: this.count, symbol: this.symbol, color: this.color}; + const snapshot = this.snapshot(); if (this.byTileCount) { this._color = NotificationColor.Red; @@ -111,10 +100,7 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, } // finally, publish an update if needed - const after = {count: this.count, symbol: this.symbol, color: this.color}; - if (JSON.stringify(before) !== JSON.stringify(after)) { - this.emit(NOTIFICATION_STATE_UPDATE); - } + this.emitIfUpdated(snapshot); } } diff --git a/src/stores/notifications/NotificationState.ts b/src/stores/notifications/NotificationState.ts new file mode 100644 index 0000000000..c8ef0ba859 --- /dev/null +++ b/src/stores/notifications/NotificationState.ts @@ -0,0 +1,87 @@ +/* +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 { EventEmitter } from "events"; +import { NotificationColor } from "./NotificationColor"; +import { IDestroyable } from "../../utils/IDestroyable"; + +export const NOTIFICATION_STATE_UPDATE = "update"; + +export abstract class NotificationState extends EventEmitter implements IDestroyable { + protected _symbol: string; + protected _count: number; + protected _color: NotificationColor; + + public get symbol(): string { + return this._symbol; + } + + public get count(): number { + return this._count; + } + + public get color(): NotificationColor { + return this._color; + } + + public get isIdle(): boolean { + return this.color <= NotificationColor.None; + } + + public get isUnread(): boolean { + return this.color >= NotificationColor.Bold; + } + + public get hasUnreadCount(): boolean { + return this.color >= NotificationColor.Grey && (!!this.count || !!this.symbol); + } + + public get hasMentions(): boolean { + return this.color >= NotificationColor.Red; + } + + protected emitIfUpdated(snapshot: NotificationStateSnapshot) { + if (snapshot.isDifferentFrom(this)) { + this.emit(NOTIFICATION_STATE_UPDATE); + } + } + + protected snapshot(): NotificationStateSnapshot { + return new NotificationStateSnapshot(this); + } + + public destroy(): void { + this.removeAllListeners(NOTIFICATION_STATE_UPDATE); + } +} + +export class NotificationStateSnapshot { + private readonly symbol: string; + private readonly count: number; + private readonly color: NotificationColor; + + constructor(state: NotificationState) { + this.symbol = state.symbol; + this.count = state.count; + this.color = state.color; + } + + public isDifferentFrom(other: NotificationState): boolean { + const before = {count: this.count, symbol: this.symbol, color: this.color}; + const after = {count: other.count, symbol: other.symbol, color: other.color}; + return JSON.stringify(before) !== JSON.stringify(after); + } +} diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts index 51355a2d4d..ab354c0e93 100644 --- a/src/stores/notifications/RoomNotificationState.ts +++ b/src/stores/notifications/RoomNotificationState.ts @@ -14,8 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventEmitter } from "events"; -import { INotificationState, NOTIFICATION_STATE_UPDATE } from "./INotificationState"; import { NotificationColor } from "./NotificationColor"; import { IDestroyable } from "../../utils/IDestroyable"; import { MatrixClientPeg } from "../../MatrixClientPeg"; @@ -25,12 +23,9 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { Room } from "matrix-js-sdk/src/models/room"; import * as RoomNotifs from '../../RoomNotifs'; import * as Unread from '../../Unread'; +import { NotificationState } from "./NotificationState"; -export class RoomNotificationState extends EventEmitter implements IDestroyable, INotificationState { - private _symbol: string; - private _count: number; - private _color: NotificationColor; - +export class RoomNotificationState extends NotificationState implements IDestroyable { constructor(public readonly room: Room) { super(); this.room.on("Room.receipt", this.handleReadReceipt); @@ -41,23 +36,12 @@ export class RoomNotificationState extends EventEmitter implements IDestroyable, this.updateNotificationState(); } - public get symbol(): string { - return this._symbol; - } - - public get count(): number { - return this._count; - } - - public get color(): NotificationColor { - return this._color; - } - private get roomIsInvite(): boolean { return getEffectiveMembership(this.room.getMyMembership()) === EffectiveMembership.Invite; } public destroy(): void { + super.destroy(); this.room.removeListener("Room.receipt", this.handleReadReceipt); this.room.removeListener("Room.timeline", this.handleRoomEventUpdate); this.room.removeListener("Room.redaction", this.handleRoomEventUpdate); @@ -87,7 +71,7 @@ export class RoomNotificationState extends EventEmitter implements IDestroyable, }; private updateNotificationState() { - const before = {count: this.count, symbol: this.symbol, color: this.color}; + const snapshot = this.snapshot(); if (RoomNotifs.getRoomNotifsState(this.room.roomId) === RoomNotifs.MUTE) { // When muted we suppress all notification states, even if we have context on them. @@ -136,9 +120,6 @@ export class RoomNotificationState extends EventEmitter implements IDestroyable, } // finally, publish an update if needed - const after = {count: this.count, symbol: this.symbol, color: this.color}; - if (JSON.stringify(before) !== JSON.stringify(after)) { - this.emit(NOTIFICATION_STATE_UPDATE); - } + this.emitIfUpdated(snapshot); } } diff --git a/src/stores/notifications/RoomNotificationStateStore.ts b/src/stores/notifications/RoomNotificationStateStore.ts new file mode 100644 index 0000000000..311dcdf2d6 --- /dev/null +++ b/src/stores/notifications/RoomNotificationStateStore.ts @@ -0,0 +1,101 @@ +/* +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 { ActionPayload } from "../../dispatcher/payloads"; +import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; +import defaultDispatcher from "../../dispatcher/dispatcher"; +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 { TagSpecificNotificationState } from "./TagSpecificNotificationState"; + +const INSPECIFIC_TAG = "INSPECIFIC_TAG"; +type INSPECIFIC_TAG = "INSPECIFIC_TAG"; + +interface IState {} + +export class RoomNotificationStateStore extends AsyncStoreWithClient { + private static internalInstance = new RoomNotificationStateStore(); + + private roomMap = new Map>(); + + private constructor() { + super(defaultDispatcher, {}); + } + + /** + * 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. + * @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. + + // 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, tagId); + }; + return new ListNotificationState(useTileCount, tagId, getRoomFn); + } + + /** + * Gets a copy of the notification state for a room. The consumer should not + * attempt to destroy the returned state as it may be shared with other + * consumers. + * @param room The room to get the notification state for. + * @param inTagId Optional tag ID to scope the notification state to. + * @returns The room's notification state. + */ + public getRoomState(room: Room, inTagId?: TagID): RoomNotificationState { + if (!this.roomMap.has(room)) { + this.roomMap.set(room, new Map()); + } + + const targetTag = inTagId ? inTagId : INSPECIFIC_TAG; + + const forRoomMap = this.roomMap.get(room); + if (!forRoomMap.has(targetTag)) { + if (inTagId) { + forRoomMap.set(inTagId, new TagSpecificNotificationState(room, inTagId)); + } else { + forRoomMap.set(INSPECIFIC_TAG, new RoomNotificationState(room)); + } + } + + return forRoomMap.get(targetTag); + } + + public static get instance(): RoomNotificationStateStore { + return RoomNotificationStateStore.internalInstance; + } + + protected async onNotReady(): Promise { + for (const roomMap of this.roomMap.values()) { + for (const roomState of roomMap.values()) { + roomState.destroy(); + } + } + } + + // We don't need this, but our contract says we do. + protected async onAction(payload: ActionPayload) { + return Promise.resolve(); + } +} diff --git a/src/stores/notifications/StaticNotificationState.ts b/src/stores/notifications/StaticNotificationState.ts index 51902688fe..0392ed3716 100644 --- a/src/stores/notifications/StaticNotificationState.ts +++ b/src/stores/notifications/StaticNotificationState.ts @@ -14,13 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventEmitter } from "events"; -import { INotificationState } from "./INotificationState"; import { NotificationColor } from "./NotificationColor"; +import { NotificationState } from "./NotificationState"; -export class StaticNotificationState extends EventEmitter implements INotificationState { - constructor(public symbol: string, public count: number, public color: NotificationColor) { +export class StaticNotificationState extends NotificationState { + constructor(symbol: string, count: number, color: NotificationColor) { super(); + this._symbol = symbol; + this._count = count; + this._color = color; } public static forCount(count: number, color: NotificationColor): StaticNotificationState {