From d89fcf17fbbda69ab82021e0ee99429113ccde45 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 22 Mar 2022 23:29:02 -0600 Subject: [PATCH] Step 8.4.2: Refactor `ActiveRoomObserver` out of existence The `RoomTile` was the last class to use it. Note that we also update the RVS to change its `instance` declaration type to fix a few tests. --- src/@types/global.d.ts | 2 - src/ActiveRoomObserver.ts | 85 ------------------------- src/components/views/rooms/RoomTile.tsx | 27 ++++---- src/stores/RoomViewStore.tsx | 38 +++++++++++ 4 files changed, 50 insertions(+), 102 deletions(-) delete mode 100644 src/ActiveRoomObserver.ts diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index c3f739c731..d0f266470c 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -29,7 +29,6 @@ import RoomListLayoutStore from "../stores/room-list/RoomListLayoutStore"; import { IntegrationManagers } from "../integrations/IntegrationManagers"; import { ModalManager } from "../Modal"; import SettingsStore from "../settings/SettingsStore"; -import { ActiveRoomObserver } from "../ActiveRoomObserver"; import { Notifier } from "../Notifier"; import type { Renderer } from "react-dom"; import RightPanelStore from "../stores/right-panel/RightPanelStore"; @@ -82,7 +81,6 @@ declare global { mxDeviceListener: DeviceListener; mxRoomListStore: RoomListStoreClass; mxRoomListLayoutStore: RoomListLayoutStore; - mxActiveRoomObserver: ActiveRoomObserver; mxPlatformPeg: PlatformPeg; mxIntegrationManagers: typeof IntegrationManagers; singletonModalManager: ModalManager; diff --git a/src/ActiveRoomObserver.ts b/src/ActiveRoomObserver.ts deleted file mode 100644 index bcdc249fea..0000000000 --- a/src/ActiveRoomObserver.ts +++ /dev/null @@ -1,85 +0,0 @@ -/* -Copyright 2017 - 2022 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 { logger } from "matrix-js-sdk/src/logger"; - -import { RoomViewStore } from './stores/RoomViewStore'; - -type Listener = (isActive: boolean) => void; - -/** - * Consumes changes from the RoomViewStore and notifies specific things - * about when the active room changes. Unlike listening for RoomViewStore - * changes, you can subscribe to only changes relevant to a particular - * room. - * - * TODO: If we introduce an observer for something else, factor out - * the adding / removing of listeners & emitting into a common class. - */ -export class ActiveRoomObserver { - private listeners: {[key: string]: Listener[]} = {}; - private _activeRoomId = RoomViewStore.instance.getRoomId(); - private readonly roomStoreToken: EventSubscription; - - constructor() { - // TODO: We could self-destruct when the last listener goes away, or at least stop listening. - this.roomStoreToken = RoomViewStore.instance.addListener(this.onRoomViewStoreUpdate); - } - - public get activeRoomId(): string { - return this._activeRoomId; - } - - public addListener(roomId, listener) { - if (!this.listeners[roomId]) this.listeners[roomId] = []; - this.listeners[roomId].push(listener); - } - - public removeListener(roomId, listener) { - if (this.listeners[roomId]) { - const i = this.listeners[roomId].indexOf(listener); - if (i > -1) { - this.listeners[roomId].splice(i, 1); - } - } else { - logger.warn("Unregistering unrecognised listener (roomId=" + roomId + ")"); - } - } - - private emit(roomId, isActive: boolean) { - if (!this.listeners[roomId]) return; - - for (const l of this.listeners[roomId]) { - l.call(null, isActive); - } - } - - private onRoomViewStoreUpdate = () => { - // emit for the old room ID - if (this._activeRoomId) this.emit(this._activeRoomId, false); - - // update our cache - this._activeRoomId = RoomViewStore.instance.getRoomId(); - - // and emit for the new one - if (this._activeRoomId) this.emit(this._activeRoomId, true); - }; -} - -if (window.mxActiveRoomObserver === undefined) { - window.mxActiveRoomObserver = new ActiveRoomObserver(); -} -export default window.mxActiveRoomObserver; diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index 6fbe956361..1d04d7be75 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -28,7 +28,6 @@ import dis from '../../../dispatcher/dispatcher'; import defaultDispatcher from '../../../dispatcher/dispatcher'; import { Action } from "../../../dispatcher/actions"; import SettingsStore from "../../../settings/SettingsStore"; -import ActiveRoomObserver from "../../../ActiveRoomObserver"; import { _t } from "../../../languageHandler"; import { ChevronFace, ContextMenuTooltipButton } from "../../structures/ContextMenu"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; @@ -61,6 +60,7 @@ import PosthogTrackers from "../../../PosthogTrackers"; import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload"; import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; import { getKeyBindingsManager } from "../../../KeyBindingsManager"; +import { RoomViewStore } from "../../../stores/RoomViewStore"; enum VoiceConnectionState { Disconnected, @@ -110,7 +110,7 @@ export default class RoomTile extends React.PureComponent { super(props); this.state = { - selected: ActiveRoomObserver.activeRoomId === this.props.room.roomId, + selected: RoomViewStore.instance.getRoomId() === this.props.room.roomId, notificationsMenuPosition: null, generalMenuPosition: null, // generatePreview() will return nothing if the user has previews disabled @@ -177,7 +177,7 @@ export default class RoomTile extends React.PureComponent { } this.updateVoiceMembers(); - ActiveRoomObserver.addListener(this.props.room.roomId, this.onActiveRoomUpdate); + RoomViewStore.instance.addRoomListener(this.props.room.roomId, this.onActiveRoomUpdate); this.dispatcherRef = defaultDispatcher.register(this.onAction); MessagePreviewStore.instance.on( MessagePreviewStore.getPreviewChangedEventName(this.props.room), @@ -185,21 +185,18 @@ export default class RoomTile extends React.PureComponent { ); this.notificationState.on(NotificationStateEvents.Update, this.onNotificationUpdate); this.roomProps.on(PROPERTY_UPDATED, this.onRoomPropertyUpdate); - this.props.room?.on(RoomEvent.Name, this.onRoomNameUpdate); - this.props.room?.currentState?.on(RoomStateEvent.Events, this.updateVoiceMembers); + this.props.room.on(RoomEvent.Name, this.onRoomNameUpdate); + this.props.room.currentState.on(RoomStateEvent.Events, this.updateVoiceMembers); } public componentWillUnmount() { - if (this.props.room) { - ActiveRoomObserver.removeListener(this.props.room.roomId, this.onActiveRoomUpdate); - MessagePreviewStore.instance.off( - MessagePreviewStore.getPreviewChangedEventName(this.props.room), - this.onRoomPreviewChanged, - ); - this.props.room.currentState.off(RoomStateEvent.Events, this.updateVoiceMembers); - this.props.room.off(RoomEvent.Name, this.onRoomNameUpdate); - } - ActiveRoomObserver.removeListener(this.props.room.roomId, this.onActiveRoomUpdate); + RoomViewStore.instance.removeRoomListener(this.props.room.roomId, this.onActiveRoomUpdate); + MessagePreviewStore.instance.off( + MessagePreviewStore.getPreviewChangedEventName(this.props.room), + this.onRoomPreviewChanged, + ); + this.props.room.off(RoomEvent.Name, this.onRoomNameUpdate); + this.props.room.currentState.off(RoomStateEvent.Events, this.updateVoiceMembers); defaultDispatcher.unregister(this.dispatcherRef); this.notificationState.off(NotificationStateEvents.Update, this.onNotificationUpdate); this.roomProps.off(PROPERTY_UPDATED, this.onRoomPropertyUpdate); diff --git a/src/stores/RoomViewStore.tsx b/src/stores/RoomViewStore.tsx index 1177444d98..e8ed6a62c4 100644 --- a/src/stores/RoomViewStore.tsx +++ b/src/stores/RoomViewStore.tsx @@ -79,20 +79,52 @@ const INITIAL_STATE = { wasContextSwitch: false, }; +type Listener = (isActive: boolean) => void; + /** * A class for storing application state for RoomView. This is the RoomView's interface * with a subset of the js-sdk. * ``` */ export class RoomViewStore extends Store { + // Important: This cannot be a dynamic getter (lazily-constructed instance) because + // otherwise we'll miss view_room dispatches during startup, breaking relaunches of + // the app. We need to eagerly create the instance. public static readonly instance = new RoomViewStore(); private state = INITIAL_STATE; // initialize state + // Keep these out of state to avoid causing excessive/recursive updates + private roomIdActivityListeners: Record = {}; + public constructor() { super(dis); } + public addRoomListener(roomId: string, fn: Listener) { + if (!this.roomIdActivityListeners[roomId]) this.roomIdActivityListeners[roomId] = []; + this.roomIdActivityListeners[roomId].push(fn); + } + + public removeRoomListener(roomId: string, fn: Listener) { + if (this.roomIdActivityListeners[roomId]) { + const i = this.roomIdActivityListeners[roomId].indexOf(fn); + if (i > -1) { + this.roomIdActivityListeners[roomId].splice(i, 1); + } + } else { + logger.warn("Unregistering unrecognised listener (roomId=" + roomId + ")"); + } + } + + private emitForRoom(roomId: string, isActive: boolean) { + if (!this.roomIdActivityListeners[roomId]) return; + + for (const fn of this.roomIdActivityListeners[roomId]) { + fn.call(null, isActive); + } + } + private setState(newState: Partial) { // If values haven't changed, there's nothing to do. // This only tries a shallow comparison, so unchanged objects will slip @@ -108,7 +140,13 @@ export class RoomViewStore extends Store { return; } + const lastRoomId = this.state.roomId; this.state = Object.assign(this.state, newState); + if (lastRoomId !== this.state.roomId) { + if (lastRoomId) this.emitForRoom(lastRoomId, false); + if (this.state.roomId) this.emitForRoom(this.state.roomId, true); + } + this.__emitChange(); }