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.

Fixes zulip#23298.
  • Loading branch information
JoeyPriceless authored and N-Shar-ma committed Mar 21, 2024
1 parent f1b8e11 commit 3e36b8e
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 96 deletions.
101 changes: 14 additions & 87 deletions web/src/message_list_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,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 @@ -1011,13 +1011,7 @@ export class MessageListView {
};
}
const new_messages_height = this._new_messages_height(new_dom_elements);
const need_user_to_scroll = this._maybe_autoscroll(new_messages_height);

if (need_user_to_scroll) {
return {
need_user_to_scroll: true,
};
}
this._maybe_autoscroll(new_messages_height);
}

return undefined;
Expand All @@ -1037,41 +1031,20 @@ 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.get_offset_to_window().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.
//
// returns `true` if we need the user to scroll
// 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.

const $selected_row = this.selected_row();
const $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 false;
return;
}

if (new_messages_height <= 0) {
return false;
return;
}

if (!activity.client_is_active) {
Expand All @@ -1082,71 +1055,25 @@ export class MessageListView {
// throttled by modern Chrome's aggressive power-saving
// features.
blueslip.log("Suppressing scroll down due to inactivity");
return false;
return;
}

// do not scroll if there are any active popovers.
if (popovers.any_active() || sidebar_ui.any_sidebar_expanded_as_overlay()) {
// 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;
return;
}

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 = $("textarea#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.
if (scroll_amount > 0) {
// Find out how close we are to the bottom of the feed.
const scroll_amount = message_viewport.offset_from_bottom($last_visible, true);
if (scroll_amount >= 0) {
// Scroll down to the bottom of the feed.
// For some reason the function doesn't scroll down to the point
// where offset_from_bottom is 0. The Error margin in the order of 0.1 - 1 px.
message_viewport.system_initiated_animate_scroll(scroll_amount);
}

return need_user_to_scroll;
}

clear_rendering_state(clear_table) {
Expand Down
33 changes: 24 additions & 9 deletions web/src/message_viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,28 +103,44 @@ export function is_below_visible_bottom(offset: number): boolean {
return offset > scrollTop() + height() - ($("#compose").height() ?? 0);
}

export function is_scrolled_up(): boolean {
export function is_scrolled_up(ignore_multiline_compose = false): boolean {
// 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);

return offset > 0;
const offset = offset_from_bottom($last_row, ignore_multiline_compose);
return offset > 1;
}

export function offset_from_bottom($last_row: JQuery): number {
export function offset_from_bottom($last_row: JQuery, ignore_multiline_compose = false): number {
// 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.get_offset_to_window().bottom;
const info = message_viewport_info();
const offset = message_bottom - info.visible_bottom;

if (ignore_multiline_compose) {
return offset - compose_added_height();
}
return offset;
}

export function compose_added_height(): number {
// 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() ?? 0;

return message_bottom - info.visible_bottom;
return compose_textarea_current_height - compose_textarea_default_height;
}

export function set_message_position(
Expand Down Expand Up @@ -359,7 +375,6 @@ export function stop_auto_scrolling(): void {
}

export function system_initiated_animate_scroll(scroll_amount: number): void {
message_scroll_state.set_update_selection_on_next_scroll(false);
const viewport_offset = scrollTop();
in_stoppable_autoscroll = true;
$scroll_container.animate({
Expand Down

0 comments on commit 3e36b8e

Please sign in to comment.