From 9a12d1798ea84d1663507272b48a37e4d9f18cf8 Mon Sep 17 00:00:00 2001
From: Sayam Samal <samal.sayam@gmail.com>
Date: Tue, 13 Feb 2024 16:16:05 +0530
Subject: [PATCH] read_receipts: Fix duplication bug when opening menu
 repeatedly.

The (1) delay in fetching the read receipts data from the api call to
`/json/messages/${message_id}/read_receipts`; followed by the execution
of the success callback function, and the (2) use of `.append()` to
render the modal and user list, together lead to duplication of the read
receipts modal and also the user list inside the read receipts menu.

This commit adds a check to set the read receipts menu contents only if
the read receipts modal for the selected message ID is open by the time
the network request is resolved.

In addition, this commit also uses the `on_shown` hook instead of the
`on_show` hook in the read receipts modal logic, to add a delay in the
calling of the read receipts API, which prevents the stacking of the
requests.
---
 templates/zerver/app/index.html       |  2 ++
 web/src/read_receipts.ts              | 14 +++++++++++---
 web/styles/dark_theme.css             |  2 +-
 web/styles/modal.css                  |  2 +-
 web/templates/read_receipts.hbs       | 14 ++++++--------
 web/templates/read_receipts_modal.hbs |  3 ++-
 6 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/templates/zerver/app/index.html b/templates/zerver/app/index.html
index 2c24692720..bbac12d393 100644
--- a/templates/zerver/app/index.html
+++ b/templates/zerver/app/index.html
@@ -161,6 +161,8 @@
 
 <div id="about-zulip-modal-container"></div>
 
+<div id="read-receipts-modal-container"></div>
+
 <audio id="user-notification-sound-audio">
     <source class="notification-sound-source-ogg" type="audio/ogg" />
     <source class="notification-sound-source-mp3" type="audio/mpeg" />
diff --git a/web/src/read_receipts.ts b/web/src/read_receipts.ts
index 1ac4c4ae01..80a8d37ca4 100644
--- a/web/src/read_receipts.ts
+++ b/web/src/read_receipts.ts
@@ -19,10 +19,10 @@ const read_receipts_api_response_schema = z.object({
 });
 
 export function show_user_list(message_id: number): void {
-    $("body").append(render_read_receipts_modal());
+    $("#read-receipts-modal-container").html(render_read_receipts_modal({message_id}));
     modals.open("read_receipts_modal", {
         autoremove: true,
-        on_show() {
+        on_shown() {
             const message = message_store.get(message_id);
             assert(message !== undefined, "message is undefined");
 
@@ -39,6 +39,14 @@ export function show_user_list(message_id: number): void {
                 void channel.get({
                     url: `/json/messages/${message_id}/read_receipts`,
                     success(raw_data) {
+                        const $modal = $("#read_receipts_modal").filter(
+                            "[data-message-id=" + message_id + "]",
+                        );
+                        // If the read receipts modal for the selected message ID is closed
+                        // by the time we receive the response, return immediately.
+                        if (!$modal.length) {
+                            return;
+                        }
                         const data = read_receipts_api_response_schema.parse(raw_data);
                         const users = data.user_ids.map((id) => {
                             const user = people.get_user_by_id_assert_valid(id);
@@ -78,7 +86,7 @@ export function show_user_list(message_id: number): void {
                             $("#read_receipts_modal .modal__container").addClass(
                                 "showing_read_receipts_list",
                             );
-                            $("#read_receipts_modal .modal__content").append(
+                            $("#read_receipts_modal .read_receipts_list").html(
                                 render_read_receipts(context),
                             );
                             new SimpleBar($("#read_receipts_modal .modal__content")[0]);
diff --git a/web/styles/dark_theme.css b/web/styles/dark_theme.css
index 671cc2ffd6..1f5a7c9005 100644
--- a/web/styles/dark_theme.css
+++ b/web/styles/dark_theme.css
@@ -1226,7 +1226,7 @@
         }
     }
 
-    #read_receipts_modal #read_receipts_list li {
+    #read_receipts_modal .read_receipts_list li {
         &:hover {
             background-color: hsl(0deg 0% 100% / 5%);
         }
diff --git a/web/styles/modal.css b/web/styles/modal.css
index 552964e666..a35e2f59fc 100644
--- a/web/styles/modal.css
+++ b/web/styles/modal.css
@@ -210,7 +210,7 @@
         margin: auto;
     }
 
-    #read_receipts_list {
+    .read_receipts_list {
         margin-left: 0;
 
         & li {
diff --git a/web/templates/read_receipts.hbs b/web/templates/read_receipts.hbs
index 53b7f1837a..0f90abb142 100644
--- a/web/templates/read_receipts.hbs
+++ b/web/templates/read_receipts.hbs
@@ -1,8 +1,6 @@
-<ul id="read_receipts_list">
-    {{#each users}}
-        <li class="view_user_profile" data-user-id="{{user_id}}" tabindex="0" role="button">
-            <img class="read_receipts_user_avatar" src="{{avatar_url}}" />
-            <span>{{full_name}}</span>
-        </li>
-    {{/each}}
-</ul>
+{{#each users}}
+    <li class="view_user_profile" data-user-id="{{user_id}}" tabindex="0" role="button">
+        <img class="read_receipts_user_avatar" src="{{avatar_url}}" />
+        <span>{{full_name}}</span>
+    </li>
+{{/each}}
diff --git a/web/templates/read_receipts_modal.hbs b/web/templates/read_receipts_modal.hbs
index 0567c2a7cc..9976971e7c 100644
--- a/web/templates/read_receipts_modal.hbs
+++ b/web/templates/read_receipts_modal.hbs
@@ -1,4 +1,4 @@
-<div class="micromodal" id="read_receipts_modal" aria-hidden="true">
+<div class="micromodal" id="read_receipts_modal" aria-hidden="true" data-message-id="{{message_id}}">
     <div class="modal__overlay" tabindex="-1">
         <div class="modal__container" role="dialog" aria-modal="true" aria-labelledby="read_receipts_modal_label">
             <header class="modal__header">
@@ -13,6 +13,7 @@
                 <div class="read_receipts_info">
                 </div>
                 <div class="loading_indicator"></div>
+                <ul class="read_receipts_list"></ul>
             </main>
         </div>
     </div>