From 5f1346568de7ff814799ac43b1981e0ff3417b5f Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 28 Jun 2021 23:18:07 +0530 Subject: [PATCH 01/11] Handle avatar error Signed-off-by: RMidhunSuresh --- src/platform/web/ui/avatar.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/platform/web/ui/avatar.js b/src/platform/web/ui/avatar.js index 8845f887..715a4890 100644 --- a/src/platform/web/ui/avatar.js +++ b/src/platform/web/ui/avatar.js @@ -44,6 +44,11 @@ export class AvatarView extends BaseUpdateView { return false; } + setAvatarError() { + this._avatarError = true; + this.update(this.value); + } + _avatarTitleChanged() { if (this.value.avatarTitle !== this._avatarTitle) { this._avatarTitle = this.value.avatarTitle; @@ -65,6 +70,8 @@ export class AvatarView extends BaseUpdateView { this._avatarLetterChanged(); this._avatarTitleChanged(); this._root = renderStaticAvatar(this.value, this._size); + const image = this._root.firstChild; + image?.addEventListener("error", () => this.setAvatarError()); // takes care of update being called when needed super.mount(options); return this._root; @@ -76,10 +83,10 @@ export class AvatarView extends BaseUpdateView { update(vm) { // important to always call _...changed for every prop - if (this._avatarUrlChanged()) { + if (this._avatarUrlChanged() || this._avatarError) { // avatarColorNumber won't change, it's based on room/user id const bgColorClass = `usercolor${vm.avatarColorNumber}`; - if (vm.avatarUrl(this._size)) { + if (vm.avatarUrl(this._size) && !this._avatarError) { this._root.replaceChild(renderImg(vm, this._size), this._root.firstChild); this._root.classList.remove(bgColorClass); } else { @@ -87,7 +94,7 @@ export class AvatarView extends BaseUpdateView { this._root.classList.add(bgColorClass); } } - const hasAvatar = !!vm.avatarUrl(this._size); + const hasAvatar = !!(vm.avatarUrl(this._size) && !vm._avatarError); if (this._avatarTitleChanged() && hasAvatar) { const img = this._root.firstChild; img.setAttribute("title", vm.avatarTitle); @@ -95,6 +102,7 @@ export class AvatarView extends BaseUpdateView { if (this._avatarLetterChanged() && !hasAvatar) { this._root.firstChild.textContent = vm.avatarLetter; } + if (this._avatarError) { this._avatarError = false;} } } @@ -104,7 +112,7 @@ export class AvatarView extends BaseUpdateView { * @return {Element} */ export function renderStaticAvatar(vm, size, extraClasses = undefined) { - const hasAvatar = !!vm.avatarUrl(size); + const hasAvatar = !!(vm.avatarUrl(size) && !vm.avatarError); let avatarClasses = classNames({ avatar: true, [`size-${size}`]: true, From 8b6ff533e8ed8604619c7cdc8ef41952fb0c9f14 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Tue, 29 Jun 2021 15:38:58 +0530 Subject: [PATCH 02/11] Add and remove opposing event listeners Signed-off-by: RMidhunSuresh --- src/platform/web/ui/avatar.js | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/platform/web/ui/avatar.js b/src/platform/web/ui/avatar.js index 715a4890..0a865963 100644 --- a/src/platform/web/ui/avatar.js +++ b/src/platform/web/ui/avatar.js @@ -44,7 +44,7 @@ export class AvatarView extends BaseUpdateView { return false; } - setAvatarError() { + _setAvatarError() { this._avatarError = true; this.update(this.value); } @@ -57,6 +57,20 @@ export class AvatarView extends BaseUpdateView { return false; } + _addListenersToAvatar(image) { + const handleAvatarError = (e) => { + const image = e.target; + image.removeEventListener("load", removeErrorHandler); + this._setAvatarError(); + }; + const removeErrorHandler = (e) => { + const image = e.target; + image.removeEventListener("error", handleAvatarError); + }; + image?.addEventListener("error", handleAvatarError); + image?.addEventListener("load", removeErrorHandler); + } + _avatarLetterChanged() { if (this.value.avatarLetter !== this._avatarLetter) { this._avatarLetter = this.value.avatarLetter; @@ -71,7 +85,7 @@ export class AvatarView extends BaseUpdateView { this._avatarTitleChanged(); this._root = renderStaticAvatar(this.value, this._size); const image = this._root.firstChild; - image?.addEventListener("error", () => this.setAvatarError()); + this._addListenersToAvatar(image); // takes care of update being called when needed super.mount(options); return this._root; From b42f7e1a361727c9fc6682efcf9d1f5f9ce0614d Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Tue, 29 Jun 2021 19:48:36 +0530 Subject: [PATCH 03/11] remove both handlers Signed-off-by: RMidhunSuresh --- src/platform/web/ui/avatar.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/platform/web/ui/avatar.js b/src/platform/web/ui/avatar.js index 0a865963..d01a026e 100644 --- a/src/platform/web/ui/avatar.js +++ b/src/platform/web/ui/avatar.js @@ -58,17 +58,14 @@ export class AvatarView extends BaseUpdateView { } _addListenersToAvatar(image) { - const handleAvatarError = (e) => { + const imageEventHandler = (e) => { + if(e.type === "error") { this._setAvatarError(); } const image = e.target; - image.removeEventListener("load", removeErrorHandler); - this._setAvatarError(); + image.removeEventListener("error", imageEventHandler); + image.removeEventListener("load", imageEventHandler); }; - const removeErrorHandler = (e) => { - const image = e.target; - image.removeEventListener("error", handleAvatarError); - }; - image?.addEventListener("error", handleAvatarError); - image?.addEventListener("load", removeErrorHandler); + image?.addEventListener("error", imageEventHandler); + image?.addEventListener("load", imageEventHandler); } _avatarLetterChanged() { From bcaf84e54587c77afab254124600209e93652b5d Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Wed, 30 Jun 2021 23:27:46 +0530 Subject: [PATCH 04/11] Revert commits This reverts commit 5f1346568de7ff814799ac43b1981e0ff3417b5f. Signed-off-by: RMidhunSuresh --- src/platform/web/ui/avatar.js | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/platform/web/ui/avatar.js b/src/platform/web/ui/avatar.js index d01a026e..8845f887 100644 --- a/src/platform/web/ui/avatar.js +++ b/src/platform/web/ui/avatar.js @@ -44,11 +44,6 @@ export class AvatarView extends BaseUpdateView { return false; } - _setAvatarError() { - this._avatarError = true; - this.update(this.value); - } - _avatarTitleChanged() { if (this.value.avatarTitle !== this._avatarTitle) { this._avatarTitle = this.value.avatarTitle; @@ -57,17 +52,6 @@ export class AvatarView extends BaseUpdateView { return false; } - _addListenersToAvatar(image) { - const imageEventHandler = (e) => { - if(e.type === "error") { this._setAvatarError(); } - const image = e.target; - image.removeEventListener("error", imageEventHandler); - image.removeEventListener("load", imageEventHandler); - }; - image?.addEventListener("error", imageEventHandler); - image?.addEventListener("load", imageEventHandler); - } - _avatarLetterChanged() { if (this.value.avatarLetter !== this._avatarLetter) { this._avatarLetter = this.value.avatarLetter; @@ -81,8 +65,6 @@ export class AvatarView extends BaseUpdateView { this._avatarLetterChanged(); this._avatarTitleChanged(); this._root = renderStaticAvatar(this.value, this._size); - const image = this._root.firstChild; - this._addListenersToAvatar(image); // takes care of update being called when needed super.mount(options); return this._root; @@ -94,10 +76,10 @@ export class AvatarView extends BaseUpdateView { update(vm) { // important to always call _...changed for every prop - if (this._avatarUrlChanged() || this._avatarError) { + if (this._avatarUrlChanged()) { // avatarColorNumber won't change, it's based on room/user id const bgColorClass = `usercolor${vm.avatarColorNumber}`; - if (vm.avatarUrl(this._size) && !this._avatarError) { + if (vm.avatarUrl(this._size)) { this._root.replaceChild(renderImg(vm, this._size), this._root.firstChild); this._root.classList.remove(bgColorClass); } else { @@ -105,7 +87,7 @@ export class AvatarView extends BaseUpdateView { this._root.classList.add(bgColorClass); } } - const hasAvatar = !!(vm.avatarUrl(this._size) && !vm._avatarError); + const hasAvatar = !!vm.avatarUrl(this._size); if (this._avatarTitleChanged() && hasAvatar) { const img = this._root.firstChild; img.setAttribute("title", vm.avatarTitle); @@ -113,7 +95,6 @@ export class AvatarView extends BaseUpdateView { if (this._avatarLetterChanged() && !hasAvatar) { this._root.firstChild.textContent = vm.avatarLetter; } - if (this._avatarError) { this._avatarError = false;} } } @@ -123,7 +104,7 @@ export class AvatarView extends BaseUpdateView { * @return {Element} */ export function renderStaticAvatar(vm, size, extraClasses = undefined) { - const hasAvatar = !!(vm.avatarUrl(size) && !vm.avatarError); + const hasAvatar = !!vm.avatarUrl(size); let avatarClasses = classNames({ avatar: true, [`size-${size}`]: true, From b469c4299f0ac60f7c70c4245f416738b25183bd Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Wed, 30 Jun 2021 23:30:44 +0530 Subject: [PATCH 05/11] implement new approach Signed-off-by: RMidhunSuresh --- src/platform/web/Platform.js | 2 ++ src/platform/web/ui/avatar.js | 20 ++++++++++++++++++-- src/platform/web/ui/css/avatar.css | 4 ++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/platform/web/Platform.js b/src/platform/web/Platform.js index fea17ba1..3d89be9a 100644 --- a/src/platform/web/Platform.js +++ b/src/platform/web/Platform.js @@ -36,6 +36,7 @@ import {BlobHandle} from "./dom/BlobHandle.js"; import {hasReadPixelPermission, ImageHandle, VideoHandle} from "./dom/ImageHandle.js"; import {downloadInIframe} from "./dom/download.js"; import {Disposables} from "../../utils/Disposables.js"; +import {handleAvatarError} from "./ui/avatar.js"; function addScript(src) { return new Promise(function (resolve, reject) { @@ -189,6 +190,7 @@ export class Platform { this._disposables.track(disposable); } } + this._container.addEventListener("error", handleAvatarError, true); window.__hydrogenViewModel = vm; const view = new RootView(vm); this._container.appendChild(view.mount()); diff --git a/src/platform/web/ui/avatar.js b/src/platform/web/ui/avatar.js index 8845f887..093cfd69 100644 --- a/src/platform/web/ui/avatar.js +++ b/src/platform/web/ui/avatar.js @@ -108,16 +108,32 @@ export function renderStaticAvatar(vm, size, extraClasses = undefined) { let avatarClasses = classNames({ avatar: true, [`size-${size}`]: true, - [`usercolor${vm.avatarColorNumber}`]: !hasAvatar, + [`usercolor${vm.avatarColorNumber}`]: true, + ['has-image']: true }); if (extraClasses) { avatarClasses += ` ${extraClasses}`; } const avatarContent = hasAvatar ? renderImg(vm, size) : text(vm.avatarLetter); - return tag.div({className: avatarClasses}, [avatarContent]); + return tag.div({className: avatarClasses, 'data-avatar-letter': vm.avatarLetter}, [avatarContent]); } function renderImg(vm, size) { const sizeStr = size.toString(); return tag.img({src: vm.avatarUrl(size), width: sizeStr, height: sizeStr, title: vm.avatarTitle}); } + +function isAvatarEvent(e) { + const element = e.target; + const parent = element.parentElement; + return element.tagName === "IMG" && parent.classList.contains("avatar"); +} + +export function handleAvatarError(e) { + if (!isAvatarEvent(e)) { return; } + const parent = e.target.parentElement; + const avatarLetter = parent.getAttribute("data-avatar-letter"); + const letterNode = document.createTextNode(avatarLetter); + parent.appendChild(letterNode); + parent.classList.remove("has-image"); +} diff --git a/src/platform/web/ui/css/avatar.css b/src/platform/web/ui/css/avatar.css index d369f85f..85d370b7 100644 --- a/src/platform/web/ui/css/avatar.css +++ b/src/platform/web/ui/css/avatar.css @@ -31,6 +31,10 @@ limitations under the License. speak: none; } +.hydrogen .avatar:not(.has-image) img{ + display: none; +} + .hydrogen .avatar img { width: 100%; height: 100%; From 168b1d6247ab57e2f65b8f80aaaaf0bd0d511526 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Wed, 30 Jun 2021 23:34:23 +0530 Subject: [PATCH 06/11] Move AvatarView to separate file Signed-off-by: RMidhunSuresh --- src/platform/web/ui/AvatarView.js | 84 ++++++++++++++++++ src/platform/web/ui/avatar.js | 85 +------------------ .../web/ui/session/leftpanel/RoomTileView.js | 2 +- .../ui/session/rightpanel/RoomDetailsView.js | 2 +- src/platform/web/ui/session/room/RoomView.js | 2 +- 5 files changed, 88 insertions(+), 87 deletions(-) create mode 100644 src/platform/web/ui/AvatarView.js diff --git a/src/platform/web/ui/AvatarView.js b/src/platform/web/ui/AvatarView.js new file mode 100644 index 00000000..22381838 --- /dev/null +++ b/src/platform/web/ui/AvatarView.js @@ -0,0 +1,84 @@ +import {BaseUpdateView} from "./general/BaseUpdateView.js"; +import {renderStaticAvatar, renderImg} from "./avatar.js"; +import {text} from "./general/html.js"; + +/* +optimization to not use a sub view when changing between img and text +because there can be many many instances of this view +*/ + +export class AvatarView extends BaseUpdateView { + /** + * @param {ViewModel} value view model with {avatarUrl, avatarColorNumber, avatarTitle, avatarLetter} + * @param {Number} size + */ + constructor(value, size) { + super(value); + this._root = null; + this._avatarUrl = null; + this._avatarTitle = null; + this._avatarLetter = null; + this._size = size; + } + + _avatarUrlChanged() { + if (this.value.avatarUrl(this._size) !== this._avatarUrl) { + this._avatarUrl = this.value.avatarUrl(this._size); + return true; + } + return false; + } + + _avatarTitleChanged() { + if (this.value.avatarTitle !== this._avatarTitle) { + this._avatarTitle = this.value.avatarTitle; + return true; + } + return false; + } + + _avatarLetterChanged() { + if (this.value.avatarLetter !== this._avatarLetter) { + this._avatarLetter = this.value.avatarLetter; + return true; + } + return false; + } + + mount(options) { + this._avatarUrlChanged(); + this._avatarLetterChanged(); + this._avatarTitleChanged(); + this._root = renderStaticAvatar(this.value, this._size); + // takes care of update being called when needed + super.mount(options); + return this._root; + } + + root() { + return this._root; + } + + update(vm) { + // important to always call _...changed for every prop + if (this._avatarUrlChanged()) { + // avatarColorNumber won't change, it's based on room/user id + const bgColorClass = `usercolor${vm.avatarColorNumber}`; + if (vm.avatarUrl(this._size)) { + this._root.replaceChild(renderImg(vm, this._size), this._root.firstChild); + this._root.classList.remove(bgColorClass); + } else { + this._root.replaceChild(text(vm.avatarLetter), this._root.firstChild); + this._root.classList.add(bgColorClass); + } + } + const hasAvatar = !!vm.avatarUrl(this._size); + if (this._avatarTitleChanged() && hasAvatar) { + const img = this._root.firstChild; + img.setAttribute("title", vm.avatarTitle); + } + if (this._avatarLetterChanged() && !hasAvatar) { + this._root.firstChild.textContent = vm.avatarLetter; + } + } +} diff --git a/src/platform/web/ui/avatar.js b/src/platform/web/ui/avatar.js index 093cfd69..bda44db0 100644 --- a/src/platform/web/ui/avatar.js +++ b/src/platform/web/ui/avatar.js @@ -15,89 +15,6 @@ limitations under the License. */ import {tag, text, classNames} from "./general/html.js"; -import {BaseUpdateView} from "./general/BaseUpdateView.js"; - -/* -optimization to not use a sub view when changing between img and text -because there can be many many instances of this view -*/ - -export class AvatarView extends BaseUpdateView { - /** - * @param {ViewModel} value view model with {avatarUrl, avatarColorNumber, avatarTitle, avatarLetter} - * @param {Number} size - */ - constructor(value, size) { - super(value); - this._root = null; - this._avatarUrl = null; - this._avatarTitle = null; - this._avatarLetter = null; - this._size = size; - } - - _avatarUrlChanged() { - if (this.value.avatarUrl(this._size) !== this._avatarUrl) { - this._avatarUrl = this.value.avatarUrl(this._size); - return true; - } - return false; - } - - _avatarTitleChanged() { - if (this.value.avatarTitle !== this._avatarTitle) { - this._avatarTitle = this.value.avatarTitle; - return true; - } - return false; - } - - _avatarLetterChanged() { - if (this.value.avatarLetter !== this._avatarLetter) { - this._avatarLetter = this.value.avatarLetter; - return true; - } - return false; - } - - mount(options) { - this._avatarUrlChanged(); - this._avatarLetterChanged(); - this._avatarTitleChanged(); - this._root = renderStaticAvatar(this.value, this._size); - // takes care of update being called when needed - super.mount(options); - return this._root; - } - - root() { - return this._root; - } - - update(vm) { - // important to always call _...changed for every prop - if (this._avatarUrlChanged()) { - // avatarColorNumber won't change, it's based on room/user id - const bgColorClass = `usercolor${vm.avatarColorNumber}`; - if (vm.avatarUrl(this._size)) { - this._root.replaceChild(renderImg(vm, this._size), this._root.firstChild); - this._root.classList.remove(bgColorClass); - } else { - this._root.replaceChild(text(vm.avatarLetter), this._root.firstChild); - this._root.classList.add(bgColorClass); - } - } - const hasAvatar = !!vm.avatarUrl(this._size); - if (this._avatarTitleChanged() && hasAvatar) { - const img = this._root.firstChild; - img.setAttribute("title", vm.avatarTitle); - } - if (this._avatarLetterChanged() && !hasAvatar) { - this._root.firstChild.textContent = vm.avatarLetter; - } - } -} - /** * @param {Object} vm view model with {avatarUrl, avatarColorNumber, avatarTitle, avatarLetter} * @param {Number} size @@ -118,7 +35,7 @@ export function renderStaticAvatar(vm, size, extraClasses = undefined) { return tag.div({className: avatarClasses, 'data-avatar-letter': vm.avatarLetter}, [avatarContent]); } -function renderImg(vm, size) { +export function renderImg(vm, size) { const sizeStr = size.toString(); return tag.img({src: vm.avatarUrl(size), width: sizeStr, height: sizeStr, title: vm.avatarTitle}); } diff --git a/src/platform/web/ui/session/leftpanel/RoomTileView.js b/src/platform/web/ui/session/leftpanel/RoomTileView.js index 84b38b62..228addba 100644 --- a/src/platform/web/ui/session/leftpanel/RoomTileView.js +++ b/src/platform/web/ui/session/leftpanel/RoomTileView.js @@ -16,7 +16,7 @@ limitations under the License. */ import {TemplateView} from "../../general/TemplateView.js"; -import {AvatarView} from "../../avatar.js"; +import {AvatarView} from "../../AvatarView.js"; export class RoomTileView extends TemplateView { render(t, vm) { diff --git a/src/platform/web/ui/session/rightpanel/RoomDetailsView.js b/src/platform/web/ui/session/rightpanel/RoomDetailsView.js index a6d1a81f..8357b722 100644 --- a/src/platform/web/ui/session/rightpanel/RoomDetailsView.js +++ b/src/platform/web/ui/session/rightpanel/RoomDetailsView.js @@ -1,6 +1,6 @@ import {TemplateView} from "../../general/TemplateView.js"; import {classNames, tag} from "../../general/html.js"; -import {AvatarView} from "../../avatar.js"; +import {AvatarView} from "../../AvatarView.js"; export class RoomDetailsView extends TemplateView { render(t, vm) { diff --git a/src/platform/web/ui/session/room/RoomView.js b/src/platform/web/ui/session/room/RoomView.js index f8e84f87..ccad448f 100644 --- a/src/platform/web/ui/session/room/RoomView.js +++ b/src/platform/web/ui/session/room/RoomView.js @@ -22,7 +22,7 @@ import {TimelineList} from "./TimelineList.js"; import {TimelineLoadingView} from "./TimelineLoadingView.js"; import {MessageComposer} from "./MessageComposer.js"; import {RoomArchivedView} from "./RoomArchivedView.js"; -import {AvatarView} from "../../avatar.js"; +import {AvatarView} from "../../AvatarView.js"; export class RoomView extends TemplateView { constructor(options) { From 9ed6cd57f3a4c7a845746fc969f47a9bfbd92ae4 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Thu, 1 Jul 2021 00:01:38 +0530 Subject: [PATCH 07/11] use textContent Signed-off-by: RMidhunSuresh --- src/platform/web/ui/avatar.js | 7 ++----- src/platform/web/ui/css/avatar.css | 4 ---- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/platform/web/ui/avatar.js b/src/platform/web/ui/avatar.js index bda44db0..36d9b172 100644 --- a/src/platform/web/ui/avatar.js +++ b/src/platform/web/ui/avatar.js @@ -25,8 +25,7 @@ export function renderStaticAvatar(vm, size, extraClasses = undefined) { let avatarClasses = classNames({ avatar: true, [`size-${size}`]: true, - [`usercolor${vm.avatarColorNumber}`]: true, - ['has-image']: true + [`usercolor${vm.avatarColorNumber}`]: true }); if (extraClasses) { avatarClasses += ` ${extraClasses}`; @@ -50,7 +49,5 @@ export function handleAvatarError(e) { if (!isAvatarEvent(e)) { return; } const parent = e.target.parentElement; const avatarLetter = parent.getAttribute("data-avatar-letter"); - const letterNode = document.createTextNode(avatarLetter); - parent.appendChild(letterNode); - parent.classList.remove("has-image"); + parent.textContent = avatarLetter; } diff --git a/src/platform/web/ui/css/avatar.css b/src/platform/web/ui/css/avatar.css index 85d370b7..d369f85f 100644 --- a/src/platform/web/ui/css/avatar.css +++ b/src/platform/web/ui/css/avatar.css @@ -31,10 +31,6 @@ limitations under the License. speak: none; } -.hydrogen .avatar:not(.has-image) img{ - display: none; -} - .hydrogen .avatar img { width: 100%; height: 100%; From 03a913629f0ef44ad120a5abc3721322c7dc237e Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Thu, 1 Jul 2021 15:25:28 +0530 Subject: [PATCH 08/11] Pass color as data attribute Signed-off-by: RMidhunSuresh --- src/platform/web/ui/avatar.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/platform/web/ui/avatar.js b/src/platform/web/ui/avatar.js index 36d9b172..c1660e70 100644 --- a/src/platform/web/ui/avatar.js +++ b/src/platform/web/ui/avatar.js @@ -25,13 +25,13 @@ export function renderStaticAvatar(vm, size, extraClasses = undefined) { let avatarClasses = classNames({ avatar: true, [`size-${size}`]: true, - [`usercolor${vm.avatarColorNumber}`]: true + [`usercolor${vm.avatarColorNumber}`]: !hasAvatar }); if (extraClasses) { avatarClasses += ` ${extraClasses}`; } const avatarContent = hasAvatar ? renderImg(vm, size) : text(vm.avatarLetter); - return tag.div({className: avatarClasses, 'data-avatar-letter': vm.avatarLetter}, [avatarContent]); + return tag.div({className: avatarClasses, 'data-avatar-letter': vm.avatarLetter, 'data-avatar-color': vm.avatarColorNumber}, [avatarContent]); } export function renderImg(vm, size) { @@ -48,6 +48,8 @@ function isAvatarEvent(e) { export function handleAvatarError(e) { if (!isAvatarEvent(e)) { return; } const parent = e.target.parentElement; + const avatarColorNumber = parent.getAttribute("data-avatar-color"); + parent.classList.add(`usercolor${avatarColorNumber}`); const avatarLetter = parent.getAttribute("data-avatar-letter"); parent.textContent = avatarLetter; } From 93e77a3fcd313d360f0258201645c3c923a96c89 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Thu, 1 Jul 2021 15:41:40 +0530 Subject: [PATCH 09/11] Only add attribute if we have avatar Signed-off-by: RMidhunSuresh --- src/platform/web/ui/avatar.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/platform/web/ui/avatar.js b/src/platform/web/ui/avatar.js index c1660e70..2e2b0142 100644 --- a/src/platform/web/ui/avatar.js +++ b/src/platform/web/ui/avatar.js @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {tag, text, classNames} from "./general/html.js"; +import {tag, text, classNames, setAttribute} from "./general/html.js"; /** * @param {Object} vm view model with {avatarUrl, avatarColorNumber, avatarTitle, avatarLetter} * @param {Number} size @@ -31,7 +31,12 @@ export function renderStaticAvatar(vm, size, extraClasses = undefined) { avatarClasses += ` ${extraClasses}`; } const avatarContent = hasAvatar ? renderImg(vm, size) : text(vm.avatarLetter); - return tag.div({className: avatarClasses, 'data-avatar-letter': vm.avatarLetter, 'data-avatar-color': vm.avatarColorNumber}, [avatarContent]); + const avatar = tag.div({className: avatarClasses}, [avatarContent]); + if (hasAvatar) { + setAttribute(avatar, "data-avatar-letter", vm.avatarLetter); + setAttribute(avatar, "data-avatar-color", vm.avatarColorNumber); + } + return avatar; } export function renderImg(vm, size) { From b8c01272f4ca3a679dd6e4084d78cf343a637442 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Thu, 1 Jul 2021 15:42:07 +0530 Subject: [PATCH 10/11] remove listener on dispose Signed-off-by: RMidhunSuresh --- src/platform/web/Platform.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/platform/web/Platform.js b/src/platform/web/Platform.js index 3d89be9a..49a90dd5 100644 --- a/src/platform/web/Platform.js +++ b/src/platform/web/Platform.js @@ -191,6 +191,7 @@ export class Platform { } } this._container.addEventListener("error", handleAvatarError, true); + this._disposables.track(() => this._container.removeEventListener("error", handleAvatarError, true)); window.__hydrogenViewModel = vm; const view = new RootView(vm); this._container.appendChild(view.mount()); From 191613adbecfcd89e667970ebd4c28435267a83c Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Thu, 1 Jul 2021 19:21:54 +0530 Subject: [PATCH 11/11] Make changes - use textContent where possible - make sure we have an image before adding title Signed-off-by: RMidhunSuresh --- src/platform/web/ui/AvatarView.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/platform/web/ui/AvatarView.js b/src/platform/web/ui/AvatarView.js index 22381838..1f6f2736 100644 --- a/src/platform/web/ui/AvatarView.js +++ b/src/platform/web/ui/AvatarView.js @@ -68,17 +68,19 @@ export class AvatarView extends BaseUpdateView { this._root.replaceChild(renderImg(vm, this._size), this._root.firstChild); this._root.classList.remove(bgColorClass); } else { - this._root.replaceChild(text(vm.avatarLetter), this._root.firstChild); + this._root.textContent = vm.avatarLetter; this._root.classList.add(bgColorClass); } } const hasAvatar = !!vm.avatarUrl(this._size); if (this._avatarTitleChanged() && hasAvatar) { - const img = this._root.firstChild; - img.setAttribute("title", vm.avatarTitle); + const element = this._root.firstChild; + if (element.tagName === "IMG") { + element.setAttribute("title", vm.avatarTitle); + } } if (this._avatarLetterChanged() && !hasAvatar) { - this._root.firstChild.textContent = vm.avatarLetter; + this._root.textContent = vm.avatarLetter; } } }