diff --git a/static/js/message_events.js b/static/js/message_events.js index 4a3758e4a9..03c913fe88 100644 --- a/static/js/message_events.js +++ b/static/js/message_events.js @@ -64,25 +64,28 @@ exports.insert_new_messages = function insert_new_messages(messages, locally_ech // other lists, so we always update this message_util.add_new_messages(messages, message_list.all); + var render_info; + if (narrow_state.active()) { // We do this NOW even though the home view is not active, // because we want the home view to load fast later. message_util.add_new_messages(messages, home_msg_list); if (narrow_state.filter().can_apply_locally()) { - message_util.add_new_messages(messages, message_list.narrowed); + render_info = message_util.add_new_messages(messages, message_list.narrowed); } else { // if we cannot apply locally, we have to wait for this callback to happen to notify maybe_add_narrowed_messages(messages, message_list.narrowed); } } else { // we're in the home view, so update its list - message_util.add_new_messages(messages, home_msg_list); + render_info = message_util.add_new_messages(messages, home_msg_list); } if (locally_echoed) { - notifications.notify_local_mixes(messages); + var need_user_to_scroll = render_info && render_info.need_user_to_scroll; + notifications.notify_local_mixes(messages, need_user_to_scroll); } activity.process_loaded_messages(messages); diff --git a/static/js/message_list.js b/static/js/message_list.js index b2180f5454..d00ca0ab59 100644 --- a/static/js/message_list.js +++ b/static/js/message_list.js @@ -45,6 +45,12 @@ exports.MessageList.prototype = { var bottom_messages = info.bottom_messages; var interior_messages = info.interior_messages; + // Currently we only need data back from rendering to + // tell us whether users needs to scroll, which only + // applies for `append_to_view`, but this may change over + // time. + var render_info; + if (interior_messages.length > 0) { self.view.rerender_the_whole_thing(); return true; @@ -54,7 +60,7 @@ exports.MessageList.prototype = { } if (bottom_messages.length > 0) { - self.append_to_view(bottom_messages, opts); + render_info = self.append_to_view(bottom_messages, opts); } if (self === exports.narrowed && !self.empty()) { @@ -69,6 +75,8 @@ exports.MessageList.prototype = { // And also select the newly arrived message. self.select_id(self.selected_id(), {then_scroll: true, use_closest: true}); } + + return render_info; }, get: function (id) { @@ -283,7 +291,8 @@ exports.MessageList.prototype = { opts = _.extend({messages_are_new: false}, opts); this.num_appends += 1; - this.view.append(messages, opts.messages_are_new); + var render_info = this.view.append(messages, opts.messages_are_new); + return render_info; }, remove_and_rerender: function MessageList_remove_and_rerender(messages) { diff --git a/static/js/message_list_view.js b/static/js/message_list_view.js index 3782233488..d8ed66f0b0 100644 --- a/static/js/message_list_view.js +++ b/static/js/message_list_view.js @@ -749,10 +749,18 @@ MessageListView.prototype = { if (list === current_msg_list && messages_are_new) { if (started_scrolled_up) { - return; + return { + need_user_to_scroll: true, + }; } var new_messages_height = self._new_messages_height(new_dom_elements); - self._maybe_autoscroll(new_messages_height); + var need_user_to_scroll = self._maybe_autoscroll(new_messages_height); + + if (need_user_to_scroll) { + return { + need_user_to_scroll: true, + }; + } } }, @@ -804,17 +812,19 @@ MessageListView.prototype = { // If we are near the bottom of our feed (the bottom is visible) and can // scroll up without moving the pointer out of the viewport, do so, by // up to the amount taken up by the new message. + // + // returns `true` if we need the user to scroll var selected_row = this.selected_row(); var last_visible = rows.last_visible(); // Make sure we have a selected row and last visible row. (defensive) if (!(selected_row && selected_row.length > 0 && last_visible)) { - return; + return false; } if (new_messages_height <= 0) { - return; + return false; } if (!activity.has_focus) { @@ -825,12 +835,15 @@ MessageListView.prototype = { // throttled by modern Chrome's aggressive power-saving // features. blueslip.log("Suppressing scrolldown due to inactivity"); - return; + return false; } // do not scroll if there are any active popovers. if (popovers.any_active()) { - return; + // If a popover is active, then we are pretty sure the + // incoming message is not from the user themselves, so + // we don't need to tell users to scroll down. + return false; } var info = message_viewport.message_viewport_info(); @@ -844,16 +857,31 @@ MessageListView.prototype = { // c) scroll amount isn't really tied to size of new messages (bad) // d) all the bad things about scrolling for users who want messages // to stay on the screen - var scroll_amount = new_messages_height; + var scroll_amount; + var need_user_to_scroll; - if (scroll_amount > scroll_limit) { + if (new_messages_height <= scroll_limit) { + // This is the happy path where we can just scroll + // automatically, and the user will see the new message. + scroll_amount = new_messages_height; + need_user_to_scroll = false; + } else { + // Sometimes we don't want to scroll the entire height of + // the message, but our callers can give appropriate + // warnings if the message is gonna be offscreen. + // (Even if we are somewhat constrained here, the message may + // still end up being visible, so we do some arithmetic.) scroll_amount = scroll_limit; + var offset = message_viewport.offset_from_bottom(last_visible); + need_user_to_scroll = offset > scroll_amount; } // Ok, we are finally ready to actually scroll. if (scroll_amount > 0) { message_viewport.system_initiated_animate_scroll(scroll_amount); } + + return need_user_to_scroll; }, @@ -1030,9 +1058,11 @@ MessageListView.prototype = { append: function (messages, messages_are_new) { var cur_window_size = this._render_win_end - this._render_win_start; + var render_info; + if (cur_window_size < this._RENDER_WINDOW_SIZE) { var slice_to_render = messages.slice(0, this._RENDER_WINDOW_SIZE - cur_window_size); - this.render(slice_to_render, 'bottom', messages_are_new); + render_info = this.render(slice_to_render, 'bottom', messages_are_new); this._render_win_end += slice_to_render.length; } @@ -1041,7 +1071,13 @@ MessageListView.prototype = { // newly received message should trigger a rerender so that // the new message, which will appear in the viewable area, // is rendered. - this.maybe_rerender(); + var needed_rerender = this.maybe_rerender(); + + if (needed_rerender) { + render_info = {need_user_to_scroll: true}; + } + + return render_info; }, prepend: function (messages) { diff --git a/static/js/message_util.js b/static/js/message_util.js index 8d81cf0108..5822b73eec 100644 --- a/static/js/message_util.js +++ b/static/js/message_util.js @@ -16,14 +16,16 @@ function add_messages(messages, msg_list, opts) { loading.destroy_indicator($('#page_loading_indicator')); $('#first_run_message').remove(); - msg_list.add_messages(messages, opts); + var render_info = msg_list.add_messages(messages, opts); + + return render_info; } exports.add_old_messages = function (messages, msg_list) { - add_messages(messages, msg_list, {messages_are_new: false}); + return add_messages(messages, msg_list, {messages_are_new: false}); }; exports.add_new_messages = function (messages, msg_list) { - add_messages(messages, msg_list, {messages_are_new: true}); + return add_messages(messages, msg_list, {messages_are_new: true}); }; diff --git a/static/js/notifications.js b/static/js/notifications.js index 89ec3628d4..488f365726 100644 --- a/static/js/notifications.js +++ b/static/js/notifications.js @@ -564,15 +564,6 @@ function get_message_header(message) { {recipient: message.display_reply_to}); } -exports.get_echo_not_in_view_reason = function () { - if (message_viewport.bottom_message_visible()) { - // If the message is visible, we do not want to notify user - return; - } - - return i18n.t("Sent! Scroll down to view your message."); -}; - exports.get_local_notify_mix_reason = function (message) { var row = current_msg_list.get_row(message.id); if (row.length > 0) { @@ -595,7 +586,7 @@ exports.get_local_notify_mix_reason = function (message) { return i18n.t("Sent! Your message is outside your current narrow."); }; -exports.notify_local_mixes = function (messages) { +exports.notify_local_mixes = function (messages, need_user_to_scroll) { /* This code should only be called when we are locally echoing messages. It notifies users that their messages aren't @@ -617,16 +608,16 @@ exports.notify_local_mixes = function (messages) { var reason = exports.get_local_notify_mix_reason(message); if (!reason) { - // Check if local_echo is not in view - reason = exports.get_echo_not_in_view_reason(); - if (reason) { + if (need_user_to_scroll) { + reason = i18n.t("Sent! Scroll down to view your message."); exports.notify_above_composebox(reason, "", null, ""); setTimeout(function () { $('#out-of-view-notification').hide(); }, 3000); } - // This is more than normal, just continue on. + // This is the HAPPY PATH--for most messages we do nothing + // other than maybe sending the above message. return; }