Skip to content

Commit

Permalink
message_view: Update autoscroll on message sent.
Browse files Browse the repository at this point in the history
Updates the autoscroll behavior when sending a message. Will now
autoscroll to the bottom of the message list if bottom of the view is
visible, moving the message selection to the message at the top of
the view.

Concerns message_list_view.js and message_viewport.js.

Fixes zulip#23298
  • Loading branch information
JoeyPriceless committed Nov 11, 2022
1 parent 23a776c commit 49fb9f5
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 79 deletions.
82 changes: 10 additions & 72 deletions static/js/message_list_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ export class MessageListView {
// the bottom message is not visible), then we will respect
// the user's current position after rendering, rather
// than auto-scrolling.
const started_scrolled_up = $message_viewport.is_scrolled_up();
const started_scrolled_up = $message_viewport.is_scrolled_up(true);

// The messages we are being asked to render are shared with between
// all messages lists. To prevent having both list views overwriting
Expand Down Expand Up @@ -974,28 +974,9 @@ export class MessageListView {
return new_messages_height;
}

_scroll_limit($selected_row, viewport_info) {
// This scroll limit is driven by the TOP of the feed, and
// it's the max amount that we can scroll down (or "skooch
// up" the messages) before knocking the selected message
// out of the feed.
const selected_row_top = $selected_row.offset().top;
let scroll_limit = selected_row_top - viewport_info.visible_top;

if (scroll_limit < 0) {
// This shouldn't happen, but if we're off by a pixel or
// something, we can deal with it, and just warn.
blueslip.warn("Selected row appears too high on screen.");
scroll_limit = 0;
}

return scroll_limit;
}

_maybe_autoscroll(new_messages_height) {
// 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.
// If we are near the bottom of our feed (the bottom is visible after the compose box is
// cleared), scroll to the bottom of the feed. Otherwise, don't scroll.
//
// returns `true` if we need the user to scroll

Expand Down Expand Up @@ -1030,60 +1011,17 @@ export class MessageListView {
return false;
}

const info = $message_viewport.message_viewport_info();
const scroll_limit = this._scroll_limit($selected_row, info);

// This next decision is fairly debatable. For a big message that
// would push the pointer off the screen, we do a partial autoscroll,
// which has the following implications:
// a) user sees scrolling (good)
// b) user's pointer stays on screen (good)
// 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
let scroll_amount;
let need_user_to_scroll;

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;
const offset = $message_viewport.offset_from_bottom($last_visible);

// For determining whether we need to show the user a "you
// need to scroll down" notification, the obvious check
// would be `offset > scroll_amount`, and that is indeed
// correct with a 1-line message in the compose box.
// However, the compose box is open with the content of
// the message just sent when this code runs, and
// `offset_from_bottom` if an offset from the top of the
// compose box, which is about to be reset to empty. So
// to compute the offset at the time the user might see
// this notification, we need to adjust by the amount that
// the current compose is bigger than the empty, open
// compose box.
const compose_textarea_default_height = 42;
const compose_textarea_current_height = $("#compose-textarea").height();
const expected_change =
compose_textarea_current_height - compose_textarea_default_height;
const expected_offset = offset - expected_change;
need_user_to_scroll = expected_offset > scroll_amount;
}

// Ok, we are finally ready to actually scroll.
// Find out how close we are to the bottom of the feed. The +1 is to make sure we get
// consistent behavior when composing a message right after an autoscroll.
const scroll_amount = $message_viewport.offset_from_bottom($last_visible, true) + 1;
if (scroll_amount > 0) {
// Scroll down to the bottom of the feed.
$message_viewport.system_initiated_animate_scroll(scroll_amount);
return false;
}

return need_user_to_scroll;
// Otherwise, don't scroll, but notify the user that they need to scroll to see the message.
return true;
}

clear_rendering_state(clear_table) {
Expand Down
30 changes: 23 additions & 7 deletions static/js/message_viewport.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,28 +96,45 @@ export function is_below_visible_bottom(offset) {
return offset > scrollTop() + height() - $("#compose").height();
}

export function is_scrolled_up() {
export function is_scrolled_up(ignore_multiline_compose = false) {
// Let's determine whether the user was already dealing
// with messages off the screen, which can guide auto
// scrolling decisions.
// scrolling decisions. If ignore_multiline_compose is true, we will only consider messages
// hidden by the default focused size of the compose box.
const $last_row = rows.last_visible();
if ($last_row.length === 0) {
return false;
}

const offset = offset_from_bottom($last_row);
const offset = offset_from_bottom($last_row, ignore_multiline_compose);

return offset > 0;
}

export function offset_from_bottom($last_row) {
export function offset_from_bottom($last_row, ignore_multiline_compose = false) {
// A positive return value here means the last row is
// below the bottom of the feed (i.e. obscured by the compose
// box or even further below the bottom).
// box or even further below the bottom). If ignore_multiline_compose is true, we will only
// consider the default focused size of the compose box.
const message_bottom = $last_row.offset().top + $last_row.height();
const info = message_viewport_info();
const offset = message_bottom - info.visible_bottom;

return message_bottom - info.visible_bottom;
if (ignore_multiline_compose) {
return offset - compose_added_height();
}
return offset;
}

export function compose_added_height() {
// Returns the difference between the compose box's current height and the height it has by
// default when it is focused.
// This is useful for auto scrolling behavior right after a message is sent. Since the compose
// box will be cleared, more of the feed will be visible, meaning we want to scroll down less.
const compose_textarea_default_height = 42;
const compose_textarea_current_height = $("#compose-textarea").height();

return compose_textarea_current_height - compose_textarea_default_height;
}

export function set_message_position(message_top, message_height, viewport_info, ratio) {
Expand Down Expand Up @@ -321,7 +338,6 @@ export function is_narrow() {
}

export function system_initiated_animate_scroll(scroll_amount) {
message_scroll.suppress_selection_update_on_next_scroll();
const viewport_offset = scrollTop();
in_stoppable_autoscroll = true;
$message_pane.animate({
Expand Down

0 comments on commit 49fb9f5

Please sign in to comment.