Repository: zulip/zulip
Pull request: Defer computing last_edit_timestr until tooltip renders.
Earlier, we used to compute last_edit_timestr as data-tippy-content when rendering the whole message feed.

This commit changes the behaviour by computing the `last_edit_timestr` when a user hovers over the message_edit_notice.

Fixes: zulip#27240.

<!-- Describe your pull request here.-->

<!-- If the PR makes UI changes, always include one or more still screenshots to demonstrate your changes. If it seems helpful, add a screen capture of the new functionality as well.

Tooling tips: https://zulip.readthedocs.io/en/latest/tutorials/screenshot-and-gif-software.html
-->

**Screenshots and screen captures:**

<details>
<summary>Self-review checklist</summary>

<!-- Prior to submitting a PR, follow our step-by-step guide to review your own code:
https://zulip.readthedocs.io/en/latest/contributing/code-reviewing.html#how-to-review-code -->

<!-- Once you create the PR, check off all the steps below that you have completed.
If any of these steps are not relevant or you have not completed, leave them unchecked.-->

- [x] [Self-reviewed](https://zulip.readthedocs.io/en/latest/contributing/code-reviewing.html#how-to-review-code) the changes for clarity and maintainability
      (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

- [x] Explains differences from previous plans (e.g., issue description).
- [x] Highlights technical choices and bugs encountered.
- [x] Calls out remaining decisions and concerns.
- [ ] Automated tests verify logic where appropriate.

Individual commits are ready for review (see [commit discipline](https://zulip.readthedocs.io/en/latest/contributing/commit-discipline.html)).

- [x] Each commit is a coherent idea.
- [x] Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

- [x] Visual appearance of the changes.
- [x] Responsiveness and internationalization.
- [x] Strings and tooltips.
- [x] End-to-end functionality of buttons, interactions and flows.
- [x] Corner cases, error conditions, and easily imagined bugs.
</details>


Problem statement:
Defer rendering last_edit_timestr until actually hovering over a message
At present, we have logic that computes the `last_edit_timestr` to display as a Tippy tooltip when hovering over a message date element:

```
$ cat web/templates/edited_notice.hbs
{{#if msg/local_edit_timestamp}}
<div class="message_edit_notice" data-tippy-content="{{t 'Last edited {last_edit_timestr}.' }}">
    {{t "SAVING" }}
</div>
{{else if moved}}
<div class="message_edit_notice" data-tippy-content="{{t 'Last moved {last_edit_timestr}.' }}">
    {{t "MOVED" }}
</div>
{{else}}
<div class="message_edit_notice" data-tippy-content="{{t 'Last edited {last_edit_timestr}.' }}">
    {{t "EDITED" }}
</div>
{{/if}}
```

When rendering a message feed containing a lot of messages that have been moved, computing all these localized time renderings in  a loop is expensive. We should start deferring that work to when the user actually hovers it.

We already have a function that is called on such a hover that could be extended to call `_get_msg_timestring` on the message object after looking it up in the message list data structure.

```
    message_list_tooltip(".message_edit_notice", {                                                      
        trigger: "mouseenter",                                                                          
        delay: LONG_HOVER_DELAY,                                                                        
        popperOptions: {                                                                                
            modifiers: [                                                                                
                {                                                                                       
                    name: "flip",                                                                       
                    options: {                                                                          
                        fallbackPlacements: "bottom",                                                   
                    },                                                                                  
                },                                                                                      
            ],                                                                                          
        },                                                                                              
        onShow(instance) {                                                                              
            const $elem = $(instance.reference);                                                        
            const edited_notice_str = $elem.attr("data-tippy-content");                                 
            instance.setContent(                                                                        
                parse_html(                                                                             
                    render_message_edit_notice_tooltip({                                                
                        edited_notice_str,                                                              
                        realm_allow_edit_history: page_params.realm_allow_edit_history,                 
                    }),                                                                                 
                ),                                                                                      
            );                                                                                          
        },                                                                                              
        onHidden(instance) {                                                                            
            instance.destroy();                                                                         
        },                                                                                              
    });                                                                                                 
```

Hints from issue discussion:

Commit message: tooltips: Defer computing last_edit_timestr until tooltip renders.

Earlier, we used to compute last_edit_timestr as data-tippy-content
when rendering the whole message feed.

This commit changes the behaviour by computing the `last_edit_timestr`
when a user hovers over the message_edit_notice.

Fixes: zulip#27240.

Patch to review:
diff --git a/web/src/message_list_tooltips.ts b/web/src/message_list_tooltips.ts
index 58992b8a4a234..6081d3db54336 100644
--- a/web/src/message_list_tooltips.ts
+++ b/web/src/message_list_tooltips.ts
@@ -6,7 +6,9 @@ import render_message_edit_notice_tooltip from "../templates/message_edit_notice
 import render_message_inline_image_tooltip from "../templates/message_inline_image_tooltip.hbs";
 import render_narrow_tooltip from "../templates/narrow_tooltip.hbs";
 
+import {$t} from "./i18n";
 import * as message_lists from "./message_lists";
+import type {Message} from "./message_store";
 import * as popover_menus from "./popover_menus";
 import * as reactions from "./reactions";
 import * as rows from "./rows";
@@ -120,6 +122,30 @@ export function destroy_all_message_list_tooltips(): void {
     message_list_tippy_instances.clear();
 }
 
+function get_last_edit_timestr(message: Message): string {
+    let last_edit_timestamp;
+    if (message.local_edit_timestamp !== undefined) {
+        last_edit_timestamp = message.local_edit_timestamp;
+    } else {
+        last_edit_timestamp = message.last_edit_timestamp!;
+    }
+    const last_edit_time = new Date(last_edit_timestamp * 1000);
+    let date = timerender.render_date(last_edit_time).textContent;
+    // If the date is today or yesterday, we don't want to show the date as capitalized.
+    // Thus, we need to check if the date string contains a digit or not using regex,
+    // since any other date except today/yesterday will contain a digit.
+    if (date && !/\d/.test(date)) {
+        date = date.toLowerCase();
+    }
+    return $t(
+        {defaultMessage: "{date} at {time}"},
+        {
+            date,
+            time: timerender.stringify_time(last_edit_time),
+        },
+    );
+}
+
 export function initialize(): void {
     message_list_tooltip(".tippy-narrow-tooltip", {
         delay: LONG_HOVER_DELAY,
@@ -338,11 +364,16 @@ export function initialize(): void {
         },
         onShow(instance) {
             const $elem = $(instance.reference);
-            const edited_notice_str = $elem.attr("data-tippy-content");
+            assert(message_lists.current !== undefined);
+            const message_id = Number($elem.closest(".message_row").attr("data-message-id"));
+            const message_container = message_lists.current.view.message_containers.get(message_id);
+            assert(message_container !== undefined);
+            const last_edit_timestr = get_last_edit_timestr(message_container.msg);
             instance.setContent(
                 parse_html(
                     render_message_edit_notice_tooltip({
-                        edited_notice_str,
+                        message_container,
+                        last_edit_timestr,
                         realm_allow_edit_history: realm.realm_allow_edit_history,
                     }),
                 ),
diff --git a/web/src/message_list_view.ts b/web/src/message_list_view.ts
index 931aac5a1396d..5b97f94d0fd32 100644
--- a/web/src/message_list_view.ts
+++ b/web/src/message_list_view.ts
@@ -52,7 +52,7 @@ export type MessageContainer = {
     include_recipient: boolean;
     include_sender: boolean;
     is_hidden: boolean;
-    last_edit_timestr: string | undefined;
+    last_edit_timestamp: number | undefined;
     mention_classname: string | undefined;
     message_edit_notices_in_left_col: boolean;
     message_edit_notices_alongside_sender: boolean;
@@ -150,7 +150,7 @@ function same_recipient(a: MessageContainer | undefined, b: MessageContainer | u
 
 function analyze_edit_history(
     message: Message,
-    last_edit_timestr: string | undefined,
+    last_edit_timestamp: number | undefined,
 ): {
     edited: boolean;
     moved: boolean;
@@ -201,9 +201,9 @@ function analyze_edit_history(
                 moved = true;
             }
         }
-    } else if (last_edit_timestr !== undefined) {
+    } else if (last_edit_timestamp !== undefined) {
         // When the edit_history is disabled for the organization, we do not receive the edit_history
-        // variable in the message object. In this case, we will check if the last_edit_timestr is
+        // variable in the message object. In this case, we will check if the last_edit_timestamp is
         // available or not. Since we don't have the edit_history, we can't determine if the message
         // was moved or edited. Therefore, we simply mark the messages as edited.
         edited = true;
@@ -582,59 +582,34 @@ export class MessageListView {
         );
     }
 
-    _get_msg_timestring(message: Message): string | undefined {
+    _get_message_edited_vars(message: Message): {
+        last_edit_timestamp: number | undefined;
+        moved: boolean;
+        modified: boolean;
+    } {
         let last_edit_timestamp;
         if (message.local_edit_timestamp !== undefined) {
             last_edit_timestamp = message.local_edit_timestamp;
         } else {
             last_edit_timestamp = message.last_edit_timestamp;
         }
-        if (last_edit_timestamp !== undefined) {
-            const last_edit_time = new Date(last_edit_timestamp * 1000);
-            let date = timerender.render_date(last_edit_time).textContent;
-            // If the date is today or yesterday, we don't want to show the date as capitalized.
-            // Thus, we need to check if the date string contains a digit or not using regex,
-            // since any other date except today/yesterday will contain a digit.
-            if (date && !/\d/.test(date)) {
-                date = date.toLowerCase();
-            }
-            return $t(
-                {defaultMessage: "{date} at {time}"},
-                {
-                    date,
-                    time: timerender.stringify_time(last_edit_time),
-                },
-            );
-        }
-        return undefined;
-    }
-
-    _get_message_edited_vars(message: Message): {
-        last_edit_timestr: string | undefined;
-        moved: boolean;
-        modified: boolean;
-    } {
-        const last_edit_timestr = this._get_msg_timestring(message);
-        const edit_history_details = analyze_edit_history(message, last_edit_timestr);
+        const edit_history_details = analyze_edit_history(message, last_edit_timestamp);
 
-        if (
-            last_edit_timestr === undefined ||
-            !(edit_history_details.moved || edit_history_details.edited)
-        ) {
+        if (!last_edit_timestamp || !(edit_history_details.moved || edit_history_details.edited)) {
             // For messages whose edit history at most includes
             // resolving topics, we don't display an EDITED/MOVED
             // notice at all. (The message actions popover will still
             // display an edit history option, so you can see when it
             // was marked as resolved if you need to).
             return {
-                last_edit_timestr: undefined,
+                last_edit_timestamp: undefined,
                 moved: false,
                 modified: false,
             };
         }
 
         return {
-            last_edit_timestr,
+            last_edit_timestamp,
             moved: edit_history_details.moved && !edit_history_details.edited,
             modified: true,
         };
@@ -659,7 +634,7 @@ export class MessageListView {
         mention_classname: string | undefined;
         include_sender: boolean;
         status_message: string | false;
-        last_edit_timestr: string | undefined;
+        last_edit_timestamp: number | undefined;
         moved: boolean;
         modified: boolean;
     } {
diff --git a/web/templates/edited_notice.hbs b/web/templates/edited_notice.hbs
index d7e25e4e190bd..a4931cfccce9e 100644
--- a/web/templates/edited_notice.hbs
+++ b/web/templates/edited_notice.hbs
@@ -1,14 +1,14 @@
 {{#if modified}}
     {{#if msg/local_edit_timestamp}}
-        <div class="message_edit_notice" data-tippy-content="{{t 'Last edited {last_edit_timestr}.'}}">
+        <div class="message_edit_notice">
             {{t "SAVING"}}
         </div>
     {{else if moved}}
-        <div class="message_edit_notice" data-tippy-content="{{t 'Last moved {last_edit_timestr}.'}}">
+        <div class="message_edit_notice">
             {{t "MOVED"}}
         </div>
     {{else}}
-        <div class="message_edit_notice" data-tippy-content="{{t 'Last edited {last_edit_timestr}.'}}">
+        <div class="message_edit_notice">
             {{t "EDITED"}}
         </div>
     {{/if}}
diff --git a/web/templates/message_edit_notice_tooltip.hbs b/web/templates/message_edit_notice_tooltip.hbs
index 69ff0dd48917b..9f0f6418ff100 100644
--- a/web/templates/message_edit_notice_tooltip.hbs
+++ b/web/templates/message_edit_notice_tooltip.hbs
@@ -1,7 +1,11 @@
 <div>
     <div>{{t "View edit history"}}</div>
     {{#if realm_allow_edit_history}}
-        <div class="tooltip-inner-content italic">{{edited_notice_str}}</div>
+        {{#if message_container/moved}}
+            <div class="tooltip-inner-content italic">{{t 'Last moved {last_edit_timestr}.'}}</div>
+        {{else}}
+            <div class="tooltip-inner-content italic">{{t 'Last edited {last_edit_timestr}.'}}</div>
+        {{/if}}
     {{/if}}
 </div>
 {{tooltip_hotkey_hints "Shift" "H"}}


Code review comments:
diff_hunk: @@ -1,7 +1,11 @@
 <div>
     <div>{{t "View edit history"}}</div>
     {{#if realm_allow_edit_history}}
path: web/templates/message_edit_notice_tooltip.hbs
text: @user2:
This preexisting check is in the wrong place. Looks like @user1 introduced this in abece73c5a580e0bc6a27c7603974c1e281df6dd, which swapped the strings around without bringing the conditionals along.