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

mention: Fix mention highlighting in unsubscribed streams. 

Rules followed:
1. Bold and highlighted background if the mention was processed
as a mention that includes you.
2. Bold personal mention (but not highlighted) if you were mentioned
but not subscribed at the time.
3. Otherwise not bold, no highlighting.

As we plan to keep the mention pill CSS the same if a user
was mentioned via that personal/wildcard/usergroup mention
irrespective of whether the user is subscribed or not, we use
usermessage flags to determine when to add 'user-mention-me' class.

Fixes #27654.
This commit is contained in:
Prakhar Pratyush 2023-11-23 21:47:37 +05:30 committed by Tim Abbott
parent 73152671e3
commit 0c159c5f47
4 changed files with 83 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
web/src/message_list_view.js
View File

@ -417,7 +417,16 @@ export class MessageListView {
// message didn't include a user mention, then it was a usergroup/wildcard
// mention (which is the only other option for `mentioned` being true).
if (message_container.msg.mentioned_me_directly && is_user_mention) {
message_container.mention_classname = "direct_mention";
// Highlight messages having personal mentions only in DMs and subscribed streams.
if (
message_container.msg.type === "private" ||
stream_data.is_user_subscribed(
message_container.msg.stream_id,
people.my_current_user_id(),
)
) {
message_container.mention_classname = "direct_mention";
}
} else {
message_container.mention_classname = "group_mention";
}

63
web/src/rendered_markdown.js
View File

@ -14,7 +14,6 @@ import * as people from "./people";
import * as realm_playground from "./realm_playground";
import * as rows from "./rows";
import * as rtl from "./rtl";
import * as stream_data from "./stream_data";
import * as sub_store from "./sub_store";
import * as timerender from "./timerender";
import * as user_groups from "./user_groups";
@ -66,6 +65,21 @@ function get_user_group_id_for_mention_button(elem) {
return undefined;
}
function get_message_for_message_content($content) {
// TODO: This selector is designed to exclude drafts/scheduled
// messages. Arguably those settings should be unconditionally
// marked with user-mention-me, but rows.id doesn't support
// those elements, and we should address that quirk for
// mentions holistically.
const $message_row = $content.closest(".message_row");
if (!$message_row.length || $message_row.closest(".overlay-message-row").length) {
// There's no containing message when rendering a preview.
return undefined;
}
const message_id = rows.id($message_row);
return message_store.get(message_id);
}
// Helper function to update a mentioned user's name.
export function set_name_in_mention_element(element, name, user_id) {
if (user_id !== undefined && people.should_add_guest_user_indicator(user_id)) {
@ -101,30 +115,24 @@ export const update_elements = ($content) => {
});
}
// personal and stream wildcard mentions
$content.find(".user-mention").each(function () {
const user_id = get_user_id_for_mention_button(this);
const message = get_message_for_message_content($content);
// We give special highlights to the mention buttons
// that refer to the current user.
if (user_id === "*" || people.is_my_user_id(user_id)) {
const $message_header_stream = $content
.closest(".recipient_row")
.find(".message_header.message_header_stream");
// If stream message
if ($message_header_stream.length) {
const stream_id = Number.parseInt(
$message_header_stream.attr("data-stream-id"),
10,
);
// Don't highlight the mention if user is not subscribed to the stream.
if (stream_data.is_user_subscribed(stream_id, people.my_current_user_id())) {
// Either a wildcard mention or us, so mark it.
$(this).addClass("user-mention-me");
}
} else {
// Always highlight wildcard mentions in direct messages.
$(this).addClass("user-mention-me");
}
if (user_id === "*" && message && message.stream_wildcard_mentioned) {
$(this).addClass("user-mention-me");
}
if (
user_id !== "*" &&
people.is_my_user_id(user_id) &&
message &&
message.mentioned_me_directly
) {
$(this).addClass("user-mention-me");
}
if (user_id && user_id !== "*" && !$(this).find(".highlight").length) {
// If it's a mention of a specific user, edit the
// mention text to show the user's current name,
@ -140,20 +148,9 @@ export const update_elements = ($content) => {
});
$content.find(".topic-mention").each(function () {
// TODO: This selector is designed to exclude drafts/scheduled
// messages. Arguably those settings should be unconditionally
// marked with user-mention-me, but rows.id doesn't support
// those elements, and we should address that quirk for
// mentions holistically.
const $message_row = $content.closest(".message_row");
if (!$message_row.length || $message_row.closest(".overlay-message-row").length) {
// There's no containing message when rendering a preview.
return;
}
const message_id = rows.id($message_row);
const message = message_store.get(message_id);
const message = get_message_for_message_content($content);
if (message.topic_wildcard_mentioned) {
if (message && message.topic_wildcard_mentioned) {
$(this).addClass("user-mention-me");
}
});

6
web/styles/rendered_markdown.css
View File

@ -185,11 +185,6 @@
&.user-mention-me {
color: var(--color-text-self-direct-mention);
font-weight: 600;
&[data-user-id="*"] {
color: var(--color-text-self-group-mention);
background-color: var(--color-background-text-group-mention);
}
}
&:hover {
@ -197,6 +192,7 @@
}
}
.user-mention[data-user-id="*"],
.user-group-mention,
.topic-mention {
color: var(--color-text-other-mention);

130
web/tests/rendered_markdown.test.js
View File

@ -88,14 +88,34 @@ const $array = (array) => {
return {each};
};
function set_closest_dot_find_result($content, value) {
$content.closest = (closest_opts) => {
assert.equal(closest_opts, ".recipient_row");
const find = (find_opts) => {
assert.equal(find_opts, ".message_header.message_header_stream");
return value;
function set_message_for_message_content($content, value) {
// no message row found
if (value === undefined) {
$content.closest = (closest_opts) => {
assert.equal(closest_opts, ".message_row");
return [];
};
return {find};
return;
}
// message row found
const $message_row = $.create(".message-row");
$content.closest = (closest_opts) => {
assert.equal(closest_opts, ".message_row");
return $message_row;
};
$message_row.length = 1;
$message_row.closest = (closest_opts) => {
assert.equal(closest_opts, ".overlay-message-row");
return [];
};
const message_id = 100;
rows.id = (message_row) => {
assert.equal(message_row, $message_row);
return message_id;
};
message_store.get = (message_id_opt) => {
assert.equal(message_id_opt, message_id);
return value;
};
}
@ -112,7 +132,7 @@ const get_content_element = () => {
$content.set_find_results("div.spoiler-header", $array([]));
$content.set_find_results("div.codehilite", $array([]));
$content.set_find_results(".message_inline_video video", $array([]));
set_closest_dot_find_result($content, []);
set_message_for_message_content($content, undefined);
// Fend off dumb security bugs by forcing devs to be
// intentional about HTML manipulation.
@ -186,12 +206,16 @@ run_test("user-mention", () => {
assert.equal($polonius.text(), "never-been-set");
rm.update_elements($content);
// Final asserts
assert.ok($iago.hasClass("user-mention-me"));
assert.ok(!$iago.hasClass("user-mention-me"));
assert.equal($iago.text(), `@${iago.full_name}`);
assert.equal($cordelia.text(), `@${cordelia.full_name}`);
assert.equal($polonius.text(), `translated: @${polonius.full_name} (guest)`);
// message row found
const message = {mentioned_me_directly: true};
set_message_for_message_content($content, message);
rm.update_elements($content);
assert.ok($iago.hasClass("user-mention-me"));
});
run_test("user-mention without guest indicator", () => {
@ -206,49 +230,20 @@ run_test("user-mention without guest indicator", () => {
assert.equal($polonius.text(), `@${polonius.full_name}`);
});
run_test("user-mention PM (wildcard)", () => {
run_test("user-mention (stream wildcard)", () => {
// Setup
const $content = get_content_element();
const $mention = $.create("mention");
$mention.attr("data-user-id", "*");
$content.set_find_results(".user-mention", $array([$mention]));
const message = {stream_wildcard_mentioned: true};
set_message_for_message_content($content, message);
assert.ok(!$mention.hasClass("user-mention-me"));
rm.update_elements($content);
assert.ok($mention.hasClass("user-mention-me"));
});
run_test("user-mention Stream subbed (wildcard)", ({override_rewire}) => {
// Setup
const $content = get_content_element();
const $mention = $.create("mention");
$mention.attr("data-user-id", "*");
$content.set_find_results(".user-mention", $array([$mention]));
const attr = () => stream.stream_id;
set_closest_dot_find_result($content, {attr, length: 1});
override_rewire(stream_data, "is_user_subscribed", () => true);
assert.ok(!$mention.hasClass("user-mention-me"));
rm.update_elements($content);
assert.ok($mention.hasClass("user-mention-me"));
});
run_test("user-mention Stream not subbed (wildcard)", ({override_rewire}) => {
// Setup
const $content = get_content_element();
const $mention = $.create("mention");
$mention.attr("data-user-id", "*");
$content.set_find_results(".user-mention", $array([$mention]));
const attr = () => 1;
set_closest_dot_find_result($content, {attr, length: 1});
override_rewire(stream_data, "is_user_subscribed", () => false);
// Don't add user-mention-me class.
assert.ok(!$mention.hasClass("user-mention-me"));
rm.update_elements($content);
assert.ok(!$mention.hasClass("user-mention-me"));
});
run_test("user-mention (email)", () => {
// Setup
const $content = get_content_element();
@ -277,39 +272,16 @@ run_test("topic-mention", () => {
const $mention = $.create("mention");
$content.set_find_results(".topic-mention", $array([$mention]));
// no message row found
$content.closest = (closest_opts) => {
assert.equal(closest_opts, ".message_row");
return [];
};
// when no message row found
assert.ok(!$mention.hasClass("user-mention-me"));
rm.update_elements($content);
assert.ok(!$mention.hasClass("user-mention-me"));
// message row found
const $message_row = $.create(".message-row");
$content.closest = (closest_opts) => {
assert.equal(closest_opts, ".message_row");
return $message_row;
};
$message_row.length = 1;
$message_row.closest = (closest_opts) => {
assert.equal(closest_opts, ".overlay-message-row");
return [];
};
const message_id = 100;
rows.id = (message_row) => {
assert.equal(message_row, $message_row);
return message_id;
};
const message = {
stream_id: 1,
topic_wildcard_mentioned: true,
};
message_store.get = (message_id_opt) => {
assert.equal(message_id_opt, message_id);
return message;
};
set_message_for_message_content($content, message);
assert.ok(!$mention.hasClass("user-mention-me"));
rm.update_elements($content);
@ -321,29 +293,11 @@ run_test("topic-mention not topic participant", () => {
const $content = get_content_element();
const $mention = $.create("mention");
$content.set_find_results(".topic-mention", $array([$mention]));
const $message_row = $.create(".message-row");
$content.closest = (closest_opts) => {
assert.equal(closest_opts, ".message_row");
return $message_row;
};
$message_row.length = 1;
$message_row.closest = (closest_opts) => {
assert.equal(closest_opts, ".overlay-message-row");
return [];
};
const message_id = 100;
rows.id = (message_row) => {
assert.equal(message_row, $message_row);
return message_id;
};
const message = {
stream_id: 1,
topic_wildcard_mentioned: false,
};
message_store.get = (message_id_opt) => {
assert.equal(message_id_opt, message_id);
return message;
};
set_message_for_message_content($content, message);
assert.ok(!$mention.hasClass("user-mention-me"));
rm.update_elements($content);