From b3cd2a0e03161fdfe02240d38c893e6f834d8194 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 7 Sep 2021 17:48:49 +0200 Subject: [PATCH] express the visible range with EventKeys rather than list indices This is less ambiguous in case the DOM and the ObservableList would be out of sync. --- src/domain/session/room/ComposerViewModel.js | 2 +- .../room/timeline/TimelineViewModel.js | 9 ++-- .../session/room/timeline/tiles/SimpleTile.js | 4 +- src/platform/web/ui/general/ListView.ts | 2 +- .../web/ui/session/room/TimelineView.ts | 41 +++++++++++++------ .../session/room/timeline/AnnouncementView.js | 3 ++ .../session/room/timeline/BaseMessageView.js | 3 ++ .../web/ui/session/room/timeline/GapView.js | 6 +++ 8 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/domain/session/room/ComposerViewModel.js b/src/domain/session/room/ComposerViewModel.js index ba4627ca..dad0fd68 100644 --- a/src/domain/session/room/ComposerViewModel.js +++ b/src/domain/session/room/ComposerViewModel.js @@ -25,7 +25,7 @@ export class ComposerViewModel extends ViewModel { } setReplyingTo(entry) { - const changed = this._replyVM?.internalId !== entry?.asEventKey().toString(); + const changed = this._replyVM?.id?.equals(entry?.asEventKey()); if (changed) { this._replyVM = this.disposeTracked(this._replyVM); if (entry) { diff --git a/src/domain/session/room/timeline/TimelineViewModel.js b/src/domain/session/room/timeline/TimelineViewModel.js index 84c3d4c9..c4c428d6 100644 --- a/src/domain/session/room/timeline/TimelineViewModel.js +++ b/src/domain/session/room/timeline/TimelineViewModel.js @@ -34,6 +34,8 @@ when loading, it just reads events from a sortkey backwards or forwards... import {TilesCollection} from "./TilesCollection.js"; import {ViewModel} from "../../../ViewModel.js"; + + export class TimelineViewModel extends ViewModel { constructor(options) { super(options); @@ -43,11 +45,8 @@ export class TimelineViewModel extends ViewModel { this._timeline.loadAtTop(50); } - setVisibleTileRange(idx, len) { - console.log("setVisibleTileRange", idx, len); - if (idx < 5) { - this._timeline.loadAtTop(10); - } + setVisibleTileRange(startId, endId) { + console.log("setVisibleTileRange", startId, endId); } get tiles() { diff --git a/src/domain/session/room/timeline/tiles/SimpleTile.js b/src/domain/session/room/timeline/tiles/SimpleTile.js index 3c370b72..412d9737 100644 --- a/src/domain/session/room/timeline/tiles/SimpleTile.js +++ b/src/domain/session/room/timeline/tiles/SimpleTile.js @@ -40,8 +40,8 @@ export class SimpleTile extends ViewModel { return false; } - get internalId() { - return this._entry.asEventKey().toString(); + get id() { + return this._entry.asEventKey(); } get isPending() { diff --git a/src/platform/web/ui/general/ListView.ts b/src/platform/web/ui/general/ListView.ts index 58ed9db3..2c45fb1c 100644 --- a/src/platform/web/ui/general/ListView.ts +++ b/src/platform/web/ui/general/ListView.ts @@ -176,7 +176,7 @@ export class ListView implements UIView { } } - protected getChildInstanceByIndex(idx: number): V | undefined { + public getChildInstanceByIndex(idx: number): V | undefined { return this._childInstances?.[idx]; } } diff --git a/src/platform/web/ui/session/room/TimelineView.ts b/src/platform/web/ui/session/room/TimelineView.ts index 6050b02a..2faad17c 100644 --- a/src/platform/web/ui/session/room/TimelineView.ts +++ b/src/platform/web/ui/session/room/TimelineView.ts @@ -26,7 +26,7 @@ import {AnnouncementView} from "./timeline/AnnouncementView.js"; import {RedactedView} from "./timeline/RedactedView.js"; import {SimpleTile} from "../../../../../domain/session/room/timeline/tiles/SimpleTile.js"; import {TimelineViewModel} from "../../../../../domain/session/room/timeline/TimelineViewModel.js"; -import {BaseObservableList as ObservableList} from "../../../../../../observable/list/BaseObservableList.js"; +import {BaseObservableList as ObservableList} from "../../../../../observable/list/BaseObservableList.js"; type TileView = GapView | AnnouncementView | TextMessageView | ImageView | VideoView | FileView | MissingAttachmentView | RedactedView; @@ -59,7 +59,8 @@ function findFirstNodeIndexAtOrBelow(tiles: HTMLElement, top: number, startIndex return i; } } - return -1; + // return first item if nothing matched before + return 0; } export class TimelineView extends TemplateView { @@ -67,20 +68,25 @@ export class TimelineView extends TemplateView { private anchoredNode?: HTMLElement; private anchoredBottom: number = 0; private stickToBottom: boolean = true; + private tilesView?: TilesListView; render(t: TemplateBuilder, vm: TimelineViewModel) { + this.tilesView = new TilesListView(vm.tiles, () => this.restoreScrollPosition()); return t.div({className: "Timeline bottom-aligned-scroll", onScroll: () => this.onScroll()}, [ - t.view(new TilesListView(vm.tiles, () => this._restoreScrollPosition())) + t.view(this.tilesView) ]); } - private _restoreScrollPosition() { + private restoreScrollPosition() { const timeline = this.root() as HTMLElement; - const tiles = timeline.firstElementChild as HTMLElement; + const tiles = this.tilesView!.root() as HTMLElement; const missingTilesHeight = timeline.clientHeight - tiles.clientHeight; if (missingTilesHeight > 0) { tiles.style.setProperty("margin-top", `${missingTilesHeight}px`); + // we don't have enough tiles to fill the viewport, so set all as visible + const len = this.value.tiles.length; + this.updateVisibleRange(0, len - 1); } else { tiles.style.removeProperty("margin-top"); if (this.stickToBottom) { @@ -102,21 +108,30 @@ export class TimelineView extends TemplateView { private onScroll(): void { const timeline = this.root() as HTMLElement; const {scrollHeight, scrollTop, clientHeight} = timeline; - const tiles = timeline.firstElementChild as HTMLElement; + const tiles = this.tilesView!.root() as HTMLElement; + let bottomNodeIndex; this.stickToBottom = Math.abs(scrollHeight - (scrollTop + clientHeight)) < 5; - if (!this.stickToBottom) { - // save bottom node position + if (this.stickToBottom) { + const len = this.value.tiles.length; + bottomNodeIndex = len - 1; + } else { const viewportBottom = scrollTop + clientHeight; const anchoredNodeIndex = findFirstNodeIndexAtOrBelow(tiles, viewportBottom); - let topNodeIndex = findFirstNodeIndexAtOrBelow(tiles, scrollTop, anchoredNodeIndex); - if (topNodeIndex === -1) { - topNodeIndex = 0; - } this.anchoredNode = tiles.childNodes[anchoredNodeIndex] as HTMLElement; this.anchoredNode.classList.add("pinned"); this.anchoredBottom = bottom(this.anchoredNode!); - this.value.setVisibleTileRange(topNodeIndex, anchoredNodeIndex - topNodeIndex); + bottomNodeIndex = anchoredNodeIndex; + } + let topNodeIndex = findFirstNodeIndexAtOrBelow(tiles, scrollTop, bottomNodeIndex); + this.updateVisibleRange(topNodeIndex, bottomNodeIndex); + } + + private updateVisibleRange(startIndex: number, endIndex: number) { + const firstVisibleChild = this.tilesView!.getChildInstanceByIndex(startIndex); + const lastVisibleChild = this.tilesView!.getChildInstanceByIndex(endIndex); + if (firstVisibleChild && lastVisibleChild) { + this.value.setVisibleTileRange(firstVisibleChild.id, lastVisibleChild.id); } } } diff --git a/src/platform/web/ui/session/room/timeline/AnnouncementView.js b/src/platform/web/ui/session/room/timeline/AnnouncementView.js index 2dd58b32..c7f47073 100644 --- a/src/platform/web/ui/session/room/timeline/AnnouncementView.js +++ b/src/platform/web/ui/session/room/timeline/AnnouncementView.js @@ -23,4 +23,7 @@ export class AnnouncementView extends TemplateView { /* This is called by the parent ListView, which just has 1 listener for the whole list */ onClick() {} + + /** Used by TimelineView to get the id of a tile when setting the visible range */ + get id() { return this.value.id; } } diff --git a/src/platform/web/ui/session/room/timeline/BaseMessageView.js b/src/platform/web/ui/session/room/timeline/BaseMessageView.js index 9469d201..8f6fa22f 100644 --- a/src/platform/web/ui/session/room/timeline/BaseMessageView.js +++ b/src/platform/web/ui/session/room/timeline/BaseMessageView.js @@ -85,6 +85,9 @@ export class BaseMessageView extends TemplateView { this._toggleMenu(evt.target); } } + + /** Used by TimelineView to get the id of a tile when setting the visible range */ + get id() { return this.value.id; } _toggleMenu(button) { if (this._menuPopup && this._menuPopup.isOpen) { diff --git a/src/platform/web/ui/session/room/timeline/GapView.js b/src/platform/web/ui/session/room/timeline/GapView.js index 1e6e0af0..0055d72c 100644 --- a/src/platform/web/ui/session/room/timeline/GapView.js +++ b/src/platform/web/ui/session/room/timeline/GapView.js @@ -29,4 +29,10 @@ export class GapView extends TemplateView { t.if(vm => vm.error, t => t.strong(vm => vm.error)) ]); } + + /* This is called by the parent ListView, which just has 1 listener for the whole list */ + onClick() {} + + /** Used by TimelineView to get the id of a tile when setting the visible range */ + get id() { return this.value.id; } }