mirror of https://github.com/zulip/zulip.git
navigate: Fix buggy detection of start/end.
I was not able to reproduce obviously badly broken behavior from these logic bugs, but after the renaming of message_viewport helpers in the last few commits, it's clear that this logic was trying to check if we're actually at the start/end of the possibly message feed, not just the rendered portion, and doing so incorrectly.
This commit is contained in:
parent
4f8da7462d
commit
d8ec141de2
|
@ -1451,6 +1451,15 @@ export class MessageListView {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
is_start_rendered() {
|
||||||
|
// Used as a helper in checks for whether a given scroll
|
||||||
|
// position is actually the very start of this view. It could
|
||||||
|
// fail to be for two reasons: Either some older messages are
|
||||||
|
// not rendered due to a render window, or we haven't finished
|
||||||
|
// fetching the oldest messages for this view from the server.
|
||||||
|
return this._render_win_start === 0 && this.list.data.fetch_status.has_found_oldest();
|
||||||
|
}
|
||||||
|
|
||||||
get_row(id) {
|
get_row(id) {
|
||||||
const $row = this._rows.get(id);
|
const $row = this._rows.get(id);
|
||||||
|
|
||||||
|
|
|
@ -100,7 +100,11 @@ export function page_down_the_right_amount() {
|
||||||
}
|
}
|
||||||
|
|
||||||
export function page_up() {
|
export function page_up() {
|
||||||
if (message_viewport.at_rendered_top() && !message_lists.current.visibly_empty()) {
|
if (
|
||||||
|
message_viewport.at_rendered_top() &&
|
||||||
|
!message_lists.current.visibly_empty() &&
|
||||||
|
message_lists.current.view.is_start_rendered()
|
||||||
|
) {
|
||||||
message_lists.current.select_id(message_lists.current.first().id, {then_scroll: false});
|
message_lists.current.select_id(message_lists.current.first().id, {then_scroll: false});
|
||||||
} else {
|
} else {
|
||||||
page_up_the_right_amount();
|
page_up_the_right_amount();
|
||||||
|
@ -108,7 +112,11 @@ export function page_up() {
|
||||||
}
|
}
|
||||||
|
|
||||||
export function page_down() {
|
export function page_down() {
|
||||||
if (message_viewport.at_rendered_bottom() && !message_lists.current.visibly_empty()) {
|
if (
|
||||||
|
message_viewport.at_rendered_bottom() &&
|
||||||
|
!message_lists.current.visibly_empty() &&
|
||||||
|
message_lists.current.view.is_end_rendered()
|
||||||
|
) {
|
||||||
message_lists.current.select_id(message_lists.current.last().id, {then_scroll: false});
|
message_lists.current.select_id(message_lists.current.last().id, {then_scroll: false});
|
||||||
unread_ops.process_visible();
|
unread_ops.process_visible();
|
||||||
} else {
|
} else {
|
||||||
|
|
|
@ -125,6 +125,7 @@ run_test("unread_ops", ({override}) => {
|
||||||
// data setup).
|
// data setup).
|
||||||
override(message_lists.current, "can_mark_messages_read", () => can_mark_messages_read);
|
override(message_lists.current, "can_mark_messages_read", () => can_mark_messages_read);
|
||||||
override(message_lists.current, "has_unread_messages", () => true);
|
override(message_lists.current, "has_unread_messages", () => true);
|
||||||
|
override(message_lists.current.view, "is_end_rendered", () => true);
|
||||||
|
|
||||||
// First, test for a message list that cannot read messages.
|
// First, test for a message list that cannot read messages.
|
||||||
can_mark_messages_read = false;
|
can_mark_messages_read = false;
|
||||||
|
@ -134,6 +135,12 @@ run_test("unread_ops", ({override}) => {
|
||||||
|
|
||||||
// Now flip the boolean, and get to the main thing we are testing.
|
// Now flip the boolean, and get to the main thing we are testing.
|
||||||
can_mark_messages_read = true;
|
can_mark_messages_read = true;
|
||||||
|
// Don't mark messages as read until all messages in the narrow are fetched and rendered.
|
||||||
|
override(message_lists.current.view, "is_end_rendered", () => false);
|
||||||
|
unread_ops.process_visible();
|
||||||
|
assert.deepEqual(channel_post_opts, undefined);
|
||||||
|
|
||||||
|
override(message_lists.current.view, "is_end_rendered", () => true);
|
||||||
unread_ops.process_visible();
|
unread_ops.process_visible();
|
||||||
|
|
||||||
// The most important side effect of the above call is that
|
// The most important side effect of the above call is that
|
||||||
|
|
Loading…
Reference in New Issue