From 9745c58144549c8b205f35054880e8ba17e2c39f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 Aug 2020 14:20:54 +0200 Subject: [PATCH 1/8] use readPath in ImageTile --- .../session/room/timeline/tiles/ImageTile.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js index bb9fb5d4..61746097 100644 --- a/src/domain/session/room/timeline/tiles/ImageTile.js +++ b/src/domain/session/room/timeline/tiles/ImageTile.js @@ -15,6 +15,7 @@ limitations under the License. */ import {MessageTile} from "./MessageTile.js"; +import {readPath, Type} from "../../../../../utils/validate.js"; const MAX_HEIGHT = 300; const MAX_WIDTH = 400; @@ -26,19 +27,21 @@ export class ImageTile extends MessageTile { } get thumbnailUrl() { - const mxcUrl = this._getContent().url; - if (mxcUrl) { + try { + const mxcUrl = readPath(this._getContent(), ["url"], Type.String); return this._room.mxcUrlThumbnail(mxcUrl, this.thumbnailWidth, this.thumbnailHeight, "scale"); + } catch (err) { + return null; } - return null; } get url() { - const mxcUrl = this._getContent().url; - if (mxcUrl) { + try { + const mxcUrl = readPath(this._getContent(), ["url"], Type.String); return this._room.mxcUrl(mxcUrl); + } catch (err) { + return null; } - return null; } _scaleFactor() { From cf0af775e3b6fcaa6201dc68f94c32c49e946b3a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 Aug 2020 15:11:39 +0200 Subject: [PATCH 2/8] make SimpleTile inherit from ViewModel to use same update mechanism and have viewmodel infra available for tile --- src/domain/ViewModel.js | 4 ++++ src/domain/session/room/timeline/tiles/GapTile.js | 6 +++--- .../session/room/timeline/tiles/MessageTile.js | 2 +- .../session/room/timeline/tiles/SimpleTile.js | 13 ++++--------- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/domain/ViewModel.js b/src/domain/ViewModel.js index 4f73702a..bc35fabd 100644 --- a/src/domain/ViewModel.js +++ b/src/domain/ViewModel.js @@ -70,6 +70,10 @@ export class ViewModel extends EventEmitter { return result; } + updateOptions(options) { + this._options = Object.assign(this._options, options); + } + emitChange(changedProps) { if (this._options.emitChange) { this._options.emitChange(changedProps); diff --git a/src/domain/session/room/timeline/tiles/GapTile.js b/src/domain/session/room/timeline/tiles/GapTile.js index 3c868a64..e3ab04b1 100644 --- a/src/domain/session/room/timeline/tiles/GapTile.js +++ b/src/domain/session/room/timeline/tiles/GapTile.js @@ -29,16 +29,16 @@ export class GapTile extends SimpleTile { // prevent doing this twice if (!this._loading) { this._loading = true; - this.emitUpdate("isLoading"); + this.emitChange("isLoading"); try { await this._timeline.fillGap(this._entry, 10); } catch (err) { console.error(`timeline.fillGap(): ${err.message}:\n${err.stack}`); this._error = err; - this.emitUpdate("error"); + this.emitChange("error"); } finally { this._loading = false; - this.emitUpdate("isLoading"); + this.emitChange("isLoading"); } } } diff --git a/src/domain/session/room/timeline/tiles/MessageTile.js b/src/domain/session/room/timeline/tiles/MessageTile.js index 802918d7..74ba202c 100644 --- a/src/domain/session/room/timeline/tiles/MessageTile.js +++ b/src/domain/session/room/timeline/tiles/MessageTile.js @@ -62,7 +62,7 @@ export class MessageTile extends SimpleTile { const isContinuation = prev && prev instanceof MessageTile && prev.sender === this.sender; if (isContinuation !== this._isContinuation) { this._isContinuation = isContinuation; - this.emitUpdate("isContinuation"); + this.emitChange("isContinuation"); } } } diff --git a/src/domain/session/room/timeline/tiles/SimpleTile.js b/src/domain/session/room/timeline/tiles/SimpleTile.js index 04ba27d6..2ccb1d17 100644 --- a/src/domain/session/room/timeline/tiles/SimpleTile.js +++ b/src/domain/session/room/timeline/tiles/SimpleTile.js @@ -15,11 +15,12 @@ limitations under the License. */ import {UpdateAction} from "../UpdateAction.js"; +import {ViewModel} from "../../../../ViewModel.js"; -export class SimpleTile { +export class SimpleTile extends ViewModel { constructor({entry}) { + super(); this._entry = entry; - this._emitUpdate = null; } // view model props for all subclasses // hmmm, could also do instanceof ... ? @@ -38,12 +39,6 @@ export class SimpleTile { return false; } - emitUpdate(paramName) { - if (this._emitUpdate) { - this._emitUpdate(this, paramName); - } - } - get internalId() { return this._entry.asEventKey().toString(); } @@ -53,7 +48,7 @@ export class SimpleTile { } // TilesCollection contract below setUpdateEmit(emitUpdate) { - this._emitUpdate = emitUpdate; + this.updateOptions({emitChange: paramName => emitUpdate(this, paramName)}); } get upperEntry() { From 1e4f3319892fe65f9e875cf91fdeffa3c26e7867 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 Aug 2020 15:13:12 +0200 Subject: [PATCH 3/8] fill top gap instead of loading more events from storage --- src/domain/session/room/timeline/TilesCollection.js | 4 ++++ src/domain/session/room/timeline/TimelineViewModel.js | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/domain/session/room/timeline/TilesCollection.js b/src/domain/session/room/timeline/TilesCollection.js index 3b8ea7b9..396aba9e 100644 --- a/src/domain/session/room/timeline/TilesCollection.js +++ b/src/domain/session/room/timeline/TilesCollection.js @@ -201,6 +201,10 @@ export class TilesCollection extends BaseObservableList { get length() { return this._tiles.length; } + + getFirst() { + return this._tiles[0]; + } } import {ObservableArray} from "../../../../observable/list/ObservableArray.js"; diff --git a/src/domain/session/room/timeline/TimelineViewModel.js b/src/domain/session/room/timeline/TimelineViewModel.js index 2ab58610..d1a4947d 100644 --- a/src/domain/session/room/timeline/TimelineViewModel.js +++ b/src/domain/session/room/timeline/TimelineViewModel.js @@ -44,7 +44,12 @@ export class TimelineViewModel { // doesn't fill gaps, only loads stored entries/tiles loadAtTop() { - return this._timeline.loadAtTop(50); + const firstTile = this._tiles.getFirst(); + if (firstTile.shape === "gap") { + return firstTile.fill(); + } else { + return this._timeline.loadAtTop(50); + } } unloadAtTop(tileAmount) { From 56efd7eee00a26aec0a2b35b3954c679ff65ce91 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 Aug 2020 15:22:25 +0200 Subject: [PATCH 4/8] don't load timeline past gaps --- src/matrix/room/timeline/persistence/TimelineReader.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/matrix/room/timeline/persistence/TimelineReader.js b/src/matrix/room/timeline/persistence/TimelineReader.js index ea40f8a4..928d6b64 100644 --- a/src/matrix/room/timeline/persistence/TimelineReader.js +++ b/src/matrix/room/timeline/persistence/TimelineReader.js @@ -60,8 +60,8 @@ export class TimelineReader { let fragmentEntry = new FragmentBoundaryEntry(fragment, direction.isBackward, this._fragmentIdComparer); // append or prepend fragmentEntry, reuse func from GapWriter? directionalAppend(entries, fragmentEntry, direction); - // don't count it in amount perhaps? or do? - if (fragmentEntry.hasLinkedFragment) { + // only continue loading if the fragment boundary can't be backfilled + if (!fragmentEntry.token && fragmentEntry.hasLinkedFragment) { const nextFragment = await fragmentStore.get(this._roomId, fragmentEntry.linkedFragmentId); this._fragmentIdComparer.add(nextFragment); const nextFragmentEntry = new FragmentBoundaryEntry(nextFragment, direction.isForward, this._fragmentIdComparer); From 2cfd38379f09e867cd4567cddc45f45e7a070ccb Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 Aug 2020 15:22:39 +0200 Subject: [PATCH 5/8] change looks of gap tile as it's auto-loaded now --- src/ui/web/css/timeline.css | 15 +++++++++++++++ src/ui/web/session/room/timeline/GapView.js | 8 +++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/ui/web/css/timeline.css b/src/ui/web/css/timeline.css index 69e96e58..4b7f572d 100644 --- a/src/ui/web/css/timeline.css +++ b/src/ui/web/css/timeline.css @@ -67,3 +67,18 @@ limitations under the License. display: flex; align-items: center; } + +.GapView { + visibility: hidden; + display: flex; + padding: 10px 20px; +} + +.GapView.isLoading { + visibility: visible; +} + +.GapView > div { + flex: 1; + margin-left: 10px; +} diff --git a/src/ui/web/session/room/timeline/GapView.js b/src/ui/web/session/room/timeline/GapView.js index b26ee83d..2b23ae3c 100644 --- a/src/ui/web/session/room/timeline/GapView.js +++ b/src/ui/web/session/room/timeline/GapView.js @@ -15,6 +15,7 @@ limitations under the License. */ import {TemplateView} from "../../../general/TemplateView.js"; +import {spinner} from "../../../common.js"; export class GapView extends TemplateView { render(t, vm) { @@ -22,12 +23,9 @@ export class GapView extends TemplateView { GapView: true, isLoading: vm => vm.isLoading }; - const label = (vm.isUp ? "🠝" : "🠟") + " fill gap"; //no binding return t.li({className}, [ - t.button({ - onClick: () => vm.fill(), - disabled: vm => vm.isLoading - }, label), + spinner(t), + t.div(vm.i18n`Loading more messages …`), t.if(vm => vm.error, t.createTemplate(t => t.strong(vm => vm.error))) ]); } From 5ae4a1aae3cc1ff93748fe195d0537d7c21fc365 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 Aug 2020 15:22:55 +0200 Subject: [PATCH 6/8] increase offset to start back-filling --- src/ui/web/session/room/TimelineList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/web/session/room/TimelineList.js b/src/ui/web/session/room/TimelineList.js index 1920290d..37b43348 100644 --- a/src/ui/web/session/room/TimelineList.js +++ b/src/ui/web/session/room/TimelineList.js @@ -39,7 +39,7 @@ export class TimelineList extends ListView { async _onScroll() { const root = this.root(); - if (root.scrollTop === 0 && !this._topLoadingPromise && this._viewModel) { + if (root.scrollTop < 100 && !this._topLoadingPromise && this._viewModel) { const beforeFromBottom = this._distanceFromBottom(); this._topLoadingPromise = this._viewModel.loadAtTop(); await this._topLoadingPromise; From 08de7c35694b6a5b4af6efb8724ff8f2f13d7cc6 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 Aug 2020 16:34:25 +0200 Subject: [PATCH 7/8] loading screen while loading timeline so we can set timelineVM directly to TimelineList --- src/domain/session/room/RoomViewModel.js | 1 + src/ui/web/css/layout.css | 2 +- src/ui/web/css/room.css | 10 +++++++ src/ui/web/session/room/RoomView.js | 21 +++++---------- src/ui/web/session/room/TimelineList.js | 14 +++++----- .../web/session/room/TimelineLoadingView.js | 27 +++++++++++++++++++ 6 files changed, 52 insertions(+), 23 deletions(-) create mode 100644 src/ui/web/session/room/TimelineLoadingView.js diff --git a/src/domain/session/room/RoomViewModel.js b/src/domain/session/room/RoomViewModel.js index c29a4cc8..5b625fe6 100644 --- a/src/domain/session/room/RoomViewModel.js +++ b/src/domain/session/room/RoomViewModel.js @@ -1,5 +1,6 @@ /* Copyright 2020 Bruno Windels +Copyright 2020 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/src/ui/web/css/layout.css b/src/ui/web/css/layout.css index 95fe71da..36385a75 100644 --- a/src/ui/web/css/layout.css +++ b/src/ui/web/css/layout.css @@ -78,7 +78,7 @@ html { height: 100%; } -.TimelinePanel ul { +.TimelinePanel .Timeline, .TimelinePanel .TimelineLoadingView { flex: 1 0 0; } diff --git a/src/ui/web/css/room.css b/src/ui/web/css/room.css index 252313a3..6bf01da7 100644 --- a/src/ui/web/css/room.css +++ b/src/ui/web/css/room.css @@ -73,3 +73,13 @@ limitations under the License. flex: 1; box-sizing: border-box; } + +.TimelineLoadingView { + display: flex; + align-items: center; + justify-content: center; +} + +.TimelineLoadingView div { + margin-left: 10px; +} diff --git a/src/ui/web/session/room/RoomView.js b/src/ui/web/session/room/RoomView.js index 0c7ace9e..de11619a 100644 --- a/src/ui/web/session/room/RoomView.js +++ b/src/ui/web/session/room/RoomView.js @@ -1,5 +1,6 @@ /* Copyright 2020 Bruno Windels +Copyright 2020 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -16,16 +17,11 @@ limitations under the License. import {TemplateView} from "../../general/TemplateView.js"; import {TimelineList} from "./TimelineList.js"; +import {TimelineLoadingView} from "./TimelineLoadingView.js"; import {MessageComposer} from "./MessageComposer.js"; export class RoomView extends TemplateView { - constructor(viewModel) { - super(viewModel); - this._timelineList = null; - } - render(t, vm) { - this._timelineList = new TimelineList(); return t.div({className: "RoomView"}, [ t.div({className: "TimelinePanel"}, [ t.div({className: "RoomHeader"}, [ @@ -36,16 +32,13 @@ export class RoomView extends TemplateView { ]), ]), t.div({className: "RoomView_error"}, vm => vm.error), - t.view(this._timelineList), + t.mapView(vm => vm.timelineViewModel, timelineViewModel => { + return timelineViewModel ? + new TimelineList(timelineViewModel) : + new TimelineLoadingView(vm); // vm is just needed for i18n + }), t.view(new MessageComposer(this.value.composerViewModel)), ]) ]); } - - update(value, prop) { - super.update(value, prop); - if (prop === "timelineViewModel") { - this._timelineList.update({viewModel: this.value.timelineViewModel}); - } - } } diff --git a/src/ui/web/session/room/TimelineList.js b/src/ui/web/session/room/TimelineList.js index 37b43348..9697bce6 100644 --- a/src/ui/web/session/room/TimelineList.js +++ b/src/ui/web/session/room/TimelineList.js @@ -21,8 +21,11 @@ import {ImageView} from "./timeline/ImageView.js"; import {AnnouncementView} from "./timeline/AnnouncementView.js"; export class TimelineList extends ListView { - constructor(options = {}) { - options.className = "Timeline"; + constructor(viewModel) { + const options = { + className: "Timeline", + list: viewModel.tiles, + } super(options, entry => { switch (entry.shape) { case "gap": return new GapView(entry); @@ -34,7 +37,7 @@ export class TimelineList extends ListView { this._atBottom = false; this._onScroll = this._onScroll.bind(this); this._topLoadingPromise = null; - this._viewModel = null; + this._viewModel = viewModel; } async _onScroll() { @@ -50,12 +53,7 @@ export class TimelineList extends ListView { } } - update(attributes) { - if(attributes.viewModel) { - this._viewModel = attributes.viewModel; - attributes.list = attributes.viewModel.tiles; } - super.update(attributes); } mount() { diff --git a/src/ui/web/session/room/TimelineLoadingView.js b/src/ui/web/session/room/TimelineLoadingView.js new file mode 100644 index 00000000..88d07f43 --- /dev/null +++ b/src/ui/web/session/room/TimelineLoadingView.js @@ -0,0 +1,27 @@ +/* +Copyright 2020 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import {TemplateView} from "../../general/TemplateView.js"; +import {spinner} from "../../common.js"; + +export class TimelineLoadingView extends TemplateView { + render(t, vm) { + return t.div({className: "TimelineLoadingView"}, [ + spinner(t), + t.div(vm.i18n`Loading messages…`) + ]); + } +} From b6cbb03eddc3273a67f4638c8e6fa6df47276f22 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 Aug 2020 16:34:58 +0200 Subject: [PATCH 8/8] keep filling gaps while viewport not filled or new content < 100px --- src/ui/web/session/room/TimelineList.js | 49 +++++++++++++++++++------ 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/src/ui/web/session/room/TimelineList.js b/src/ui/web/session/room/TimelineList.js index 9697bce6..49e254cf 100644 --- a/src/ui/web/session/room/TimelineList.js +++ b/src/ui/web/session/room/TimelineList.js @@ -40,19 +40,38 @@ export class TimelineList extends ListView { this._viewModel = viewModel; } - async _onScroll() { - const root = this.root(); - if (root.scrollTop < 100 && !this._topLoadingPromise && this._viewModel) { - const beforeFromBottom = this._distanceFromBottom(); - this._topLoadingPromise = this._viewModel.loadAtTop(); - await this._topLoadingPromise; - const fromBottom = this._distanceFromBottom(); - const amountGrown = fromBottom - beforeFromBottom; - root.scrollTop = root.scrollTop + amountGrown; + async _loadAtTopWhile(predicate) { + try { + while (predicate()) { + // fill, not enough content to fill timeline + this._topLoadingPromise = this._viewModel.loadAtTop(); + await this._topLoadingPromise; + } + } + catch (err) { + //ignore error, as it is handled in the VM + } + finally { this._topLoadingPromise = null; } } + async _onScroll() { + const PAGINATE_OFFSET = 100; + const root = this.root(); + if (root.scrollTop < PAGINATE_OFFSET && !this._topLoadingPromise && this._viewModel) { + // to calculate total amountGrown to check when we stop loading + let beforeContentHeight = root.scrollHeight; + // to adjust scrollTop every time + let lastContentHeight = beforeContentHeight; + // load until pagination offset is reached again + this._loadAtTopWhile(() => { + const contentHeight = root.scrollHeight; + const amountGrown = contentHeight - beforeContentHeight; + root.scrollTop = root.scrollTop + (contentHeight - lastContentHeight); + lastContentHeight = contentHeight; + return amountGrown < PAGINATE_OFFSET; + }); } } @@ -70,7 +89,15 @@ export class TimelineList extends ListView { loadList() { super.loadList(); const root = this.root(); - root.scrollTop = root.scrollHeight; + const {scrollHeight, clientHeight} = root; + if (scrollHeight > clientHeight) { + root.scrollTop = root.scrollHeight; + } + // load while viewport is not filled + this._loadAtTopWhile(() => { + const {scrollHeight, clientHeight} = root; + return scrollHeight <= clientHeight; + }); } onBeforeListChanged() { @@ -84,8 +111,8 @@ export class TimelineList extends ListView { } onListChanged() { + const root = this.root(); if (this._atBottom) { - const root = this.root(); root.scrollTop = root.scrollHeight; } }