From 3e34ccb7e1253f38311cf348fa90fc976826d8b7 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 19 Oct 2020 12:52:57 +0200 Subject: [PATCH 1/5] =?UTF-8?q?rename,=20I=20think,=20last=20occurrence=20?= =?UTF-8?q?of=20brawl=20to=20hydrogen=20=F0=9F=91=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.js b/src/main.js index 7bf7d5af..2626b2f9 100644 --- a/src/main.js +++ b/src/main.js @@ -149,7 +149,7 @@ export async function main(container, paths, legacyExtras) { navigation, updateService: serviceWorkerHandler }); - window.__brawlViewModel = vm; + window.__hydrogenViewModel = vm; await vm.load(); // TODO: replace with platform.createAndMountRootView(vm, container); const view = new RootView(vm); From fb273782bf010aecb051cf2882ff52e5381921fa Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 19 Oct 2020 12:55:10 +0200 Subject: [PATCH 2/5] use handleEvent in History so we don't have to bind --- src/ui/web/dom/History.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/ui/web/dom/History.js b/src/ui/web/dom/History.js index 61f04444..92927d3f 100644 --- a/src/ui/web/dom/History.js +++ b/src/ui/web/dom/History.js @@ -17,14 +17,11 @@ limitations under the License. import {BaseObservableValue} from "../../../observable/ObservableValue.js"; export class History extends BaseObservableValue { - constructor() { - super(); - this._boundOnHashChange = null; - } - - _onHashChange() { - this.emit(this.get()); - this._storeHash(this.get()); + handleEvent(event) { + if (event.type === "hashchange") { + this.emit(this.get()); + this._storeHash(this.get()); + } } get() { @@ -60,13 +57,11 @@ export class History extends BaseObservableValue { } onSubscribeFirst() { - this._boundOnHashChange = this._onHashChange.bind(this); - window.addEventListener('hashchange', this._boundOnHashChange); + window.addEventListener('hashchange', this); } onUnsubscribeLast() { - window.removeEventListener('hashchange', this._boundOnHashChange); - this._boundOnHashChange = null; + window.removeEventListener('hashchange', this); } _storeHash(hash) { From 83572601337952f7294580700519f875bc8b7df7 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 19 Oct 2020 12:55:42 +0200 Subject: [PATCH 3/5] fix an applying an url also pushing a copy of the url back on history it should replace instead, as it is a redirect --- src/domain/navigation/URLRouter.js | 33 ++++++++++++++++++------------ 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/domain/navigation/URLRouter.js b/src/domain/navigation/URLRouter.js index 082be2b9..36a1c244 100644 --- a/src/domain/navigation/URLRouter.js +++ b/src/domain/navigation/URLRouter.js @@ -22,22 +22,13 @@ export class URLRouter { this._stringifyPath = stringifyPath; this._subscription = null; this._pathSubscription = null; + this._isApplyingUrl = false; } attach() { - this._subscription = this._history.subscribe(url => { - const redirectedUrl = this._applyUrl(url); - if (redirectedUrl !== url) { - this._history.replaceUrlSilently(redirectedUrl); - } - }); + this._subscription = this._history.subscribe(url => this._applyUrl(url)); this._applyUrl(this._history.get()); - this._pathSubscription = this._navigation.pathObservable.subscribe(path => { - const url = this.urlForPath(path); - if (url !== this._history.get()) { - this._history.pushUrlSilently(url); - } - }); + this._pathSubscription = this._navigation.pathObservable.subscribe(path => this._applyNavigationPath(path)); } dispose() { @@ -45,11 +36,27 @@ export class URLRouter { this._pathSubscription = this._pathSubscription(); } + _applyNavigationPath(path) { + const url = this.urlForPath(path); + if (url !== this._history.get()) { + if (this._isApplyingUrl) { + // redirect + this._history.replaceUrlSilently(url); + } else { + this._history.pushUrlSilently(url); + } + } + } + _applyUrl(url) { const urlPath = this._history.urlAsPath(url) const navPath = this._navigation.pathFrom(this._parseUrlPath(urlPath, this._navigation.path)); + // this will cause _applyNavigationPath to be called, + // so set a flag whether this request came from ourselves + // (in which case it is a redirect if the url does not match the current one) + this._isApplyingUrl = true; this._navigation.applyPath(navPath); - return this._history.pathAsUrl(this._stringifyPath(navPath)); + this._isApplyingUrl = false; } pushUrl(url) { From cdcdc07c06a2785d22603fdd2d945e88d7428f51 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 19 Oct 2020 12:57:21 +0200 Subject: [PATCH 4/5] fix a crash when switching rooms before the messages have loaded as we were not disposing the timeline view model (but still not leaking though) --- src/domain/session/room/RoomViewModel.js | 10 ++++------ src/domain/session/room/timeline/TimelineViewModel.js | 6 +++++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/domain/session/room/RoomViewModel.js b/src/domain/session/room/RoomViewModel.js index d401c843..188f3d1c 100644 --- a/src/domain/session/room/RoomViewModel.js +++ b/src/domain/session/room/RoomViewModel.js @@ -25,7 +25,6 @@ export class RoomViewModel extends ViewModel { const {room, ownUserId} = options; this._room = room; this._ownUserId = ownUserId; - this._timeline = null; this._timelineVM = null; this._onRoomChange = this._onRoomChange.bind(this); this._timelineError = null; @@ -42,13 +41,12 @@ export class RoomViewModel extends ViewModel { async load() { this._room.on("change", this._onRoomChange); try { - this._timeline = this.track(this._room.openTimeline()); - await this._timeline.load(); - this._timelineVM = new TimelineViewModel(this.childOptions({ + this._timelineVM = this.track(new TimelineViewModel(this.childOptions({ room: this._room, - timeline: this._timeline, + timeline: this._room.openTimeline(), ownUserId: this._ownUserId, - })); + }))); + await this._timelineVM.load(); this.emitChange("timelineViewModel"); } catch (err) { console.error(`room.openTimeline(): ${err.message}:\n${err.stack}`); diff --git a/src/domain/session/room/timeline/TimelineViewModel.js b/src/domain/session/room/timeline/TimelineViewModel.js index 0f7464f8..5faa23b3 100644 --- a/src/domain/session/room/timeline/TimelineViewModel.js +++ b/src/domain/session/room/timeline/TimelineViewModel.js @@ -39,13 +39,17 @@ export class TimelineViewModel extends ViewModel { constructor(options) { super(options); const {room, timeline, ownUserId} = options; - this._timeline = timeline; + this._timeline = this.track(timeline); // once we support sending messages we could do // timeline.entries.concat(timeline.pendingEvents) // for an ObservableList that also contains local echos this._tiles = new TilesCollection(timeline.entries, tilesCreator({room, ownUserId, clock: this.clock})); } + async load() { + await this._timeline.load(); + } + /** * @return {bool} startReached if the start of the timeline was reached */ From 6bf8e976cb05bf728e2aa165946c0eea64ac6f1f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 19 Oct 2020 13:39:19 +0200 Subject: [PATCH 5/5] don't assign timelineVM before loaded, so prop doesn't return it ... if the binding happens to be evaluating (which it was during mount) follow-up from bwindels/fix-crash-switch-rooms-too-fast --- src/domain/session/room/RoomViewModel.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/domain/session/room/RoomViewModel.js b/src/domain/session/room/RoomViewModel.js index 188f3d1c..5aa20a2a 100644 --- a/src/domain/session/room/RoomViewModel.js +++ b/src/domain/session/room/RoomViewModel.js @@ -41,12 +41,13 @@ export class RoomViewModel extends ViewModel { async load() { this._room.on("change", this._onRoomChange); try { - this._timelineVM = this.track(new TimelineViewModel(this.childOptions({ + const timelineVM = this.track(new TimelineViewModel(this.childOptions({ room: this._room, timeline: this._room.openTimeline(), ownUserId: this._ownUserId, }))); - await this._timelineVM.load(); + await timelineVM.load(); + this._timelineVM = timelineVM; this.emitChange("timelineViewModel"); } catch (err) { console.error(`room.openTimeline(): ${err.message}:\n${err.stack}`);