From f72910822666471267e77d3c2192d2b5f7304c16 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Sat, 21 Mar 2020 14:28:09 +0100 Subject: [PATCH] pass emit update fn through setter so we control when tile can update --- .../session/room/timeline/TilesCollection.js | 35 ++++++++++++++----- .../session/room/timeline/tiles/SimpleTile.js | 13 +++++-- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/domain/session/room/timeline/TilesCollection.js b/src/domain/session/room/timeline/TilesCollection.js index de2cb324..cb2621d6 100644 --- a/src/domain/session/room/timeline/TilesCollection.js +++ b/src/domain/session/room/timeline/TilesCollection.js @@ -2,6 +2,10 @@ import BaseObservableList from "../../../../observable/list/BaseObservableList.j import sortedIndex from "../../../../utils/sortedIndex.js"; // maps 1..n entries to 0..1 tile. Entries are what is stored in the timeline, either an event or fragmentboundary +// for now, tileCreator should be stable in whether it returns a tile or not. +// e.g. the decision to create a tile or not should be based on properties +// not updated later on (e.g. event type) +// also see big comment in onUpdate export default class TilesCollection extends BaseObservableList { constructor(entries, tileCreator) { super(); @@ -28,7 +32,7 @@ export default class TilesCollection extends BaseObservableList { let currentTile = null; for (let entry of this._entries) { if (!currentTile || !currentTile.tryIncludeEntry(entry)) { - currentTile = this._tileCreator(entry, this._emitSpontanousUpdate); + currentTile = this._tileCreator(entry); if (currentTile) { this._tiles.push(currentTile); } @@ -45,6 +49,11 @@ export default class TilesCollection extends BaseObservableList { if (prevTile) { prevTile.updateNextSibling(null); } + // now everything is wired up, + // allow tiles to emit updates + for (const tile of this._tiles) { + tile.setUpdateEmit(this._emitSpontanousUpdate); + } } _findTileIdx(entry) { @@ -93,10 +102,11 @@ export default class TilesCollection extends BaseObservableList { return; } - const newTile = this._tileCreator(entry, this._emitSpontanousUpdate); + const newTile = this._tileCreator(entry); if (newTile) { if (prevTile) { prevTile.updateNextSibling(newTile); + // this emits an update while the add hasn't been emitted yet newTile.updatePreviousSibling(prevTile); } if (nextTile) { @@ -105,6 +115,9 @@ export default class TilesCollection extends BaseObservableList { } this._tiles.splice(tileIdx, 0, newTile); this.emitAdd(tileIdx, newTile); + // add event is emitted, now the tile + // can emit updates + newTile.setUpdateEmit(this._emitSpontanousUpdate); } // find position by sort key // ask siblings to be included? both? yes, twice: a (insert c here) b, ask a(c), if yes ask b(a), else ask b(c)? if yes then b(a)? @@ -141,6 +154,7 @@ export default class TilesCollection extends BaseObservableList { this._tiles.splice(tileIdx, 1); prevTile && prevTile.updateNextSibling(nextTile); nextTile && nextTile.updatePreviousSibling(prevTile); + tile.setUpdateEmit(null); this.emitRemove(tileIdx, tile); } @@ -177,10 +191,12 @@ import ObservableArray from "../../../../observable/list/ObservableArray.js"; import UpdateAction from "./UpdateAction.js"; export function tests() { - class TestTile { - constructor(entry, update) { + constructor(entry) { this.entry = entry; + this.update = null; + } + setUpdateEmit(update) { this.update = update; } tryIncludeEntry() { @@ -211,15 +227,15 @@ export function tests() { class UpdateOnSiblingTile extends TestTile { updateNextSibling() { // this happens with isContinuation - this.update(this, "next"); + this.update && this.update(this, "next"); } updatePreviousSibling() { // this happens with isContinuation - this.update(this, "previous"); + this.update && this.update(this, "previous"); } } const entries = new ObservableArray([{n: 5}, {n: 10}]); - const tiles = new TilesCollection(entries, (e, u) => new UpdateOnSiblingTile(e, u)); + const tiles = new TilesCollection(entries, entry => new UpdateOnSiblingTile(entry)); let receivedAdd = false; tiles.subscribe({ onAdd(idx, tile) { @@ -227,8 +243,9 @@ export function tests() { receivedAdd = true; }, onUpdate(idx, tile) { - assert(tile.entry.n, 7); - assert(!receivedAdd, "receiving update before add"); + if (tile.entry.n === 7) { + assert(!receivedAdd, "receiving update before add"); + } } }); entries.insert(1, {n: 7}); diff --git a/src/domain/session/room/timeline/tiles/SimpleTile.js b/src/domain/session/room/timeline/tiles/SimpleTile.js index 8733c0dc..7b1ed91f 100644 --- a/src/domain/session/room/timeline/tiles/SimpleTile.js +++ b/src/domain/session/room/timeline/tiles/SimpleTile.js @@ -1,9 +1,9 @@ import UpdateAction from "../UpdateAction.js"; export default class SimpleTile { - constructor({entry, emitUpdate}) { + constructor({entry}) { this._entry = entry; - this._emitUpdate = emitUpdate; + this._emitUpdate = null; } // view model props for all subclasses // hmmm, could also do instanceof ... ? @@ -23,7 +23,9 @@ export default class SimpleTile { } emitUpdate(paramName) { - this._emitUpdate(this, paramName); + if (this._emitUpdate) { + this._emitUpdate(this, paramName); + } } get internalId() { @@ -33,6 +35,11 @@ export default class SimpleTile { get isPending() { return this._entry.isPending; } + // TilesCollection contract below + setUpdateEmit(emitUpdate) { + this._emitUpdate = emitUpdate; + } + get upperEntry() { return this._entry; }