compose: Jump to conversation where message was sent.

Removes two 'narrow_to_recipient' banners which were
shown after sending a message to a different conversation.

Now, the sender is narrowed to the conversation where
message was sent.

Fixes #29186.
This commit is contained in:
Prakhar Pratyush 2024-03-06 17:42:08 +05:30 committed by Tim Abbott
parent e959a392c9
commit e4cbca698d
10 changed files with 59 additions and 77 deletions

View File

@ -35,6 +35,7 @@ async function test_send_messages(page: Page): Promise<void> {
{recipient: "cordelia@zulip.com", content: "Compose direct message reply test"}, {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); assert.equal((await page.$$(".message-list .message_row")).length, initial_msgs_count + 2);
} }

View File

@ -150,6 +150,7 @@ async function copy_paste_test(page: Page): Promise<void> {
{stream_name: "Verona", topic: "copy-paste-topic #3", content: "copy paste test G"}, {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); const message_list_id = await common.get_current_msg_list_id(page, true);
await common.check_messages_sent(page, message_list_id, [ await common.check_messages_sent(page, message_list_id, [
["Verona > copy-paste-topic #1", ["copy paste test A", "copy paste test B"]], ["Verona > copy-paste-topic #1", ["copy paste test A", "copy paste test B"]],

View File

@ -306,9 +306,7 @@ async function drafts_test(page: Page): Promise<void> {
outside_view: true, outside_view: true,
}); });
await create_private_message_draft(page); await create_private_message_draft(page);
// Narrow to the conversation so that the compose box will restore it, // Close and try restoring it by opening the composebox again.
// then close and try restoring it by opening the composebox again.
await page.click("#compose .narrow_to_compose_recipients");
await page.click("#compose_close"); await page.click("#compose_close");
await test_restore_private_message_draft_by_opening_composebox(page); await test_restore_private_message_draft_by_opening_composebox(page);

View File

@ -29,7 +29,7 @@ async function edit_stream_message(page: Page, content: string): Promise<void> {
await common.wait_for_fully_processed_message(page, content); await common.wait_for_fully_processed_message(page, content);
} }
async function test_stream_message_edit(page: Page, message_list_id: number): Promise<void> { async function test_stream_message_edit(page: Page): Promise<void> {
await common.send_message(page, "stream", { await common.send_message(page, "stream", {
stream_name: "Verona", stream_name: "Verona",
topic: "edits", 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"); 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"]]]); 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<void> {
); );
} }
async function test_edit_private_message(page: Page, message_list_id: number): Promise<void> { async function test_edit_private_message(page: Page): Promise<void> {
await common.send_message(page, "private", { await common.send_message(page, "private", {
recipient: "cordelia@zulip.com", recipient: "cordelia@zulip.com",
content: "test editing pm", 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 page.click(".message_edit_save");
await common.wait_for_fully_processed_message(page, "test edited pm"); 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, [ await common.check_messages_sent(page, message_list_id, [
["You and Cordelia, Lear's daughter", ["test edited pm"]], ["You and Cordelia, Lear's daughter", ["test edited pm"]],
]); ]);
@ -96,11 +98,10 @@ async function edit_tests(page: Page): Promise<void> {
await common.log_in(page); await common.log_in(page);
await page.click("#left-sidebar-navigation-list .top_left_all_messages"); await page.click("#left-sidebar-navigation-list .top_left_all_messages");
await page.waitForSelector(".message-list .message_row", {visible: true}); 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_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); common.run_test(edit_tests);

View File

@ -553,6 +553,7 @@ async function message_basic_tests(page: Page): Promise<void> {
{recipient: "cordelia@zulip.com", content: "direct message e"}, {recipient: "cordelia@zulip.com", content: "direct message e"},
]); ]);
await page.click("#left-sidebar-navigation-list .top_left_all_messages");
await expect_home(page); await expect_home(page);
await test_navigations_from_home(page); await test_navigations_from_home(page);

View File

@ -44,7 +44,7 @@ async function test_narrow_to_starred_messages(page: Page): Promise<void> {
async function stars_test(page: Page): Promise<void> { async function stars_test(page: Page): Promise<void> {
await common.log_in(page); await common.log_in(page);
await page.click("#left-sidebar-navigation-list .top_left_all_messages"); 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( await page.waitForSelector(
`.message-list[data-message-list-id='${message_list_id}'] .message_row`, `.message-list[data-message-list-id='${message_list_id}'] .message_row`,
{visible: true}, {visible: true},
@ -60,6 +60,8 @@ async function stars_test(page: Page): Promise<void> {
assert.strictEqual(await stars_count(page), 0, "Unexpected already starred message(s)."); assert.strictEqual(await stars_count(page), 0, "Unexpected already starred message(s).");
await toggle_test_star_message(page); 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( await page.waitForSelector(
`.message-list[data-message-list-id='${message_list_id}'] .zulip-icon-star-filled`, `.message-list[data-message-list-id='${message_list_id}'] .zulip-icon-star-filled`,
{visible: true}, {visible: true},

View File

@ -21,7 +21,6 @@ export const INFO = "info";
const MESSAGE_SENT_CLASSNAMES = { const MESSAGE_SENT_CLASSNAMES = {
sent_scroll_to_view: "sent_scroll_to_view", sent_scroll_to_view: "sent_scroll_to_view",
narrow_to_recipient: "narrow_to_recipient",
message_scheduled_success_compose_banner: "message_scheduled_success_compose_banner", message_scheduled_success_compose_banner: "message_scheduled_success_compose_banner",
automatic_new_visibility_policy: "automatic_new_visibility_policy", 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)) { 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(); $(`#compose_banners .${CSS.escape(classname)}`).remove();
} }
if (include_unmute_banner) { if (include_unmute_banner) {

View File

@ -116,35 +116,28 @@ export function get_muted_narrow(message: Message): string | undefined {
return 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) { if (message_lists.current === undefined) {
// For non-message list views like Inbox, the message is not visible after sending it. // 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(); const current_filter = narrow_state.filter();
if ( const $row = message_lists.current.get_row(message.id);
current_filter && if (current_filter && current_filter.is_conversation_view() && $row.length > 0) {
current_filter.can_apply_locally() && // If our message is in the current conversation view, we do
!current_filter.predicate()(message) // not have a mix, so we are happy.
) { return false;
return $t({defaultMessage: "Sent! Your message is outside your current view."});
} }
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 This code should only be called when we are displaying
messages sent by current client. It notifies users that 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; continue;
} }
let banner_text = get_local_notify_mix_reason(message); const local_mix = is_local_mix(message);
const link_msg_id = message.id; const link_msg_id = message.id;
if (!banner_text) { if (!local_mix) {
if (need_user_to_scroll) { 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."}); const link_text = $t({defaultMessage: "Scroll down to view your message."});
notify_above_composebox( notify_above_composebox(
banner_text, banner_text,
@ -197,18 +190,7 @@ export function notify_local_mixes(messages: Message[], need_user_to_scroll: boo
continue; continue;
} }
const link_text = $t( narrow_to_recipient(link_msg_id);
{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,
);
} }
} }
@ -225,28 +207,6 @@ function get_above_composebox_narrow_url(message: Message): string {
return above_composebox_narrow_url; 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 { export function reify_message_id(opts: {old_id: number; new_id: number}): void {
const old_id = opts.old_id; const old_id = opts.old_id;
const new_id = opts.new_id; const new_id = opts.new_id;

View File

@ -56,14 +56,10 @@ function maybe_add_narrowed_messages(messages, msg_list, callback, attempt = 1)
} }
let new_messages = []; let new_messages = [];
const elsewhere_messages = [];
for (const elem of messages) { for (const elem of messages) {
if (Object.hasOwn(data.messages, elem.id)) { if (Object.hasOwn(data.messages, elem.id)) {
util.set_match_data(elem, data.messages[elem.id]); util.set_match_data(elem, data.messages[elem.id]);
new_messages.push(elem); 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); callback(new_messages, msg_list);
unread_ops.process_visible(); unread_ops.process_visible();
compose_notifications.notify_messages_outside_current_search(elsewhere_messages);
}, },
error(xhr) { error(xhr) {
if (!narrow_state.is_message_feed_visible() || msg_list !== message_lists.current) { 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 // were sent by this client; notifications.notify_local_mixes
// will filter out any not sent by us. // will filter out any not sent by us.
if (sent_by_this_client) { 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) { if (any_untracked_unread_messages) {

View File

@ -59,7 +59,7 @@ import * as util from "./util";
const LARGER_THAN_MAX_MESSAGE_ID = 10000000000000000; 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 // Resets the state of various visual UI elements that are
// a function of the current narrow. // a function of the current narrow.
narrow_banner.hide_empty_narrow_message(); narrow_banner.hide_empty_narrow_message();
@ -70,7 +70,11 @@ export function reset_ui_state() {
compose_state.allow_draft_restoring(); compose_state.allow_draft_restoring();
// Most users aren't going to send a bunch of a out-of-narrow messages // 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. // 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) { export function changehash(newhash, trigger) {
@ -485,7 +489,7 @@ export function activate(raw_terms, opts) {
// this point. This is important to prevent calling such functions // this point. This is important to prevent calling such functions
// more than once in the event that we call narrow.activate // more than once in the event that we call narrow.activate
// recursively. // recursively.
reset_ui_state(); reset_ui_state(opts);
if (coming_from_recent_view) { if (coming_from_recent_view) {
recent_view_ui.hide(); recent_view_ui.hide();