diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index 77ad3fd89f..68e514fc0b 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -2,7 +2,7 @@ Copyright 2017 Vector Creations Ltd Copyright 2018 New Vector Ltd Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> -Copyright 2020 The Matrix.org Foundation C.I.C. +Copyright 2020 - 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. @@ -23,7 +23,6 @@ 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 AccessibleButton from './AccessibleButton'; import { _t } from '../../../languageHandler'; @@ -118,17 +117,40 @@ export default class AppTile extends React.Component { showLayoutButtons: true, }; + // We track a count of all "live" `AppTile`s for a given widget ID. + // For this purpose, an `AppTile` is considered live from the time it is + // constructed until it is unmounted. This is used to aid logic around when + // to tear down the widget iframe. See `componentWillUnmount` for details. + private static liveTilesById: { [key: string]: number } = {}; + + public static addLiveTile(widgetId: string): void { + const refs = this.liveTilesById[widgetId] || 0; + this.liveTilesById[widgetId] = refs + 1; + } + + public static removeLiveTile(widgetId: string): void { + const refs = this.liveTilesById[widgetId] || 0; + this.liveTilesById[widgetId] = refs - 1; + } + + public static isLive(widgetId: string): boolean { + const refs = this.liveTilesById[widgetId] || 0; + return refs > 0; + } + private contextMenuButton = createRef(); private iframe: HTMLIFrameElement; // ref to the iframe (callback style) private allowedWidgetsWatchRef: string; private persistKey: string; private sgWidget: StopGapWidget; private dispatcherRef: string; - private roomStoreToken: EventSubscription; + private unmounted: boolean; constructor(props: IProps) { super(props); + AppTile.addLiveTile(this.props.app.id); + // The key used for PersistedElement this.persistKey = getPersistKey(this.props.app.id); try { @@ -165,23 +187,6 @@ export default class AppTile extends React.Component { return !!currentlyAllowedWidgets[props.app.eventId]; }; - private onWidgetLayoutChange = () => { - const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id); - const isVisibleOnScreen = WidgetLayoutStore.instance.isVisibleOnScreen(this.props.room, this.props.app.id); - if (!isVisibleOnScreen && !isActiveWidget) { - this.endWidgetActions(); - } - }; - - private onRoomViewStoreUpdate = () => { - if (this.props.room?.roomId === RoomViewStore.getRoomId()) return; - const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id); - // Stop the widget if it's not the active (persistent) widget and it's not a user widget - if (!isActiveWidget && !this.props.userWidget) { - this.endWidgetActions(); - } - }; - private onUserLeftRoom() { const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id); if (isActiveWidget) { @@ -263,28 +268,46 @@ export default class AppTile extends React.Component { this.watchUserReady(); if (this.props.room) { - const emitEvent = WidgetLayoutStore.emissionForRoom(this.props.room); - WidgetLayoutStore.instance.on(emitEvent, this.onWidgetLayoutChange); this.context.on("Room.myMembership", this.onMyMembership); } - this.roomStoreToken = RoomViewStore.addListener(this.onRoomViewStoreUpdate); this.allowedWidgetsWatchRef = SettingsStore.watchSetting("allowedWidgets", null, this.onAllowedWidgetsChange); // Widget action listeners this.dispatcherRef = dis.register(this.onAction); } public componentWillUnmount(): void { + this.unmounted = true; + + // It might seem simplest to always tear down the widget itself here, + // and indeed that would be a bit easier to reason about... however, we + // support moving widgets between containers (e.g. top <-> center). + // During such a move, this component will unmount from the old + // container and remount in the new container. By keeping the widget + // iframe loaded across this transition, the widget doesn't notice that + // anything happened, which improves overall widget UX. During this kind + // of movement between containers, the new `AppTile` for the new + // container is constructed before the old one unmounts. By counting the + // mounted `AppTile`s for each widget, we know to only tear down the + // widget iframe when the last the `AppTile` unmounts. + AppTile.removeLiveTile(this.props.app.id); + + // We also support a separate "persistence" mode where a single widget + // can request to be "sticky" and follow you across rooms in a PIP + // container. + const isActiveWidget = ActiveWidgetStore.instance.getWidgetPersistence(this.props.app.id); + + if (!AppTile.isLive(this.props.app.id) && !isActiveWidget) { + this.endWidgetActions(); + } + // Widget action listeners if (this.dispatcherRef) dis.unregister(this.dispatcherRef); if (this.props.room) { - const emitEvent = WidgetLayoutStore.emissionForRoom(this.props.room); - WidgetLayoutStore.instance.off(emitEvent, this.onWidgetLayoutChange); this.context.off("Room.myMembership", this.onMyMembership); } - this.roomStoreToken?.remove(); SettingsStore.unwatchSetting(this.allowedWidgetsWatchRef); OwnProfileStore.instance.removeListener(UPDATE_EVENT, this.onUserReady); } @@ -319,6 +342,7 @@ export default class AppTile extends React.Component { private startWidget(): void { this.sgWidget.prepare().then(() => { + if (this.unmounted) return; this.setState({ initialising: false }); }); } @@ -333,6 +357,7 @@ export default class AppTile extends React.Component { private iframeRefChange = (ref: HTMLIFrameElement): void => { this.iframe = ref; + if (this.unmounted) return; if (ref) { this.startMessaging(); } else { @@ -668,6 +693,7 @@ export default class AppTile extends React.Component { "mx_AppTileMenuBar_iconButton_maximise": !isMaximised, }); layoutButtons.push( { "mx_AppTileMenuBar_iconButton_pin": !isPinned, }); layoutButtons.push( { private tooltipContainer: HTMLElement; - private tooltip: void | Element | Component; private parent: Element; // XXX: This is because some components (Field) are unable to `import` the Tooltip class, @@ -178,7 +177,7 @@ export default class Tooltip extends React.Component { ); // Render the tooltip manually, as we wish it not to be rendered within the parent - this.tooltip = ReactDOM.render(tooltip, this.tooltipContainer); + ReactDOM.render(tooltip, this.tooltipContainer); }; public render() { diff --git a/src/components/views/voip/PipView.tsx b/src/components/views/voip/PipView.tsx index 384f67601e..4dbe5c2111 100644 --- a/src/components/views/voip/PipView.tsx +++ b/src/components/views/voip/PipView.tsx @@ -33,10 +33,8 @@ import { Action } from "../../../dispatcher/actions"; import { WidgetLayoutStore } from '../../../stores/widgets/WidgetLayoutStore'; import CallViewHeader from './CallView/CallViewHeader'; import ActiveWidgetStore, { ActiveWidgetStoreEvent } from '../../../stores/ActiveWidgetStore'; -import { UPDATE_EVENT } from '../../../stores/AsyncStore'; -import { RightPanelPhases } from '../../../stores/right-panel/RightPanelStorePhases'; -import RightPanelStore from '../../../stores/right-panel/RightPanelStore'; import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload"; +import AppTile from '../elements/AppTile'; const SHOW_CALL_IN_STATES = [ CallState.Connected, @@ -63,7 +61,6 @@ interface IState { // widget candidate to be displayed in the pip view. persistentWidgetId: string; showWidgetInPip: boolean; - rightPanelPhase: RightPanelPhases; moving: boolean; } @@ -125,7 +122,6 @@ export default class PipView extends React.Component { primaryCall: primaryCall, secondaryCall: secondaryCalls[0], persistentWidgetId: ActiveWidgetStore.instance.getPersistentWidgetId(), - rightPanelPhase: RightPanelStore.instance.currentCard.phase, showWidgetInPip: false, }; } @@ -139,7 +135,6 @@ export default class PipView extends React.Component { if (room) { WidgetLayoutStore.instance.on(WidgetLayoutStore.emissionForRoom(room), this.updateCalls); } - RightPanelStore.instance.on(UPDATE_EVENT, this.onRightPanelStoreUpdate); ActiveWidgetStore.instance.on(ActiveWidgetStoreEvent.Update, this.onActiveWidgetStoreUpdate); document.addEventListener("mouseup", this.onEndMoving.bind(this)); } @@ -154,7 +149,6 @@ export default class PipView extends React.Component { if (room) { WidgetLayoutStore.instance.off(WidgetLayoutStore.emissionForRoom(room), this.updateCalls); } - RightPanelStore.instance.off(UPDATE_EVENT, this.onRightPanelStoreUpdate); ActiveWidgetStore.instance.off(ActiveWidgetStoreEvent.Update, this.onActiveWidgetStoreUpdate); document.removeEventListener("mouseup", this.onEndMoving.bind(this)); } @@ -192,13 +186,6 @@ export default class PipView extends React.Component { this.updateShowWidgetInPip(); }; - private onRightPanelStoreUpdate = () => { - this.setState({ - rightPanelPhase: RightPanelStore.instance.currentCard.phase, - }); - this.updateShowWidgetInPip(); - }; - private onActiveWidgetStoreUpdate = (): void => { this.updateShowWidgetInPip(ActiveWidgetStore.instance.getPersistentWidgetId()); }; @@ -247,14 +234,15 @@ export default class PipView extends React.Component { // Sanity check the room - the widget may have been destroyed between render cycles, and // thus no room is associated anymore. if (persistentWidgetInRoom) { - const wls = WidgetLayoutStore.instance; - notVisible = !wls.isVisibleOnScreen(persistentWidgetInRoom, persistentWidgetId); + notVisible = !AppTile.isLive(persistentWidgetId); fromAnotherRoom = this.state.viewedRoomId !== persistentWidgetInRoomId; } } - // 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. + // 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 || notVisible; this.setState({ showWidgetInPip, persistentWidgetId }); diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index e00e311473..71754d2bc6 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -31,7 +31,6 @@ import { convertToStorePanel, IRightPanelForRoom, } from './RightPanelStoreIPanelState'; -import { MatrixClientPeg } from "../../MatrixClientPeg"; import RoomViewStore from '../RoomViewStore'; const GROUP_PHASES = [ @@ -71,7 +70,7 @@ export default class RightPanelStore extends ReadyWatchingStore { protected async onReady(): Promise { this.isReady = true; this.roomStoreToken = RoomViewStore.addListener(this.onRoomViewStoreUpdate); - MatrixClientPeg.get().on("crypto.verification.request", this.onVerificationRequestUpdate); + this.matrixClient.on("crypto.verification.request", this.onVerificationRequestUpdate); this.viewedRoomId = RoomViewStore.getRoomId(); this.loadCacheFromSettings(); this.emitAndUpdateSettings(); @@ -85,7 +84,7 @@ export default class RightPanelStore extends ReadyWatchingStore { protected async onNotReady(): Promise { this.isReady = false; - MatrixClientPeg.get().off("crypto.verification.request", this.onVerificationRequestUpdate); + this.matrixClient.off("crypto.verification.request", this.onVerificationRequestUpdate); this.roomStoreToken.remove(); } diff --git a/src/stores/widgets/WidgetLayoutStore.ts b/src/stores/widgets/WidgetLayoutStore.ts index 896b755648..ef0df52efc 100644 --- a/src/stores/widgets/WidgetLayoutStore.ts +++ b/src/stores/widgets/WidgetLayoutStore.ts @@ -1,5 +1,5 @@ /* - * Copyright 2021 The Matrix.org Foundation C.I.C. + * Copyright 2021 - 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. @@ -28,8 +28,6 @@ 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"; @@ -354,22 +352,6 @@ export class WidgetLayoutStore extends ReadyWatchingStore { return this.getContainerWidgets(room, container).some(w => w.id === widget.id); } - public isVisibleOnScreen(room: Optional, 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; diff --git a/test/components/structures/RightPanel-test.tsx b/test/components/structures/RightPanel-test.tsx index f2ecba7f07..aa6c526494 100644 --- a/test/components/structures/RightPanel-test.tsx +++ b/test/components/structures/RightPanel-test.tsx @@ -18,7 +18,6 @@ import React from "react"; import TestRenderer from "react-test-renderer"; import { jest } from "@jest/globals"; import { Room } from "matrix-js-sdk/src/models/room"; -import { SyncState } from "matrix-js-sdk/src/sync"; // We can't use the usual `skinned-sdk`, as it stubs out the RightPanel import "../../minimal-sdk"; @@ -34,6 +33,7 @@ import SettingsStore from "../../../src/settings/SettingsStore"; import { RightPanelPhases } from "../../../src/stores/right-panel/RightPanelStorePhases"; import RightPanelStore from "../../../src/stores/right-panel/RightPanelStore"; import { UPDATE_EVENT } from "../../../src/stores/AsyncStore"; +import { WidgetLayoutStore } from "../../../src/stores/widgets/WidgetLayoutStore"; describe("RightPanel", () => { it("renders info from only one room during room changes", async () => { @@ -72,13 +72,13 @@ describe("RightPanel", () => { return null; }); - // Wake up any stores waiting for a client - dis.dispatch({ - action: "MatrixActions.sync", - prevState: SyncState.Prepared, - state: SyncState.Syncing, - matrixClient: cli, - }); + // Wake up various stores we rely on + WidgetLayoutStore.instance.useUnitTestClient(cli); + // @ts-ignore + await WidgetLayoutStore.instance.onReady(); + RightPanelStore.instance.useUnitTestClient(cli); + // @ts-ignore + await RightPanelStore.instance.onReady(); const resizeNotifier = new ResizeNotifier(); @@ -139,7 +139,11 @@ describe("RightPanel", () => { await rendered; }); - afterAll(() => { + afterAll(async () => { + // @ts-ignore + await WidgetLayoutStore.instance.onNotReady(); + // @ts-ignore + await RightPanelStore.instance.onNotReady(); jest.restoreAllMocks(); }); }); diff --git a/test/components/views/elements/AppTile-test.tsx b/test/components/views/elements/AppTile-test.tsx new file mode 100644 index 0000000000..aef240655f --- /dev/null +++ b/test/components/views/elements/AppTile-test.tsx @@ -0,0 +1,216 @@ +/* +Copyright 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 React from "react"; +import TestRenderer from "react-test-renderer"; +import { jest } from "@jest/globals"; +import { Room } from "matrix-js-sdk/src/models/room"; +import { MatrixWidgetType } from "matrix-widget-api"; + +// We can't use the usual `skinned-sdk`, as it stubs out the RightPanel +import "../../../minimal-sdk"; +import RightPanel from "../../../../src/components/structures/RightPanel"; +import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; +import ResizeNotifier from "../../../../src/utils/ResizeNotifier"; +import { stubClient } from "../../../test-utils"; +import { Action } from "../../../../src/dispatcher/actions"; +import dis from "../../../../src/dispatcher/dispatcher"; +import DMRoomMap from "../../../../src/utils/DMRoomMap"; +import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; +import SettingsStore from "../../../../src/settings/SettingsStore"; +import { RightPanelPhases } from "../../../../src/stores/right-panel/RightPanelStorePhases"; +import RightPanelStore from "../../../../src/stores/right-panel/RightPanelStore"; +import { UPDATE_EVENT } from "../../../../src/stores/AsyncStore"; +import WidgetStore, { IApp } from "../../../../src/stores/WidgetStore"; +import AppTile from "../../../../src/components/views/elements/AppTile"; +import { Container, WidgetLayoutStore } from "../../../../src/stores/widgets/WidgetLayoutStore"; +import AppsDrawer from "../../../../src/components/views/rooms/AppsDrawer"; + +describe("AppTile", () => { + let cli; + let r1; + let r2; + const resizeNotifier = new ResizeNotifier(); + let app: IApp; + + beforeAll(async () => { + stubClient(); + cli = MatrixClientPeg.get(); + cli.hasLazyLoadMembersEnabled = () => false; + + // Init misc. startup deps + DMRoomMap.makeShared(); + + r1 = new Room("r1", cli, "@name:example.com"); + r2 = new Room("r2", cli, "@name:example.com"); + + jest.spyOn(cli, "getRoom").mockImplementation(roomId => { + if (roomId === "r1") return r1; + if (roomId === "r2") return r2; + return null; + }); + jest.spyOn(cli, "getVisibleRooms").mockImplementation(() => { + return [r1, r2]; + }); + + // Adjust various widget stores to add a mock app + app = { + id: "1", + eventId: "1", + roomId: "r1", + type: MatrixWidgetType.Custom, + url: "https://example.com", + name: "Example", + creatorUserId: cli.getUserId(), + avatar_url: null, + }; + jest.spyOn(WidgetStore.instance, "getApps").mockReturnValue([app]); + + // Wake up various stores we rely on + WidgetLayoutStore.instance.useUnitTestClient(cli); + // @ts-ignore + await WidgetLayoutStore.instance.onReady(); + RightPanelStore.instance.useUnitTestClient(cli); + // @ts-ignore + await RightPanelStore.instance.onReady(); + }); + + it("destroys non-persisted right panel widget on room change", async () => { + // Set up right panel state + const realGetValue = SettingsStore.getValue; + const mockSettings = jest.spyOn(SettingsStore, "getValue").mockImplementation((name, roomId) => { + if (name !== "RightPanel.phases") return realGetValue(name, roomId); + if (roomId === "r1") { + return { + history: [{ + phase: RightPanelPhases.Widget, + state: { + widgetId: "1", + }, + }], + isOpen: true, + }; + } + return null; + }); + + // Run initial render with room 1, and also running lifecycle methods + const renderer = TestRenderer.create( + + ); + // Wait for RPS room 1 updates to fire + const rpsUpdated = new Promise(resolve => { + const update = () => { + if ( + RightPanelStore.instance.currentCardForRoom("r1").phase !== + RightPanelPhases.Widget + ) return; + RightPanelStore.instance.off(UPDATE_EVENT, update); + resolve(); + }; + RightPanelStore.instance.on(UPDATE_EVENT, update); + }); + dis.dispatch({ + action: Action.ViewRoom, + room_id: "r1", + }); + await rpsUpdated; + + expect(AppTile.isLive("1")).toBe(true); + + // We want to verify that as we change to room 2, we should close the + // right panel and destroy the widget. + const instance = renderer.root.findByType(AppTile).instance; + const endWidgetActions = jest.spyOn(instance, "endWidgetActions"); + + console.log("Switch to room 2"); + dis.dispatch({ + action: Action.ViewRoom, + room_id: "r2", + }); + renderer.update( + + ); + + expect(endWidgetActions.mock.calls.length).toBe(1); + expect(AppTile.isLive("1")).toBe(false); + + mockSettings.mockRestore(); + }); + + it("preserves non-persisted widget on container move", async () => { + // Set up widget in top container + const realGetValue = SettingsStore.getValue; + const mockSettings = jest.spyOn(SettingsStore, "getValue").mockImplementation((name, roomId) => { + if (name !== "Widgets.layout") return realGetValue(name, roomId); + if (roomId === "r1") { + return { + widgets: { + 1: { + container: Container.Top, + }, + }, + }; + } + return null; + }); + + TestRenderer.act(() => { + WidgetLayoutStore.instance.recalculateRoom(r1); + }); + + // Run initial render with room 1, and also running lifecycle methods + const renderer = TestRenderer.create( + + ); + + expect(AppTile.isLive("1")).toBe(true); + + // We want to verify that as we move the widget to the center container, + // the widget frame remains running. + const instance = renderer.root.findByType(AppTile).instance; + const endWidgetActions = jest.spyOn(instance, "endWidgetActions"); + + console.log("Move widget to center"); + + // Stop mocking settings so that the widget move can take effect + mockSettings.mockRestore(); + TestRenderer.act(() => { + WidgetLayoutStore.instance.moveToContainer(r1, app, Container.Center); + }); + + expect(endWidgetActions.mock.calls.length).toBe(0); + expect(AppTile.isLive("1")).toBe(true); + }); + + afterAll(async () => { + // @ts-ignore + await WidgetLayoutStore.instance.onNotReady(); + // @ts-ignore + await RightPanelStore.instance.onNotReady(); + jest.restoreAllMocks(); + }); +}); diff --git a/test/test-utils.js b/test/test-utils.js index 665def5e18..46fc588967 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -56,8 +56,9 @@ export function createTestClient() { getGroups: jest.fn().mockReturnValue([]), loginFlows: jest.fn(), on: eventEmitter.on.bind(eventEmitter), - emit: eventEmitter.emit.bind(eventEmitter), + off: eventEmitter.off.bind(eventEmitter), removeListener: eventEmitter.removeListener.bind(eventEmitter), + emit: eventEmitter.emit.bind(eventEmitter), isRoomEncrypted: jest.fn().mockReturnValue(false), peekInRoom: jest.fn().mockResolvedValue(mkStubRoom()),