From 8bde627cdbcb131fc226ee14b6ed698b7a632338 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 29 Apr 2020 10:10:20 +0200 Subject: [PATCH] more WIP --- doc/impl-thoughts/RECONNECTING.md | 2 + src/domain/LoginViewModel.js | 131 ++++---------------- src/domain/SessionLoadViewModel.js | 39 ++++-- src/domain/SessionPickerViewModel.js | 7 +- src/observable/BaseObservable.js | 20 +-- src/ui/web/css/main.css | 3 +- src/ui/web/login/LoginView.js | 6 +- src/ui/web/login/SessionPickerView.js | 10 +- src/ui/web/session/SyncStatusBar.js | 2 +- src/ui/web/session/room/timeline/GapView.js | 2 +- 10 files changed, 79 insertions(+), 143 deletions(-) diff --git a/doc/impl-thoughts/RECONNECTING.md b/doc/impl-thoughts/RECONNECTING.md index fc7a3b41..310889e1 100644 --- a/doc/impl-thoughts/RECONNECTING.md +++ b/doc/impl-thoughts/RECONNECTING.md @@ -75,3 +75,5 @@ rooms should report how many messages they have queued up, and each time they se thought: do we want to retry a request a couple of times when we can't reach the server before handing it over to the reconnector? Not that some requests may succeed while others may fail, like when matrix.org is really slow, some requests may timeout and others may not. Although starting a service like sync while it is still succeeding should be mostly fine. Perhaps we can pass a canRetry flag to the HomeServerApi that if we get a ConnectionError, we will retry. Only when the flag is not set, we'd call the Reconnector. The downside of this is that if 2 parts are doing requests, 1 retries and 1 does not, and the both requests fail, the other part of the code would still be retrying when the reconnector already kicked in. The HomeServerApi should perhaps tell the retryer if it should give up if a non-retrying request already caused the reconnector to kick in? + +CatchupSync should also use timeout 0, in case there is nothing to report we spend 30s with a catchup spinner. Riot-web sync also says something about using a 0 timeout until there are no more to_device messages as they are queued up by the server and not all returned at once if there are a lot? This is needed for crypto to be aware of all to_device messages. diff --git a/src/domain/LoginViewModel.js b/src/domain/LoginViewModel.js index 3d7853f3..11d43dc9 100644 --- a/src/domain/LoginViewModel.js +++ b/src/domain/LoginViewModel.js @@ -1,9 +1,5 @@ import {EventEmitter} from "../utils/EventEmitter.js"; -import {LoadStatus, LoginFailure} from "../matrix/SessionContainer.js"; -import {AbortError} from "../utils/error.js"; -import {loadLabel} from "./common.js"; - - +import {SessionLoadViewModel} from "./SessionLoadViewModel.js"; export class LoginViewModel extends EventEmitter { constructor({sessionCallback, defaultHomeServer, createSessionContainer}) { @@ -11,121 +7,42 @@ export class LoginViewModel extends EventEmitter { this._createSessionContainer = createSessionContainer; this._sessionCallback = sessionCallback; this._defaultHomeServer = defaultHomeServer; - this._homeserver = null; - this._sessionContainer = null; - this._loadWaitHandle = null; - this._loading = false; - this._error = null; + this._loadViewModel = null; } get usernamePlaceholder() { return "Username"; } get passwordPlaceholder() { return "Password"; } get hsPlaceholder() { return "Your matrix homeserver"; } get defaultHomeServer() { return this._defaultHomeServer; } - get loading() {return this._loading} - get showLoadLabel() { - return this._loading || this._sessionContainer || this._error; - } + get loadViewModel() {return this._loadViewModel; } async login(username, password, homeserver) { - try { - this._loading = true; - this.emit("change", "loading"); - this._homeserver = homeserver; - this._sessionContainer = this._createSessionContainer(); - this._sessionContainer.startWithLogin(homeserver, username, password); - this._loadWaitHandle = this._sessionContainer.loadStatus.waitFor(s => { - this.emit("change", "loadLabel"); - return s === LoadStatus.Ready || - s === LoadStatus.LoginFailed || - s === LoadStatus.Error; - }); - try { - await this._loadWaitHandle.promise; - } catch (err) { - if (err instanceof AbortError) { - // login was cancelled + this._loadViewModel = new SessionLoadViewModel({ + createAndStartSessionContainer: () => { + const sessionContainer = this._createSessionContainer(); + sessionContainer.startWithLogin(homeserver, username, password); + return sessionContainer; + }, + sessionCallback: sessionContainer => { + if (sessionContainer) { + // make parent view model move away + this._sessionCallback(sessionContainer); } else { - throw err; + // show list of session again + this._loadViewModel = null; + this.emit("change", "loadViewModel"); } - } - this._loadWaitHandle = null; - if (this._sessionContainer.loadStatus.get() === LoadStatus.Ready) { - this._sessionCallback(this._sessionContainer); - // wait for parent view model to switch away here - } else { - this._loading = false; - this.emit("change", "loading"); - if (this._sessionContainer.loadError) { - console.error(this._sessionContainer.loadError); - } - } - } catch (err) { - this._error = err; - this._loading = false; - this.emit("change", "loading"); - } + }, + deleteSessionOnCancel: true, + homeserver, + }); + this._loadViewModel.start(); + this.emit("change", "loadViewModel"); } - get loadLabel() { - const sc = this._sessionContainer; - const error = this._error || (sc && sc.loadError); - - if (error || (sc && sc.loadStatus.get() === LoadStatus.Error)) { - return `Something went wrong: ${error && error.message}.`; - } - if (loadStatus) { - switch (loadStatus.get()) { - case LoadStatus.NotLoading: - return `Preparing…`; - case LoadStatus.Login: - return `Checking your login and password…`; - case LoadStatus.Loading: - return `Loading your conversations…`; - case LoadStatus.FirstSync: - return `Getting your conversations from the server…`; - default: - return this._sessionContainer.loadStatus.get(); - } - } - return `Preparing…`; - - if (this._error) { - return loadLabel(null, this._error); - } - if (this.showLoadLabel) { - const sc = this._sessionContainer; - return loadLoginLabel( - sc && sc.loadStatus, - sc && sc.loadError, - sc && sc.loginFailure, - this._homeserver - ); - } - return null; - } - - async cancel() { - if (!this._loading) { - return; - } - this._loading = false; - this.emit("change", "loading"); - if (this._sessionContainer) { - this._sessionContainer.stop(); - await this._sessionContainer.deleteSession(); - this._sessionContainer = null; - } - if (this._loadWaitHandle) { - // rejects with AbortError - this._loadWaitHandle.dispose(); - this._loadWaitHandle = null; - } - } - - goBack() { - if (!this._loading) { + cancel() { + if (!this._loadViewModel) { this._sessionCallback(); } } diff --git a/src/domain/SessionLoadViewModel.js b/src/domain/SessionLoadViewModel.js index a5d2e971..659abeea 100644 --- a/src/domain/SessionLoadViewModel.js +++ b/src/domain/SessionLoadViewModel.js @@ -3,11 +3,12 @@ import {LoadStatus, LoginFailure} from "../matrix/SessionContainer.js"; import {SyncStatus} from "../matrix/Sync.js"; export class SessionLoadViewModel extends EventEmitter { - constructor({createAndStartSessionContainer, sessionCallback, homeserver}) { + constructor({createAndStartSessionContainer, sessionCallback, homeserver, deleteSessionOnCancel}) { super(); this._createAndStartSessionContainer = createAndStartSessionContainer; this._sessionCallback = sessionCallback; this._homeserver = homeserver; + this._deleteSessionOnCancel = deleteSessionOnCancel; this._loading = false; this._error = null; } @@ -33,7 +34,7 @@ export class SessionLoadViewModel extends EventEmitter { try { await this._waitHandle.promise; } catch (err) { - // swallow AbortError + return; // aborted by goBack } // TODO: should we deal with no connection during initial sync // and we're retrying as well here? @@ -53,19 +54,31 @@ export class SessionLoadViewModel extends EventEmitter { } } - get loading() { - return this._loading; + + async cancel() { + try { + if (this._sessionContainer) { + this._sessionContainer.stop(); + if (this._deleteSessionOnCancel) { + await this._sessionContainer.deletSession(); + } + this._sessionContainer = null; + } + if (this._waitHandle) { + // rejects with AbortError + this._waitHandle.dispose(); + this._waitHandle = null; + } + this._sessionCallback(); + } catch (err) { + this._error = err; + this.emit("change"); + } } - goBack() { - if (this._sessionContainer) { - this._sessionContainer.stop(); - this._sessionContainer = null; - if (this._waitHandle) { - this._waitHandle.dispose(); - } - } - this._sessionCallback(); + // to show a spinner or not + get loading() { + return this._loading; } get loadLabel() { diff --git a/src/domain/SessionPickerViewModel.js b/src/domain/SessionPickerViewModel.js index 1959ff85..8274106a 100644 --- a/src/domain/SessionPickerViewModel.js +++ b/src/domain/SessionPickerViewModel.js @@ -1,5 +1,6 @@ import {SortedArray} from "../observable/index.js"; import {EventEmitter} from "../utils/EventEmitter.js"; +import {SessionLoadViewModel} from "./SessionLoadViewModel.js"; class SessionItemViewModel extends EventEmitter { constructor(sessionInfo, pickerVM) { @@ -127,7 +128,7 @@ export class SessionPickerViewModel extends EventEmitter { } const sessionVM = this._sessions.array.find(s => s.id === id); if (sessionVM) { - this._loadViewModel = new LoadViewModel({ + this._loadViewModel = new SessionLoadViewModel({ createAndStartSessionContainer: () => { const sessionContainer = this._createSessionContainer(); sessionContainer.startWithExistingSession(sessionVM.id); @@ -182,6 +183,8 @@ export class SessionPickerViewModel extends EventEmitter { } cancel() { - this._sessionCallback(); + if (!this._loadViewModel) { + this._sessionCallback(); + } } } diff --git a/src/observable/BaseObservable.js b/src/observable/BaseObservable.js index 75d8023f..3afdd4c0 100644 --- a/src/observable/BaseObservable.js +++ b/src/observable/BaseObservable.js @@ -17,17 +17,21 @@ export class BaseObservable { this.onSubscribeFirst(); } return () => { - if (handler) { - this._handlers.delete(handler); - if (this._handlers.size === 0) { - this.onUnsubscribeLast(); - } - handler = null; - } - return null; + return this.unsubscribe(handler); }; } + unsubscribe(handler) { + if (handler) { + this._handlers.delete(handler); + if (this._handlers.size === 0) { + this.onUnsubscribeLast(); + } + handler = null; + } + return null; + } + // Add iterator over handlers here } diff --git a/src/ui/web/css/main.css b/src/ui/web/css/main.css index 1ba316a7..e01e3fc6 100644 --- a/src/ui/web/css/main.css +++ b/src/ui/web/css/main.css @@ -6,7 +6,8 @@ body { margin: 0; - font-family: sans-serif; + font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Ubuntu, Cantarell, sans-serif, + "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji"; background-color: black; color: white; } diff --git a/src/ui/web/login/LoginView.js b/src/ui/web/login/LoginView.js index c4b68477..b472e436 100644 --- a/src/ui/web/login/LoginView.js +++ b/src/ui/web/login/LoginView.js @@ -13,7 +13,7 @@ export class LoginView extends TemplateView { const homeserver = t.input({type: "text", placeholder: vm.hsPlaceholder, value: vm.defaultHomeServer, disabled}); return t.div({className: "LoginView form"}, [ t.h1(["Log in to your homeserver"]), - t.if(vm => vm.error, t => t.div({className: "error"}, vm => vm.error)), + t.if(vm => vm.error, t.createTemplate(t => t.div({className: "error"}, vm => vm.error))), t.div(username), t.div(password), t.div(homeserver), @@ -22,7 +22,7 @@ export class LoginView extends TemplateView { disabled }, "Log In")), t.div(t.button({onClick: () => vm.goBack(), disabled}, ["Pick an existing session"])), - t.if(vm => vm.showLoadLabel, renderLoadProgress), + t.if(vm => vm.loadViewModel, vm => new SessionLoadView(vm.loadViewModel)), t.p(brawlGithubLink(t)) ]); } @@ -32,6 +32,6 @@ function renderLoadProgress(t) { return t.div({className: "loadProgress"}, [ t.div({className: "spinner"}), t.p(vm => vm.loadLabel), - t.if(vm => vm.loading, t => t.button({onClick: vm => vm.cancel()}, "Cancel login")) + t.if(vm => vm.loading, t.template(t => t.button({onClick: vm => vm.cancel()}, "Cancel login"))) ]); } diff --git a/src/ui/web/login/SessionPickerView.js b/src/ui/web/login/SessionPickerView.js index ed335033..e443ed81 100644 --- a/src/ui/web/login/SessionPickerView.js +++ b/src/ui/web/login/SessionPickerView.js @@ -27,10 +27,6 @@ function selectFileAsText(mimeType) { class SessionPickerItemView extends TemplateView { - constructor(vm) { - super(vm, true); - } - _onDeleteClick() { if (confirm("Are you sure?")) { this.viewModel.delete(); @@ -50,16 +46,16 @@ class SessionPickerItemView extends TemplateView { disabled: vm => vm.isClearing, onClick: () => this.viewModel.export(), }, "Export"); - const downloadExport = t.if(vm => vm.exportDataUrl, (t, vm) => { + const downloadExport = t.if(vm => vm.exportDataUrl, t.template((t, vm) => { return t.a({ href: vm.exportDataUrl, download: `brawl-session-${this.viewModel.id}.json`, onClick: () => setTimeout(() => this.viewModel.clearExport(), 100), }, "Download"); - }); + })); const userName = t.span({className: "userId"}, vm => vm.label); - const errorMessage = t.if(vm => vm.error, t => t.span({className: "error"}, vm => vm.error)); + const errorMessage = t.if(vm => vm.error, t.template(t => t.span({className: "error"}, vm => vm.error))); return t.li([t.div({className: "sessionInfo"}, [ userName, errorMessage, diff --git a/src/ui/web/session/SyncStatusBar.js b/src/ui/web/session/SyncStatusBar.js index e8d4e95c..c9d22a8c 100644 --- a/src/ui/web/session/SyncStatusBar.js +++ b/src/ui/web/session/SyncStatusBar.js @@ -11,7 +11,7 @@ export class SyncStatusBar extends TemplateView { "SyncStatusBar_shown": true, }}, [ vm => vm.status, - t.if(vm => !vm.isSyncing, t => t.button({onClick: () => vm.trySync()}, "Try syncing")), + t.if(vm => !vm.isSyncing, t.template(t => t.button({onClick: () => vm.trySync()}, "Try syncing"))), window.DEBUG ? t.button({id: "showlogs"}, "Show logs") : "" ]); } diff --git a/src/ui/web/session/room/timeline/GapView.js b/src/ui/web/session/room/timeline/GapView.js index db79f161..a0daef24 100644 --- a/src/ui/web/session/room/timeline/GapView.js +++ b/src/ui/web/session/room/timeline/GapView.js @@ -12,7 +12,7 @@ export class GapView extends TemplateView { onClick: () => this.viewModel.fill(), disabled: vm => vm.isLoading }, label), - t.if(vm => vm.error, t => t.strong(vm => vm.error)) + t.if(vm => vm.error, t.template(t => t.strong(vm => vm.error))) ]); } }