From e63ba2075f565a952b3ae5eebcd21beda4fad836 Mon Sep 17 00:00:00 2001
From: JC Brand <jc@opkode.com>
Date: Tue, 13 Dec 2022 09:54:13 +0100
Subject: [PATCH] Use `repeat` directive to render roster and MUC occupant
 items

If we don't use `repeat`, a DOM node may be reused with different state
(e.g. the `model` it receives originally changes upon next render).

https://lit.dev/docs/templates/lists/#when-to-use-map-or-repeat

Fixes #2816
---
 CHANGES.md                                     |  1 +
 src/headless/plugins/disco/tests/disco.js      |  2 +-
 src/plugins/muc-views/templates/muc-list.js    |  4 ++--
 src/plugins/muc-views/templates/muc-sidebar.js |  7 ++++---
 src/plugins/rosterview/templates/group.js      |  3 ++-
 src/plugins/rosterview/templates/roster.js     | 12 +++---------
 6 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index a5ad03b76..7538cf6ad 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- #2816 Chat highlight behaves odd
 - #2925 File upload is not always enabled
 - Add a "Add to Contacts" button in MUC occupant modals
 
diff --git a/src/headless/plugins/disco/tests/disco.js b/src/headless/plugins/disco/tests/disco.js
index 5cf038ddf..b5b38c2a6 100644
--- a/src/headless/plugins/disco/tests/disco.js
+++ b/src/headless/plugins/disco/tests/disco.js
@@ -4,7 +4,7 @@ describe("Service Discovery", function () {
 
     describe("Whenever a server is queried for its features", function () {
 
-        fit("stores the features it receives",
+        it("stores the features it receives",
             mock.initConverse(
                 ['discoInitialized'], {},
                 async function (_converse) {
diff --git a/src/plugins/muc-views/templates/muc-list.js b/src/plugins/muc-views/templates/muc-list.js
index 9f2d0c3d0..918cd0252 100644
--- a/src/plugins/muc-views/templates/muc-list.js
+++ b/src/plugins/muc-views/templates/muc-list.js
@@ -54,9 +54,9 @@ export default (o) => {
     return html`
         ${o.show_form ? form(o) : '' }
         <ul class="available-chatrooms list-group">
-            ${ o.loading_items ? html`<li class="list-group-item"> ${spinner()} </li>` : '' }
+            ${ o.loading_items ? html`<li class="list-group-item"> ${ spinner() } </li>` : '' }
             ${ o.feedback_text ? html`<li class="list-group-item active">${ o.feedback_text }</li>` : '' }
-            ${repeat(o.items, item => item.jid, item => tpl_item(o, item))}
+            ${ repeat(o.items, (item) => item.jid, (item) => tpl_item(o, item)) }
         </ul>
     `;
 }
diff --git a/src/plugins/muc-views/templates/muc-sidebar.js b/src/plugins/muc-views/templates/muc-sidebar.js
index e0f3b9c97..b48174357 100644
--- a/src/plugins/muc-views/templates/muc-sidebar.js
+++ b/src/plugins/muc-views/templates/muc-sidebar.js
@@ -1,6 +1,7 @@
-import { html } from "lit";
-import { __ } from 'i18n';
 import tpl_occupant from "./occupant.js";
+import { __ } from 'i18n';
+import { html } from "lit";
+import { repeat } from 'lit/directives/repeat.js';
 
 
 export default (o) => {
@@ -15,6 +16,6 @@ export default (o) => {
             </div>
         </div>
         <div class="dragresize dragresize-occupants-left"></div>
-        <ul class="occupant-list">${o.occupants.map(occ => tpl_occupant(occ, o))}</ul>
+        <ul class="occupant-list">${ repeat(o.occupants, (occ) => occ.get('jid'), (occ) => tpl_occupant(occ, o)) }</ul>
     `;
 }
diff --git a/src/plugins/rosterview/templates/group.js b/src/plugins/rosterview/templates/group.js
index 9078120d3..562bfe63d 100644
--- a/src/plugins/rosterview/templates/group.js
+++ b/src/plugins/rosterview/templates/group.js
@@ -3,6 +3,7 @@ import { __ } from 'i18n';
 import { _converse, converse } from "@converse/headless/core";
 import { html } from "lit";
 import { isUniView } from '@converse/headless/utils/core.js';
+import { repeat } from 'lit/directives/repeat.js';
 import { toggleGroup } from '../utils.js';
 
 const { u } = converse.env;
@@ -56,7 +57,7 @@ export default  (o) => {
                 <converse-icon color="var(--chat-head-color-dark)" size="1em" class="fa ${ (collapsed.includes(o.name)) ? 'fa-caret-right' : 'fa-caret-down' }"></converse-icon> ${o.name}
             </a>
             <ul class="items-list roster-group-contacts ${ (collapsed.includes(o.name)) ? 'collapsed' : '' }" data-group="${o.name}">
-                ${ o.contacts.map(renderContact) }
+                ${ repeat(o.contacts, (c) => c.get('jid'), renderContact) }
             </ul>
         </div>`;
 }
diff --git a/src/plugins/rosterview/templates/roster.js b/src/plugins/rosterview/templates/roster.js
index a15642787..17773461a 100644
--- a/src/plugins/rosterview/templates/roster.js
+++ b/src/plugins/rosterview/templates/roster.js
@@ -44,19 +44,13 @@ export default (el) => {
                     <converse-icon class="fa fa-user-plus right" size="1.25em"></converse-icon>
                 </a>` : '' }
         </div>
+
         <div class="list-container roster-contacts ${ is_closed ? 'hidden' : '' }">
             <converse-roster-filter @update=${() => el.requestUpdate()}></converse-roster-filter>
-            ${ repeat(groupnames, n => n, name => {
+            ${ repeat(groupnames, (n) => n, (name) => {
                 const contacts = contacts_map[name].filter(c => shouldShowContact(c, name));
                 contacts.sort(contactsComparator);
-                if (contacts.length) {
-                    return tpl_group({
-                        'contacts': contacts,
-                        'name': name,
-                    });
-                } else {
-                    return '';
-                }
+                return contacts.length ? tpl_group({ contacts, name }) : '';
             }) }
         </div>
     `;