unread: Add UI support for marking messages as unread.

Fixes zulip#2676. Co-authored-by: Aman Agrawal <[email protected]> Co-authored-by: Tim Abbott <[email protected]>
timrichardt · Sep 23, 2022 · 8fc811d · 8fc811d
1 parent e7b97e2
commit 8fc811d
Show file tree

Hide file tree

Showing 15 changed files with 160 additions and 3 deletions.
diff --git a/frontend_tests/node_tests/hotkey.js b/frontend_tests/node_tests/hotkey.js
@@ -262,7 +262,7 @@ run_test("allow normal typing when processing text", ({override, override_rewire
     // Unmapped keys should immediately return false, without
     // calling any functions outside of hotkey.js.
     assert_unmapped("bfmoyz");
-    assert_unmapped("BEFHILNOQTUWXYZ");
+    assert_unmapped("BEFHILNOQTWXYZ");
 
     // All letters should return false if we are composing text.
     override_rewire(hotkey, "processing_text", () => true);

diff --git a/frontend_tests/node_tests/message_flags.js b/frontend_tests/node_tests/message_flags.js
@@ -318,3 +318,24 @@ run_test("collapse_and_uncollapse", ({override}) => {
         },
     });
 });
+
+run_test("mark_as_unread", ({override}) => {
+    // Way to capture posted info in every request
+    let channel_post_opts;
+    override(channel, "post", (opts) => {
+        channel_post_opts = opts;
+    });
+
+    const msg = {id: 5};
+
+    message_flags.mark_as_unread([msg.id]);
+
+    assert.deepEqual(channel_post_opts, {
+        url: "/json/messages/flags",
+        data: {
+            messages: "[5]",
+            op: "remove",
+            flag: "read",
+        },
+    });
+});
diff --git a/frontend_tests/node_tests/popovers.js b/frontend_tests/node_tests/popovers.js
@@ -27,6 +27,11 @@ const message_lists = mock_esm("../../static/js/message_lists", {
         view: {
             message_containers: {},
         },
+        data: {
+            fetch_status: {
+                has_found_newest: () => true,
+            },
+        },
     },
 });
 mock_esm("../../static/js/message_viewport", {

diff --git a/static/js/hotkey.js b/static/js/hotkey.js
@@ -41,6 +41,7 @@ import * as stream_popover from "./stream_popover";
 import * as stream_settings_ui from "./stream_settings_ui";
 import * as topic_zoom from "./topic_zoom";
 import * as ui from "./ui";
+import * as unread_ops from "./unread_ops";
 import {user_settings} from "./user_settings";
 
 function do_narrow_action(action) {
@@ -135,6 +136,7 @@ const keypress_mappings = {
     80: {name: "narrow_private", message_view_only: true}, // 'P'
     82: {name: "respond_to_author", message_view_only: true}, // 'R'
     83: {name: "narrow_by_topic", message_view_only: true}, // 'S'
+    85: {name: "mark_unread", message_view_only: true}, // 'U'
     86: {name: "view_selected_stream", message_view_only: false}, // 'V'
     97: {name: "all_messages", message_view_only: true}, // 'a'
     99: {name: "compose", message_view_only: true}, // 'c'
@@ -940,6 +942,9 @@ export function process_hotkey(e, hotkey) {
         case "toggle_message_collapse":
             condense.toggle_collapse(msg);
             return true;
+        case "mark_unread":
+            unread_ops.mark_as_unread_from_here(msg.id);
+            return true;
         case "compose_quote_reply": // > : respond to selected message with quote
             compose_actions.quote_and_reply({trigger: "hotkey"});
             return true;

diff --git a/static/js/message_flags.js b/static/js/message_flags.js
@@ -66,6 +66,10 @@ export const send_read = (function () {
     return add;
 })();
 
+export function mark_as_unread(message_ids) {
+    send_flag_update_for_messages(message_ids, "read", "remove");
+}
+
 export function save_collapsed(message) {
     send_flag_update_for_messages([message.id], "collapsed", "add");
 }

diff --git a/static/js/message_list.js b/static/js/message_list.js
@@ -34,10 +34,19 @@ export class MessageList {
         this.table_name = table_name;
         this.narrowed = this.table_name === "zfilt";
         this.num_appends = 0;
+        this.reading_prevented = false;
 
         return this;
     }
 
+    prevent_reading() {
+        this.reading_prevented = true;
+    }
+
+    resume_reading() {
+        this.reading_prevented = false;
+    }
+
     add_messages(messages, opts) {
         // This adds all messages to our data, but only returns
         // the currently viewable ones.
@@ -100,6 +109,10 @@ export class MessageList {
         return this.data.last();
     }
 
+    ids_greater_or_equal_than(id) {
+        return this.data.ids_greater_or_equal_than(id);
+    }
+
     prev() {
         return this.data.prev();
     }
@@ -121,7 +134,14 @@ export class MessageList {
     }
 
     can_mark_messages_read() {
-        return this.data.can_mark_messages_read();
+        /* Automatically marking messages as read can be disabled for
+           two different reasons:
+           * The view is structurally a search view, encoded in the
+             properties of the message_list_data object.
+           * The user recently marked messages in the view as unread, and
+             we don't want to lose that state.
+        */
+        return this.data.can_mark_messages_read() && !this.reading_prevented;
     }
 
     clear({clear_selected_id = true} = {}) {

diff --git a/static/js/message_list_data.js b/static/js/message_list_data.js
@@ -42,6 +42,21 @@ export class MessageListData {
         return this._items.at(-1);
     }
 
+    ids_greater_or_equal_than(my_id) {
+        const result = [];
+
+        for (let i = this._items.length - 1; i >= 0; i -= 1) {
+            const message_id = this._items[i].id;
+            if (message_id >= my_id) {
+                result.push(message_id);
+            } else {
+                continue;
+            }
+        }
+
+        return result;
+    }
+
     select_idx() {
         if (this._selected_id === -1) {
             return undefined;

diff --git a/static/js/popovers.js b/static/js/popovers.js
@@ -49,6 +49,7 @@ import * as settings_data from "./settings_data";
 import * as settings_users from "./settings_users";
 import * as stream_popover from "./stream_popover";
 import * as ui_report from "./ui_report";
+import * as unread_ops from "./unread_ops";
 import * as user_groups from "./user_groups";
 import * as user_profile from "./user_profile";
 import {user_settings} from "./user_settings";
@@ -479,6 +480,17 @@ export function toggle_actions_popover(element, id) {
             editability_menu_item = $t({defaultMessage: "View source"});
         }
 
+        // Theoretically, it could be useful to offer this even for a
+        // message that is already unread, so you can mark those below
+        // it as unread; but that's an unlikely situation, and showing
+        // it can be a confusing source of clutter.
+        //
+        // To work around #22893, we also only offer the option if the
+        // fetch_status data structure means we'll be able to mark
+        // everything below the current message as read correctly.
+        const should_display_mark_as_unread =
+            message_lists.current.data.fetch_status.has_found_newest() && !message.unread;
+
         const should_display_edit_history_option =
             message.edit_history &&
             message.edit_history.some(
@@ -521,6 +533,7 @@ export function toggle_actions_popover(element, id) {
             stream_id: message.stream_id,
             use_edit_icon,
             editability_menu_item,
+            should_display_mark_as_unread,
             should_display_collapse,
             should_display_uncollapse,
             should_display_add_reaction_option: message.sent_by_me,
@@ -1101,6 +1114,16 @@ export function register_click_handlers() {
         current_user_sidebar_popover = $target.data("popover");
     });
 
+    $("body").on("click", ".mark_as_unread", (e) => {
+        hide_actions_popover();
+        const message_id = $(e.currentTarget).data("message-id");
+
+        unread_ops.mark_as_unread_from_here(message_id);
+
+        e.stopPropagation();
+        e.preventDefault();
+    });
+
     $("body").on("click", ".respond_button", (e) => {
         // Arguably, we should fetch the message ID to respond to from
         // e.target, but that should always be the current selected

diff --git a/static/js/unread_ops.js b/static/js/unread_ops.js
@@ -40,6 +40,23 @@ function process_newly_read_message(message, options) {
     recent_topics_ui.update_topic_unread_count(message);
 }
 
+export function mark_as_unread_from_here(message_id) {
+    /* TODO: This algorithm is not correct if we don't have full data for
+       the current narrow loaded from the server.
+
+       Currently, we turn off the feature when fetch_status suggests
+       we are in that that situation, but we plan to replace this
+       logic with asking the server to do the marking as unread.
+     */
+    const message_ids = message_lists.current.ids_greater_or_equal_than(message_id);
+    message_lists.current.prevent_reading();
+    message_flags.mark_as_unread(message_ids);
+}
+
+export function resume_reading() {
+    message_lists.current.resume_reading();
+}
+
 export function process_read_messages_event(message_ids) {
     /*
         This code has a lot in common with notify_server_messages_read,
@@ -81,6 +98,10 @@ export function process_unread_messages_event({message_ids, message_details}) {
         return;
     }
 
+    if (message_lists.current === message_list.narrowed) {
+        unread.set_messages_read_in_narrow(false);
+    }
+
     for (const message_id of message_ids) {
         const message = message_store.get(message_id);
 

diff --git a/static/js/unread_ui.js b/static/js/unread_ui.js
@@ -125,6 +125,8 @@ export function initialize() {
             .all_messages()
             .filter((message) => unread.message_unread(message));
         notify_server_messages_read(unread_messages);
+        // New messages received may be marked as read based on narrow type.
+        message_lists.current.resume_reading();
 
         hide_mark_as_read_turned_off_banner();
     });

diff --git a/static/styles/dark_theme.css b/static/styles/dark_theme.css
@@ -292,7 +292,10 @@ body.dark-theme {
         color: hsl(236, 33%, 90%);
     }
 
-    .unread_count {
+    .unread_count,
+    /* The actions_popover unread_count object has its own variable CSS,
+       and thus needs to be repeated here with all three classes for precedence) */
+    .actions_popover .mark_as_unread .unread_count {
         background-color: hsla(105, 2%, 50%, 0.5);
     }
 

diff --git a/static/styles/popovers.css b/static/styles/popovers.css
@@ -145,6 +145,28 @@ ul {
     }
 }
 
+.actions_popover {
+    .mark_as_unread {
+        .unread_count {
+            /* The icon for this menu item is in the form of an unread count from
+               the left sidebar. We reuse much of the shared styling,
+               but need to override some of the defaults set in app_components.css. */
+            display: inline;
+            float: unset;
+            line-height: 14px;
+            font-size: 11px;
+            font-weight: 550;
+            margin-right: 2px;
+            background-color: hsl(200, 100%, 40%);
+            /* Override random undesired bootstrap style */
+            text-shadow: none;
+            /* Not center aligned but looks better. */
+            position: relative;
+            top: -1px;
+        }
+    }
+}
+
 .user_popover {
     width: 240px;
 

diff --git a/static/templates/actions_popover_content.hbs b/static/templates/actions_popover_content.hbs
@@ -27,6 +27,16 @@
     </li>
     {{/if}}
 
+    {{#if should_display_mark_as_unread}}
+        <li>
+            <a class="mark_as_unread" data-message-id="{{message_id}}">
+                <span class="unread_count">1</span>
+                {{t "Mark as unread from here" }}
+                <span class="hotkey-hint">(U)</span>
+            </a>
+        </li>
+    {{/if}}
+
     {{#if should_display_reminder_option}}
     <li>
         <a class='reminder_button' data-message-id="{{message_id}}" tabindex="0">

diff --git a/static/templates/keyboard_shortcuts.hbs b/static/templates/keyboard_shortcuts.hbs
@@ -234,6 +234,10 @@
                     </td>
                     <td><span class="hotkey"><kbd>+</kbd></span></td>
                 </tr>
+                <tr>
+                    <td class="definition">{{t 'Mark as unread from selected message' }}</td>
+                    <td><span class="hotkey"><kbd>Shift</kbd> + <kbd>U</kbd></span></td>
+                </tr>
                 <tr>
                     <td class="definition">{{t 'Collapse/show selected message' }}</td>
                     <td><span class="hotkey"><kbd>-</kbd></span></td>

diff --git a/templates/zerver/help/keyboard-shortcuts.md b/templates/zerver/help/keyboard-shortcuts.md
@@ -151,6 +151,8 @@ below, and add more to your repertoire as needed.
   src="/static/generated/emoji/images/emoji/unicode/1f44d.png"
   title="thumbs up"/>**: <kbd>+</kbd>
 
+* **Mark as unread from selected message**: <kbd>Shift</kbd> + <kbd>U</kbd>
+
 * **Collapse/show message**: <kbd>-</kbd>
 
 * **Toggle topic mute**: <kbd>Shift</kbd> + <kbd>M</kbd> — Muted topics