From 0596ca06b156722fbe455a2b10772e5d4697349f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 31 May 2021 11:56:41 +0200 Subject: [PATCH] emit remove before linking up sibling tiles otherwise emitting the update from updatePreviousSibling has the wrong idx --- .../session/room/timeline/TilesCollection.js | 31 +++++++++++++++++-- src/observable/list/ObservableArray.js | 5 +++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/domain/session/room/timeline/TilesCollection.js b/src/domain/session/room/timeline/TilesCollection.js index f8c55e3a..0acb2859 100644 --- a/src/domain/session/room/timeline/TilesCollection.js +++ b/src/domain/session/room/timeline/TilesCollection.js @@ -195,11 +195,14 @@ export class TilesCollection extends BaseObservableList { _removeTile(tileIdx, tile) { const prevTile = this._getTileAtIdx(tileIdx - 1); const nextTile = this._getTileAtIdx(tileIdx + 1); + // applying and emitting the remove should happen + // atomically, as updateNext/PreviousSibling might + // emit an update with the wrong index otherwise this._tiles.splice(tileIdx, 1); - prevTile && prevTile.updateNextSibling(nextTile); - nextTile && nextTile.updatePreviousSibling(prevTile); tile.dispose(); this.emitRemove(tileIdx, tile); + prevTile?.updateNextSibling(nextTile); + nextTile?.updatePreviousSibling(prevTile); } // would also be called when unloading a part of the timeline @@ -301,5 +304,29 @@ export function tests() { entries.insert(1, {n: 7}); assert(receivedAdd); }, + "emit update with correct index in updatePreviousSibling during remove": assert => { + class UpdateOnSiblingTile extends TestTile { + updatePreviousSibling() { + this.update?.(this, "previous"); + } + } + const entries = new ObservableArray([{n: 5}, {n: 10}, {n: 15}]); + const tiles = new TilesCollection(entries, entry => new UpdateOnSiblingTile(entry)); + const events = []; + tiles.subscribe({ + onUpdate(idx, tile) { + assert.equal(idx, 1); + assert.equal(tile.entry.n, 15); + events.push("update"); + }, + onRemove(idx, tile) { + assert.equal(idx, 1); + assert.equal(tile.entry.n, 10); + events.push("remove"); + } + }); + entries.remove(1); + assert.deepEqual(events, ["remove", "update"]); + } } } diff --git a/src/observable/list/ObservableArray.js b/src/observable/list/ObservableArray.js index ca33fcce..37802587 100644 --- a/src/observable/list/ObservableArray.js +++ b/src/observable/list/ObservableArray.js @@ -27,6 +27,11 @@ export class ObservableArray extends BaseObservableList { this.emitAdd(this._items.length - 1, item); } + remove(idx) { + const [item] = this._items.splice(idx, 1); + this.emitRemove(idx, item); + } + insertMany(idx, items) { for(let item of items) { this.insert(idx, item);