From 1f3367abfb6a90ac6db0c0672bc5736213fdcad6 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Fri, 13 Jan 2023 14:58:45 +0000 Subject: [PATCH] message_list_view: Remove dead next_is_same_sender code. The `next_is_same_sender` has no effect on the CSS of the message displayed and the JS changes seem to have no effect too. See cc8021a742491a04519544bda571206e0e23a842 for more details. --- .../node_tests/message_list_view.js | 16 ----------- static/js/message_list_view.js | 27 ------------------- static/styles/zulip.css | 5 ---- static/templates/single_message.hbs | 2 +- 4 files changed, 1 insertion(+), 49 deletions(-) diff --git a/frontend_tests/node_tests/message_list_view.js b/frontend_tests/node_tests/message_list_view.js index ac667b13aa..e30d2b0770 100644 --- a/frontend_tests/node_tests/message_list_view.js +++ b/frontend_tests/node_tests/message_list_view.js @@ -486,7 +486,6 @@ test("merge_message_groups", () => { assert.deepEqual(result.prepend_groups, []); assert.deepEqual(result.rerender_groups, []); assert.deepEqual(result.append_messages, []); - assert.deepEqual(result.rerender_messages_next_same_sender, []); })(); (function test_append_message_same_topic() { @@ -506,7 +505,6 @@ test("merge_message_groups", () => { assert.deepEqual(result.prepend_groups, []); assert.deepEqual(result.rerender_groups, []); assert_message_list_equal(result.append_messages, [message2]); - assert_message_list_equal(result.rerender_messages_next_same_sender, [message1]); })(); (function test_append_message_different_topic() { @@ -525,7 +523,6 @@ test("merge_message_groups", () => { assert.deepEqual(result.prepend_groups, []); assert.deepEqual(result.rerender_groups, []); assert.deepEqual(result.append_messages, []); - assert.deepEqual(result.rerender_messages_next_same_sender, []); })(); (function test_append_message_different_topic_and_days() { @@ -543,7 +540,6 @@ test("merge_message_groups", () => { assert.deepEqual(result.prepend_groups, []); assert.deepEqual(result.rerender_groups, []); assert.deepEqual(result.append_messages, []); - assert.deepEqual(result.rerender_messages_next_same_sender, []); assert.equal(message_group2.group_date_divider_html, "900000000 - 1000000"); })(); @@ -562,7 +558,6 @@ test("merge_message_groups", () => { assert.deepEqual(result.prepend_groups, []); assert.deepEqual(result.rerender_groups, []); assert.deepEqual(result.append_messages, [message2]); - assert.deepEqual(result.rerender_messages_next_same_sender, [message1]); assert.ok(list._message_groups[0].message_containers[1].want_date_divider); })(); @@ -582,7 +577,6 @@ test("merge_message_groups", () => { assert.deepEqual(result.prepend_groups, []); assert.deepEqual(result.rerender_groups, []); assert.deepEqual(result.append_messages, []); - assert.deepEqual(result.rerender_messages_next_same_sender, []); })(); (function test_append_message_same_topic_me_message() { @@ -603,7 +597,6 @@ test("merge_message_groups", () => { assert.deepEqual(result.prepend_groups, []); assert.deepEqual(result.rerender_groups, []); assert_message_list_equal(result.append_messages, [message2]); - assert_message_list_equal(result.rerender_messages_next_same_sender, [message1]); })(); (function test_prepend_message_same_topic() { @@ -625,7 +618,6 @@ test("merge_message_groups", () => { build_message_group([message2, message1]), ]); assert.deepEqual(result.append_messages, []); - assert.deepEqual(result.rerender_messages_next_same_sender, []); })(); (function test_prepend_message_different_topic() { @@ -643,7 +635,6 @@ test("merge_message_groups", () => { assert_message_groups_list_equal(result.prepend_groups, [message_group2]); assert.deepEqual(result.rerender_groups, []); assert.deepEqual(result.append_messages, []); - assert.deepEqual(result.rerender_messages_next_same_sender, []); })(); (function test_prepend_message_different_topic_and_day() { @@ -663,7 +654,6 @@ test("merge_message_groups", () => { assert_message_groups_list_equal(result.prepend_groups, [message_group2]); assert.deepEqual(result.rerender_groups, [message_group1]); assert.deepEqual(result.append_messages, []); - assert.deepEqual(result.rerender_messages_next_same_sender, []); })(); (function test_prepend_message_different_day() { @@ -683,7 +673,6 @@ test("merge_message_groups", () => { assert.deepEqual(result.prepend_groups, []); assert_message_groups_list_equal(result.rerender_groups, [message_group2]); assert.deepEqual(result.append_messages, []); - assert.deepEqual(result.rerender_messages_next_same_sender, []); })(); (function test_prepend_message_historical() { @@ -702,14 +691,9 @@ test("merge_message_groups", () => { assert_message_groups_list_equal(result.prepend_groups, [message_group2]); assert.deepEqual(result.rerender_groups, []); assert.deepEqual(result.append_messages, []); - assert.deepEqual(result.rerender_messages_next_same_sender, []); })(); }); -// TODO: Add a test suite for rerender_messages_next_same_sender() that includes cases -// where new messages added via local echo have a different date from -// the older messages. - test("render_windows", () => { // We only render up to 400 messages at a time in our message list, // and we only change the window (which is a range, really, with diff --git a/static/js/message_list_view.js b/static/js/message_list_view.js index b60510cb08..cb35e6d089 100644 --- a/static/js/message_list_view.js +++ b/static/js/message_list_view.js @@ -426,9 +426,6 @@ export class MessageListView { let prev; const add_message_container_to_group = (message_container) => { - if (same_sender(prev, message_container)) { - prev.next_is_same_sender = true; - } current_group.message_containers.push(message_container); }; @@ -530,9 +527,6 @@ export class MessageListView { ) { first_msg_container.include_sender = false; } - if (same_sender(last_msg_container, first_msg_container)) { - last_msg_container.next_is_same_sender = true; - } first_group.message_containers = first_group.message_containers.concat( second_group.message_containers, ); @@ -563,7 +557,6 @@ export class MessageListView { prepend_groups: [], rerender_groups: [], append_messages: [], - rerender_messages_next_same_sender: [], }; let first_group; let second_group; @@ -623,7 +616,6 @@ export class MessageListView { } else { if (was_joined) { // rerender the last message - message_actions.rerender_messages_next_same_sender.push(prev_msg_container); message_actions.append_messages = new_message_groups[0].message_containers; new_message_groups = new_message_groups.slice(1); } else if (first_group !== undefined && second_group !== undefined) { @@ -837,25 +829,6 @@ export class MessageListView { } } - // Update the rendering for message rows which used to be last - // and now know whether the following message has the same - // sender. - // - // It is likely the case that we can just remove the block - // entirely, since it appears the next_is_same_sender CSS - // class doesn't do anything. - if (message_actions.rerender_messages_next_same_sender.length > 0) { - const targets = message_actions.rerender_messages_next_same_sender; - - for (const message_container of targets) { - const $row = this.get_row(message_container.msg.id); - $row.find("div.messagebox").toggleClass( - "next_is_same_sender", - message_container.next_is_same_sender, - ); - } - } - // Insert new messages in to the last message group if (message_actions.append_messages.length > 0) { $last_message_row = $table.find(".message_row").last().expectOne(); diff --git a/static/styles/zulip.css b/static/styles/zulip.css index 19d8678cc3..c13435c13e 100644 --- a/static/styles/zulip.css +++ b/static/styles/zulip.css @@ -1711,11 +1711,6 @@ div.focused_table { background-color: transparent; } -.next_is_same_sender { - border-bottom: 0; - padding-bottom: 0; -} - .inline_profile_picture { display: inline-block; width: 35px; diff --git a/static/templates/single_message.hbs b/static/templates/single_message.hbs index 5ec3d91713..7c6ecf2ca2 100644 --- a/static/templates/single_message.hbs +++ b/static/templates/single_message.hbs @@ -5,7 +5,7 @@ {{#if want_date_divider}}
{{{date_divider_html}}}
{{/if}} -
{{> message_body}}