Merge pull request #1548 from matrix-org/rxl881/widgetrendering

Improve widget rendering on prop updates

package.json

@@ -74,6 +74,7 @@
     "matrix-js-sdk": "0.8.5",
     "optimist": "^0.6.1",
     "prop-types": "^15.5.8",
+    "querystring": "^0.2.0",
     "react": "^15.4.0",
     "react-addons-css-transition-group": "15.3.2",
     "react-dom": "^15.4.0",

src/components/views/elements/AppTile.js

@@ -17,6 +17,7 @@ limitations under the License.
 'use strict';
 
 import url from 'url';
+import qs from 'querystring';
 import React from 'react';
 import MatrixClientPeg from '../../../MatrixClientPeg';
 import PlatformPeg from '../../../PlatformPeg';
@@ -51,42 +52,63 @@ export default React.createClass({
         creatorUserId: React.PropTypes.string,
     },
 
-    getDefaultProps: function() {
+    getDefaultProps() {
         return {
             url: "",
         };
     },
 
-    getInitialState: function() {
+    /**
-        const widgetPermissionId = [this.props.room.roomId, encodeURIComponent(this.props.url)].join('_');
+     * Set initial component state when the App wUrl (widget URL) is being updated.
+     * Component props *must* be passed (rather than relying on this.props).
+     * @param  {Object} newProps The new properties of the component
+     * @return {Object} Updated component state to be set with setState
+     */
+    _getNewState(newProps) {
+        const widgetPermissionId = [newProps.room.roomId, encodeURIComponent(newProps.url)].join('_');
         const hasPermissionToLoad = localStorage.getItem(widgetPermissionId);
         return {
-            loading: false,
+            initialising: true,   // True while we are mangling the widget URL
-            widgetUrl: this.props.url,
+            loading: true,        // True while the iframe content is loading
+            widgetUrl: newProps.url,
             widgetPermissionId: widgetPermissionId,
-            // Assume that widget has permission to load if we are the user who added it to the room, or if explicitly granted by the user
+            // Assume that widget has permission to load if we are the user who
-            hasPermissionToLoad: hasPermissionToLoad === 'true' || this.props.userId === this.props.creatorUserId,
+            // added it to the room, or if explicitly granted by the user
+            hasPermissionToLoad: hasPermissionToLoad === 'true' || newProps.userId === newProps.creatorUserId,
             error: null,
             deleting: false,
         };
     },
 
-    // Returns true if props.url is a scalar URL, typically https://scalar.vector.im/api
+    getInitialState() {
-    isScalarUrl: function() {
+        return this._getNewState(this.props);
+    },
+
+    /**
+     * Returns true if specified url is a scalar URL, typically https://scalar.vector.im/api
+     * @param  {[type]}  url URL to check
+     * @return {Boolean} True if specified URL is a scalar URL
+     */
+    isScalarUrl(url) {
+        if (!url) {
+            console.error('Scalar URL check failed. No URL specified');
+            return false;
+        }
+
         let scalarUrls = SdkConfig.get().integrations_widgets_urls;
         if (!scalarUrls || scalarUrls.length == 0) {
             scalarUrls = [SdkConfig.get().integrations_rest_url];
         }
 
         for (let i = 0; i < scalarUrls.length; i++) {
-            if (this.props.url.startsWith(scalarUrls[i])) {
+            if (url.startsWith(scalarUrls[i])) {
                 return true;
             }
         }
         return false;
     },
 
-    isMixedContent: function() {
+    isMixedContent() {
         const parentContentProtocol = window.location.protocol;
         const u = url.parse(this.props.url);
         const childContentProtocol = u.protocol;
@@ -98,43 +120,73 @@ export default React.createClass({
         return false;
     },
 
-    componentWillMount: function() {
+    componentWillMount() {
-        if (!this.isScalarUrl()) {
+        window.addEventListener('message', this._onMessage, false);
+        this.setScalarToken();
+    },
+
+    /**
+     * Adds a scalar token to the widget URL, if required
+     * Component initialisation is only complete when this function has resolved
+     */
+    setScalarToken() {
+        this.setState({initialising: true});
+
+        if (!this.isScalarUrl(this.props.url)) {
+            console.warn('Non-scalar widget, not setting scalar token!', url);
+            this.setState({
+                error: null,
+                widgetUrl: this.props.url,
+                initialising: false,
+            });
             return;
         }
-        // Fetch the token before loading the iframe as we need to mangle the URL
+
-        this.setState({
+        // Fetch the token before loading the iframe as we need it to mangle the URL
-            loading: true,
+        if (!this._scalarClient) {
-        });
+            this._scalarClient = new ScalarAuthClient();
-        this._scalarClient = new ScalarAuthClient();
+        }
         this._scalarClient.getScalarToken().done((token) => {
-            // Append scalar_token as a query param
+            // Append scalar_token as a query param if not already present
             this._scalarClient.scalarToken = token;
             const u = url.parse(this.props.url);
-            if (!u.search) {
+            const params = qs.parse(u.query);
-                u.search = "?scalar_token=" + encodeURIComponent(token);
+            if (!params.scalar_token) {
-            } else {
+                params.scalar_token = encodeURIComponent(token);
-                u.search += "&scalar_token=" + encodeURIComponent(token);
+                // u.search must be set to undefined, so that u.format() uses query paramerters - https://nodejs.org/docs/latest/api/url.html#url_url_format_url_options
+                u.search = undefined;
+                u.query = params;
             }
 
             this.setState({
                 error: null,
                 widgetUrl: u.format(),
-                loading: false,
+                initialising: false,
             });
         }, (err) => {
+            console.error("Failed to get scalar_token", err);
             this.setState({
                 error: err.message,
-                loading: false,
+                initialising: false,
             });
         });
-        window.addEventListener('message', this._onMessage, false);
     },
 
     componentWillUnmount() {
         window.removeEventListener('message', this._onMessage);
     },
 
+    componentWillReceiveProps(nextProps) {
+        if (nextProps.url !== this.props.url) {
+            this._getNewState(nextProps);
+            this.setScalarToken();
+        } else if (nextProps.show && !this.props.show) {
+            this.setState({
+                loading: true,
+            });
+        }
+    },
+
     _onMessage(event) {
         if (this.props.type !== 'jitsi') {
             return;
@@ -154,11 +206,11 @@ export default React.createClass({
         }
     },
 
-    _canUserModify: function() {
+    _canUserModify() {
         return WidgetUtils.canUserModifyWidgets(this.props.room.roomId);
     },
 
-    _onEditClick: function(e) {
+    _onEditClick(e) {
         console.log("Edit widget ID ", this.props.id);
         const IntegrationsManager = sdk.getComponent("views.settings.IntegrationsManager");
         const src = this._scalarClient.getScalarInterfaceUrlForRoom(
@@ -168,9 +220,10 @@ export default React.createClass({
         }, "mx_IntegrationsManager");
     },
 
-    /* If user has permission to modify widgets, delete the widget, otherwise revoke access for the widget to load in the user's browser
+    /* If user has permission to modify widgets, delete the widget,
+     * otherwise revoke access for the widget to load in the user's browser
     */
-    _onDeleteClick: function() {
+    _onDeleteClick() {
         if (this._canUserModify()) {
             // Show delete confirmation dialog
             const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog");
@@ -202,6 +255,10 @@ export default React.createClass({
         }
     },
 
+    _onLoaded() {
+        this.setState({loading: false});
+    },
+
     // Widget labels to render, depending upon user permissions
     // These strings are translated at the point that they are inserted in to the DOM, in the render method
     _deleteWidgetLabel() {
@@ -224,7 +281,7 @@ export default React.createClass({
         this.setState({hasPermissionToLoad: false});
     },
 
-    formatAppTileName: function() {
+    formatAppTileName() {
         let appTileName = "No name";
         if(this.props.name && this.props.name.trim()) {
             appTileName = this.props.name.trim();
@@ -232,7 +289,7 @@ export default React.createClass({
         return appTileName;
     },
 
-    onClickMenuBar: function(ev) {
+    onClickMenuBar(ev) {
         ev.preventDefault();
 
         // Ignore clicks on menu bar children
@@ -247,7 +304,7 @@ export default React.createClass({
         });
     },
 
-    render: function() {
+    render() {
         let appTileBody;
 
         // Don't render widget if it is in the process of being deleted
@@ -269,29 +326,30 @@ export default React.createClass({
         }
 
         if (this.props.show) {
-            if (this.state.loading) {
+            const loadingElement = (
-                appTileBody = (
+                <div className='mx_AppTileBody mx_AppLoading'>
-                    <div className='mx_AppTileBody mx_AppLoading'>
+                    <MessageSpinner msg='Loading...' />
-                        <MessageSpinner msg='Loading...' />
+                </div>
-                    </div>
+            );
-                );
+            if (this.state.initialising) {
+                appTileBody = loadingElement;
             } else if (this.state.hasPermissionToLoad == true) {
                 if (this.isMixedContent()) {
                     appTileBody = (
                         <div className="mx_AppTileBody">
-                            <AppWarning
+                            <AppWarning errorMsg="Error - Mixed content" />
-                                errorMsg="Error - Mixed content"
-                            />
                         </div>
                     );
                 } else {
                     appTileBody = (
-                        <div className="mx_AppTileBody">
+                        <div className={this.state.loading ? 'mx_AppTileBody mx_AppLoading' : 'mx_AppTileBody'}>
+                            { this.state.loading && loadingElement }
                             <iframe
                                 ref="appFrame"
                                 src={safeWidgetUrl}
                                 allowFullScreen="true"
                                 sandbox={sandboxFlags}
+                                onLoad={this._onLoaded}
                             ></iframe>
                         </div>
                     );