From 6804647dda850b7454bf48ba8214f80617c5887d Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 23 Jul 2018 17:11:53 +0100 Subject: [PATCH 1/5] Destroy widget when its permission is revoked --- src/components/views/elements/AppTile.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 534a490a32..3b8105a0fe 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -161,14 +161,18 @@ export default class AppTile extends React.Component { // if it's not remaining on screen, get rid of the PersistedElement container if (!ActiveWidgetStore.getWidgetPersistence(this.props.id)) { // FIXME: ActiveWidgetStore should probably worry about this? - const PersistedElement = sdk.getComponent("elements.PersistedElement"); - PersistedElement.destroyElement(this._persistKey); - ActiveWidgetStore.delWidgetMessaging(this.props.id); - ActiveWidgetStore.delWidgetCapabilities(this.props.id); - ActiveWidgetStore.delRoomId(this.props.id); + this._destroyPersistentWidget(); } } + _destroyPersistentWidget() { + const PersistedElement = sdk.getComponent("elements.PersistedElement"); + PersistedElement.destroyElement(this._persistKey); + ActiveWidgetStore.delWidgetMessaging(this.props.id); + ActiveWidgetStore.delWidgetCapabilities(this.props.id); + ActiveWidgetStore.delRoomId(this.props.id); + } + /** * Adds a scalar token to the widget URL, if required * Component initialisation is only complete when this function has resolved @@ -439,6 +443,10 @@ export default class AppTile extends React.Component { console.warn('Revoking permission to load widget - ', this.state.widgetUrl); localStorage.removeItem(this.state.widgetPermissionId); this.setState({hasPermissionToLoad: false}); + + // Force the widget to be non-persistent + ActiveWidgetStore.setWidgetPersistence(this.props.id, false); + this._destroyPersistentWidget(); } formatAppTileName() { From ec4c7ffb71605d29ba4f3fbee77257241b39d7d1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Jul 2018 16:21:43 +0100 Subject: [PATCH 2/5] Make ActiveWidgetStore clear persistent widgets ActiveWidgetStore is now reponsible for removing the current persistent widget from the store if it's been removed from whatever room it was in. As per comment, this leaves us with the store updating itself in this case but in all other cases, views call setters on the store to update its state. We should make it so the store keeps itself up to date and views aren't responsible for keeping the store up to date. The store now emits events so it can notify PersistentApp when it changes. Fixes https://github.com/vector-im/riot-web/issues/7076 --- src/Lifecycle.js | 3 ++ .../views/elements/PersistentApp.js | 13 ++++- src/stores/ActiveWidgetStore.js | 47 ++++++++++++++++++- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/Lifecycle.js b/src/Lifecycle.js index 7378e982ef..a618cbeadf 100644 --- a/src/Lifecycle.js +++ b/src/Lifecycle.js @@ -30,6 +30,7 @@ import DMRoomMap from './utils/DMRoomMap'; import RtsClient from './RtsClient'; import Modal from './Modal'; import sdk from './index'; +import ActiveWidgetStore from './stores/ActiveWidgetStore'; /** * Called at startup, to attempt to build a logged-in Matrix session. It tries @@ -436,6 +437,7 @@ async function startMatrixClient() { UserActivity.start(); Presence.start(); DMRoomMap.makeShared().start(); + ActiveWidgetStore.start(); await MatrixClientPeg.start(); @@ -488,6 +490,7 @@ export function stopMatrixClient() { Notifier.stop(); UserActivity.stop(); Presence.stop(); + ActiveWidgetStore.start(); if (DMRoomMap.shared()) DMRoomMap.shared().stop(); const cli = MatrixClientPeg.get(); if (cli) { diff --git a/src/components/views/elements/PersistentApp.js b/src/components/views/elements/PersistentApp.js index bbb3b2a9c8..facf5d1179 100644 --- a/src/components/views/elements/PersistentApp.js +++ b/src/components/views/elements/PersistentApp.js @@ -27,17 +27,20 @@ module.exports = React.createClass({ getInitialState: function() { return { roomId: RoomViewStore.getRoomId(), + persistentWidgetId: ActiveWidgetStore.getPersistentWidgetId(), }; }, componentWillMount: function() { this._roomStoreToken = RoomViewStore.addListener(this._onRoomViewStoreUpdate); + ActiveWidgetStore.on('update', this._onActiveWidgetStoreUpdate); }, componentWillUnmount: function() { if (this._roomStoreToken) { this._roomStoreToken.remove(); } + ActiveWidgetStore.removeListener('update', this._onActiveWidgetStoreUpdate); }, _onRoomViewStoreUpdate: function(payload) { @@ -47,9 +50,15 @@ module.exports = React.createClass({ }); }, + _onActiveWidgetStoreUpdate: function() { + this.setState({ + persistentWidgetId: ActiveWidgetStore.getPersistentWidgetId(), + }); + }, + render: function() { - if (ActiveWidgetStore.getPersistentWidgetId()) { - const persistentWidgetInRoomId = ActiveWidgetStore.getRoomId(ActiveWidgetStore.getPersistentWidgetId()); + if (this.state.persistentWidgetId) { + const persistentWidgetInRoomId = ActiveWidgetStore.getRoomId(this.state.persistentWidgetId); if (this.state.roomId !== persistentWidgetInRoomId) { const persistentWidgetInRoom = MatrixClientPeg.get().getRoom(persistentWidgetInRoomId); // get the widget data diff --git a/src/stores/ActiveWidgetStore.js b/src/stores/ActiveWidgetStore.js index 01d5f15601..e1b1efaace 100644 --- a/src/stores/ActiveWidgetStore.js +++ b/src/stores/ActiveWidgetStore.js @@ -14,14 +14,20 @@ See the License for the specific language governing permissions and limitations under the License. */ +import EventEmitter from 'events'; + +import MatrixClientPeg from '../MatrixClientPeg'; +import sdk from '../index'; + /** * Stores information about the widgets active in the app right now: * * What widget is set to remain always-on-screen, if any * Only one widget may be 'always on screen' at any one time. * * Negotiated capabilities for active apps */ -class ActiveWidgetStore { +class ActiveWidgetStore extends EventEmitter { constructor() { + super(); this._persistentWidgetId = null; // A list of negotiated capabilities for each widget, by ID @@ -35,6 +41,38 @@ class ActiveWidgetStore { // What room ID each widget is associated with (if it's a room widget) this._roomIdByWidgetId = {}; + + this.onRoomStateEvents = this.onRoomStateEvents.bind(this); + + this.dispatcherRef = null; + } + + start() { + MatrixClientPeg.get().on('RoomState.events', this.onRoomStateEvents); + } + + stop() { + MatrixClientPeg.get().removeListener('RoomState.events', this.onRoomStateEvents); + this._capsByWidgetId = {}; + this._widgetMessagingByWidgetId = {}; + this._roomIdByWidgetId = {}; + } + + onRoomStateEvents(ev, state) { + // XXX: This listens for state events in order to remove the active widget. + // Everything else relies on views listening for events and calling setters + // on this class which is terrible. This store should just listen for events + // and keep itself up to date. + if (ev.getType() !== 'im.vector.modular.widgets') return; + + if (ev.getStateKey() === this._persistentWidgetId) { + const PersistedElement = sdk.getComponent("elements.PersistedElement"); + PersistedElement.destroyElement('widget_' + ev.getStateKey()); + this.setWidgetPersistence(ev.getStateKey(), false); + this.delWidgetMessaging(ev.getStateKey()); + this.delWidgetCapabilities(ev.getStateKey()); + this.delRoomId(ev.getStateKey()); + } } setWidgetPersistence(widgetId, val) { @@ -43,6 +81,7 @@ class ActiveWidgetStore { } else if (this._persistentWidgetId !== widgetId && val) { this._persistentWidgetId = widgetId; } + this.emit('update'); } getWidgetPersistence(widgetId) { @@ -55,6 +94,7 @@ class ActiveWidgetStore { setWidgetCapabilities(widgetId, caps) { this._capsByWidgetId[widgetId] = caps; + this.emit('update'); } widgetHasCapability(widgetId, cap) { @@ -63,10 +103,12 @@ class ActiveWidgetStore { delWidgetCapabilities(widgetId) { delete this._capsByWidgetId[widgetId]; + this.emit('update'); } setWidgetMessaging(widgetId, wm) { this._widgetMessagingByWidgetId[widgetId] = wm; + this.emit('update'); } getWidgetMessaging(widgetId) { @@ -81,6 +123,7 @@ class ActiveWidgetStore { console.error('Failed to stop listening for widgetMessaging events', e.message); } delete this._widgetMessagingByWidgetId[widgetId]; + this.emit('update'); } } @@ -90,10 +133,12 @@ class ActiveWidgetStore { setRoomId(widgetId, roomId) { this._roomIdByWidgetId[widgetId] = roomId; + this.emit('update'); } delRoomId(widgetId) { delete this._roomIdByWidgetId[widgetId]; + this.emit('update'); } } From acc767a479fe9c3cae4ac198f1ec052cde662f67 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Jul 2018 16:39:30 +0100 Subject: [PATCH 3/5] s/start/stop/ --- src/Lifecycle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lifecycle.js b/src/Lifecycle.js index a618cbeadf..f32f105889 100644 --- a/src/Lifecycle.js +++ b/src/Lifecycle.js @@ -490,7 +490,7 @@ export function stopMatrixClient() { Notifier.stop(); UserActivity.stop(); Presence.stop(); - ActiveWidgetStore.start(); + ActiveWidgetStore.stop(); if (DMRoomMap.shared()) DMRoomMap.shared().stop(); const cli = MatrixClientPeg.get(); if (cli) { From 7044410a138047b93d83223152ee91911388fe4d Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Jul 2018 16:50:34 +0100 Subject: [PATCH 4/5] Move destroyPersistentWidget to store --- src/components/views/elements/AppTile.js | 14 ++------------ src/stores/ActiveWidgetStore.js | 18 ++++++++++++------ src/stores/LifecycleStore.js | 1 + 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 3b8105a0fe..bd88327b7f 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -160,19 +160,10 @@ export default class AppTile extends React.Component { // if it's not remaining on screen, get rid of the PersistedElement container if (!ActiveWidgetStore.getWidgetPersistence(this.props.id)) { - // FIXME: ActiveWidgetStore should probably worry about this? - this._destroyPersistentWidget(); + ActiveWidgetStore.destroyPersistentWidget(); } } - _destroyPersistentWidget() { - const PersistedElement = sdk.getComponent("elements.PersistedElement"); - PersistedElement.destroyElement(this._persistKey); - ActiveWidgetStore.delWidgetMessaging(this.props.id); - ActiveWidgetStore.delWidgetCapabilities(this.props.id); - ActiveWidgetStore.delRoomId(this.props.id); - } - /** * Adds a scalar token to the widget URL, if required * Component initialisation is only complete when this function has resolved @@ -445,8 +436,7 @@ export default class AppTile extends React.Component { this.setState({hasPermissionToLoad: false}); // Force the widget to be non-persistent - ActiveWidgetStore.setWidgetPersistence(this.props.id, false); - this._destroyPersistentWidget(); + ActiveWidgetStore.destroyPersistentWidget(); } formatAppTileName() { diff --git a/src/stores/ActiveWidgetStore.js b/src/stores/ActiveWidgetStore.js index e1b1efaace..53c17d0ee6 100644 --- a/src/stores/ActiveWidgetStore.js +++ b/src/stores/ActiveWidgetStore.js @@ -66,15 +66,21 @@ class ActiveWidgetStore extends EventEmitter { if (ev.getType() !== 'im.vector.modular.widgets') return; if (ev.getStateKey() === this._persistentWidgetId) { - const PersistedElement = sdk.getComponent("elements.PersistedElement"); - PersistedElement.destroyElement('widget_' + ev.getStateKey()); - this.setWidgetPersistence(ev.getStateKey(), false); - this.delWidgetMessaging(ev.getStateKey()); - this.delWidgetCapabilities(ev.getStateKey()); - this.delRoomId(ev.getStateKey()); + this.destroyPersistentWidget(); } } + destroyPersistentWidget() { + const toDeleteId = this._persistentWidgetId; + + const PersistedElement = sdk.getComponent("elements.PersistedElement"); + PersistedElement.destroyElement('widget_' + toDeleteId); + this.setWidgetPersistence(toDeleteId, false); + this.delWidgetMessaging(toDeleteId); + this.delWidgetCapabilities(toDeleteId); + this.delRoomId(toDeleteId); + } + setWidgetPersistence(widgetId, val) { if (this._persistentWidgetId === widgetId && !val) { this._persistentWidgetId = null; diff --git a/src/stores/LifecycleStore.js b/src/stores/LifecycleStore.js index 0d76f06e72..2ce3be5a33 100644 --- a/src/stores/LifecycleStore.js +++ b/src/stores/LifecycleStore.js @@ -1,5 +1,6 @@ /* Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From cdd73e6e1f318fefe82fd0fd8bf3d0180f1dbf64 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Jul 2018 16:55:45 +0100 Subject: [PATCH 5/5] Check matrix client exists when stopping --- src/stores/ActiveWidgetStore.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/stores/ActiveWidgetStore.js b/src/stores/ActiveWidgetStore.js index 53c17d0ee6..cc27febaf9 100644 --- a/src/stores/ActiveWidgetStore.js +++ b/src/stores/ActiveWidgetStore.js @@ -52,7 +52,9 @@ class ActiveWidgetStore extends EventEmitter { } stop() { - MatrixClientPeg.get().removeListener('RoomState.events', this.onRoomStateEvents); + if (MatrixClientPeg.get()) { + MatrixClientPeg.get().removeListener('RoomState.events', this.onRoomStateEvents); + } this._capsByWidgetId = {}; this._widgetMessagingByWidgetId = {}; this._roomIdByWidgetId = {};