From 0ee064fbe159ea8b15e3a9d4045f034a96b4e701 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 12 Apr 2021 14:54:08 +0200 Subject: [PATCH 1/4] fix: this var does not exist and should be a member became apparent during testing when no native platform is returned --- src/open/ClientViewModel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/open/ClientViewModel.js b/src/open/ClientViewModel.js index e1b3db5..cf47c23 100644 --- a/src/open/ClientViewModel.js +++ b/src/open/ClientViewModel.js @@ -43,7 +43,7 @@ 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(); From d8f1371b608bc8cb9b6be2f6727e30e30a25a62b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 12 Apr 2021 15:09:39 +0200 Subject: [PATCH 2/4] update TemplateView to add if and map fns --- src/utils/TemplateView.js | 59 +++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/src/utils/TemplateView.js b/src/utils/TemplateView.js index c414c37..349907c 100644 --- a/src/utils/TemplateView.js +++ b/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 - if(fn, viewCreator) { + // Special case of mapView for a TemplateView. + // 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; From ef2acf81efe4c4f8f6da8c16f0735f88abcb7487 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 12 Apr 2021 15:10:02 +0200 Subject: [PATCH 3/4] use map rather than mapView where it makes sense --- src/open/ClientListView.js | 10 ++++------ src/preview/PreviewView.js | 6 +++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/open/ClientListView.js b/src/open/ClientListView.js index 3fc43ab..8834cdf 100644 --- a/src/open/ClientListView.js +++ b/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, () => { - return new TemplateView(vm, t => { - return t.div({className: "list"}, vm.clientList.map(clientViewModel => { - return t.view(new ClientView(clientViewModel)); - })); - }); + t.map(vm => vm.clientList, (clientList, t) => { + return t.div({className: "list"}, clientList.map(clientViewModel => { + return t.view(new ClientView(clientViewModel)); + })); }), t.div(t.label([ t.input({ diff --git a/src/preview/PreviewView.js b/src/preview/PreviewView.js index 4d16075..0fedfb6 100644 --- a/src/preview/PreviewView.js +++ b/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([ From f02dc5aae972044f05f14298cec483208e38cdbd Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 12 Apr 2021 15:15:21 +0200 Subject: [PATCH 4/4] also show install options for https deeplinks for native app --- src/open/ClientView.js | 8 ++------ src/open/ClientViewModel.js | 23 ++++++++++++++++++++--- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/open/ClientView.js b/src/open/ClientView.js index 408ba87..8924054 100644 --- a/src/open/ClientView.js +++ b/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 => { - switch (stage) { - case "open": return new OpenClientView(vm); - case "install": return new InstallClientView(vm); - } - }), + t.ifView(vm => vm.showOpen, vm => new OpenClientView(vm)), + t.ifView(vm => vm.showInstall, vm => new InstallClientView(vm)) ]); } } diff --git a/src/open/ClientViewModel.js b/src/open/ClientViewModel.js index cf47c23..2c54232 100644 --- a/src/open/ClientViewModel.js +++ b/src/open/ClientViewModel.js @@ -49,6 +49,15 @@ export class ClientViewModel extends ViewModel { 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() { - return this._showOpen ? "open" : "install"; - } + get showOpen() { + 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);