Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

message_list: Don't always cache "All messages" view. #29740

Merged
merged 1 commit into from Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions tools/test-js-with-node
Expand Up @@ -108,6 +108,7 @@ EXEMPT_FILES = make_set(
"web/src/emojisets.ts",
"web/src/favicon.ts",
"web/src/feedback_widget.ts",
"web/src/fetch_status.ts",
"web/src/flatpickr.ts",
"web/src/gear_menu.js",
"web/src/giphy.js",
Expand Down
11 changes: 7 additions & 4 deletions web/e2e-tests/message-basics.test.ts
Expand Up @@ -11,6 +11,12 @@ async function get_stream_li(page: Page, stream_name: string): Promise<string> {

async function expect_home(page: Page): Promise<void> {
const message_list_id = await common.get_current_msg_list_id(page, true);
await page.waitForSelector(`.message-list[data-message-list-id='${message_list_id}']`, {
visible: true,
});
// Assert that there is only one message list.
assert.equal((await page.$$(".message-list")).length, 1);
assert.strictEqual(await page.title(), "Combined feed - Zulip Dev - Zulip");
await common.check_messages_sent(page, message_list_id, [
["Verona > test", ["verona test a", "verona test b"]],
["Verona > other topic", ["verona other topic c"]],
Expand Down Expand Up @@ -107,10 +113,6 @@ async function un_narrow(page: Page): Promise<void> {
await page.keyboard.press("Escape");
}
await page.click("#left-sidebar-navigation-list .top_left_all_messages");
await page.waitForSelector(".message-list .message_row", {visible: true});
// Assert that there is only one message list.
assert.equal((await page.$$(".message-list")).length, 1);
assert.strictEqual(await page.title(), "Combined feed - Zulip Dev - Zulip");
}

async function un_narrow_by_clicking_org_icon(page: Page): Promise<void> {
Expand Down Expand Up @@ -322,6 +324,7 @@ async function test_narrow_by_clicking_the_left_sidebar(page: Page): Promise<voi
await expect_all_direct_messages(page);

await un_narrow(page);
await expect_home(page);
}

async function arrow(page: Page, direction: "Up" | "Down"): Promise<void> {
Expand Down
3 changes: 2 additions & 1 deletion web/src/compose_tooltips.ts
Expand Up @@ -206,7 +206,8 @@ export function initialize(): void {
const narrow_filter = narrow_state.filter();
let display_current_view;
if (narrow_state.is_message_feed_visible()) {
if (narrow_filter === undefined) {
assert(narrow_filter !== undefined);
if (narrow_filter.is_in_home()) {
display_current_view = $t({
defaultMessage: "Currently viewing your combined feed.",
});
Expand Down
2 changes: 0 additions & 2 deletions web/src/hashchange.js
Expand Up @@ -12,7 +12,6 @@ import * as inbox_ui from "./inbox_ui";
import * as inbox_util from "./inbox_util";
import * as info_overlay from "./info_overlay";
import * as message_fetch from "./message_fetch";
import * as message_lists from "./message_lists";
import * as message_viewport from "./message_viewport";
import * as modals from "./modals";
import * as narrow from "./narrow";
Expand Down Expand Up @@ -161,7 +160,6 @@ function do_hashchange_normal(from_reload) {
if (from_reload) {
blueslip.debug("We are narrowing as part of a reload.");
if (message_fetch.initial_narrow_pointer !== undefined) {
message_lists.home.pre_narrow_offset = message_fetch.initial_offset;
narrow_opts.then_select_id = message_fetch.initial_narrow_pointer;
narrow_opts.then_select_offset = message_fetch.initial_narrow_offset;
}
Expand Down
2 changes: 1 addition & 1 deletion web/src/message_edit.js
Expand Up @@ -1241,7 +1241,7 @@ export function delete_topic(stream_id, topic_name, failures = 0) {
});
}

export function handle_narrow_deactivated() {
export function restore_edit_state_after_message_view_change() {
assert(message_lists.current !== undefined);
for (const [idx, elem] of currently_editing_messages) {
if (message_lists.current.get(idx) !== undefined) {
Expand Down
13 changes: 9 additions & 4 deletions web/src/message_events.js
Expand Up @@ -166,7 +166,6 @@ export function insert_new_messages(messages, sent_by_this_client) {
message_notifications.received_messages(messages);
stream_list.update_streams_sidebar();
pm_list.update_private_messages();
recent_view_ui.process_messages(messages);
}

export function update_messages(events) {
Expand Down Expand Up @@ -532,7 +531,6 @@ export function update_messages(events) {
// propagated edits to be updated (since the topic edits can have
// changed the correct grouping of messages).
if (any_topic_edited || any_stream_changed) {
message_lists.home.update_muting_and_rerender();
// However, we don't need to rerender message_list if
// we just changed the narrow earlier in this function.
//
Expand All @@ -542,8 +540,15 @@ export function update_messages(events) {
// edit. Doing so could save significant work, since most
// topic edits will not match the current topic narrow in
// large organizations.
if (!changed_narrow && message_lists.current?.narrowed) {
message_lists.current.update_muting_and_rerender();

for (const list of message_lists.all_rendered_message_lists()) {
if (changed_narrow && list === message_lists.current) {
// Avoid updating current message list if user switched to a different narrow and
// we don't want to preserver the rendered state for the current one.
continue;
}

list.view.rerender_messages(messages_to_rerender, any_message_content_edited);
}
} else {
// If the content of the message was edited, we do a special animation.
Expand Down