message_edit: Reimplement message history modal as an overlay.

We redesign the message history modal to make it look similar to the drafts and scheduled messages, using the shared styling/rendering logic for that those existing elements to have a less goofy widget. Fixes #28695.
2025-11-01 20:44:04 +00:00 · 2024-03-01 19:35:41 +05:30
parent 3324721eb7
commit e7adde96e0
11 changed files with 220 additions and 101 deletions
--- a/templates/zerver/app/index.html
+++ b/templates/zerver/app/index.html
@@ -147,6 +147,7 @@
 <div id="scheduled_messages_overlay_container"></div>
 <div id="settings_overlay_container" class="overlay" data-overlay="settings" aria-hidden="true">
 </div>
+<div id="message-edit-history-overlay-container"></div>
 <div class="informational-overlays overlay new-style" data-overlay="informationalOverlays" aria-hidden="true">
    <div class="overlay-content overlay-container">
        <div class="overlay-tabs">
--- a/web/src/hotkey.js
+++ b/web/src/hotkey.js
@@ -755,6 +755,10 @@ export function process_hotkey(e, hotkey) {
                scheduled_messages_overlay_ui.handle_keyboard_events(event_name);
                return true;
            }
+            if (overlays.message_edit_history_open()) {
+                message_edit_history.handle_keyboard_events(event_name);
+                return true;
+            }
    }

    if (hotkey.message_view_only && overlays.any_active()) {
@@ -1161,8 +1165,8 @@ export function process_hotkey(e, hotkey) {
        }
        case "view_edit_history": {
            if (realm.realm_allow_edit_history) {
-                message_edit_history.show_history(msg);
-                $("#message-history-cancel").trigger("focus");
+                message_edit_history.fetch_and_render_message_history(msg);
+                $("#message-history-overlay .exit-sign").trigger("focus");
                return true;
            }
            return false;
--- a/web/src/message_edit_history.ts
+++ b/web/src/message_edit_history.ts
@@ -3,36 +3,40 @@ import assert from "minimalistic-assert";
 import {z} from "zod";

 import render_message_edit_history from "../templates/message_edit_history.hbs";
-import render_message_history_modal from "../templates/message_history_modal.hbs";
+import render_message_history_overlay from "../templates/message_history_overlay.hbs";

+import {exit_overlay} from "./browser_history";
 import * as channel from "./channel";
-import * as dialog_widget from "./dialog_widget";
 import {$t, $t_html} from "./i18n";
 import * as message_lists from "./message_lists";
 import type {Message} from "./message_store";
+import * as messages_overlay_ui from "./messages_overlay_ui";
+import * as overlays from "./overlays";
 import {page_params} from "./page_params";
 import * as people from "./people";
 import * as rendered_markdown from "./rendered_markdown";
 import * as rows from "./rows";
 import * as spectators from "./spectators";
 import {realm} from "./state_data";
+import {get_recipient_bar_color} from "./stream_color";
+import {get_color} from "./stream_data";
 import * as sub_store from "./sub_store";
-import {is_same_day} from "./time_zone_util";
 import * as timerender from "./timerender";
 import * as ui_report from "./ui_report";
-import {user_settings} from "./user_settings";

 type EditHistoryEntry = {
-    timestamp: string;
-    display_date: string;
-    show_date_row: boolean;
+    edited_at_time: string;
    edited_by_notice: string;
+    timestamp: number; // require to set data-message-id for overlay message row
+    is_stream: boolean;
+    recipient_bar_color?: string;
    body_to_render?: string;
    topic_edited?: boolean;
    prev_topic?: string;
    new_topic?: string;
    stream_changed?: boolean;
    prev_stream?: string;
+    prev_stream_id?: number;
    new_stream?: string;
 };

@@ -54,7 +58,37 @@ const server_message_history_schema = z.object({
    ),
 });

+// This will be used to handle up and down keyws
+const keyboard_handling_context: messages_overlay_ui.Context = {
+    items_container_selector: "message-edit-history-container",
+    items_list_selector: "message-edit-history-list",
+    row_item_selector: "overlay-message-row",
+    box_item_selector: "overlay-message-info-box",
+    id_attribute_name: "data-message-edit-history-id",
+    get_items_ids(): number[] {
+        const edited_messages_ids: number[] = [];
+        const $message_history_list: JQuery = $(
+            "#message-history-overlay .message-edit-history-list",
+        );
+        for (const message of $message_history_list.children()) {
+            const data_message_edit_history_id = $(message).attr("data-message-edit-history-id");
+            assert(data_message_edit_history_id !== undefined);
+            edited_messages_ids.push(Number(data_message_edit_history_id));
+        }
+        return edited_messages_ids;
+    },
+    on_enter() {
+        return;
+    },
+    on_delete() {
+        return;
+    },
+};
+
 export function fetch_and_render_message_history(message: Message): void {
+    $("#message-edit-history-overlay-container").empty();
+    $("#message-edit-history-overlay-container").append(render_message_history_overlay());
+    open_overlay();
    void channel.get({
        url: "/json/messages/" + message.id + "/history",
        data: {message_id: JSON.stringify(message.id)},
@@ -62,22 +96,11 @@ export function fetch_and_render_message_history(message: Message): void {
            const clean_data = server_message_history_schema.parse(data);

            const content_edit_history: EditHistoryEntry[] = [];
-            let prev_time = null;
            let prev_stream_item: EditHistoryEntry | null = null;
-
-            const date_time_format = new Intl.DateTimeFormat(user_settings.default_language, {
-                year: "numeric",
-                month: "long",
-                day: "numeric",
-            });
            for (const [index, msg] of clean_data.message_history.entries()) {
                // Format times and dates nicely for display
                const time = new Date(msg.timestamp * 1000);
-                const timestamp = timerender.stringify_time(time);
-                const display_date = date_time_format.format(time);
-                const show_date_row =
-                    prev_time === null ||
-                    !is_same_day(time, prev_time, timerender.display_time_zone);
+                const edited_at_time = timerender.get_full_datetime(time, "time");

                if (!msg.user_id) {
                    continue;
@@ -93,6 +116,7 @@ export function fetch_and_render_message_history(message: Message): void {
                let new_topic;
                let stream_changed;
                let prev_stream;
+                let prev_stream_id;

                if (index === 0) {
                    edited_by_notice = $t({defaultMessage: "Posted by {full_name}"}, {full_name});
@@ -110,6 +134,7 @@ export function fetch_and_render_message_history(message: Message): void {
                    prev_topic = msg.prev_topic;
                    new_topic = msg.topic;
                    stream_changed = true;
+                    prev_stream_id = msg.prev_stream;
                    if (!sub) {
                        prev_stream = $t({defaultMessage: "Unknown stream"});
                    } else {
@@ -129,6 +154,7 @@ export function fetch_and_render_message_history(message: Message): void {
                    const sub = sub_store.get(msg.prev_stream);
                    edited_by_notice = $t({defaultMessage: "Moved by {full_name}"}, {full_name});
                    stream_changed = true;
+                    prev_stream_id = msg.prev_stream;
                    if (!sub) {
                        prev_stream = $t({defaultMessage: "Unknown stream"});
                    } else {
@@ -144,18 +170,19 @@ export function fetch_and_render_message_history(message: Message): void {
                    edited_by_notice = $t({defaultMessage: "Edited by {full_name}"}, {full_name});
                    body_to_render = msg.content_html_diff;
                }
-
                const item: EditHistoryEntry = {
-                    timestamp,
-                    display_date,
-                    show_date_row,
+                    edited_at_time,
                    edited_by_notice,
+                    timestamp: msg.timestamp,
+                    is_stream: message.is_stream,
+                    recipient_bar_color: undefined,
                    body_to_render,
                    topic_edited,
                    prev_topic,
                    new_topic,
                    stream_changed,
                    prev_stream,
+                    prev_stream_id,
                    new_stream: undefined,
                };

@@ -164,56 +191,80 @@ export function fetch_and_render_message_history(message: Message): void {
                }

                content_edit_history.push(item);
-                prev_time = time;
            }
            if (prev_stream_item !== null) {
                assert(message.type === "stream");
                prev_stream_item.new_stream = sub_store.maybe_get_stream_name(message.stream_id);
            }
-            $("#message-history").attr("data-message-id", message.id);
-            $("#message-history").html(
-                render_message_edit_history({
-                    edited_messages: content_edit_history,
-                }),
-            );
+
+            // In order to correctly compute the recipient_bar_color
+            // values, it is convenient to iterate through the array of edit history
+            // entries in reverse chronological order.
+            if (message.is_stream) {
+                // Start with the message's current location.
+                let stream_color: string = get_color(message.stream_id);
+                let recipient_bar_color: string = get_recipient_bar_color(stream_color);
+                for (const edit_history_entry of content_edit_history.toReversed()) {
+                    // The stream following this move is the one whose color we already have.
+                    edit_history_entry.recipient_bar_color = recipient_bar_color;
+                    if (edit_history_entry.stream_changed) {
+                        // If this event moved the message, then immediately
+                        // prior to this event, the message must have been in
+                        // edit_history_event.prev_stream_id; fetch its color.
+                        assert(edit_history_entry.prev_stream_id !== undefined);
+                        stream_color = get_color(edit_history_entry.prev_stream_id);
+                        recipient_bar_color = get_recipient_bar_color(stream_color);
+                    }
+                }
+            }
+            const rendered_list: string = render_message_edit_history({
+                edited_messages: content_edit_history,
+            });
+            $("#message-history-overlay").attr("data-message-id", message.id);
+            $("#message-history-overlay .overlay-messages-list").append(rendered_list);
+
            // Pass the history through rendered_markdown.ts
            // to update dynamic_elements in the content.
-            $("#message-history")
+            $("#message-history-overlay")
                .find(".rendered_markdown")
                .each(function () {
                    rendered_markdown.update_elements($(this));
                });
+            const first_element_id = content_edit_history[0].timestamp;
+            messages_overlay_ui.set_initial_element(
+                String(first_element_id),
+                keyboard_handling_context,
+            );
        },
        error(xhr) {
            ui_report.error(
-                $t_html({defaultMessage: "Error fetching message edit history"}),
+                $t_html({defaultMessage: "Error fetching message edit history."}),
                xhr,
-                $("#dialog_error"),
+                $("#message-history-overlay #message-history-error"),
            );
+            $("#message-history-error").show();
        },
    });
 }

-export function show_history(message: Message): void {
-    const rendered_message_history = render_message_history_modal();
-
-    dialog_widget.launch({
-        html_heading: $t_html({defaultMessage: "Message edit history"}),
-        html_body: rendered_message_history,
-        html_submit_button: $t_html({defaultMessage: "Close"}),
-        id: "message-edit-history",
-        on_click() {
-            /* do nothing */
-        },
-        close_on_submit: true,
-        focus_submit_on_open: true,
-        single_footer_button: true,
-        post_render() {
-            fetch_and_render_message_history(message);
+export function open_overlay(): void {
+    if (overlays.any_active()) {
+        return;
+    }
+    overlays.open_overlay({
+        name: "message_edit_history",
+        $overlay: $("#message-history-overlay"),
+        on_close() {
+            exit_overlay();
+            $("#message-edit-history-overlay-container").empty();
        },
    });
 }

+export function handle_keyboard_events(event_key: string): void {
+    messages_overlay_ui.modals_handle_events(event_key, keyboard_handling_context);
+}
+
 export function initialize(): void {
    $("body").on("mouseenter", ".message_edit_notice", (e) => {
        if (realm.realm_allow_edit_history) {
@@ -244,8 +295,12 @@ export function initialize(): void {
        }

        if (realm.realm_allow_edit_history) {
-            show_history(message);
-            $("#message-history-cancel").trigger("focus");
+            fetch_and_render_message_history(message);
+            $("#message-history-overlay .exit-sign").trigger("focus");
        }
    });
+
+    $("body").on("focus", "#message-history-overlay .overlay-message-info-box", (e) => {
+        messages_overlay_ui.activate_element(e.target, keyboard_handling_context);
+    });
 }
--- a/web/src/messages_overlay_ui.ts
+++ b/web/src/messages_overlay_ui.ts
@@ -1,7 +1,7 @@
 import $ from "jquery";
 import assert from "minimalistic-assert";

-type Context = {
+export type Context = {
    items_container_selector: string;
    items_list_selector: string;
    row_item_selector: string;
--- a/web/src/overlays.ts
+++ b/web/src/overlays.ts
@@ -73,6 +73,10 @@ export function scheduled_messages_open(): boolean {
    return open_overlay_name === "scheduled";
 }

+export function message_edit_history_open(): boolean {
+    return open_overlay_name === "message_edit_history";
+}
+
 export function open_overlay(opts: OverlayOptions): void {
    call_hooks(pre_open_hooks);

--- a/web/styles/dark_theme.css
+++ b/web/styles/dark_theme.css
@@ -596,7 +596,8 @@
    }

    #draft_overlay,
-    #scheduled_messages_overlay_container {
+    #scheduled_messages_overlay_container,
+    #message-edit-history-overlay-container {
        .flex.overlay-content > div {
            box-shadow: 0 0 30px hsl(213deg 31% 0%);
            background-color: var(--color-background);
@@ -833,7 +834,7 @@
        border-color: hsl(217deg 64% 59% / 70%);
    }

-    #message-edit-history {
+    #message-edit-history-overlay-container {
        .message_edit_history_content {
            .highlight_text_inserted {
                color: hsl(122deg 100% 81%);
--- a/web/styles/message_edit_history.css
+++ b/web/styles/message_edit_history.css
@@ -1,37 +1,26 @@
-#message-edit-history {
-    .message_top_line {
-        float: right;
-    }
-
-    .date_row > span {
+.message-edit-history-container {
+    .header-body {
        display: flex;
        align-items: center;
-        white-space: nowrap;
+        flex-direction: row;
+        justify-content: space-between;
+        gap: 5px;

-        &::before,
-        &::after {
-            width: 100%;
-            margin: 0;
+        @media (width < $lg_min) {
+            display: block;
        }
    }

-    .message_time {
-        position: static;
-    }
-
-    .message_author {
-        position: relative;
-    }
-
-    .author_details {
-        display: block;
-        font-size: 12px;
-        padding: 1px;
-        text-align: right;
-    }
-
-    .messagebox-content {
-        padding: 0 10px;
+    .message-edit-history-list {
+        /*
+        styles are based on drafts-list
+        see web/styles/drafts.css
+        */
+        & h2 {
+            font-size: 1.1em;
+            line-height: normal;
+            margin-bottom: 5px;
+        }
    }

    .message_edit_history_content {
@@ -47,4 +36,18 @@
            word-break: break-all;
        }
    }
+
+    .messagebox-content {
+        display: block !important;
+    }
+
+    #message-history-error {
+        /*
+        styles are based on .model_content
+        see web/styles/modal.css
+        */
+        font-size: 1rem;
+        display: none;
+        margin: 10px;
+    }
 }
--- a/web/templates/message_edit_history.hbs
+++ b/web/templates/message_edit_history.hbs
@@ -1,21 +1,55 @@
 {{! Client-side Handlebars template for viewing message edit history. }}

 {{#each edited_messages}}
-    {{#if show_date_row}}
-    <div class="date_row"><span>{{ display_date }}</span></div>
-    {{/if}}
-    <div class="messagebox-content">
-        <div class="message_top_line"><span class="message_time">{{ timestamp }}</span></div>
-        {{#if topic_edited}}
-        <div class="message_content message_edit_history_content"><p>Topic: <span class="highlight_text_inserted">{{ new_topic }}</span> <span class="highlight_text_deleted">{{ prev_topic }}</span></p></div>
-        {{/if}}
-        {{#if stream_changed}}
-        <div class="message_content message_edit_history_content"><p>Stream: <span class="highlight_text_inserted">{{ new_stream }}</span> <span class="highlight_text_deleted">{{ prev_stream }}</span></p></div>
-        {{/if}}
-        {{#if body_to_render}}
-        <div class="message_content rendered_markdown message_edit_history_content">{{rendered_markdown body_to_render}}</div>
-        {{/if}}
-        <div class="message_author"><div class="author_details">{{ edited_by_notice }}</div></div>
+    <div class="overlay-message-row" data-message-edit-history-id="{{timestamp}}">
+        <div class="overlay-message-info-box" tabindex="0">
+            {{#if is_stream }}
+            <div class="message_header message_header_stream">
+                <div class="message-header-contents" style="background: {{recipient_bar_color}};">
+                    <div class="message_label_clickable stream_label">
+                        <span class="private_message_header_name">{{ edited_by_notice }}</span>
+                    </div>
+                    <div class="recipient_row_date" title="{{t 'Last modified'}}">{{t "{edited_at_time}" }}
+
+                    </div>
+                </div>
+            </div>
+            {{else}}
+            <div class="message_header message_header_private_message">
+                <div class="message-header-contents">
+                    <div class="message_label_clickable stream_label">
+                        <span class="private_message_header_name">{{ edited_by_notice }}</span>
+                    </div>
+                    <div class="recipient_row_date" title="{{t 'Last modified'}}">{{t "{edited_at_time}" }}</div>
+                </div>
+            </div>
+            {{/if}}
+            <div class="message_row{{^is_stream}} private-message{{/is_stream}}" role="listitem">
+                <div class="messagebox">
+                    <div class="messagebox-content">
+                        {{#if topic_edited}}
+                        <div class="message_content message_edit_history_content">
+                            <p>Topic: <span class="highlight_text_inserted">{{ new_topic }}</span>
+                                <span class="highlight_text_deleted">{{ prev_topic}}</span>
+                            </p>
+                        </div>
+                        {{/if}}
+                        {{#if stream_changed}}
+                        <div class="message_content message_edit_history_content">
+                            <p>Stream: <span class="highlight_text_inserted">{{ new_stream }}</span>
+                                <span class="highlight_text_deleted">{{ prev_stream }}</span>
+                            </p>
+                        </div>
+                        {{/if}}
+                        {{#if body_to_render}}
+                        <div class="message_content rendered_markdown message_edit_history_content">
+                            {{ rendered_markdown body_to_render}}
+                        </div>
+                        {{/if}}
+                    </div>
+                </div>
+            </div>
+        </div>
    </div>
-    <hr />
 {{/each}}
+
--- a/web/templates/message_history_modal.hbs
+++ b/web/templates/message_history_modal.hbs
@@ -1 +0,0 @@
-<div class="controls" id="message-history"></div>
--- a/web/templates/message_history_overlay.hbs
+++ b/web/templates/message_history_overlay.hbs
@@ -0,0 +1,17 @@
+<div id="message-history-overlay" class="overlay new-style" data-overlay="message_edit_history">
+    <div class="flex overlay-content">
+        <div class="message-edit-history-container overlay-messages-container overlay-container">
+            <div class="overlay-messages-header">
+                <h1>{{t "Message edit history" }}</h1>
+                <div class="exit">
+                    <span class="exit-sign">&times;</span>
+                </div>
+            </div>
+            <div class="message-edit-history-list overlay-messages-list">
+            </div>
+            <div id="message-history-error" class="alert">
+            </div>
+        </div>
+    </div>
+</div>
+
--- a/web/tests/hotkey.test.js
+++ b/web/tests/hotkey.test.js
@@ -64,6 +64,7 @@ const overlays = mock_esm("../src/overlays", {
    drafts_open: () => false,
    scheduled_messages_open: () => false,
    info_overlay_open: () => false,
+    message_edit_history_open: () => false,
 });
 const popovers = mock_esm("../src/user_card_popover", {
    manage_menu: {
				`@@ -1 +0,0 @@`
				`<div class="controls" id="message-history"></div>`