From df180f7bd5fb8f226d277d2436a7e76450194e9b Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Wed, 10 Jan 2024 08:31:36 +0000 Subject: [PATCH] single_message: Use data-message-id instead of zid. --- web/src/echo.js | 3 ++- web/src/lightbox.js | 8 ++++---- web/src/message_list_view.js | 5 +++-- web/src/reactions.js | 8 +++++--- web/src/rows.ts | 22 +++++++++++----------- web/src/user_card_popover.js | 2 +- web/templates/single_message.hbs | 2 +- web/tests/lightbox.test.js | 8 ++++---- web/tests/reactions.test.js | 8 ++++++-- 9 files changed, 37 insertions(+), 29 deletions(-) diff --git a/web/src/echo.js b/web/src/echo.js index 27482892f1..0bdcf28b07 100644 --- a/web/src/echo.js +++ b/web/src/echo.js @@ -461,7 +461,8 @@ function abort_message(message) { } export function display_slow_send_loading_spinner(message) { - const $row = $(`div[zid="${CSS.escape(message.id)}"]`); + const message_list_id = message_lists.current.id; + const $row = $(`#message-row-${message_list_id}-${CSS.escape(message.id)}`); if (message.locally_echoed && !message.failed_request) { $row.find(".slow-send-spinner").removeClass("hidden"); // We don't need to do anything special to ensure this gets diff --git a/web/src/lightbox.js b/web/src/lightbox.js index a9c7061ee6..73904a6771 100644 --- a/web/src/lightbox.js +++ b/web/src/lightbox.js @@ -491,11 +491,11 @@ export function parse_media_data(media) { if (is_compose_preview_media) { sender_full_name = people.my_full_name(); } else { - const $message = $parent.closest("[zid]"); - const zid = rows.id($message); - const message = message_store.get(zid); + const $message = $parent.closest(".message_row"); + const message_id = rows.id($message); + const message = message_store.get(message_id); if (message === undefined) { - blueslip.error("Lightbox for unknown message", {zid}); + blueslip.error("Lightbox for unknown message", {message_id}); } else { sender_full_name = message.sender_full_name; } diff --git a/web/src/message_list_view.js b/web/src/message_list_view.js index bd54debbe6..350b7f61b5 100644 --- a/web/src/message_list_view.js +++ b/web/src/message_list_view.js @@ -1500,7 +1500,7 @@ export class MessageListView { const $row = this._rows.get(old_id); this._rows.delete(old_id); - $row.attr("zid", new_id); + $row.attr("data-message-id", new_id); $row.attr("id", `message-row-${this.list.id}-` + new_id); $row.removeClass("local"); this._rows.set(new_id, $row); @@ -1688,7 +1688,8 @@ export class MessageListView { show_messages_as_unread(message_ids) { const $rows_to_show_as_unread = this.$list.find(".message_row").filter((_index, $row) => { - const message_id = Number.parseFloat($row.getAttribute("zid")); + // eslint-disable-next-line unicorn/prefer-dom-node-dataset + const message_id = Number.parseFloat($row.getAttribute("data-message-id")); return message_ids.includes(message_id); }); $rows_to_show_as_unread.addClass("unread"); diff --git a/web/src/reactions.js b/web/src/reactions.js index a2e6910c98..03fc00867b 100644 --- a/web/src/reactions.js +++ b/web/src/reactions.js @@ -6,6 +6,7 @@ import * as blueslip from "./blueslip"; import * as channel from "./channel"; import * as emoji from "./emoji"; import {$t} from "./i18n"; +import * as message_lists from "./message_lists"; import * as message_store from "./message_store"; import {page_params} from "./page_params"; import * as people from "./people"; @@ -204,9 +205,10 @@ export function get_reaction_title_data(message_id, local_id) { } export function get_reaction_section(message_id) { - const $message_element = $(".message-list").find(`[zid='${CSS.escape(message_id)}']`); - const $section = $message_element.find(".message_reactions"); - return $section; + const message_list_id = message_lists.current.id; + return $(`#message-row-${message_list_id}-${CSS.escape(message_id)}`).find( + ".message_reactions", + ); } export function find_reaction(message_id, local_id) { diff --git a/web/src/rows.ts b/web/src/rows.ts index 0131e1f4af..cc13d6dfac 100644 --- a/web/src/rows.ts +++ b/web/src/rows.ts @@ -80,7 +80,7 @@ export function is_overlay_row($row: JQuery): boolean { export function id($message_row: JQuery): number | undefined { if (is_overlay_row($message_row)) { - blueslip.error("Drafts and scheduled messages have no zid"); + blueslip.error("Drafts and scheduled messages have no message id."); return undefined; } @@ -88,28 +88,28 @@ export function id($message_row: JQuery): number | undefined { throw new Error("Caller should pass in a single row."); } - const zid = $message_row.attr("zid"); + const message_id = $message_row.attr("data-message-id"); - if (zid === undefined) { - throw new Error("Calling code passed rows.id a row with no zid attr."); + if (message_id === undefined) { + throw new Error("Calling code passed rows.id a row with no `data-message-id` attr."); } - return Number.parseFloat(zid); + return Number.parseFloat(message_id); } export function local_echo_id($message_row: JQuery): string | undefined { - const zid = $message_row.attr("zid"); + const message_id = $message_row.attr("data-message-id"); - if (zid === undefined) { - blueslip.error("Calling code passed rows.local_id a row with no zid attr."); + if (message_id === undefined) { + blueslip.error("Calling code passed rows.local_id a row with no `data-message-id` attr."); return undefined; } - if (!zid.includes(".0")) { - blueslip.error("Trying to get local_id from row that has reified message id", {zid}); + if (!message_id.includes(".0")) { + blueslip.error("Trying to get local_id from row that has reified message id", {message_id}); } - return zid; + return message_id; } export function get_message_id(elem: string): number | undefined { diff --git a/web/src/user_card_popover.js b/web/src/user_card_popover.js index 769da6abe4..af89dc9b3f 100644 --- a/web/src/user_card_popover.js +++ b/web/src/user_card_popover.js @@ -527,7 +527,7 @@ function toggle_user_card_popover_for_message(element, user, message, on_mount) // This is never supposed to happen, not even for deactivated // users, so we'll need to debug this error if it occurs. blueslip.error("Bad sender in message", { - zid: message.id, + message_id: message.id, sender_id: message.sender_id, }); return; diff --git a/web/templates/single_message.hbs b/web/templates/single_message.hbs index 6adad889fb..46e0bfb810 100644 --- a/web/templates/single_message.hbs +++ b/web/templates/single_message.hbs @@ -1,4 +1,4 @@ -
{{#if want_date_divider}} diff --git a/web/tests/lightbox.test.js b/web/tests/lightbox.test.js index a5c282834c..f637e60d06 100644 --- a/web/tests/lightbox.test.js +++ b/web/tests/lightbox.test.js @@ -46,10 +46,10 @@ test("pan_and_zoom", ({override}) => { $img.attr("src", "example"); - let fetched_zid; + let fetched_message_id; - message_store.get = (zid) => { - fetched_zid = zid; + message_store.get = (message_id) => { + fetched_message_id = message_id; return "message-stub"; }; @@ -60,7 +60,7 @@ test("pan_and_zoom", ({override}) => { const open_image = lightbox.build_open_media_function(); open_image($img); - assert.equal(fetched_zid, 1234); + assert.equal(fetched_message_id, 1234); }); test("youtube", ({override}) => { diff --git a/web/tests/reactions.test.js b/web/tests/reactions.test.js index 3faf703a30..2c3e50c17d 100644 --- a/web/tests/reactions.test.js +++ b/web/tests/reactions.test.js @@ -44,6 +44,11 @@ const settings_data = mock_esm("../src/settings_data"); const spectators = mock_esm("../src/spectators", { login_to_access() {}, }); +mock_esm("../src/message_lists", { + current: { + id: 1, + }, +}); set_global("document", "document-stub"); @@ -410,9 +415,8 @@ test("prevent_simultaneous_requests_updating_reaction", ({override, override_rew }); function stub_reactions(message_id) { - const $message_row = $.create("message-row-stub"); const $message_reactions = $.create("reactions-stub"); - $(".message-list").set_find_results(`[zid='${CSS.escape(message_id)}']`, $message_row); + const $message_row = $.create(`#message-row-1-${CSS.escape(message_id)}`); $message_row.set_find_results(".message_reactions", $message_reactions); return $message_reactions; }