message_list: Extract visibly_empty helper method.

This function will allow us to adjust the codebase to write what it
means semantically -- whether a check is for the message list being
visibly empty, or completely empty.

In this commit, we leave the .empty() method incorrect, because
several other adjustments need to be made atomically with fixing it.
This commit is contained in:
Tim Abbott 2023-05-01 19:04:46 -07:00
parent 8f1c3a0fa1
commit 804f473214
13 changed files with 41 additions and 30 deletions

View File

@ -14,7 +14,7 @@ export function get_recipient_label(message) {
// passed in.
if (message === undefined) {
if (message_lists.current.empty()) {
if (message_lists.current.visibly_empty()) {
// For empty narrows where there's a clear reply target,
// i.e. stream+topic or a single PM conversation, we label
// the button as replying to the thread.

View File

@ -890,7 +890,7 @@ export function process_hotkey(e, hotkey) {
// Hotkeys below this point are for the message feed, and so
// should only function if the message feed is visible and nonempty.
if (!narrow_state.is_message_feed_visible() || message_lists.current.empty()) {
if (!narrow_state.is_message_feed_visible() || message_lists.current.visibly_empty()) {
return false;
}

View File

@ -57,7 +57,7 @@ function process_result(data, opts) {
if (
opts.msg_list === message_lists.current &&
opts.msg_list.narrowed &&
opts.msg_list.empty()
opts.msg_list.visibly_empty()
) {
// Even after loading more messages, we have
// no messages to display in this narrow.
@ -410,7 +410,7 @@ export function initialize(home_view_loaded) {
function load_more(data) {
// If we haven't selected a message in the home view yet, and
// the home view isn't empty, we select the anchor message here.
if (message_lists.home.selected_id() === -1 && !message_lists.home.empty()) {
if (message_lists.home.selected_id() === -1 && !message_lists.home.visibly_empty()) {
// We fall back to the closest selected id, as the user
// may have removed a stream from the home view while we
// were loading data.

View File

@ -116,14 +116,14 @@ export class MessageList {
render_info = this.append_to_view(bottom_messages, opts);
}
if (this.narrowed && !this.empty()) {
if (this.narrowed && !this.visibly_empty()) {
// If adding some new messages to the message tables caused
// our current narrow to no longer be empty, hide the empty
// feed placeholder text.
narrow_banner.hide_empty_narrow_message();
}
if (this.narrowed && !this.empty() && this.selected_id() === -1) {
if (this.narrowed && !this.visibly_empty() && this.selected_id() === -1) {
// The message list was previously empty, but now isn't
// due to adding these messages, and we need to select a
// message. Regardless of whether the messages are new or
@ -147,6 +147,10 @@ export class MessageList {
return this.data.empty();
}
visibly_empty() {
return this.data.visibly_empty();
}
first() {
return this.data.first();
}
@ -437,7 +441,7 @@ export class MessageList {
this.view.update_render_window(this.selected_idx(), false);
if (this.narrowed) {
if (this.empty()) {
if (this.visibly_empty()) {
narrow_banner.show_empty_narrow_message();
} else {
narrow_banner.hide_empty_narrow_message();

View File

@ -63,7 +63,14 @@ export class MessageListData {
return this._items.length;
}
// The message list is completely empty.
empty() {
// BUG: This should be checking _all_items.
return this._items.length === 0;
}
// The message list appears empty, but might contain messages that hidden by muting.
visibly_empty() {
return this._items.length === 0;
}

View File

@ -139,7 +139,7 @@ export function hide_top_of_narrow_notices() {
let hide_scroll_to_bottom_timer;
export function hide_scroll_to_bottom() {
const $show_scroll_to_bottom_button = $("#scroll-to-bottom-button-container");
if (message_viewport.bottom_message_visible() || message_lists.current.empty()) {
if (message_viewport.bottom_message_visible() || message_lists.current.visibly_empty()) {
// If last message is visible, just hide the
// scroll to bottom button.
$show_scroll_to_bottom_button.removeClass("show");

View File

@ -620,14 +620,14 @@ function min_defined(a, b) {
function load_local_messages(msg_data) {
// This little helper loads messages into our narrow message
// data and returns true unless it's empty. We use this for
// data and returns true unless it's visibly empty. We use this for
// cases when our local cache (all_messages_data) has at least
// one message the user will expect to see in the new narrow.
const in_msgs = all_messages_data.all_messages();
msg_data.add_messages(in_msgs);
return !msg_data.empty();
return !msg_data.visibly_empty();
}
export function maybe_add_local_messages(opts) {
@ -761,7 +761,7 @@ export function maybe_add_local_messages(opts) {
//
// And similarly for `near: max_int` with has_found_newest.
if (
all_messages_data.empty() ||
all_messages_data.visibly_empty() ||
id_info.target_id < all_messages_data.first().id ||
id_info.target_id > all_messages_data.last().id
) {
@ -795,7 +795,7 @@ export function update_selection(opts) {
return;
}
if (message_lists.current.empty()) {
if (message_lists.current.visibly_empty()) {
// There's nothing to select if there are no messages.
return;
}

View File

@ -101,7 +101,7 @@ export function page_down_the_right_amount() {
}
export function page_up() {
if (message_viewport.at_top() && !message_lists.current.empty()) {
if (message_viewport.at_top() && !message_lists.current.visibly_empty()) {
message_lists.current.select_id(message_lists.current.first().id, {then_scroll: false});
} else {
page_up_the_right_amount();
@ -109,7 +109,7 @@ export function page_up() {
}
export function page_down() {
if (message_viewport.at_bottom() && !message_lists.current.empty()) {
if (message_viewport.at_bottom() && !message_lists.current.visibly_empty()) {
message_lists.current.select_id(message_lists.current.last().id, {then_scroll: false});
unread_ops.process_scrolled_to_bottom();
} else {

View File

@ -119,7 +119,7 @@ function get_events_success(events) {
}
}
if (message_lists.home.selected_id() === -1 && !message_lists.home.empty()) {
if (message_lists.home.selected_id() === -1 && !message_lists.home.visibly_empty()) {
message_lists.home.select_id(message_lists.home.first().id, {then_scroll: false});
}

View File

@ -113,7 +113,7 @@ run_test("test_custom_message_input", () => {
});
run_test("empty_narrow", () => {
message_lists.current.empty = () => true;
message_lists.current.visibly_empty = () => true;
compose_closed_ui.update_reply_recipient_label();
const label = $(".compose_reply_button_label").text();
assert.equal(label, "translated: Compose message");

View File

@ -89,7 +89,7 @@ const stream_popover = mock_esm("../src/stream_popover", {
});
message_lists.current = {
empty() {
visibly_empty() {
return false;
},
selected_id() {
@ -343,7 +343,7 @@ run_test("misc", ({override}) => {
// Check that they do nothing without a selected message
with_overrides(({override}) => {
override(message_lists.current, "empty", () => true);
override(message_lists.current, "visibly_empty", () => true);
assert_unmapped(message_view_only_keys);
});
@ -470,7 +470,7 @@ run_test("motion_keys", () => {
}
list_util.inside_list = () => false;
message_lists.current.empty = () => true;
message_lists.current.visibly_empty = () => true;
overlays.settings_open = () => false;
overlays.streams_open = () => false;
overlays.lightbox_open = () => false;
@ -488,7 +488,7 @@ run_test("motion_keys", () => {
assert_mapping("down_arrow", list_util, "go_down");
list_util.inside_list = () => false;
message_lists.current.empty = () => false;
message_lists.current.visibly_empty = () => false;
assert_mapping("down_arrow", navigate, "down");
assert_mapping("end", navigate, "to_end");
assert_mapping("home", navigate, "to_home");

View File

@ -129,8 +129,8 @@ function stub_message_list() {
return this.data.get(msg_id);
}
empty() {
return this.data.empty();
visibly_empty() {
return this.data.visibly_empty();
}
select_id(msg_id) {
@ -166,7 +166,7 @@ run_test("basics", () => {
all_messages_data.all_messages_data = {
all_messages: () => messages,
empty: () => false,
visibly_empty: () => false,
first: () => ({id: 900}),
last: () => ({id: 1100}),
};

View File

@ -43,7 +43,7 @@ function test_with(fixture) {
fetch_status: {
has_found_newest: () => fixture.has_found_newest,
},
empty: () => fixture.empty,
visibly_empty: () => fixture.visibly_empty,
all_messages() {
assert.notEqual(fixture.all_messages, undefined);
return fixture.all_messages;
@ -159,7 +159,7 @@ run_test("near with no unreads", () => {
flavor: "not_found",
},
has_found_newest: false,
empty: true,
visibly_empty: true,
expected_id_info: {
target_id: 42,
final_select_id: 42,
@ -222,7 +222,7 @@ run_test("is:private with no unreads before fetch", () => {
flavor: "not_found",
},
has_found_newest: false,
empty: true,
visibly_empty: true,
expected_id_info: {
target_id: undefined,
final_select_id: undefined,
@ -242,7 +242,7 @@ run_test("is:private with target and no unreads", () => {
flavor: "not_found",
},
has_found_newest: true,
empty: false,
visibly_empty: false,
all_messages: [
{id: 350},
{id: 400, type: "private", to_user_ids: "1,2"},
@ -401,7 +401,7 @@ run_test("stream, no unread, not in all_messages", () => {
flavor: "not_found",
},
has_found_newest: true,
empty: false,
visibly_empty: false,
all_messages: [{id: 400}, {id: 500}],
expected_id_info: {
target_id: 450,
@ -424,7 +424,7 @@ run_test("search, stream, not in all_messages", () => {
flavor: "cannot_compute",
},
has_found_newest: true,
empty: false,
visibly_empty: false,
all_messages: [{id: 400}, {id: 500}],
expected_id_info: {
target_id: undefined,
@ -476,7 +476,7 @@ run_test("final corner case", () => {
flavor: "not_found",
},
has_found_newest: true,
empty: false,
visibly_empty: false,
all_messages: [
{id: 400, topic: "whatever"},
{id: 425, topic: "whatever", starred: true},