Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

js: Break cyclic dependency between unread_ops and recent_topics_ui. #21

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion web/src/compose_actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,11 @@ export function respond_to_message(opts) {
}

if (message_lists.current.can_mark_messages_read()) {
unread_ops.notify_server_message_read(message);
unread_ops.notify_server_message_read(
message,
{},
recent_topics_ui.update_topic_unread_count,
);
}
}

Expand Down
4 changes: 2 additions & 2 deletions web/src/message_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function maybe_add_narrowed_messages(messages, msg_list, callback, attempt = 1)
);

callback(new_messages, msg_list);
unread_ops.process_visible();
unread_ops.process_visible(recent_topics_ui.update_topic_unread_count);
notifications.notify_messages_outside_current_search(elsewhere_messages);
},
error(xhr) {
Expand Down Expand Up @@ -159,7 +159,7 @@ export function insert_new_messages(messages, sent_by_this_client) {
unread_ui.update_unread_counts();
}

unread_ops.process_visible();
unread_ops.process_visible(recent_topics_ui.update_topic_unread_count);
notifications.received_messages(messages);
stream_list.update_streams_sidebar();
pm_list.update_private_messages();
Expand Down
3 changes: 2 additions & 1 deletion web/src/message_flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import _ from "lodash";

import * as channel from "./channel";
import * as message_store from "./message_store";
import * as recent_topics_ui from "./recent_topics_ui";
import * as starred_messages from "./starred_messages";
import * as ui from "./ui";
import * as unread_ops from "./unread_ops";
Expand Down Expand Up @@ -104,7 +105,7 @@ export function toggle_starred_and_update_server(message) {
// msg_list.can_mark_messages_read, because starring a message is an
// explicit interaction and we'd like to preserve the user
// expectation invariant that all starred messages are read.
unread_ops.notify_server_message_read(message);
unread_ops.notify_server_message_read(message, {}, recent_topics_ui.update_topic_unread_count);
ui.update_starred_view(message.id, message.starred);

if (message.starred) {
Expand Down
9 changes: 7 additions & 2 deletions web/src/message_scroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as message_lists from "./message_lists";
import * as message_viewport from "./message_viewport";
import * as narrow_banner from "./narrow_banner";
import * as narrow_state from "./narrow_state";
import * as recent_topics_ui from "./recent_topics_ui";
import * as recent_topics_util from "./recent_topics_util";
import * as unread from "./unread";
import * as unread_ops from "./unread_ops";
Expand Down Expand Up @@ -246,7 +247,7 @@ export function initialize() {
message_viewport.$message_pane.on(
"scroll",
_.throttle(() => {
unread_ops.process_visible();
unread_ops.process_visible(recent_topics_ui.update_topic_unread_count);
scroll_finish();
}, 50),
);
Expand All @@ -266,7 +267,11 @@ export function initialize() {
messages = event.msg_list.message_range(event.previously_selected_id, event.id);
}
if (event.msg_list.can_mark_messages_read()) {
unread_ops.notify_server_messages_read(messages, {from: "pointer"});
unread_ops.notify_server_messages_read(
messages,
{from: "pointer"},
recent_topics_ui.update_topic_unread_count,
);
} else if (
unread.get_unread_messages(messages).length !== 0 &&
// The below checks might seem redundant, but it's
Expand Down
6 changes: 3 additions & 3 deletions web/src/narrow.js
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,7 @@ export function update_selection(opts) {
// narrowing
message_lists.current.view.set_message_offset(select_offset);
}
unread_ops.process_visible();
unread_ops.process_visible(recent_topics_ui.update_topic_unread_count);
}

export function activate_stream_for_cycle_hotkey(stream_name) {
Expand Down Expand Up @@ -923,7 +923,7 @@ export function by_topic(target_id, opts) {
// We don't check msg_list.can_mark_messages_read here only because
// the target msg_list isn't initialized yet; in any case, the
// message is about to be marked read in the new view.
unread_ops.notify_server_message_read(original);
unread_ops.notify_server_message_read(original, {}, recent_topics_ui.update_topic_unread_count);

const search_terms = [
{operator: "stream", operand: original.stream},
Expand All @@ -942,7 +942,7 @@ export function by_recipient(target_id, opts) {
// We don't check msg_list.can_mark_messages_read here only because
// the target msg_list isn't initialized yet; in any case, the
// message is about to be marked read in the new view.
unread_ops.notify_server_message_read(message);
unread_ops.notify_server_message_read(message, {}, recent_topics_ui.update_topic_unread_count);

switch (message.type) {
case "private":
Expand Down
3 changes: 2 additions & 1 deletion web/src/notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import * as navigate from "./navigate";
import {page_params} from "./page_params";
import * as people from "./people";
import {realm_user_settings_defaults} from "./realm_user_settings_defaults";
import * as recent_topics_ui from "./recent_topics_ui";
import * as settings_config from "./settings_config";
import * as spoilers from "./spoilers";
import * as stream_data from "./stream_data";
Expand Down Expand Up @@ -85,7 +86,7 @@ export function initialize() {

// Update many places on the DOM to reflect unread
// counts.
unread_ops.process_visible();
unread_ops.process_visible(recent_topics_ui.update_topic_unread_count);
})
.on("blur", () => {
window_focused = false;
Expand Down
22 changes: 16 additions & 6 deletions web/src/server_events_dispatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import * as realm_icon from "./realm_icon";
import * as realm_logo from "./realm_logo";
import * as realm_playground from "./realm_playground";
import {realm_user_settings_defaults} from "./realm_user_settings_defaults";
import * as recent_topics_ui from "./recent_topics_ui";
import * as reload from "./reload";
import * as scroll_bar from "./scroll_bar";
import * as settings_account from "./settings_account";
Expand Down Expand Up @@ -102,7 +103,10 @@ export function dispatch_normal_event(event) {
// message is passed to unread.get_unread_messages,
// which returns all the unread messages out of a given list.
// So double marking something as read would not occur
unread_ops.process_read_messages_event(msg_ids);
unread_ops.process_read_messages_event(
msg_ids,
recent_topics_ui.update_topic_unread_count,
);
// This methods updates message_list too and since stream_topic_history relies on it
// this method should be called first.
message_events.remove_messages(msg_ids);
Expand Down Expand Up @@ -758,12 +762,18 @@ export function dispatch_normal_event(event) {
break;
case "read":
if (event.op === "add") {
unread_ops.process_read_messages_event(event.messages);
unread_ops.process_read_messages_event(
event.messages,
recent_topics_ui.update_topic_unread_count,
);
} else {
unread_ops.process_unread_messages_event({
message_ids: event.messages,
message_details: event.message_details,
});
unread_ops.process_unread_messages_event(
{
message_ids: event.messages,
message_details: event.message_details,
},
recent_topics_ui.complete_rerender,
);
}
break;
}
Expand Down
33 changes: 18 additions & 15 deletions web/src/unread_ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import * as message_viewport from "./message_viewport";
import * as narrow_state from "./narrow_state";
import * as notifications from "./notifications";
import * as people from "./people";
import * as recent_topics_ui from "./recent_topics_ui";
import * as ui_report from "./ui_report";
import * as unread from "./unread";
import * as unread_ui from "./unread_ui";
Expand Down Expand Up @@ -148,12 +147,12 @@ export function mark_all_as_read(args = {}) {
});
}

function process_newly_read_message(message, options) {
function process_newly_read_message(message, options, update_topic_unread_count) {
for (const msg_list of message_lists.all_rendered_message_lists()) {
msg_list.show_message_as_read(message, options);
}
notifications.close_notification(message);
recent_topics_ui.update_topic_unread_count(message);
update_topic_unread_count(message);
}

export function mark_as_unread_from_here(
Expand Down Expand Up @@ -257,7 +256,7 @@ export function resume_reading() {
message_lists.current.resume_reading();
}

export function process_read_messages_event(message_ids) {
export function process_read_messages_event(message_ids, update_topic_unread_count) {
/*
This code has a lot in common with notify_server_messages_read,
but there are subtle differences due to the fact that the
Expand Down Expand Up @@ -287,14 +286,14 @@ export function process_read_messages_event(message_ids) {
// recent conversations per message, not a single global
// rerender or one per conversation.
if (message) {
process_newly_read_message(message, options);
process_newly_read_message(message, options, update_topic_unread_count);
}
}

unread_ui.update_unread_counts();
}

export function process_unread_messages_event({message_ids, message_details}) {
export function process_unread_messages_event({message_ids, message_details}, complete_rerender) {
// This is the reverse of process_unread_messages_event.
message_ids = unread.get_read_message_ids(message_ids);
if (message_ids.length === 0) {
Expand Down Expand Up @@ -347,7 +346,7 @@ export function process_unread_messages_event({message_ids, message_details}) {
user-visible in the form of users' avatars flickering.
*/
message_live_update.rerender_messages_view_by_message_ids(message_ids);
recent_topics_ui.complete_rerender();
complete_rerender();

if (
!message_lists.current.can_mark_messages_read() &&
Expand All @@ -361,7 +360,7 @@ export function process_unread_messages_event({message_ids, message_details}) {

// Takes a list of messages and marks them as read.
// Skips any messages that are already marked as read.
export function notify_server_messages_read(messages, options = {}) {
export function notify_server_messages_read(messages, options = {}, update_topic_unread_count) {
messages = unread.get_unread_messages(messages);
if (messages.length === 0) {
return;
Expand All @@ -375,7 +374,7 @@ export function notify_server_messages_read(messages, options = {}) {
}

unread.mark_as_read(message.id);
process_newly_read_message(message, options);
process_newly_read_message(message, options, update_topic_unread_count);
}

unread_ui.update_unread_counts();
Expand All @@ -385,14 +384,14 @@ export function notify_server_message_read(message, options) {
notify_server_messages_read([message], options);
}

export function process_scrolled_to_bottom() {
export function process_scrolled_to_bottom(update_topic_unread_count) {
if (!narrow_state.is_message_feed_visible()) {
// First, verify the current message list is visible.
return;
}

if (message_lists.current.can_mark_messages_read()) {
mark_current_list_as_read();
mark_current_list_as_read({}, update_topic_unread_count);
return;
}

Expand All @@ -406,14 +405,18 @@ export function process_scrolled_to_bottom() {

// If we ever materially change the algorithm for this function, we
// may need to update notifications.received_messages as well.
export function process_visible() {
export function process_visible(update_topic_unread_count) {
if (message_viewport.is_visible_and_focused() && message_viewport.bottom_message_visible()) {
process_scrolled_to_bottom();
process_scrolled_to_bottom(update_topic_unread_count);
}
}

export function mark_current_list_as_read(options) {
notify_server_messages_read(message_lists.current.all_messages(), options);
export function mark_current_list_as_read(options, update_topic_unread_count) {
notify_server_messages_read(
message_lists.current.all_messages(),
options,
update_topic_unread_count,
);
}

export function mark_stream_as_read(stream_id, cont) {
Expand Down
5 changes: 3 additions & 2 deletions web/tests/example7.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ message_lists.home = {};
message_lists.all_rendered_message_lists = () => [message_lists.home, message_lists.current];

const message_store = zrequire("message_store");
const recent_topics_ui = zrequire("recent_topics_ui");
const recent_topics_util = zrequire("recent_topics_util");
const stream_data = zrequire("stream_data");
const unread = zrequire("unread");
Expand Down Expand Up @@ -129,13 +130,13 @@ run_test("unread_ops", ({override}) => {

// First, test for a message list that cannot read messages.
can_mark_messages_read = false;
unread_ops.process_visible();
unread_ops.process_visible(recent_topics_ui.update_topic_unread_count);

assert.deepEqual(channel_post_opts, undefined);

// Now flip the boolean, and get to the main thing we are testing.
can_mark_messages_read = true;
unread_ops.process_visible();
unread_ops.process_visible(recent_topics_ui.update_topic_unread_count);

// The most important side effect of the above call is that
// we post info to the server. We can verify that the correct
Expand Down