From fb273782bf010aecb051cf2882ff52e5381921fa Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 19 Oct 2020 12:55:10 +0200 Subject: [PATCH 1/2] 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 2/2] 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) {