From 447b98ce6ca73c85c304d4333a66184723b16b39 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 31 May 2021 11:57:17 +0200 Subject: [PATCH] don't use subviews for showing/hiding avatar & sender on continuation --- src/platform/web/ui/general/TemplateView.js | 20 +++++++++++++++ .../session/room/timeline/BaseMessageView.js | 25 ++++++++++++++----- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/platform/web/ui/general/TemplateView.js b/src/platform/web/ui/general/TemplateView.js index fd27eafd..f3425136 100644 --- a/src/platform/web/ui/general/TemplateView.js +++ b/src/platform/web/ui/general/TemplateView.js @@ -344,6 +344,26 @@ class TemplateBuilder { if(predicate, renderFn) { return this.ifView(predicate, vm => new TemplateView(vm, renderFn)); } + + /** You probably are looking for something else, like map or mapView. + This is an escape hatch that allows you to do manual DOM manipulations + as a reaction to a binding change. + This should only be used if the side-effect won't add any bindings, + event handlers, ... + You should not call the TemplateBuilder (e.g. `t.xxx()`) at all from the side effect, + instead use tags from html.js to help you construct any DOM you need. */ + mapSideEffect(mapFn, sideEffect) { + let prevValue = mapFn(this._value); + const binding = () => { + const newValue = mapFn(this._value); + if (prevValue !== newValue) { + sideEffect(newValue, prevValue); + prevValue = newValue; + } + }; + this._addBinding(binding); + sideEffect(prevValue, undefined); + } } diff --git a/src/platform/web/ui/session/room/timeline/BaseMessageView.js b/src/platform/web/ui/session/room/timeline/BaseMessageView.js index b90c89b8..caf9e961 100644 --- a/src/platform/web/ui/session/room/timeline/BaseMessageView.js +++ b/src/platform/web/ui/session/room/timeline/BaseMessageView.js @@ -16,6 +16,7 @@ limitations under the License. */ import {renderStaticAvatar} from "../../../avatar.js"; +import {tag} from "../../../general/html.js"; import {TemplateView} from "../../../general/TemplateView.js"; import {Popup} from "../../../general/Popup.js"; import {Menu} from "../../../general/Menu.js"; @@ -27,20 +28,32 @@ export class BaseMessageView extends TemplateView { } render(t, vm) { - const classes = { + const li = t.li({className: { "Timeline_message": true, own: vm.isOwn, unsent: vm.isUnsent, unverified: vm.isUnverified, continuation: vm => vm.isContinuation, - }; - return t.li({className: classes}, [ - t.if(vm => !vm.isContinuation, () => renderStaticAvatar(vm, 30, "Timeline_messageAvatar")), - t.if(vm => !vm.isContinuation, t => t.div({className: `Timeline_messageSender usercolor${vm.avatarColorNumber}`}, vm.displayName)), + }}, [ this.renderMessageBody(t, vm), // should be after body as it is overlayed on top t.button({className: "Timeline_messageOptions"}, "⋯"), - ]); + ]); + // given that there can be many tiles, we don't add + // unneeded DOM nodes in case of a continuation, and we add it + // with a side-effect binding to not have to create sub views, + // as the avatar or sender doesn't need any bindings or event handlers. + // don't use `t` from within the side-effect callback + t.mapSideEffect(vm => vm.isContinuation, (isContinuation, wasContinuation) => { + if (isContinuation && wasContinuation === false) { + li.removeChild(li.querySelector(".Timeline_messageAvatar")); + li.removeChild(li.querySelector(".Timeline_messageSender")); + } else if (!isContinuation) { + li.appendChild(renderStaticAvatar(vm, 30, "Timeline_messageAvatar")); + li.appendChild(tag.div({className: `Timeline_messageSender usercolor${vm.avatarColorNumber}`}, vm.displayName)); + } + }); + return li; } /* This is called by the parent ListView, which just has 1 listener for the whole list */