recent_topics: Add mention indicator in row for unread topics.

Fixes #22984

Add an `@` icon in unread topics where user is mentioned.

We track a new set of `stream_id:topic` pairs for the unread mentions
so that recent topics instantly knows if a topic is unread and mentioned
or not.
This commit is contained in:
Aman Agrawal 2022-10-14 15:37:47 +00:00 committed by Tim Abbott
parent 228ec4bde4
commit e3f22a9907
12 changed files with 125 additions and 5 deletions

View File

@ -84,6 +84,7 @@ run_test("insert_message", ({override}) => {
sender_id: isaac.user_id, sender_id: isaac.user_id,
id: 1001, id: 1001,
content: "example content", content: "example content",
topic: "Foo",
}; };
assert.equal(message_store.get(new_message.id), undefined); assert.equal(message_store.get(new_message.id), undefined);

View File

@ -94,6 +94,7 @@ run_test("unread_ops", ({override}) => {
// Make our test message appear to be unread, so that // Make our test message appear to be unread, so that
// we then need to subsequently process them as read. // we then need to subsequently process them as read.
message_store.update_message_cache(test_messages[0]);
unread.process_loaded_messages(test_messages); unread.process_loaded_messages(test_messages);
// Make our message_viewport appear visible. // Make our message_viewport appear visible.

View File

@ -150,6 +150,7 @@ mock_esm("../../static/js/unread", {
} }
return 1; return 1;
}, },
topic_has_any_unread_mentions: () => false,
}); });
const {all_messages_data} = zrequire("all_messages_data"); const {all_messages_data} = zrequire("all_messages_data");
@ -293,6 +294,7 @@ function generate_topic_data(topic_info_array) {
conversation_key: get_topic_key(stream_id, topic), conversation_key: get_topic_key(stream_id, topic),
topic_url: "https://www.example.com", topic_url: "https://www.example.com",
unread_count, unread_count,
mention_in_unread: false,
muted, muted,
topic_muted: muted, topic_muted: muted,
participated, participated,

View File

@ -10,6 +10,7 @@ const message_util = mock_esm("../../static/js/message_util");
const all_messages_data = zrequire("all_messages_data"); const all_messages_data = zrequire("all_messages_data");
const unread = zrequire("unread"); const unread = zrequire("unread");
const message_store = zrequire("message_store");
const stream_data = zrequire("stream_data"); const stream_data = zrequire("stream_data");
const stream_topic_history = zrequire("stream_topic_history"); const stream_topic_history = zrequire("stream_topic_history");
const stream_topic_history_util = zrequire("stream_topic_history_util"); const stream_topic_history_util = zrequire("stream_topic_history_util");
@ -266,6 +267,7 @@ test("test_unread_logic", () => {
msg.type = "stream"; msg.type = "stream";
msg.stream_id = stream_id; msg.stream_id = stream_id;
msg.unread = true; msg.unread = true;
message_store.update_message_cache(msg);
} }
unread.process_loaded_messages(msgs); unread.process_loaded_messages(msgs);

View File

@ -7,6 +7,14 @@ const _ = require("lodash");
const {mock_esm, zrequire} = require("../zjsunit/namespace"); const {mock_esm, zrequire} = require("../zjsunit/namespace");
const {run_test} = require("../zjsunit/test"); const {run_test} = require("../zjsunit/test");
mock_esm("../../static/js/message_store", {
get() {
return {
stream_id: 556,
topic: "general",
};
},
});
const user_topics = mock_esm("../../static/js/user_topics", { const user_topics = mock_esm("../../static/js/user_topics", {
is_topic_muted() { is_topic_muted() {
return false; return false;

View File

@ -4,12 +4,13 @@ const {strict: assert} = require("assert");
const _ = require("lodash"); const _ = require("lodash");
const {zrequire} = require("../zjsunit/namespace"); const {zrequire, set_global} = require("../zjsunit/namespace");
const {run_test} = require("../zjsunit/test"); const {run_test} = require("../zjsunit/test");
const {page_params, user_settings} = require("../zjsunit/zpage_params"); const {page_params, user_settings} = require("../zjsunit/zpage_params");
page_params.realm_push_notifications_enabled = false; page_params.realm_push_notifications_enabled = false;
set_global("document", "document-stub");
const {FoldDict} = zrequire("fold_dict"); const {FoldDict} = zrequire("fold_dict");
const message_store = zrequire("message_store"); const message_store = zrequire("message_store");
const user_topics = zrequire("user_topics"); const user_topics = zrequire("user_topics");
@ -98,6 +99,8 @@ test("changing_topics", () => {
topic: "lunCH", topic: "lunCH",
unread: true, unread: true,
}; };
message_store.update_message_cache(message);
message_store.update_message_cache(other_message);
assert.deepEqual(unread.get_read_message_ids([15, 16]), [15, 16]); assert.deepEqual(unread.get_read_message_ids([15, 16]), [15, 16]);
assert.deepEqual(unread.get_unread_message_ids([15, 16]), []); assert.deepEqual(unread.get_unread_message_ids([15, 16]), []);
@ -266,6 +269,7 @@ test("num_unread_for_topic", () => {
let i; let i;
for (i = num_msgs; i > 0; i -= 1) { for (i = num_msgs; i > 0; i -= 1) {
message.id = i; message.id = i;
message_store.update_message_cache(message);
unread.process_loaded_messages([message]); unread.process_loaded_messages([message]);
} }
@ -363,7 +367,7 @@ test("phantom_messages", () => {
stream_id: 555, stream_id: 555,
topic: "phantom", topic: "phantom",
}; };
message_store.update_message_cache(message);
unread.mark_as_read(message.id); unread.mark_as_read(message.id);
const counts = unread.get_counts(); const counts = unread.get_counts();
assert.equal(counts.home_unread_messages, 0); assert.equal(counts.home_unread_messages, 0);
@ -557,6 +561,7 @@ test("mention updates", () => {
id: 17, id: 17,
unread: false, unread: false,
type: "stream", type: "stream",
topic: "hello",
}; };
function test_counted(counted) { function test_counted(counted) {

View File

@ -183,8 +183,6 @@ export function update_messages(events) {
message_store.update_booleans(msg, event.flags); message_store.update_booleans(msg, event.flags);
unread.update_message_for_mention(msg);
condense.un_cache_message_content_height(msg.id); condense.un_cache_message_content_height(msg.id);
if (event.rendered_content !== undefined) { if (event.rendered_content !== undefined) {
@ -228,6 +226,8 @@ export function update_messages(events) {
msg.raw_content = event.content; msg.raw_content = event.content;
} }
unread.update_message_for_mention(msg, any_message_content_edited);
// new_topic will be undefined if the topic is unchanged. // new_topic will be undefined if the topic is unchanged.
const new_topic = util.get_edit_event_topic(event); const new_topic = util.get_edit_event_topic(event);
// new_stream_id will be undefined if the stream is unchanged. // new_stream_id will be undefined if the stream is unchanged.
@ -489,6 +489,7 @@ export function update_messages(events) {
new_stream_id: post_edit_stream_id, new_stream_id: post_edit_stream_id,
new_topic: post_edit_topic, new_topic: post_edit_topic,
}); });
unread.clear_and_populate_unread_mention_topics();
recent_topics_ui.process_topic_edit(...args); recent_topics_ui.process_topic_edit(...args);
} }

View File

@ -343,6 +343,10 @@ function format_conversation(conversation_data) {
context.topic_muted = Boolean(user_topics.is_topic_muted(context.stream_id, context.topic)); context.topic_muted = Boolean(user_topics.is_topic_muted(context.stream_id, context.topic));
const stream_muted = stream_data.is_muted(context.stream_id); const stream_muted = stream_data.is_muted(context.stream_id);
context.muted = context.topic_muted || stream_muted; context.muted = context.topic_muted || stream_muted;
context.mention_in_unread = unread.topic_has_any_unread_mentions(
context.stream_id,
context.topic,
);
// Display in most recent sender first order // Display in most recent sender first order
all_senders = recent_senders.get_topic_recent_senders(context.stream_id, context.topic); all_senders = recent_senders.get_topic_recent_senders(context.stream_id, context.topic);

View File

@ -2,6 +2,8 @@ import {FoldDict} from "./fold_dict";
import * as message_store from "./message_store"; import * as message_store from "./message_store";
import {page_params} from "./page_params"; import {page_params} from "./page_params";
import * as people from "./people"; import * as people from "./people";
import * as recent_topics_ui from "./recent_topics_ui";
import * as recent_topics_util from "./recent_topics_util";
import * as settings_config from "./settings_config"; import * as settings_config from "./settings_config";
import * as stream_data from "./stream_data"; import * as stream_data from "./stream_data";
import * as sub_store from "./sub_store"; import * as sub_store from "./sub_store";
@ -30,6 +32,38 @@ export function set_messages_read_in_narrow(value) {
export const unread_mentions_counter = new Set(); export const unread_mentions_counter = new Set();
const unread_messages = new Set(); const unread_messages = new Set();
// Map with keys of the form "{stream_id}:{topic.toLowerCase()}" and
// values being Sets of message IDs for unread messages mentioning the
// user within that topic. Use `recent_topics_util.get_topic_key` to
// calculate keys.
//
// Functionally a cache; see clear_and_populate_unread_mention_topics
// for how we can refresh it efficiently.
export const unread_mention_topics = new Map();
function add_message_to_unread_mention_topics(message_id) {
const message = message_store.get(message_id);
if (message.type !== "stream") {
return;
}
const topic_key = recent_topics_util.get_topic_key(message.stream_id, message.topic);
if (unread_mention_topics.has(topic_key)) {
unread_mention_topics.get(topic_key).add(message_id);
}
unread_mention_topics.set(topic_key, new Set([message_id]));
}
function remove_message_from_unread_mention_topics(message_id) {
const message = message_store.get(message_id);
if (message.type !== "stream") {
return;
}
const topic_key = recent_topics_util.get_topic_key(message.stream_id, message.topic);
if (unread_mention_topics.has(topic_key)) {
unread_mention_topics.get(topic_key).delete(message_id);
}
}
class Bucketer { class Bucketer {
// Maps item_id => bucket_key for items present in a bucket. // Maps item_id => bucket_key for items present in a bucket.
reverse_lookup = new Map(); reverse_lookup = new Map();
@ -433,6 +467,36 @@ class UnreadTopicCounter {
} }
const unread_topic_counter = new UnreadTopicCounter(); const unread_topic_counter = new UnreadTopicCounter();
export function clear_and_populate_unread_mention_topics() {
// The unread_mention_topics is an important data structure for
// efficiently querying whether a given stream/topic pair contains
// unread mentions.
//
// It is effectively a cache, since it can be reconstructed from
// unread_mentions_counter (IDs for all unread mentions) and
// unread_topic_counter (Streams/topics for all unread messages).
//
// Since this function runs in O(unread mentions) time, we can use
// it in topic editing code paths where it might be onerous to
// write custom live-update code; but we should avoid calling it
// in loops.
unread_mention_topics.clear();
for (const message_id of unread_mentions_counter) {
const stream_id = unread_topic_counter.bucketer.reverse_lookup.get(message_id);
if (!stream_id) {
continue;
}
const per_stream_bucketer = unread_topic_counter.bucketer.get_bucket(stream_id);
const topic = per_stream_bucketer.reverse_lookup.get(message_id);
const topic_key = recent_topics_util.get_topic_key(stream_id, topic);
if (unread_mention_topics.has(topic_key)) {
unread_mention_topics.get(topic_key).add(message_id);
}
unread_mention_topics.set(topic_key, new Set([message_id]));
}
}
export function message_unread(message) { export function message_unread(message) {
if (message === undefined) { if (message === undefined) {
return false; return false;
@ -518,9 +582,10 @@ export function process_unread_message(message) {
update_message_for_mention(message); update_message_for_mention(message);
} }
export function update_message_for_mention(message) { export function update_message_for_mention(message, content_edited = false) {
if (!message.unread) { if (!message.unread) {
unread_mentions_counter.delete(message.id); unread_mentions_counter.delete(message.id);
remove_message_from_unread_mention_topics(message.id);
return; return;
} }
@ -531,8 +596,17 @@ export function update_message_for_mention(message) {
if (is_unmuted_mention || message.mentioned_me_directly) { if (is_unmuted_mention || message.mentioned_me_directly) {
unread_mentions_counter.add(message.id); unread_mentions_counter.add(message.id);
add_message_to_unread_mention_topics(message.id);
} else { } else {
unread_mentions_counter.delete(message.id); unread_mentions_counter.delete(message.id);
remove_message_from_unread_mention_topics(message.id);
}
if (content_edited && message.type === "stream") {
// We only need to update recent topics here if this was a content change in an unread
// mention, since in other cases recent topics gets rerendered by other functions.
const topic_key = recent_topics_util.get_topic_key(message.stream_id, message.topic);
recent_topics_ui.inplace_rerender(topic_key);
} }
} }
@ -544,6 +618,7 @@ export function mark_as_read(message_id) {
unread_topic_counter.delete(message_id); unread_topic_counter.delete(message_id);
unread_mentions_counter.delete(message_id); unread_mentions_counter.delete(message_id);
unread_messages.delete(message_id); unread_messages.delete(message_id);
remove_message_from_unread_mention_topics(message_id);
const message = message_store.get(message_id); const message = message_store.get(message_id);
if (message) { if (message) {
@ -556,6 +631,7 @@ export function declare_bankruptcy() {
unread_topic_counter.clear(); unread_topic_counter.clear();
unread_mentions_counter.clear(); unread_mentions_counter.clear();
unread_messages.clear(); unread_messages.clear();
unread_mention_topics.clear();
} }
export function get_counts() { export function get_counts() {
@ -619,10 +695,19 @@ export function num_unread_for_topic(stream_id, topic_name) {
} }
export function stream_has_any_unread_mentions(stream_id) { export function stream_has_any_unread_mentions(stream_id) {
// This function is somewhat inefficient and thus should not be
// called in loops, since runs in O(total unread mentions) time.
const streams_with_mentions = unread_topic_counter.get_streams_with_unread_mentions(); const streams_with_mentions = unread_topic_counter.get_streams_with_unread_mentions();
return streams_with_mentions.has(stream_id); return streams_with_mentions.has(stream_id);
} }
export function topic_has_any_unread_mentions(stream_id, topic) {
// Because this function is called in a loop for every displayed
// Recent Topics row, it's important for it to run in O(1) time.
const topic_key = stream_id + ":" + topic.toLowerCase();
return unread_mention_topics.get(topic_key) && unread_mention_topics.get(topic_key).size > 0;
}
export function topic_has_any_unread(stream_id, topic) { export function topic_has_any_unread(stream_id, topic) {
return unread_topic_counter.topic_has_any_unread(stream_id, topic); return unread_topic_counter.topic_has_any_unread(stream_id, topic);
} }
@ -685,6 +770,7 @@ export function initialize() {
for (const message_id of unread_msgs.mentions) { for (const message_id of unread_msgs.mentions) {
unread_mentions_counter.add(message_id); unread_mentions_counter.add(message_id);
} }
clear_and_populate_unread_mention_topics();
for (const obj of unread_msgs.huddles) { for (const obj of unread_msgs.huddles) {
for (const message_id of obj.unread_message_ids) { for (const message_id of obj.unread_message_ids) {

View File

@ -170,6 +170,10 @@
opacity: 0.7; opacity: 0.7;
} }
.unread_mention_info:not(:empty) {
margin-right: -5px;
}
.unread_hidden { .unread_hidden {
visibility: hidden; visibility: hidden;
} }
@ -209,6 +213,10 @@
flex-flow: row nowrap; flex-flow: row nowrap;
} }
.mention_in_unread {
opacity: 0.7;
}
.recent_topic_actions .recent_topics_focusable { .recent_topic_actions .recent_topics_focusable {
display: flex; display: flex;
align-items: center; align-items: center;

View File

@ -39,6 +39,7 @@
{{#if is_private}} {{#if is_private}}
<span class="unread_count unread_count_pm {{#unless unread_count}}unread_hidden{{/unless}}">{{unread_count}}</span> <span class="unread_count unread_count_pm {{#unless unread_count}}unread_hidden{{/unless}}">{{unread_count}}</span>
{{else}} {{else}}
<span class="unread_mention_info {{#unless mention_in_unread}}unread_hidden{{/unless}}">@</span>
<div class="recent_topic_actions"> <div class="recent_topic_actions">
<div class="recent_topics_focusable hidden-for-spectators"> <div class="recent_topics_focusable hidden-for-spectators">
<span class="unread_count {{#unless unread_count}}unread_hidden{{/unless}} tippy-zulip-tooltip on_hover_topic_read" data-stream-id="{{stream_id}}" data-topic-name="{{topic}}" data-tippy-content="{{t 'Mark as read' }}" role="button" tabindex="0" aria-label="{{t 'Mark as read' }}">{{unread_count}}</span> <span class="unread_count {{#unless unread_count}}unread_hidden{{/unless}} tippy-zulip-tooltip on_hover_topic_read" data-stream-id="{{stream_id}}" data-topic-name="{{topic}}" data-tippy-content="{{t 'Mark as read' }}" role="button" tabindex="0" aria-label="{{t 'Mark as read' }}">{{unread_count}}</span>

View File

@ -197,6 +197,7 @@ EXEMPT_FILES = make_set(
"static/js/ui_init.js", "static/js/ui_init.js",
"static/js/ui_report.ts", "static/js/ui_report.ts",
"static/js/ui_util.ts", "static/js/ui_util.ts",
"static/js/unread.js",
"static/js/unread_ops.js", "static/js/unread_ops.js",
"static/js/unread_ui.js", "static/js/unread_ui.js",
"static/js/upload_widget.ts", "static/js/upload_widget.ts",