diff --git a/web/e2e-tests/compose.test.ts b/web/e2e-tests/compose.test.ts index 81d5ae3861..6dde2d37c6 100644 --- a/web/e2e-tests/compose.test.ts +++ b/web/e2e-tests/compose.test.ts @@ -35,6 +35,7 @@ async function test_send_messages(page: Page): Promise { {recipient: "cordelia@zulip.com", content: "Compose direct message reply test"}, ]); + await page.click("#left-sidebar-navigation-list .top_left_all_messages"); assert.equal((await page.$$(".message-list .message_row")).length, initial_msgs_count + 2); } diff --git a/web/e2e-tests/copy-and-paste.test.ts b/web/e2e-tests/copy-and-paste.test.ts index 321465514d..b5335394f9 100644 --- a/web/e2e-tests/copy-and-paste.test.ts +++ b/web/e2e-tests/copy-and-paste.test.ts @@ -150,6 +150,7 @@ async function copy_paste_test(page: Page): Promise { {stream_name: "Verona", topic: "copy-paste-topic #3", content: "copy paste test G"}, ]); + await page.click("#left-sidebar-navigation-list .top_left_all_messages"); const message_list_id = await common.get_current_msg_list_id(page, true); await common.check_messages_sent(page, message_list_id, [ ["Verona > copy-paste-topic #1", ["copy paste test A", "copy paste test B"]], diff --git a/web/e2e-tests/drafts.test.ts b/web/e2e-tests/drafts.test.ts index 15817d5759..83b371139e 100644 --- a/web/e2e-tests/drafts.test.ts +++ b/web/e2e-tests/drafts.test.ts @@ -300,9 +300,7 @@ async function drafts_test(page: Page): Promise { outside_view: true, }); await create_private_message_draft(page); - // Narrow to the conversation so that the compose box will restore it, - // then close and try restoring it by opening the composebox again. - await page.click("#compose .narrow_to_compose_recipients"); + // Close and try restoring it by opening the composebox again. await page.click("#compose_close"); await test_restore_private_message_draft_by_opening_composebox(page); diff --git a/web/e2e-tests/edit.test.ts b/web/e2e-tests/edit.test.ts index 9134487e96..d26835a1e1 100644 --- a/web/e2e-tests/edit.test.ts +++ b/web/e2e-tests/edit.test.ts @@ -29,7 +29,7 @@ async function edit_stream_message(page: Page, content: string): Promise { await common.wait_for_fully_processed_message(page, content); } -async function test_stream_message_edit(page: Page, message_list_id: number): Promise { +async function test_stream_message_edit(page: Page): Promise { await common.send_message(page, "stream", { stream_name: "Verona", topic: "edits", @@ -38,6 +38,7 @@ async function test_stream_message_edit(page: Page, message_list_id: number): Pr await edit_stream_message(page, "test edited"); + const message_list_id = await common.get_current_msg_list_id(page, true); await common.check_messages_sent(page, message_list_id, [["Verona > edits", ["test edited"]]]); } @@ -76,7 +77,7 @@ async function test_edit_message_with_slash_me(page: Page): Promise { ); } -async function test_edit_private_message(page: Page, message_list_id: number): Promise { +async function test_edit_private_message(page: Page): Promise { await common.send_message(page, "private", { recipient: "cordelia@zulip.com", content: "test editing pm", @@ -87,6 +88,7 @@ async function test_edit_private_message(page: Page, message_list_id: number): P await page.click(".message_edit_save"); await common.wait_for_fully_processed_message(page, "test edited pm"); + const message_list_id = await common.get_current_msg_list_id(page, true); await common.check_messages_sent(page, message_list_id, [ ["You and Cordelia, Lear's daughter", ["test edited pm"]], ]); @@ -96,11 +98,10 @@ async function edit_tests(page: Page): Promise { await common.log_in(page); await page.click("#left-sidebar-navigation-list .top_left_all_messages"); await page.waitForSelector(".message-list .message_row", {visible: true}); - const message_list_id = await common.get_current_msg_list_id(page, true); - await test_stream_message_edit(page, message_list_id); + await test_stream_message_edit(page); await test_edit_message_with_slash_me(page); - await test_edit_private_message(page, message_list_id); + await test_edit_private_message(page); } common.run_test(edit_tests); diff --git a/web/e2e-tests/message-basics.test.ts b/web/e2e-tests/message-basics.test.ts index 7d4914ede2..50f333cb55 100644 --- a/web/e2e-tests/message-basics.test.ts +++ b/web/e2e-tests/message-basics.test.ts @@ -553,6 +553,7 @@ async function message_basic_tests(page: Page): Promise { {recipient: "cordelia@zulip.com", content: "direct message e"}, ]); + await page.click("#left-sidebar-navigation-list .top_left_all_messages"); await expect_home(page); await test_navigations_from_home(page); diff --git a/web/e2e-tests/stars.test.ts b/web/e2e-tests/stars.test.ts index c48d9ba1a4..e8a4cbda86 100644 --- a/web/e2e-tests/stars.test.ts +++ b/web/e2e-tests/stars.test.ts @@ -44,7 +44,7 @@ async function test_narrow_to_starred_messages(page: Page): Promise { async function stars_test(page: Page): Promise { await common.log_in(page); await page.click("#left-sidebar-navigation-list .top_left_all_messages"); - const message_list_id = await common.get_current_msg_list_id(page, true); + let message_list_id = await common.get_current_msg_list_id(page, true); await page.waitForSelector( `.message-list[data-message-list-id='${message_list_id}'] .message_row`, {visible: true}, @@ -60,6 +60,8 @@ async function stars_test(page: Page): Promise { assert.strictEqual(await stars_count(page), 0, "Unexpected already starred message(s)."); await toggle_test_star_message(page); + await page.click("#left-sidebar-navigation-list .top_left_all_messages"); + message_list_id = await common.get_current_msg_list_id(page, true); await page.waitForSelector( `.message-list[data-message-list-id='${message_list_id}'] .zulip-icon-star-filled`, {visible: true}, diff --git a/web/src/compose_banner.ts b/web/src/compose_banner.ts index 2723be3afd..ec013a6509 100644 --- a/web/src/compose_banner.ts +++ b/web/src/compose_banner.ts @@ -21,7 +21,6 @@ export const INFO = "info"; const MESSAGE_SENT_CLASSNAMES = { sent_scroll_to_view: "sent_scroll_to_view", - narrow_to_recipient: "narrow_to_recipient", message_scheduled_success_compose_banner: "message_scheduled_success_compose_banner", automatic_new_visibility_policy: "automatic_new_visibility_policy", }; @@ -87,8 +86,24 @@ export function update_or_append_banner( } } -export function clear_message_sent_banners(include_unmute_banner = true): void { +export function clear_message_sent_banners( + include_unmute_banner = true, + skip_automatic_new_visibility_policy_banner = false, +): void { for (const classname of Object.values(MESSAGE_SENT_CLASSNAMES)) { + if ( + skip_automatic_new_visibility_policy_banner && + classname === MESSAGE_SENT_CLASSNAMES.automatic_new_visibility_policy + ) { + // Handles the case where this banner shouldn't be cleared in the + // race condition where the response from `POST /messages` for a + // not locally echoed message (composed from a different view) wins + // over the event received for the same. + // Otherwise, the response will lead to this banner, and the event + // will narrow the sender to the new conversation, leading to this + // banner being visible for a fraction of seconds. + continue; + } $(`#compose_banners .${CSS.escape(classname)}`).remove(); } if (include_unmute_banner) { diff --git a/web/src/compose_notifications.ts b/web/src/compose_notifications.ts index 50def9c81a..a49ef67901 100644 --- a/web/src/compose_notifications.ts +++ b/web/src/compose_notifications.ts @@ -116,35 +116,28 @@ export function get_muted_narrow(message: Message): string | undefined { return undefined; } -export function get_local_notify_mix_reason(message: Message): string | undefined { +export function is_local_mix(message: Message): boolean { if (message_lists.current === undefined) { // For non-message list views like Inbox, the message is not visible after sending it. - return undefined; + return true; } - const $row = message_lists.current.get_row(message.id); - if ($row.length > 0) { - // If our message is in the current message list, we do - // not have a mix, so we are happy. - return undefined; - } - - // offscreen because it is outside narrow - // we can only look for these on non-search (can_apply_locally) messages - // see also: exports.notify_messages_outside_current_search const current_filter = narrow_state.filter(); - if ( - current_filter && - current_filter.can_apply_locally() && - !current_filter.predicate()(message) - ) { - return $t({defaultMessage: "Sent! Your message is outside your current view."}); + const $row = message_lists.current.get_row(message.id); + if (current_filter && current_filter.is_conversation_view() && $row.length > 0) { + // If our message is in the current conversation view, we do + // not have a mix, so we are happy. + return false; } - return undefined; + return true; } -export function notify_local_mixes(messages: Message[], need_user_to_scroll: boolean): void { +export function notify_local_mixes( + messages: Message[], + need_user_to_scroll: boolean, + {narrow_to_recipient}: {narrow_to_recipient: (message_id: number) => void}, +): void { /* This code should only be called when we are displaying messages sent by current client. It notifies users that @@ -173,13 +166,13 @@ export function notify_local_mixes(messages: Message[], need_user_to_scroll: boo continue; } - let banner_text = get_local_notify_mix_reason(message); + const local_mix = is_local_mix(message); const link_msg_id = message.id; - if (!banner_text) { + if (!local_mix) { if (need_user_to_scroll) { - banner_text = $t({defaultMessage: "Sent!"}); + const banner_text = $t({defaultMessage: "Sent!"}); const link_text = $t({defaultMessage: "Scroll down to view your message."}); notify_above_composebox( banner_text, @@ -197,18 +190,7 @@ export function notify_local_mixes(messages: Message[], need_user_to_scroll: boo continue; } - const link_text = $t( - {defaultMessage: "Go to {message_recipient}"}, - {message_recipient: get_message_header(message)}, - ); - - notify_above_composebox( - banner_text, - compose_banner.CLASSNAMES.narrow_to_recipient, - get_above_composebox_narrow_url(message), - link_msg_id, - link_text, - ); + narrow_to_recipient(link_msg_id); } } @@ -225,28 +207,6 @@ function get_above_composebox_narrow_url(message: Message): string { return above_composebox_narrow_url; } -// for callback when we have to check with the server if a message should be in -// the message_lists.current (!can_apply_locally; a.k.a. "a search"). -export function notify_messages_outside_current_search(messages: Message[]): void { - for (const message of messages) { - if (!people.is_current_user(message.sender_email)) { - continue; - } - const above_composebox_narrow_url = get_above_composebox_narrow_url(message); - const link_text = $t( - {defaultMessage: "Narrow to {message_recipient}"}, - {message_recipient: get_message_header(message)}, - ); - notify_above_composebox( - $t({defaultMessage: "Sent! Your recent message is outside the current search."}), - compose_banner.CLASSNAMES.narrow_to_recipient, - above_composebox_narrow_url, - message.id, - link_text, - ); - } -} - export function reify_message_id(opts: {old_id: number; new_id: number}): void { const old_id = opts.old_id; const new_id = opts.new_id; diff --git a/web/src/message_events.js b/web/src/message_events.js index e4e05b3ed9..e11e22125f 100644 --- a/web/src/message_events.js +++ b/web/src/message_events.js @@ -56,14 +56,10 @@ function maybe_add_narrowed_messages(messages, msg_list, callback, attempt = 1) } let new_messages = []; - const elsewhere_messages = []; - for (const elem of messages) { if (Object.hasOwn(data.messages, elem.id)) { util.set_match_data(elem, data.messages[elem.id]); new_messages.push(elem); - } else { - elsewhere_messages.push(elem); } } @@ -81,7 +77,6 @@ function maybe_add_narrowed_messages(messages, msg_list, callback, attempt = 1) callback(new_messages, msg_list); unread_ops.process_visible(); - compose_notifications.notify_messages_outside_current_search(elsewhere_messages); }, error(xhr) { if (!narrow_state.is_message_feed_visible() || msg_list !== message_lists.current) { @@ -155,7 +150,11 @@ export function insert_new_messages(messages, sent_by_this_client) { // were sent by this client; notifications.notify_local_mixes // will filter out any not sent by us. if (sent_by_this_client) { - compose_notifications.notify_local_mixes(messages, need_user_to_scroll); + compose_notifications.notify_local_mixes(messages, need_user_to_scroll, { + narrow_to_recipient(message_id) { + narrow.by_topic(message_id, {trigger: "outside_current_view"}); + }, + }); } if (any_untracked_unread_messages) { diff --git a/web/src/narrow.js b/web/src/narrow.js index 861f5e156e..a7a728b19e 100644 --- a/web/src/narrow.js +++ b/web/src/narrow.js @@ -59,7 +59,7 @@ import * as util from "./util"; const LARGER_THAN_MAX_MESSAGE_ID = 10000000000000000; -export function reset_ui_state() { +export function reset_ui_state(opts) { // Resets the state of various visual UI elements that are // a function of the current narrow. narrow_banner.hide_empty_narrow_message(); @@ -70,7 +70,11 @@ export function reset_ui_state() { compose_state.allow_draft_restoring(); // Most users aren't going to send a bunch of a out-of-narrow messages // and expect to visit a list of narrows, so let's get these out of the way. - compose_banner.clear_message_sent_banners(); + let skip_automatic_new_visibility_policy_banner = false; + if (opts && opts.trigger === "outside_current_view") { + skip_automatic_new_visibility_policy_banner = true; + } + compose_banner.clear_message_sent_banners(true, skip_automatic_new_visibility_policy_banner); } export function changehash(newhash, trigger) { @@ -449,7 +453,7 @@ export function activate(raw_terms, opts) { // this point. This is important to prevent calling such functions // more than once in the event that we call narrow.activate // recursively. - reset_ui_state(); + reset_ui_state(opts); if (coming_from_recent_view) { recent_view_ui.hide();