Merge pull request #202 from matrix-org/bwindels/fix-201

Show both open and install options when deeplink for native app is https link

src/open/ClientListView.js

@@ -33,12 +33,10 @@ class AllClientsView extends TemplateView {
 	render(t, vm) {
 		return t.div({className: "ClientListView"}, [
 			t.h2("Choose an app to continue"),
-			t.mapView(vm => vm.clientList, () => {
+			t.map(vm => vm.clientList, (clientList, t) => {
-				return new TemplateView(vm, t => {
+				return t.div({className: "list"}, clientList.map(clientViewModel => {
-					return t.div({className: "list"}, vm.clientList.map(clientViewModel => {
+					return t.view(new ClientView(clientViewModel));
-						return t.view(new ClientView(clientViewModel));
+				}));
-					}));
-				});
 			}),
 			t.div(t.label([
 				t.input({

src/open/ClientView.js

@@ -53,12 +53,8 @@ export class ClientView extends TemplateView {
 				]),
 				t.img({className: "clientIcon", src: vm.iconUrl})
 			]),
-			t.mapView(vm => vm.stage, stage => {
+            t.ifView(vm => vm.showOpen, vm => new OpenClientView(vm)),
-				switch (stage) {
+            t.ifView(vm => vm.showInstall, vm => new InstallClientView(vm))
-					case "open": return new OpenClientView(vm);
-					case "install": return new InstallClientView(vm);
-				}
-			}),
 		]);
 	}
 }

src/open/ClientViewModel.js

@@ -43,12 +43,21 @@ export class ClientViewModel extends ViewModel {
 		this._webPlatform = matchingPlatforms.find(p => isWebPlatform(p));
 		this._nativePlatform = matchingPlatforms.find(p => !isWebPlatform(p));
         const preferredPlatform = matchingPlatforms.find(p => p === this.preferences.platform);
-		this._proposedPlatform = preferredPlatform || this._nativePlatform || webPlatform;
+		this._proposedPlatform = preferredPlatform || this._nativePlatform || this._webPlatform;
 
         this.openActions = this._createOpenActions();
 		this.installActions = this._createInstallActions();
 		this._clientCanIntercept = !!(this._nativePlatform && this._client.canInterceptMatrixToLinks(this._nativePlatform));
 		this._showOpen = this.openActions.length && !this._clientCanIntercept;
+        const proposedDeepLink = this._client.getDeepLink(this._proposedPlatform, this._link);
+        this._openWillNavigateIfNotInstalled = false;
+        if (this._showOpen && !isWebPlatform(this._proposedPlatform)) {
+            try {
+                if (new URL(proposedDeepLink).protocol === "https:") {
+                    this._openWillNavigateIfNotInstalled = true;
+                }
+            } catch (err) {}
+        }
     }
 
     // these are only shown in the open stage
@@ -170,9 +179,17 @@ export class ClientViewModel extends ViewModel {
         return this._client.icon;
     }
 
-	get stage() {
+    get showOpen() {
-		return this._showOpen ? "open" : "install";
+        return this._showOpen;
-	}
+    }
+
+    get showInstall() {
+        // also show install options in open screen if the deeplink is
+        // a https link that should be intercepted by the native app
+        // because if it isn't installed, you will just go to that
+        // website and never see the install options here.
+        return !this._showOpen || this._openWillNavigateIfNotInstalled;
+    }
 
 	get textInstructions() {
         let instructions = this._client.getLinkInstructions(this._proposedPlatform, this._link);

src/preview/PreviewView.js

@@ -44,11 +44,11 @@ class LoadingPreviewView extends TemplateView {
 
 class LoadedPreviewView extends TemplateView {
 	render(t, vm) {
-        const avatar = t.mapView(vm => vm.avatarUrl, avatarUrl => {
+        const avatar = t.map(vm => vm.avatarUrl, (avatarUrl, t) => {
             if (avatarUrl) {
-                return new TemplateView(avatarUrl, (t, src) => t.img({className: "avatar", src}));
+                return t.img({className: "avatar", src: avatarUrl});
             } else {
-                return new TemplateView(null, t => t.div({className: "defaultAvatar"}));
+                return t.div({className: "defaultAvatar"});
             }
         });
 		return t.div([

src/utils/TemplateView.js

@@ -15,7 +15,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { setAttribute, text, isChildren, classNames, TAG_NAMES, HTML_NS, tag } from "./html.js";
+import { setAttribute, text, isChildren, classNames, TAG_NAMES, HTML_NS } from "./html.js";
 
 /**
     Bindable template. Renders once, and allows bindings for given nodes. If you need
@@ -33,11 +33,13 @@ import { setAttribute, text, isChildren, classNames, TAG_NAMES, HTML_NS, tag } f
 export class TemplateView {
     constructor(value, render = undefined) {
         this._value = value;
+        // TODO: can avoid this if we have a separate class for inline templates vs class template views
         this._render = render;
         this._eventListeners = null;
         this._bindings = null;
         this._subViews = null;
         this._root = null;
+        // TODO: can avoid this if we adopt the handleEvent pattern in our EventListener
         this._boundUpdateFromValue = null;
     }
 
@@ -108,6 +110,10 @@ export class TemplateView {
         return this._root;
     }
 
+    _updateFromValue(changedProps) {
+        this.update(this._value, changedProps);
+    }
+
     update(value) {
         this._value = value;
         if (this._bindings) {
@@ -117,10 +123,6 @@ export class TemplateView {
         }
     }
 
-    _updateFromValue(changedProps) {
-        this.update(this._value, changedProps);
-    }
-
     _addEventListener(node, name, fn, useCapture = false) {
         if (!this._eventListeners) {
             this._eventListeners = [];
@@ -135,12 +137,19 @@ export class TemplateView {
         this._bindings.push(bindingFn);
     }
 
-    _addSubView(view) {
+    addSubView(view) {
         if (!this._subViews) {
             this._subViews = [];
         }
         this._subViews.push(view);
     }
+
+    removeSubView(view) {
+        const idx = this._subViews.indexOf(view);
+        if (idx !== -1) {
+            this._subViews.splice(idx, 1);
+        }
+    }
 }
 
 // what is passed to render
@@ -238,8 +247,6 @@ class TemplateBuilder {
                 const newNode = renderNode(node);
                 if (node.parentNode) {
                     node.parentNode.replaceChild(newNode, node);
-                } else {
-                    console.warn("Could not update parent of node binding");
                 }
                 node = newNode;
             }
@@ -279,15 +286,10 @@ class TemplateBuilder {
         } catch (err) {
             return errorToDOM(err);
         }
-        this._templateView._addSubView(view);
+        this._templateView.addSubView(view);
         return root;
     }
 
-    // sugar
-    createTemplate(render) {
-        return vm => new TemplateView(vm, render);
-    }
-
     // map a value to a view, every time the value changes
     mapView(mapFn, viewCreator) {
         return this._addReplaceNodeBinding(mapFn, (prevNode) => {
@@ -308,15 +310,36 @@ class TemplateBuilder {
         });
     }
 
-    // creates a conditional subtemplate
+    // Special case of mapView for a TemplateView.
-    if(fn, viewCreator) {
+    // Always creates a TemplateView, if this is optional depending
+    // on mappedValue, use `if` or `mapView`
+    map(mapFn, renderFn) {
+        return this.mapView(mapFn, mappedValue => {
+            return new TemplateView(this._value, (t, vm) => {
+                const rootNode = renderFn(mappedValue, t, vm);
+                if (!rootNode) {
+                    // TODO: this will confuse mapView which assumes that
+                    // a comment node means there is no view to clean up
+                    return document.createComment("map placeholder");
+                }
+                return rootNode;
+            });
+        });
+    }
+
+    ifView(predicate, viewCreator) {
         return this.mapView(
-            value => !!fn(value),
+            value => !!predicate(value),
             enabled => enabled ? viewCreator(this._value) : null
         );
     }
-}
 
+    // creates a conditional subtemplate
+    // use mapView if you need to map to a different view class
+    if(predicate, renderFn) {
+        return this.ifView(predicate, vm => new TemplateView(vm, renderFn));
+    }
+}
 
 function errorToDOM(error) {
     const stack = new Error().stack;