From a4f76670c4ef0efb4942c69b7499ee8a428b01f0 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 1 Apr 2019 19:47:28 -0600 Subject: [PATCH 1/4] Don't re-init the stickerpicker unless something actually changes Fixes https://github.com/vector-im/riot-web/issues/9354 https://github.com/matrix-org/matrix-react-sdk/pull/2801 introduced a change which tried to make sure that when the widget URL was changed that the picker would be re-mounted, however it accidentally introduced a regression. While it effectively did the task it wanted to, it failed to keep the previously-mounted sticker picker alive. This is because the Stickerpicker component is remounted when opened, and the _updateWidget function is called. This results in this.state not having the "current" widget, meaning a URL change is always detected when the component is remounted (room changes, open sticker picker). Instead of remounting always, we'll instead track which sticker picker widget is being used out of band. This therefore means that whenever the Stickerpicker component is mounted it doesn't create a whole new widget, and the existing (background) picker can be used. This also fixes the loading screen that people would see when opening the sticker picker after switching rooms, something which the persistent widget stuff is supposed to solve. --- src/components/views/rooms/Stickerpicker.js | 8 +++++++- src/stores/ActiveWidgetStore.js | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/Stickerpicker.js b/src/components/views/rooms/Stickerpicker.js index 9d8912954c..d9b23b0734 100644 --- a/src/components/views/rooms/Stickerpicker.js +++ b/src/components/views/rooms/Stickerpicker.js @@ -130,8 +130,13 @@ export default class Stickerpicker extends React.Component { _updateWidget() { const stickerpickerWidget = WidgetUtils.getStickerpickerWidgets()[0]; + if (!stickerpickerWidget) { + ActiveWidgetStore.delStickerPickerWidget(); + this.setState({stickerpickerWidget: null, widgetId: null}); + return; + } - const currentWidget = this.state.stickerpickerWidget; + const currentWidget = ActiveWidgetStore.getStickerPickerWidget(); let currentUrl = null; if (currentWidget && currentWidget.content && currentWidget.content.url) { currentUrl = currentWidget.content.url; @@ -147,6 +152,7 @@ export default class Stickerpicker extends React.Component { PersistedElement.destroyElement(PERSISTED_ELEMENT_KEY); } + ActiveWidgetStore.setStickerPickerWidget(stickerpickerWidget); this.setState({ stickerpickerWidget, widgetId: stickerpickerWidget ? stickerpickerWidget.id : null, diff --git a/src/stores/ActiveWidgetStore.js b/src/stores/ActiveWidgetStore.js index 89fa6e6936..1211195203 100644 --- a/src/stores/ActiveWidgetStore.js +++ b/src/stores/ActiveWidgetStore.js @@ -23,6 +23,7 @@ import MatrixClientPeg from '../MatrixClientPeg'; * * 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 + * * Which stickerpicker the app is using, if any */ class ActiveWidgetStore extends EventEmitter { constructor() { @@ -41,6 +42,9 @@ class ActiveWidgetStore extends EventEmitter { // What room ID each widget is associated with (if it's a room widget) this._roomIdByWidgetId = {}; + // The sticker picker widget definition the app is currently using, if any + this._stickerPickerWidget = null; + this.onRoomStateEvents = this.onRoomStateEvents.bind(this); this.dispatcherRef = null; @@ -145,6 +149,18 @@ class ActiveWidgetStore extends EventEmitter { delete this._roomIdByWidgetId[widgetId]; this.emit('update'); } + + getStickerPickerWidget() { + return this._stickerPickerWidget; + } + + setStickerPickerWidget(widget) { + this._stickerPickerWidget = widget; + } + + delStickerPickerWidget() { + this._stickerPickerWidget = null; + } } if (global.singletonActiveWidgetStore === undefined) { From 51750cf153561b1aa9761cbcf2ca2c0b5cbfaa7a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 1 Apr 2019 19:50:05 -0600 Subject: [PATCH 2/4] General cleanup of handling for WidgetMessaging instances When setting, delete the old one. Because delete checks if it exists first, we can safely call this. The change in FromWidgetPostMessageApi is just something noticed while debugging. --- src/FromWidgetPostMessageApi.js | 6 ++---- src/components/views/elements/AppTile.js | 4 +--- src/stores/ActiveWidgetStore.js | 2 ++ 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/FromWidgetPostMessageApi.js b/src/FromWidgetPostMessageApi.js index 4dd3ea6e6d..61c51d4a20 100644 --- a/src/FromWidgetPostMessageApi.js +++ b/src/FromWidgetPostMessageApi.js @@ -116,10 +116,8 @@ export default class FromWidgetPostMessageApi { const origin = u.protocol + '//' + u.host; if (this.widgetMessagingEndpoints && this.widgetMessagingEndpoints.length > 0) { const length = this.widgetMessagingEndpoints.length; - this.widgetMessagingEndpoints = this.widgetMessagingEndpoints. - filter(function(endpoint) { - return (endpoint.widgetId != widgetId || endpoint.endpointUrl != origin); - }); + this.widgetMessagingEndpoints = this.widgetMessagingEndpoints + .filter((endpoint) => endpoint.widgetId !== widgetId || endpoint.endpointUrl !== origin); return (length > this.widgetMessagingEndpoints.length); } return false; diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 708eb39bd3..959cee7ace 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -339,9 +339,7 @@ export default class AppTile extends React.Component { // Destroy the old widget messaging before starting it back up again. Some widgets // have startup routines that run when they are loaded, so we just need to reinitialize // the messaging for them. - if (ActiveWidgetStore.getWidgetMessaging(this.props.id)) { - ActiveWidgetStore.delWidgetMessaging(this.props.id); - } + ActiveWidgetStore.delWidgetMessaging(this.props.id); this._setupWidgetMessaging(); ActiveWidgetStore.setRoomId(this.props.id, this.props.room.roomId); diff --git a/src/stores/ActiveWidgetStore.js b/src/stores/ActiveWidgetStore.js index 1211195203..2d80a763d0 100644 --- a/src/stores/ActiveWidgetStore.js +++ b/src/stores/ActiveWidgetStore.js @@ -116,6 +116,8 @@ class ActiveWidgetStore extends EventEmitter { } setWidgetMessaging(widgetId, wm) { + // Stop any existing widget messaging first + this.delWidgetMessaging(widgetId); this._widgetMessagingByWidgetId[widgetId] = wm; this.emit('update'); } From 10369170edc461538fe8afb4dd273d067f28bd35 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 3 Apr 2019 17:16:56 -0600 Subject: [PATCH 3/4] Use a static prop on Stickerpicker to track the current widget --- src/components/views/rooms/Stickerpicker.js | 9 ++++++--- src/stores/ActiveWidgetStore.js | 15 --------------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/components/views/rooms/Stickerpicker.js b/src/components/views/rooms/Stickerpicker.js index cab1b5c146..55f34cc42b 100644 --- a/src/components/views/rooms/Stickerpicker.js +++ b/src/components/views/rooms/Stickerpicker.js @@ -37,6 +37,9 @@ const STICKERPICKER_Z_INDEX = 3500; const PERSISTED_ELEMENT_KEY = "stickerPicker"; export default class Stickerpicker extends React.Component { + + static currentWidget; + constructor(props) { super(props); this._onShowStickersClick = this._onShowStickersClick.bind(this); @@ -131,12 +134,12 @@ export default class Stickerpicker extends React.Component { _updateWidget() { const stickerpickerWidget = WidgetUtils.getStickerpickerWidgets()[0]; if (!stickerpickerWidget) { - ActiveWidgetStore.delStickerPickerWidget(); + Stickerpicker.currentWidget = null; this.setState({stickerpickerWidget: null, widgetId: null}); return; } - const currentWidget = ActiveWidgetStore.getStickerPickerWidget(); + const currentWidget = Stickerpicker.currentWidget; let currentUrl = null; if (currentWidget && currentWidget.content && currentWidget.content.url) { currentUrl = currentWidget.content.url; @@ -152,7 +155,7 @@ export default class Stickerpicker extends React.Component { PersistedElement.destroyElement(PERSISTED_ELEMENT_KEY); } - ActiveWidgetStore.setStickerPickerWidget(stickerpickerWidget); + Stickerpicker.currentWidget = stickerpickerWidget; this.setState({ stickerpickerWidget, widgetId: stickerpickerWidget ? stickerpickerWidget.id : null, diff --git a/src/stores/ActiveWidgetStore.js b/src/stores/ActiveWidgetStore.js index 2d80a763d0..94b1b8e79f 100644 --- a/src/stores/ActiveWidgetStore.js +++ b/src/stores/ActiveWidgetStore.js @@ -42,9 +42,6 @@ class ActiveWidgetStore extends EventEmitter { // What room ID each widget is associated with (if it's a room widget) this._roomIdByWidgetId = {}; - // The sticker picker widget definition the app is currently using, if any - this._stickerPickerWidget = null; - this.onRoomStateEvents = this.onRoomStateEvents.bind(this); this.dispatcherRef = null; @@ -151,18 +148,6 @@ class ActiveWidgetStore extends EventEmitter { delete this._roomIdByWidgetId[widgetId]; this.emit('update'); } - - getStickerPickerWidget() { - return this._stickerPickerWidget; - } - - setStickerPickerWidget(widget) { - this._stickerPickerWidget = widget; - } - - delStickerPickerWidget() { - this._stickerPickerWidget = null; - } } if (global.singletonActiveWidgetStore === undefined) { From f2b7042a33f404f33370a7c9c66f13ac46f4e60d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 4 Apr 2019 09:30:15 -0600 Subject: [PATCH 4/4] misc cleanup --- src/components/views/rooms/Stickerpicker.js | 1 - src/stores/ActiveWidgetStore.js | 1 - 2 files changed, 2 deletions(-) diff --git a/src/components/views/rooms/Stickerpicker.js b/src/components/views/rooms/Stickerpicker.js index 55f34cc42b..a0e3f1b7a9 100644 --- a/src/components/views/rooms/Stickerpicker.js +++ b/src/components/views/rooms/Stickerpicker.js @@ -37,7 +37,6 @@ const STICKERPICKER_Z_INDEX = 3500; const PERSISTED_ELEMENT_KEY = "stickerPicker"; export default class Stickerpicker extends React.Component { - static currentWidget; constructor(props) { diff --git a/src/stores/ActiveWidgetStore.js b/src/stores/ActiveWidgetStore.js index 94b1b8e79f..f0a9f718a2 100644 --- a/src/stores/ActiveWidgetStore.js +++ b/src/stores/ActiveWidgetStore.js @@ -23,7 +23,6 @@ import MatrixClientPeg from '../MatrixClientPeg'; * * 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 - * * Which stickerpicker the app is using, if any */ class ActiveWidgetStore extends EventEmitter { constructor() {