Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

reactions: Optimize DOM cost of message reactions via dynamic rendering. 

Before, the message reactions section along with the add reaction button
was being rendered for every message even when there were no reactions
present - this led to additional DOM cost.

This commit adds the message reactions section only when there is
at least a single reaction on the message, and follows up with a cleanup
of the message reactions section when there are no reactions.

Fixes #31137.

Co-authored-by: Anmol-dev45 <basnetanmol2020@gmail.com>
This commit is contained in:
Sayam Samal 2024-10-02 12:46:01 +05:30 committed by Tim Abbott
parent 1b4e02c5d0
commit b09dfb782d
5 changed files with 178 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
web/src/reactions.ts
View File

@ -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.

9
web/styles/reactions.css
View File

@ -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(

6
web/templates/message_body.hbs
View File

@ -69,6 +69,6 @@
<div class="message_length_controller"></div>
{{#unless is_hidden}}
<div class="message_reactions">{{> message_reactions }}</div>
{{/unless}}
{{#if (and (not is_hidden) msg.message_reactions)}}
{{> message_reactions }}
{{/if}}

16
web/templates/message_reactions.hbs
View File

@ -1,9 +1,11 @@
{{#each this/msg/message_reactions}}
{{> message_reaction}}
{{/each}}
<div class="reaction_button" role="button" aria-haspopup="true" data-tooltip-template-id="add-emoji-tooltip-template" aria-label="{{t 'Add emoji reaction' }} (:)">
<div class="emoji-message-control-button-container">
<i class="zulip-icon zulip-icon-smile" tabindex="0"></i>
<div class="message_reaction_count">+</div>
<div class="message_reactions">
{{#each this/msg/message_reactions}}
{{> message_reaction}}
{{/each}}
<div class="reaction_button" role="button" aria-haspopup="true" data-tooltip-template-id="add-emoji-tooltip-template" aria-label="{{t 'Add emoji reaction' }} (:)">
<div class="emoji-message-control-button-container">
<i class="zulip-icon zulip-icon-smile" tabindex="0"></i>
<div class="message_reaction_count">+</div>
</div>
</div>
</div>

144
web/tests/reactions.test.js
View File

@ -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 "<msg-reactions-section-stub>";
});
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, "<msg-reactions-section-stub>");
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);