From 7ad3225ecc9b68e034dd1c53040ffad2265e5163 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Wed, 31 May 2023 17:40:51 -0700 Subject: [PATCH] starred_messages: Extract starred_messages_ui.js. --- web/src/click_handlers.js | 4 +- web/src/hotkey.js | 4 +- web/src/message_events.js | 2 + web/src/message_flags.js | 45 +--------------------- web/src/server_events_dispatch.js | 8 ++-- web/src/starred_messages.js | 22 ----------- web/src/starred_messages_ui.js | 62 ++++++++++++++++++++++++++++++ web/tests/message_flags.test.js | 11 ++++-- web/tests/starred_messages.test.js | 5 ++- 9 files changed, 85 insertions(+), 78 deletions(-) diff --git a/web/src/click_handlers.js b/web/src/click_handlers.js index 69677f0b11..4ffa9ccb05 100644 --- a/web/src/click_handlers.js +++ b/web/src/click_handlers.js @@ -20,7 +20,6 @@ import * as emoji_picker from "./emoji_picker"; import * as hash_util from "./hash_util"; import * as hashchange from "./hashchange"; import * as message_edit from "./message_edit"; -import * as message_flags from "./message_flags"; import * as message_lists from "./message_lists"; import * as message_store from "./message_store"; import * as muted_topics_ui from "./muted_topics_ui"; @@ -37,6 +36,7 @@ import * as settings_display from "./settings_display"; import * as settings_panel_menu from "./settings_panel_menu"; import * as settings_toggle from "./settings_toggle"; import * as spectators from "./spectators"; +import * as starred_messages_ui from "./starred_messages_ui"; import * as stream_list from "./stream_list"; import * as stream_popover from "./stream_popover"; import * as topic_list from "./topic_list"; @@ -226,7 +226,7 @@ export function initialize() { const message_id = rows.id($(this).closest(".message_row")); const message = message_store.get(message_id); - message_flags.toggle_starred_and_update_server(message); + starred_messages_ui.toggle_starred_and_update_server(message); }); $("#main_div").on("click", ".message_reaction", function (e) { diff --git a/web/src/hotkey.js b/web/src/hotkey.js index 28389059b1..7a54966566 100644 --- a/web/src/hotkey.js +++ b/web/src/hotkey.js @@ -23,7 +23,6 @@ import * as hotspots from "./hotspots"; import * as lightbox from "./lightbox"; import * as list_util from "./list_util"; import * as message_edit from "./message_edit"; -import * as message_flags from "./message_flags"; import * as message_lists from "./message_lists"; import * as message_scroll from "./message_scroll"; import * as message_view_header from "./message_view_header"; @@ -42,6 +41,7 @@ import * as scheduled_messages_overlay_ui from "./scheduled_messages_overlay_ui" import * as search from "./search"; import * as settings_data from "./settings_data"; import * as spectators from "./spectators"; +import * as starred_messages_ui from "./starred_messages_ui"; import * as stream_list from "./stream_list"; import * as stream_popover from "./stream_popover"; import * as stream_settings_ui from "./stream_settings_ui"; @@ -959,7 +959,7 @@ export function process_hotkey(e, hotkey) { case "message_actions": return popover_menus.toggle_message_actions_menu(msg); case "star_message": - message_flags.toggle_starred_and_update_server(msg); + starred_messages_ui.toggle_starred_and_update_server(msg); return true; case "toggle_conversation_view": if (narrow_state.narrowed_by_topic_reply()) { diff --git a/web/src/message_events.js b/web/src/message_events.js index 73283e1636..241b9b7902 100644 --- a/web/src/message_events.js +++ b/web/src/message_events.js @@ -25,6 +25,7 @@ import * as recent_senders from "./recent_senders"; import * as recent_topics_ui from "./recent_topics_ui"; import * as recent_topics_util from "./recent_topics_util"; import * as starred_messages from "./starred_messages"; +import * as starred_messages_ui from "./starred_messages_ui"; import * as stream_list from "./stream_list"; import * as stream_topic_history from "./stream_topic_history"; import * as sub_store from "./sub_store"; @@ -575,4 +576,5 @@ export function remove_messages(message_ids) { recent_senders.update_topics_of_deleted_message_ids(message_ids); recent_topics_ui.update_topics_of_deleted_message_ids(message_ids); starred_messages.remove(message_ids); + starred_messages_ui.rerender_ui(); } diff --git a/web/src/message_flags.js b/web/src/message_flags.js index d34e5e9494..cbcbfe04ac 100644 --- a/web/src/message_flags.js +++ b/web/src/message_flags.js @@ -1,12 +1,9 @@ import _ from "lodash"; import * as channel from "./channel"; -import * as message_live_update from "./message_live_update"; -import * as message_store from "./message_store"; import * as starred_messages from "./starred_messages"; -import * as unread_ops from "./unread_ops"; -function send_flag_update_for_messages(msg_ids, flag, op) { +export function send_flag_update_for_messages(msg_ids, flag, op) { channel.post({ url: "/json/messages/flags", data: { @@ -76,46 +73,6 @@ export function save_uncollapsed(message) { send_flag_update_for_messages([message.id], "collapsed", "remove"); } -// This updates the state of the starred flag in local data -// structures, and triggers a UI rerender. -export function update_starred_flag(message_id, new_value) { - const message = message_store.get(message_id); - if (message === undefined) { - // If we don't have the message locally, do nothing; if later - // we fetch it, it'll come with the correct `starred` state. - return; - } - message.starred = new_value; - message_live_update.update_starred_view(message_id, new_value); -} - -export function toggle_starred_and_update_server(message) { - if (message.locally_echoed) { - // This is defensive code for when you hit the "*" key - // before we get a server ack. It's rare that somebody - // can star this quickly, and we don't have a good way - // to tell the server which message was starred. - return; - } - - message.starred = !message.starred; - - // Unlike most calls to mark messages as read, we don't check - // 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); - message_live_update.update_starred_view(message.id, message.starred); - - if (message.starred) { - send_flag_update_for_messages([message.id], "starred", "add"); - starred_messages.add([message.id]); - } else { - send_flag_update_for_messages([message.id], "starred", "remove"); - starred_messages.remove([message.id]); - } -} - export function unstar_all_messages() { const starred_msg_ids = starred_messages.get_starred_msg_ids(); send_flag_update_for_messages(starred_msg_ids, "starred", "remove"); diff --git a/web/src/server_events_dispatch.js b/web/src/server_events_dispatch.js index fca3d160de..c33041395c 100644 --- a/web/src/server_events_dispatch.js +++ b/web/src/server_events_dispatch.js @@ -21,7 +21,6 @@ import * as hotspots from "./hotspots"; import * as linkifiers from "./linkifiers"; import * as message_edit from "./message_edit"; import * as message_events from "./message_events"; -import * as message_flags from "./message_flags"; import * as message_lists from "./message_lists"; import * as message_live_update from "./message_live_update"; import * as muted_topics_ui from "./muted_topics_ui"; @@ -61,6 +60,7 @@ import * as settings_streams from "./settings_streams"; import * as settings_user_groups from "./settings_user_groups_legacy"; import * as settings_users from "./settings_users"; import * as starred_messages from "./starred_messages"; +import * as starred_messages_ui from "./starred_messages_ui"; import * as stream_data from "./stream_data"; import * as stream_events from "./stream_events"; import * as stream_list from "./stream_list"; @@ -761,7 +761,7 @@ export function dispatch_normal_event(event) { }, 300); } if (event.property === "starred_message_counts") { - starred_messages.rerender_ui(); + starred_messages_ui.rerender_ui(); } if (event.property === "fluid_layout_width") { scroll_bar.set_layout_width(); @@ -816,13 +816,15 @@ export function dispatch_normal_event(event) { switch (event.flag) { case "starred": for (const message_id of event.messages) { - message_flags.update_starred_flag(message_id, new_value); + starred_messages_ui.update_starred_flag(message_id, new_value); } if (event.op === "add") { starred_messages.add(event.messages); + starred_messages_ui.rerender_ui(); } else { starred_messages.remove(event.messages); + starred_messages_ui.rerender_ui(); } break; case "read": diff --git a/web/src/starred_messages.js b/web/src/starred_messages.js index 80d103b4c8..8f4262eec9 100644 --- a/web/src/starred_messages.js +++ b/web/src/starred_messages.js @@ -1,8 +1,5 @@ import * as message_store from "./message_store"; import {page_params} from "./page_params"; -import * as popover_menus from "./popover_menus"; -import * as top_left_corner from "./top_left_corner"; -import {user_settings} from "./user_settings"; export const starred_ids = new Set(); @@ -12,24 +9,18 @@ export function initialize() { for (const id of page_params.starred_messages) { starred_ids.add(id); } - - rerender_ui(); } export function add(ids) { for (const id of ids) { starred_ids.add(id); } - - rerender_ui(); } export function remove(ids) { for (const id of ids) { starred_ids.delete(id); } - - rerender_ui(); } export function get_count() { @@ -71,16 +62,3 @@ export function get_count_in_topic(stream_id, topic) { return messages.length; } - -export function rerender_ui() { - let count = get_count(); - - if (!user_settings.starred_message_counts) { - // This essentially hides the count - count = 0; - } - - popover_menus.get_topic_menu_popover()?.hide(); - popover_menus.get_starred_messages_popover()?.hide(); - top_left_corner.update_starred_count(count); -} diff --git a/web/src/starred_messages_ui.js b/web/src/starred_messages_ui.js index ab5c036b2c..77ced4e828 100644 --- a/web/src/starred_messages_ui.js +++ b/web/src/starred_messages_ui.js @@ -4,7 +4,69 @@ import render_confirm_unstar_all_messages_in_topic from "../templates/confirm_di import * as confirm_dialog from "./confirm_dialog"; import {$t_html} from "./i18n"; import * as message_flags from "./message_flags"; +import * as message_live_update from "./message_live_update"; +import * as message_store from "./message_store"; +import * as popover_menus from "./popover_menus"; +import * as starred_messages from "./starred_messages"; import * as sub_store from "./sub_store"; +import * as top_left_corner from "./top_left_corner"; +import * as unread_ops from "./unread_ops"; +import {user_settings} from "./user_settings"; + +export function toggle_starred_and_update_server(message) { + if (message.locally_echoed) { + // This is defensive code for when you hit the "*" key + // before we get a server ack. It's rare that somebody + // can star this quickly, and we don't have a good way + // to tell the server which message was starred. + return; + } + + message.starred = !message.starred; + + // Unlike most calls to mark messages as read, we don't check + // 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); + message_live_update.update_starred_view(message.id, message.starred); + + if (message.starred) { + message_flags.send_flag_update_for_messages([message.id], "starred", "add"); + starred_messages.add([message.id]); + rerender_ui(); + } else { + message_flags.send_flag_update_for_messages([message.id], "starred", "remove"); + starred_messages.remove([message.id]); + rerender_ui(); + } +} + +// This updates the state of the starred flag in local data +// structures, and triggers a UI rerender. +export function update_starred_flag(message_id, new_value) { + const message = message_store.get(message_id); + if (message === undefined) { + // If we don't have the message locally, do nothing; if later + // we fetch it, it'll come with the correct `starred` state. + return; + } + message.starred = new_value; + message_live_update.update_starred_view(message_id, new_value); +} + +export function rerender_ui() { + let count = starred_messages.get_count(); + + if (!user_settings.starred_message_counts) { + // This essentially hides the count + count = 0; + } + + popover_menus.get_topic_menu_popover()?.hide(); + popover_menus.get_starred_messages_popover()?.hide(); + top_left_corner.update_starred_count(count); +} export function confirm_unstar_all_messages() { const html_body = render_confirm_unstar_all_messages(); diff --git a/web/tests/message_flags.test.js b/web/tests/message_flags.test.js index e18e05a73f..66556e704d 100644 --- a/web/tests/message_flags.test.js +++ b/web/tests/message_flags.test.js @@ -12,11 +12,16 @@ set_global("document", {hasFocus: () => true}); mock_esm("../src/starred_messages", { add() {}, + get_count: () => 5, get_starred_msg_ids: () => [1, 2, 3, 4, 5], remove() {}, }); +mock_esm("../src/top_left_corner", { + update_starred_count() {}, +}); const message_flags = zrequire("message_flags"); +const starred_messages_ui = zrequire("starred_messages_ui"); run_test("starred", ({override}) => { const message = { @@ -35,7 +40,7 @@ run_test("starred", ({override}) => { posted_data = opts.data; }); - message_flags.toggle_starred_and_update_server(message); + starred_messages_ui.toggle_starred_and_update_server(message); assert.ok(ui_updated); @@ -52,7 +57,7 @@ run_test("starred", ({override}) => { ui_updated = false; - message_flags.toggle_starred_and_update_server(message); + starred_messages_ui.toggle_starred_and_update_server(message); assert.ok(ui_updated); @@ -76,7 +81,7 @@ run_test("starring local echo", () => { locally_echoed: true, }; - message_flags.toggle_starred_and_update_server(locally_echoed_message); + starred_messages_ui.toggle_starred_and_update_server(locally_echoed_message); // message_live_update.update_starred_view not called diff --git a/web/tests/starred_messages.test.js b/web/tests/starred_messages.test.js index a8ccb05d0d..3088c6e73f 100644 --- a/web/tests/starred_messages.test.js +++ b/web/tests/starred_messages.test.js @@ -12,6 +12,7 @@ const top_left_corner = mock_esm("../src/top_left_corner", { }); const message_store = zrequire("message_store"); const starred_messages = zrequire("starred_messages"); +const starred_messages_ui = zrequire("starred_messages_ui"); run_test("add starred", () => { starred_messages.starred_ids.clear(); @@ -97,7 +98,7 @@ run_test("rerender_ui", () => { with_overrides(({override}) => { const stub = make_stub(); override(top_left_corner, "update_starred_count", stub.f); - starred_messages.rerender_ui(); + starred_messages_ui.rerender_ui(); assert.equal(stub.num_calls, 1); const args = stub.get_args("count"); assert.equal(args.count, 3); @@ -107,7 +108,7 @@ run_test("rerender_ui", () => { with_overrides(({override}) => { const stub = make_stub(); override(top_left_corner, "update_starred_count", stub.f); - starred_messages.rerender_ui(); + starred_messages_ui.rerender_ui(); assert.equal(stub.num_calls, 1); const args = stub.get_args("count"); assert.equal(args.count, 0);