From 139a87de994463f4ce95efb498832be4b5b2a49c Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Sun, 8 May 2022 19:14:51 +0530 Subject: [PATCH 1/4] Pass a copy of the options to the tiles --- src/domain/session/room/timeline/TilesCollection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/domain/session/room/timeline/TilesCollection.js b/src/domain/session/room/timeline/TilesCollection.js index 173b0cf6..d8dd660d 100644 --- a/src/domain/session/room/timeline/TilesCollection.js +++ b/src/domain/session/room/timeline/TilesCollection.js @@ -35,7 +35,7 @@ export class TilesCollection extends BaseObservableList { _createTile(entry) { const Tile = this._tileOptions.tileClassForEntry(entry); if (Tile) { - return new Tile(entry, this._tileOptions); + return new Tile(entry, { ...this._tileOptions }); } } From 6beff7e55255a8c6635dd855d2906113d360c261 Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Mon, 9 May 2022 14:09:45 +0200 Subject: [PATCH 2/4] override emitChange so no need to clone option object for all tiles instead, we don't store the emitChange in the options but rather on the tile itself. --- .../session/room/timeline/TilesCollection.js | 2 +- .../session/room/timeline/tiles/SimpleTile.js | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/domain/session/room/timeline/TilesCollection.js b/src/domain/session/room/timeline/TilesCollection.js index d8dd660d..173b0cf6 100644 --- a/src/domain/session/room/timeline/TilesCollection.js +++ b/src/domain/session/room/timeline/TilesCollection.js @@ -35,7 +35,7 @@ export class TilesCollection extends BaseObservableList { _createTile(entry) { const Tile = this._tileOptions.tileClassForEntry(entry); if (Tile) { - return new Tile(entry, { ...this._tileOptions }); + return new Tile(entry, this._tileOptions); } } diff --git a/src/domain/session/room/timeline/tiles/SimpleTile.js b/src/domain/session/room/timeline/tiles/SimpleTile.js index b8a7121e..04141576 100644 --- a/src/domain/session/room/timeline/tiles/SimpleTile.js +++ b/src/domain/session/room/timeline/tiles/SimpleTile.js @@ -22,6 +22,7 @@ export class SimpleTile extends ViewModel { constructor(entry, options) { super(options); this._entry = entry; + this._emitUpdate = undefined; } // view model props for all subclasses // hmmm, could also do instanceof ... ? @@ -67,16 +68,20 @@ export class SimpleTile extends ViewModel { // TilesCollection contract below setUpdateEmit(emitUpdate) { - this.updateOptions({emitChange: paramName => { + this._emitUpdate = emitUpdate; + } + + /** overrides the emitChange in ViewModel to also emit the update over the tiles collection */ + emitChange(changedProps) { + if (this._emitUpdate) { // it can happen that after some network call // we switched away from the room and the response // comes in, triggering an emitChange in a tile that // has been disposed already (and hence the change // callback has been cleared by dispose) We should just ignore this. - if (emitUpdate) { - emitUpdate(this, paramName); - } - }}); + this._emitUpdate(this, changedProps); + } + super.emitChange(changedProps); } get upperEntry() { From 3888291758a82fd59d20c31549861a048b2fe67c Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Mon, 9 May 2022 14:10:50 +0200 Subject: [PATCH 3/4] updateOptions is unused,not the best idea since options is/can be shared --- src/domain/ViewModel.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/domain/ViewModel.ts b/src/domain/ViewModel.ts index 8b8581ae..8c12829a 100644 --- a/src/domain/ViewModel.ts +++ b/src/domain/ViewModel.ts @@ -115,10 +115,6 @@ export class ViewModel extends EventEmitter<{change return result; } - updateOptions(options: O): void { - this._options = Object.assign(this._options, options); - } - emitChange(changedProps: any): void { if (this._options.emitChange) { this._options.emitChange(changedProps); From e903d3a6a47200d950b180ffab6d7d9a8226b3bf Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Mon, 9 May 2022 14:12:31 +0200 Subject: [PATCH 4/4] mark options as readonly --- src/domain/ViewModel.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/domain/ViewModel.ts b/src/domain/ViewModel.ts index 8c12829a..0bc52f6e 100644 --- a/src/domain/ViewModel.ts +++ b/src/domain/ViewModel.ts @@ -40,9 +40,9 @@ export type Options = { export class ViewModel extends EventEmitter<{change: never}> { private disposables?: Disposables; private _isDisposed = false; - private _options: O; + private _options: Readonly; - constructor(options: O) { + constructor(options: Readonly) { super(); this._options = options; } @@ -51,7 +51,7 @@ export class ViewModel extends EventEmitter<{change return Object.assign({}, this._options, explicitOptions); } - get options(): O { return this._options; } + get options(): Readonly { return this._options; } // makes it easier to pass through dependencies of a sub-view model getOption(name: N): O[N] {