From 518f6d220ee86886af03c6c85e33b41dfb4aae3f Mon Sep 17 00:00:00 2001 From: Timo K Date: Tue, 18 Jan 2022 19:21:29 +0100 Subject: [PATCH 01/10] make widgets persistent between center and top container - this also fixes a really nasty bug where the widget layout was updated far too often --- src/components/views/elements/AppTile.tsx | 52 +++++++++++++++++------ src/components/views/rooms/AppsDrawer.tsx | 2 +- src/components/views/voip/PipView.tsx | 16 ++----- src/stores/widgets/WidgetLayoutStore.ts | 39 +++++++++++++---- 4 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index 8a17cbe8122..06d2f5a9929 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -23,6 +23,7 @@ import classNames from 'classnames'; import { MatrixCapabilities } from "matrix-widget-api"; import { Room } from "matrix-js-sdk/src/models/room"; import { logger } from "matrix-js-sdk/src/logger"; +import { EventSubscription } from 'fbemitter'; import { MatrixClientPeg } from '../../../MatrixClientPeg'; import AccessibleButton from './AccessibleButton'; @@ -46,6 +47,7 @@ import { IApp } from "../../../stores/WidgetStore"; import { WidgetLayoutStore, Container } from "../../../stores/widgets/WidgetLayoutStore"; import { OwnProfileStore } from '../../../stores/OwnProfileStore'; import { UPDATE_EVENT } from '../../../stores/AsyncStore'; +import RoomViewStore from '../../../stores/RoomViewStore'; interface IProps { app: IApp; @@ -116,6 +118,7 @@ export default class AppTile extends React.Component { private persistKey: string; private sgWidget: StopGapWidget; private dispatcherRef: string; + private roomStoreToken: EventSubscription; constructor(props: IProps) { super(props); @@ -134,9 +137,6 @@ export default class AppTile extends React.Component { } this.state = this.getNewState(props); - this.watchUserReady(); - - this.allowedWidgetsWatchRef = SettingsStore.watchSetting("allowedWidgets", null, this.onAllowedWidgetsChange); } private watchUserReady = () => { @@ -162,6 +162,33 @@ export default class AppTile extends React.Component { return !!currentlyAllowedWidgets[props.app.eventId]; }; + private onWidgetLayoutChange = () => { + const room = MatrixClientPeg.get().getRoom(this.props.room.roomId); + const app = this.props.app; + const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(app.id); + const isVisibleOnScreen = WidgetLayoutStore.instance.isVisibleOnScreen(room, app.id); + if (!isVisibleOnScreen && !isActiveWidget) { + ActiveWidgetStore.instance.destroyPersistentWidget(app.id); + PersistedElement.destroyElement(this.persistKey); + if (this.sgWidget) { + this.sgWidget.stop(); + } + } + }; + + private onRoomViewStoreUpdate = () => { + if (this.props.room.roomId == RoomViewStore.getRoomId()) return; + const app = this.props.app; + const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(app.id); + if (!isActiveWidget) { + ActiveWidgetStore.instance.destroyPersistentWidget(app.id); + PersistedElement.destroyElement(this.persistKey); + if (this.sgWidget) { + this.sgWidget.stop(); + } + } + }; + /** * Set initial component state when the App wUrl (widget URL) is being updated. * Component props *must* be passed (rather than relying on this.props). @@ -217,7 +244,11 @@ export default class AppTile extends React.Component { if (this.sgWidget && this.state.hasPermissionToLoad) { this.startWidget(); } + this.watchUserReady(); + WidgetLayoutStore.instance.on(WidgetLayoutStore.emissionForRoom(this.props.room), this.onWidgetLayoutChange); + this.roomStoreToken = RoomViewStore.addListener(this.onRoomViewStoreUpdate); + this.allowedWidgetsWatchRef = SettingsStore.watchSetting("allowedWidgets", null, this.onAllowedWidgetsChange); // Widget action listeners this.dispatcherRef = dis.register(this.onAction); } @@ -226,16 +257,8 @@ export default class AppTile extends React.Component { // Widget action listeners if (this.dispatcherRef) dis.unregister(this.dispatcherRef); - // if it's not remaining on screen, get rid of the PersistedElement container - if (!ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id)) { - ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id); - PersistedElement.destroyElement(this.persistKey); - } - - if (this.sgWidget) { - this.sgWidget.stop(); - } - + WidgetLayoutStore.instance.off(WidgetLayoutStore.emissionForRoom(this.props.room), this.onWidgetLayoutChange); + this.roomStoreToken?.remove(); SettingsStore.unwatchSetting(this.allowedWidgetsWatchRef); OwnProfileStore.instance.removeListener(UPDATE_EVENT, this.onUserReady); } @@ -527,7 +550,7 @@ export default class AppTile extends React.Component { // Also wrap the PersistedElement in a div to fix the height, otherwise // AppTile's border is in the wrong place - // For persistent apps in PiP we want the zIndex to be higher then for other persistent apps (100) + // For persisted apps in PiP we want the zIndex to be higher then for other persisted apps (100) // otherwise there are issues that the PiP view is drawn UNDER another widget (Persistent app) when dragged around. const zIndexAboveOtherPersistentElements = 101; @@ -564,6 +587,7 @@ export default class AppTile extends React.Component { /> ); } + let maxMinButton; if (!this.props.hideMaximiseButton) { const widgetIsMaximised = WidgetLayoutStore.instance. diff --git a/src/components/views/rooms/AppsDrawer.tsx b/src/components/views/rooms/AppsDrawer.tsx index e6d2157f975..9981cb916b0 100644 --- a/src/components/views/rooms/AppsDrawer.tsx +++ b/src/components/views/rooms/AppsDrawer.tsx @@ -189,7 +189,7 @@ export default class AppsDrawer extends React.Component { private onAction = (action: ActionPayload): void => { const hideWidgetKey = this.props.room.roomId + '_hide_widget_drawer'; switch (action.action) { - case 'appsDrawer': + case "appsDrawer": // Note: these booleans are awkward because localstorage is fundamentally // string-based. We also do exact equality on the strings later on. if (action.show) { diff --git a/src/components/views/voip/PipView.tsx b/src/components/views/voip/PipView.tsx index a4093d063aa..7cb1abd34e2 100644 --- a/src/components/views/voip/PipView.tsx +++ b/src/components/views/voip/PipView.tsx @@ -242,9 +242,7 @@ export default class PipView extends React.Component { let userIsPartOfTheRoom = false; let fromAnotherRoom = false; - let notInRightPanel = false; - let notInCenterContainer = false; - let notInTopContainer = false; + let notVisible = false; if (wId) { const persistentWidgetInRoomId = ActiveWidgetStore.instance.getRoomId(wId); const persistentWidgetInRoom = MatrixClientPeg.get().getRoom(persistentWidgetInRoomId); @@ -254,24 +252,16 @@ export default class PipView extends React.Component { if (!persistentWidgetInRoom) return null; const wls = WidgetLayoutStore.instance; - + notVisible = !wls.isVisibleOnScreen(persistentWidgetInRoom, wId); userIsPartOfTheRoom = persistentWidgetInRoom.getMyMembership() == "join"; fromAnotherRoom = this.state.viewedRoomId !== persistentWidgetInRoomId; - - notInRightPanel = - !(RightPanelStore.instance.currentCard.phase == RightPanelPhases.Widget && - wId == RightPanelStore.instance.currentCard.state?.widgetId); - notInCenterContainer = - !wls.getContainerWidgets(persistentWidgetInRoom, Container.Center).some((app) => app.id == wId); - notInTopContainer = - !wls.getContainerWidgets(persistentWidgetInRoom, Container.Top).some(app => app.id == wId); } // The widget should only be shown as a persistent app (in a floating pip container) if it is not visible on screen // either, because we are viewing a different room OR because it is in none of the possible containers of the room view. const showWidgetInPip = (fromAnotherRoom && userIsPartOfTheRoom) || - (notInRightPanel && notInCenterContainer && notInTopContainer && userIsPartOfTheRoom); + (notVisible && userIsPartOfTheRoom); this.setState({ showWidgetInPip }); } diff --git a/src/stores/widgets/WidgetLayoutStore.ts b/src/stores/widgets/WidgetLayoutStore.ts index ba98d48feb4..02c09784aca 100644 --- a/src/stores/widgets/WidgetLayoutStore.ts +++ b/src/stores/widgets/WidgetLayoutStore.ts @@ -27,6 +27,8 @@ import { SettingLevel } from "../../settings/SettingLevel"; import { arrayFastClone } from "../../utils/arrays"; import { UPDATE_EVENT } from "../AsyncStore"; import { compare } from "../../utils/strings"; +import RightPanelStore from "../right-panel/RightPanelStore"; +import { RightPanelPhases } from "../right-panel/RightPanelStorePhases"; export const WIDGET_LAYOUT_EVENT_TYPE = "io.element.widgets.layout"; @@ -351,6 +353,22 @@ export class WidgetLayoutStore extends ReadyWatchingStore { return this.getContainerWidgets(room, container).some(w => w.id === widget.id); } + public isVisibleOnScreen(room: Room, widgetId: string) { + const wId = widgetId; + const inRightPanel = + (RightPanelStore.instance.currentCard.phase == RightPanelPhases.Widget && + wId == RightPanelStore.instance.currentCard.state?.widgetId); + const inCenterContainer = + this.getContainerWidgets(room, Container.Center).some((app) => app.id == wId); + const inTopContainer = + this.getContainerWidgets(room, Container.Top).some(app => app.id == wId); + + // The widget should only be shown as a persistent app (in a floating pip container) if it is not visible on screen + // either, because we are viewing a different room OR because it is in none of the possible containers of the room view. + const isVisibleOnScreen = (inRightPanel || inCenterContainer || inTopContainer); + return isVisibleOnScreen; + } + public canAddToContainer(room: Room, container: Container): boolean { switch (container) { case Container.Top: return this.getContainerWidgets(room, container).length < MAX_PINNED; @@ -440,7 +458,8 @@ export class WidgetLayoutStore extends ReadyWatchingStore { public moveToContainer(room: Room, widget: IApp, toContainer: Container) { const allWidgets = this.getAllWidgets(room); if (!allWidgets.some(([w]) => w.id === widget.id)) return; // invalid - // Prepare other containers (potentially move widgets to obay the following rules) + // Prepare other containers (potentially move widgets to obey the following rules) + const newLayout = {}; switch (toContainer) { case Container.Right: // new "right" widget @@ -448,24 +467,28 @@ export class WidgetLayoutStore extends ReadyWatchingStore { case Container.Center: // new "center" widget => all other widgets go into "right" for (const w of this.getContainerWidgets(room, Container.Top)) { - this.moveToContainer(room, w, Container.Right); + // skipping the update here, so that we + newLayout[w.id] = { container: Container.Right }; + // this.moveToContainer(room, w, Container.Right, true); } for (const w of this.getContainerWidgets(room, Container.Center)) { - this.moveToContainer(room, w, Container.Right); + newLayout[w.id] = { container: Container.Right }; + // this.moveToContainer(room, w, Container.Right, true); } break; case Container.Top: // new "top" widget => the center widget moves into "right" if (this.hasMaximisedWidget(room)) { - this.moveToContainer(room, this.getContainerWidgets(room, Container.Center)[0], Container.Right); + const centerWidget = this.getContainerWidgets(room, Container.Center)[0]; + newLayout[centerWidget.id] = { container: Container.Right }; } break; } - // move widgets into requested container. - this.updateUserLayout(room, { - [widget.id]: { container: toContainer }, - }); + newLayout[widget.id] = { container: toContainer }; + + // move widgets into requested containers. + this.updateUserLayout(room, newLayout); } public hasMaximisedWidget(room: Room) { From 0afc987cf99e9c9a38d60a652165a3386f9fb845 Mon Sep 17 00:00:00 2001 From: Timo K Date: Tue, 18 Jan 2022 19:28:16 +0100 Subject: [PATCH 02/10] linter --- src/components/views/voip/PipView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/voip/PipView.tsx b/src/components/views/voip/PipView.tsx index 7cb1abd34e2..d6d1e9ac9ff 100644 --- a/src/components/views/voip/PipView.tsx +++ b/src/components/views/voip/PipView.tsx @@ -30,7 +30,7 @@ import { replaceableComponent } from "../../../utils/replaceableComponent"; import PictureInPictureDragger from './PictureInPictureDragger'; import dis from '../../../dispatcher/dispatcher'; import { Action } from "../../../dispatcher/actions"; -import { Container, WidgetLayoutStore } from '../../../stores/widgets/WidgetLayoutStore'; +import { WidgetLayoutStore } from '../../../stores/widgets/WidgetLayoutStore'; import CallViewHeader from './CallView/CallViewHeader'; import ActiveWidgetStore, { ActiveWidgetStoreEvent } from '../../../stores/ActiveWidgetStore'; import { UPDATE_EVENT } from '../../../stores/AsyncStore'; From e1613c4d0e9733b77e4961680844fa3f87979720 Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 19 Jan 2022 10:41:51 +0100 Subject: [PATCH 03/10] sgWidget?.stop --- src/components/views/elements/AppTile.tsx | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index 06d2f5a9929..eb4adcbb47c 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -170,9 +170,7 @@ export default class AppTile extends React.Component { if (!isVisibleOnScreen && !isActiveWidget) { ActiveWidgetStore.instance.destroyPersistentWidget(app.id); PersistedElement.destroyElement(this.persistKey); - if (this.sgWidget) { - this.sgWidget.stop(); - } + this.sgWidget?.stop(); } }; @@ -183,9 +181,7 @@ export default class AppTile extends React.Component { if (!isActiveWidget) { ActiveWidgetStore.instance.destroyPersistentWidget(app.id); PersistedElement.destroyElement(this.persistKey); - if (this.sgWidget) { - this.sgWidget.stop(); - } + this.sgWidget?.stop(); } }; @@ -221,7 +217,7 @@ export default class AppTile extends React.Component { // Force the widget to be non-persistent (able to be deleted/forgotten) ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id); PersistedElement.destroyElement(this.persistKey); - if (this.sgWidget) this.sgWidget.stop(); + this.sgWidget?.stop(); } this.setState({ hasPermissionToLoad }); @@ -265,7 +261,7 @@ export default class AppTile extends React.Component { private resetWidget(newProps: IProps): void { if (this.sgWidget) { - this.sgWidget.stop(); + this.sgWidget?.stop(); } try { this.sgWidget = new StopGapWidget(newProps); From 5a42bb69779d2f2b6468b1ae3419c221afc44396 Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 19 Jan 2022 10:42:45 +0100 Subject: [PATCH 04/10] remove unecassary comments --- src/stores/widgets/WidgetLayoutStore.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/stores/widgets/WidgetLayoutStore.ts b/src/stores/widgets/WidgetLayoutStore.ts index 02c09784aca..71c0fccbb04 100644 --- a/src/stores/widgets/WidgetLayoutStore.ts +++ b/src/stores/widgets/WidgetLayoutStore.ts @@ -467,13 +467,10 @@ export class WidgetLayoutStore extends ReadyWatchingStore { case Container.Center: // new "center" widget => all other widgets go into "right" for (const w of this.getContainerWidgets(room, Container.Top)) { - // skipping the update here, so that we newLayout[w.id] = { container: Container.Right }; - // this.moveToContainer(room, w, Container.Right, true); } for (const w of this.getContainerWidgets(room, Container.Center)) { newLayout[w.id] = { container: Container.Right }; - // this.moveToContainer(room, w, Container.Right, true); } break; case Container.Top: From c4dab35f60a4cc557fe58d42815c296230d6894b Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 20 Jan 2022 18:08:41 +0100 Subject: [PATCH 05/10] more unecassary if's --- src/components/views/elements/AppTile.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index eb4adcbb47c..fca7cfa50d8 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -260,9 +260,7 @@ export default class AppTile extends React.Component { } private resetWidget(newProps: IProps): void { - if (this.sgWidget) { - this.sgWidget?.stop(); - } + this.sgWidget?.stop(); try { this.sgWidget = new StopGapWidget(newProps); this.sgWidget.on("preparing", this.onWidgetPreparing); @@ -284,9 +282,7 @@ export default class AppTile extends React.Component { this.iframe = ref; if (ref) { try { - if (this.sgWidget) { - this.sgWidget.start(ref); - } + this.sgWidget?.start(ref); } catch (e) { logger.error("Failed to start widget", e); } @@ -339,7 +335,7 @@ export default class AppTile extends React.Component { PersistedElement.destroyElement(this.persistKey); ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id); - if (this.sgWidget) this.sgWidget.stop({ forceDestroy: true }); + this.sgWidget?.stop({ forceDestroy: true }); } private onWidgetPreparing = (): void => { From f136cc390cc97e685b3b5f76b4e5bf696f368eb0 Mon Sep 17 00:00:00 2001 From: Timo K Date: Sat, 22 Jan 2022 00:22:48 +0100 Subject: [PATCH 06/10] rename start/stop in StopGapWidget - and add documentation --- src/components/views/elements/AppTile.tsx | 12 ++++++------ src/stores/widgets/StopGapWidget.ts | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index fca7cfa50d8..ba16f01f0dc 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -170,7 +170,7 @@ export default class AppTile extends React.Component { if (!isVisibleOnScreen && !isActiveWidget) { ActiveWidgetStore.instance.destroyPersistentWidget(app.id); PersistedElement.destroyElement(this.persistKey); - this.sgWidget?.stop(); + this.sgWidget?.stopMessaging(); } }; @@ -181,7 +181,7 @@ export default class AppTile extends React.Component { if (!isActiveWidget) { ActiveWidgetStore.instance.destroyPersistentWidget(app.id); PersistedElement.destroyElement(this.persistKey); - this.sgWidget?.stop(); + this.sgWidget?.stopMessaging(); } }; @@ -217,7 +217,7 @@ export default class AppTile extends React.Component { // Force the widget to be non-persistent (able to be deleted/forgotten) ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id); PersistedElement.destroyElement(this.persistKey); - this.sgWidget?.stop(); + this.sgWidget?.stopMessaging(); } this.setState({ hasPermissionToLoad }); @@ -260,7 +260,7 @@ export default class AppTile extends React.Component { } private resetWidget(newProps: IProps): void { - this.sgWidget?.stop(); + this.sgWidget?.stopMessaging(); try { this.sgWidget = new StopGapWidget(newProps); this.sgWidget.on("preparing", this.onWidgetPreparing); @@ -282,7 +282,7 @@ export default class AppTile extends React.Component { this.iframe = ref; if (ref) { try { - this.sgWidget?.start(ref); + this.sgWidget?.startMessaging(ref); } catch (e) { logger.error("Failed to start widget", e); } @@ -335,7 +335,7 @@ export default class AppTile extends React.Component { PersistedElement.destroyElement(this.persistKey); ActiveWidgetStore.instance.destroyPersistentWidget(this.props.app.id); - this.sgWidget?.stop({ forceDestroy: true }); + this.sgWidget?.stopMessaging({ forceDestroy: true }); } private onWidgetPreparing = (): void => { diff --git a/src/stores/widgets/StopGapWidget.ts b/src/stores/widgets/StopGapWidget.ts index 369cafab5b4..807917b2a8f 100644 --- a/src/stores/widgets/StopGapWidget.ts +++ b/src/stores/widgets/StopGapWidget.ts @@ -255,8 +255,12 @@ export class StopGapWidget extends EventEmitter { }); } }; - - public start(iframe: HTMLIFrameElement) { + /** + * This starts the messaging for the widget if it is not in the state `started` yet. + * @param iframe the iframe the widget should use + * @returns + */ + public startMessaging(iframe: HTMLIFrameElement): any { if (this.started) return; const allowedCapabilities = this.appTileProps.whitelistCapabilities || []; const driver = new StopGapWidgetDriver(allowedCapabilities, this.mockWidget, this.kind, this.roomId); @@ -407,7 +411,13 @@ export class StopGapWidget extends EventEmitter { } } - public stop(opts = { forceDestroy: false }) { + /** + * Stops the widget messaging for if it is started. Skips stopping if it is an active + * widget. + * @param opts + * @returns + */ + public stopMessaging(opts = { forceDestroy: false }) { if (!opts?.forceDestroy && ActiveWidgetStore.instance.getPersistentWidgetId() === this.mockWidget.id) { logger.log("Skipping destroy - persistent widget"); return; From 5e0187e1ada7362cfc04e880c96274c920a1e0da Mon Sep 17 00:00:00 2001 From: Timo K Date: Sat, 22 Jan 2022 00:23:11 +0100 Subject: [PATCH 07/10] remove unecassary logic --- src/components/views/rooms/AppsDrawer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/AppsDrawer.tsx b/src/components/views/rooms/AppsDrawer.tsx index 9981cb916b0..eebba4f67cc 100644 --- a/src/components/views/rooms/AppsDrawer.tsx +++ b/src/components/views/rooms/AppsDrawer.tsx @@ -279,7 +279,7 @@ export default class AppsDrawer extends React.Component { drawer = Date: Sat, 22 Jan 2022 00:32:39 +0100 Subject: [PATCH 08/10] fix emission on dispatch!! This is bad (for widgets at least) - this might break group right panel persistence but hopefully not. --- src/stores/right-panel/RightPanelStore.ts | 64 ++++++++++++----------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index 9f6903d3572..2387f78a7c8 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { EventSubscription } from 'fbemitter'; import { logger } from "matrix-js-sdk/src/logger"; import defaultDispatcher from '../../dispatcher/dispatcher'; @@ -21,7 +22,6 @@ import { pendingVerificationRequestForUser } from '../../verification'; import SettingsStore from "../../settings/SettingsStore"; import { RightPanelPhases } from "./RightPanelStorePhases"; import { ActionPayload } from "../../dispatcher/payloads"; -import { Action } from '../../dispatcher/actions'; import { SettingLevel } from "../../settings/SettingLevel"; import { UPDATE_EVENT } from '../AsyncStore'; import { ReadyWatchingStore } from '../ReadyWatchingStore'; @@ -32,8 +32,7 @@ import { IRightPanelForRoom, } from './RightPanelStoreIPanelState'; import { MatrixClientPeg } from "../../MatrixClientPeg"; -// import RoomViewStore from '../RoomViewStore'; - +import RoomViewStore from '../RoomViewStore'; const GROUP_PHASES = [ RightPanelPhases.GroupMemberList, RightPanelPhases.GroupRoomList, @@ -41,11 +40,11 @@ const GROUP_PHASES = [ RightPanelPhases.GroupMemberInfo, ]; -const MEMBER_INFO_PHASES = [ - RightPanelPhases.RoomMemberInfo, - RightPanelPhases.Room3pidMemberInfo, - RightPanelPhases.EncryptionPanel, -]; +// const MEMBER_INFO_PHASES = [ +// RightPanelPhases.RoomMemberInfo, +// RightPanelPhases.Room3pidMemberInfo, +// RightPanelPhases.EncryptionPanel, +// ]; /** * A class for tracking the state of the right panel between layouts and @@ -68,6 +67,8 @@ export default class RightPanelStore extends ReadyWatchingStore { [roomId: string]: IRightPanelForRoom; } = {}; + private roomStoreToken: EventSubscription; + private constructor() { super(defaultDispatcher); this.dispatcherRefRightPanelStore = defaultDispatcher.register(this.onDispatch); @@ -76,8 +77,9 @@ export default class RightPanelStore extends ReadyWatchingStore { protected async onReady(): Promise { this.isReady = true; // TODO RightPanelStore (will be addressed when dropping groups): This should be used instead of the onDispatch callback when groups are removed. - // RoomViewStore.on(UPDATE_EVENT, this.onRoomViewStoreUpdate); + this.roomStoreToken = RoomViewStore.addListener(this.onRoomViewStoreUpdate); MatrixClientPeg.get().on("crypto.verification.request", this.onVerificationRequestUpdate); + this.viewedRoomId = RoomViewStore.getRoomId(); this.loadCacheFromSettings(); this.emitAndUpdateSettings(); } @@ -91,8 +93,7 @@ export default class RightPanelStore extends ReadyWatchingStore { protected async onNotReady(): Promise { this.isReady = false; MatrixClientPeg.get().off("crypto.verification.request", this.onVerificationRequestUpdate); - // TODO RightPanelStore (will be addressed when dropping groups): User this instead of the dispatcher. - // RoomViewStore.off(UPDATE_EVENT, this.onRoomViewStoreUpdate); + this.roomStoreToken.remove(); } // Getters @@ -373,45 +374,46 @@ export default class RightPanelStore extends ReadyWatchingStore { }; onRoomViewStoreUpdate = () => { - // TODO: use this function instead of the onDispatch (the whole onDispatch can get removed!) as soon groups are removed - // this.viewedRoomId = RoomViewStore.getRoomId(); - // this.isViewingRoom = true; // Is viewing room will of course be removed when removing groups - // // load values from byRoomCache with the viewedRoomId. - // this.loadCacheFromSettings(); + // TODO: only use this function instead of the onDispatch (the whole onDispatch can get removed!) as soon groups are removed + this.viewedRoomId = RoomViewStore.getRoomId(); + this.isViewingRoom = true; // Is viewing room will of course be removed when removing groups + // load values from byRoomCache with the viewedRoomId. + this.loadCacheFromSettings(); + this.emitAndUpdateSettings(); }; onDispatch = (payload: ActionPayload) => { switch (payload.action) { - case 'view_group': - case Action.ViewRoom: { + case 'view_group': { if (payload.room_id === this.viewedRoomId) break; // skip this transition, probably a permalink // Put group in the same/similar view to what was open from the previously viewed room // Is contradictory to the new "per room" philosophy but it is the legacy behavior for groups. - if ((this.isViewingRoom ? Action.ViewRoom : "view_group") != payload.action) { - if (payload.action == Action.ViewRoom && MEMBER_INFO_PHASES.includes(this.currentCard?.phase)) { - // switch from group to room - this.setRightPanelCache({ phase: RightPanelPhases.RoomMemberList, state: {} }); - } else if ( - payload.action == "view_group" && - this.currentCard?.phase === RightPanelPhases.GroupMemberInfo - ) { - // switch from room to group - this.setRightPanelCache({ phase: RightPanelPhases.GroupMemberList, state: {} }); - } + + if ( + this.currentCard?.phase === RightPanelPhases.GroupMemberInfo + ) { + // switch from room to group + this.setRightPanelCache({ phase: RightPanelPhases.GroupMemberList, state: {} }); } // Update the current room here, so that all the other functions dont need to be room dependant. // The right panel store always will return the state for the current room. this.viewedRoomId = payload.room_id; - this.isViewingRoom = payload.action == Action.ViewRoom; + this.isViewingRoom = false; // load values from byRoomCache with the viewedRoomId. if (this.isReady) { // we need the client to be ready to get the events form the ids of the settings // the loading will be done in the onReady function (to catch up with the changes done here before it was ready) // all the logic in this case is not necessary anymore as soon as groups are dropped and we use: onRoomViewStoreUpdate this.loadCacheFromSettings(); - this.emitAndUpdateSettings(); + + /* + DO NOT EMIT. Emitting breaks iframe refs by triggering a render + for the room view and calling the iframe ref changed + function + */ + // this.emitAndUpdateSettings(); } break; } From 16ed299175d99f565758996a9eaa8e8d004c6c35 Mon Sep 17 00:00:00 2001 From: Timo K Date: Sat, 22 Jan 2022 00:45:44 +0100 Subject: [PATCH 09/10] remove MEMBER_INFO_PHASES --- src/stores/right-panel/RightPanelStore.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index 2387f78a7c8..2316b677720 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -40,12 +40,6 @@ const GROUP_PHASES = [ RightPanelPhases.GroupMemberInfo, ]; -// const MEMBER_INFO_PHASES = [ -// RightPanelPhases.RoomMemberInfo, -// RightPanelPhases.Room3pidMemberInfo, -// RightPanelPhases.EncryptionPanel, -// ]; - /** * A class for tracking the state of the right panel between layouts and * sessions. This state includes a history for each room. Each history element From 686a6c32bd448cb4b793170b9a36d113ca4fc74e Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Mon, 24 Jan 2022 14:41:24 +0000 Subject: [PATCH 10/10] Apply suggestions from code review --- src/stores/right-panel/RightPanelStore.ts | 9 +++------ src/stores/widgets/StopGapWidget.ts | 2 -- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index 2316b677720..3f76cbb7bbf 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -33,6 +33,7 @@ import { } from './RightPanelStoreIPanelState'; import { MatrixClientPeg } from "../../MatrixClientPeg"; import RoomViewStore from '../RoomViewStore'; + const GROUP_PHASES = [ RightPanelPhases.GroupMemberList, RightPanelPhases.GroupRoomList, @@ -70,7 +71,6 @@ export default class RightPanelStore extends ReadyWatchingStore { protected async onReady(): Promise { this.isReady = true; - // TODO RightPanelStore (will be addressed when dropping groups): This should be used instead of the onDispatch callback when groups are removed. this.roomStoreToken = RoomViewStore.addListener(this.onRoomViewStoreUpdate); MatrixClientPeg.get().on("crypto.verification.request", this.onVerificationRequestUpdate); this.viewedRoomId = RoomViewStore.getRoomId(); @@ -402,11 +402,8 @@ export default class RightPanelStore extends ReadyWatchingStore { // all the logic in this case is not necessary anymore as soon as groups are dropped and we use: onRoomViewStoreUpdate this.loadCacheFromSettings(); - /* - DO NOT EMIT. Emitting breaks iframe refs by triggering a render - for the room view and calling the iframe ref changed - function - */ + // DO NOT EMIT. Emitting breaks iframe refs by triggering a render + // for the room view and calling the iframe ref changed function // this.emitAndUpdateSettings(); } break; diff --git a/src/stores/widgets/StopGapWidget.ts b/src/stores/widgets/StopGapWidget.ts index 807917b2a8f..5d1f1412182 100644 --- a/src/stores/widgets/StopGapWidget.ts +++ b/src/stores/widgets/StopGapWidget.ts @@ -258,7 +258,6 @@ export class StopGapWidget extends EventEmitter { /** * This starts the messaging for the widget if it is not in the state `started` yet. * @param iframe the iframe the widget should use - * @returns */ public startMessaging(iframe: HTMLIFrameElement): any { if (this.started) return; @@ -415,7 +414,6 @@ export class StopGapWidget extends EventEmitter { * Stops the widget messaging for if it is started. Skips stopping if it is an active * widget. * @param opts - * @returns */ public stopMessaging(opts = { forceDestroy: false }) { if (!opts?.forceDestroy && ActiveWidgetStore.instance.getPersistentWidgetId() === this.mockWidget.id) {