diff --git a/web/src/reactions.ts b/web/src/reactions.ts index 19b0a21417..fa6df71a61 100644 --- a/web/src/reactions.ts +++ b/web/src/reactions.ts @@ -3,6 +3,7 @@ import assert from "minimalistic-assert"; import {z} from "zod"; import render_message_reaction from "../templates/message_reaction.hbs"; +import render_message_reactions from "../templates/message_reactions.hbs"; import * as blueslip from "./blueslip"; import * as channel from "./channel"; @@ -369,11 +370,23 @@ export function insert_new_reaction( class: reaction_class, }; - const $new_reaction = $(render_message_reaction(context)); - - // Now insert it before the add button. - const $reaction_button_element = get_add_reaction_button(message.id); - $new_reaction.insertBefore($reaction_button_element); + // If the given reaction is the first reaction in a message, then we add + // the whole message reactions section along with the new reaction. + // Else, we insert the new reaction before the add reaction button. + if (message.clean_reactions.size - 1 === 0) { + const $rows = message_lists.all_rendered_row_for_message_id(message.id); + const reaction_section_context = { + msg: { + message_reactions: [context], + }, + }; + const $msg_reaction_section = render_message_reactions(reaction_section_context); + $rows.find(".messagebox-content").append($msg_reaction_section); + } else { + const $new_reaction = $(render_message_reaction(context)); + const $reaction_button_element = get_add_reaction_button(message.id); + $new_reaction.insertBefore($reaction_button_element); + } update_vote_text_on_message(message); } @@ -424,6 +437,14 @@ export function remove_reaction_from_view( const $reaction = find_reaction(message.id, local_id); const reaction_count = clean_reaction_object.user_ids.length; + // Cleanup: If the reaction being removed is the last reaction on the + // message, we remove the whole message reaction section and exit. + if (message.clean_reactions.size === 0) { + const $msg_reaction_section = get_reaction_sections(message.id); + $msg_reaction_section.remove(); + return; + } + if (reaction_count === 0) { // If this user was the only one reacting for this emoji, we simply // remove the reaction and exit. diff --git a/web/styles/reactions.css b/web/styles/reactions.css index 4f4deed425..55b02cb072 100644 --- a/web/styles/reactions.css +++ b/web/styles/reactions.css @@ -121,15 +121,6 @@ color: var(--color-message-reaction-button-text-hover); } - /* Configure the reaction button to appear if and only if there's an - existing reaction to the message. We reference first-child - rather than only-child because tooltips may be appended to - the DOM after this element, whereas actual reactions are - appended before it. */ - &:first-child { - display: none; - } - &:hover { color: var(--color-message-reaction-button-text-hover); background-color: var( diff --git a/web/templates/message_body.hbs b/web/templates/message_body.hbs index 1d888bac0c..10d16b8ff8 100644 --- a/web/templates/message_body.hbs +++ b/web/templates/message_body.hbs @@ -69,6 +69,6 @@
-{{#unless is_hidden}} -
{{> message_reactions }}
-{{/unless}} +{{#if (and (not is_hidden) msg.message_reactions)}} + {{> message_reactions }} +{{/if}} diff --git a/web/templates/message_reactions.hbs b/web/templates/message_reactions.hbs index 878f0883db..7828520d98 100644 --- a/web/templates/message_reactions.hbs +++ b/web/templates/message_reactions.hbs @@ -1,9 +1,11 @@ -{{#each this/msg/message_reactions}} - {{> message_reaction}} -{{/each}} -
-
- -
+
+
+ {{#each this/msg/message_reactions}} + {{> message_reaction}} + {{/each}} +
+
+ +
+
+
diff --git a/web/tests/reactions.test.js b/web/tests/reactions.test.js index 226f6dc3bf..bd1eeefc46 100644 --- a/web/tests/reactions.test.js +++ b/web/tests/reactions.test.js @@ -837,6 +837,74 @@ test("add_reaction/remove_reaction", ({override, override_rewire}) => { }); }); +test("insert_new_reaction (first reaction)", ({mock_template, override_rewire}) => { + const clean_reaction_object = { + class: "message_reaction", + count: 1, + emoji_alt_code: false, + emoji_code: "1f3b1", + emoji_name: "8ball", + is_realm_emoji: false, + label: "translated: You (click to remove) reacted with :8ball:", + local_id: "unicode_emoji,1f3b1", + reaction_type: "unicode_emoji", + user_ids: [alice.user_id], + }; + const message_id = 501; + + mock_template("message_reactions.hbs", false, (data) => { + assert.deepEqual(data, { + msg: { + message_reactions: [ + { + count: 1, + emoji_alt_code: false, + emoji_name: "8ball", + emoji_code: "1f3b1", + local_id: "unicode_emoji,1f3b1", + class: "message_reaction reacted", + message_id, + label: "translated: You (click to remove) reacted with :8ball:", + reaction_type: clean_reaction_object.reaction_type, + is_realm_emoji: false, + vote_text: "", + }, + ], + }, + }); + return ""; + }); + + const $rows = $.create("rows-stub"); + message_lists.all_rendered_row_for_message_id = () => $rows; + + const $messagebox_content = $.create("messagebox-content-stub"); + $rows.set_find_results(".messagebox-content", $messagebox_content); + + let append_called = false; + $messagebox_content.append = (element) => { + assert.equal(element, ""); + append_called = true; + }; + + const message = { + id: message_id, + reactions: [ + { + emoji_name: "8ball", + user_id: alice.user_id, + reaction_type: "unicode_emoji", + emoji_code: "1f3b1", + }, + ], + }; + + override_rewire(reactions, "update_vote_text_on_message", noop); + convert_reactions_to_clean_reactions(message); + reactions.insert_new_reaction(clean_reaction_object, message, alice.user_id); + assert.ok(append_called); +}); + test("insert_new_reaction (me w/unicode emoji)", ({mock_template}) => { const clean_reaction_object = { class: "message_reaction", @@ -855,9 +923,10 @@ test("insert_new_reaction (me w/unicode emoji)", ({mock_template}) => { const $message_reactions = stub_reactions(message_id); const $reaction_button = $.create("reaction-button-stub"); $message_reactions.find = () => $reaction_button; + const $message_reactions_count = $.create("message-reaction-count-stub"); $reaction_button.find = (selector) => { assert.equal(selector, ".message_reaction_count"); - return $.create("message-reaction-count-stub"); + return $message_reactions_count; }; mock_template("message_reaction.hbs", false, (data) => { @@ -886,6 +955,12 @@ test("insert_new_reaction (me w/unicode emoji)", ({mock_template}) => { const message = { id: message_id, reactions: [ + { + emoji_name: "+1", + user_id: bob.user_id, + reaction_type: "unicode_emoji", + emoji_code: "1f44d", + }, { emoji_name: "8ball", user_id: alice.user_id, @@ -894,8 +969,8 @@ test("insert_new_reaction (me w/unicode emoji)", ({mock_template}) => { }, ], }; - convert_reactions_to_clean_reactions(message); + convert_reactions_to_clean_reactions(message); reactions.insert_new_reaction(clean_reaction_object, message, alice.user_id); assert.ok(insert_called); }); @@ -918,9 +993,10 @@ test("insert_new_reaction (them w/zulip emoji)", ({mock_template}) => { const $message_reactions = stub_reactions(message_id); const $reaction_button = $.create("reaction-button-stub"); $message_reactions.find = () => $reaction_button; + const $message_reactions_count = $.create("message-reaction-count-stub"); $reaction_button.find = (selector) => { assert.equal(selector, ".message_reaction_count"); - return $.create("message-reaction-count-stub"); + return $message_reactions_count; }; mock_template("message_reaction.hbs", false, (data) => { @@ -951,6 +1027,12 @@ test("insert_new_reaction (them w/zulip emoji)", ({mock_template}) => { const message = { id: message_id, reactions: [ + { + emoji_name: "+1", + user_id: bob.user_id, + reaction_type: "unicode_emoji", + emoji_code: "1f44d", + }, { emoji_name: "8ball", user_id: bob.user_id, @@ -1155,7 +1237,7 @@ test("remove_reaction_from_view (them)", () => { ); }); -test("remove_reaction_from_view (last person)", () => { +test("remove_reaction_from_view (last person to react)", ({override_rewire}) => { const clean_reaction_object = { class: "message_reaction", count: 1, @@ -1170,12 +1252,66 @@ test("remove_reaction_from_view (last person)", () => { const message_id = 507; const $our_reaction = stub_reaction(message_id, "unicode_emoji,1f3b1"); + override_rewire(reactions, "find_reaction", (message_id_param, local_id) => { + assert.equal(message_id_param, message_id); + assert.equal(local_id, "unicode_emoji,1f3b1"); + return $our_reaction; + }); let removed; $our_reaction.remove = () => { removed = true; }; + const message = { + id: message_id, + reactions: [ + { + emoji_code: "1f3b1", + emoji_name: "8ball", + reaction_type: "unicode_emoji", + user_id: bob.user_id, + }, + { + emoji_code: "1f44d", + emoji_name: "thumbs_up", + reaction_type: "unicode_emoji", + user_id: alice.user_id, + }, + ], + }; + + override_rewire(reactions, "update_vote_text_on_message", noop); + convert_reactions_to_clean_reactions(message); + reactions.remove_reaction_from_view(clean_reaction_object, message, bob.user_id); + assert.ok(removed); +}); + +test("remove_reaction_from_view (last reaction)", () => { + const clean_reaction_object = { + class: "message_reaction", + count: 1, + emoji_alt_code: false, + emoji_code: "1f3b1", + emoji_name: "8ball", + is_realm_emoji: false, + local_id: "unicode_emoji,1f3b1", + reaction_type: "unicode_emoji", + user_ids: [], + }; + const message_id = 507; + + const $message_reactions = stub_reactions(message_id); + + // Create a stub for the specific reaction + const $specific_reaction = $.create("specific-reaction-stub"); + $message_reactions.find = () => $specific_reaction; + + let removed = false; + $message_reactions.remove = () => { + removed = true; + }; + const message = {id: message_id, reactions: []}; convert_reactions_to_clean_reactions(message); reactions.remove_reaction_from_view(clean_reaction_object, message, bob.user_id);