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 Prakhar Pratyush
parent 05513c90f0
commit a2ef1d7e93
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"},
]);
await page.click("#left-sidebar-navigation-list .top_left_all_messages");
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"},
]);
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"]],

View File

@ -300,9 +300,7 @@ async function drafts_test(page: Page): Promise<void> {
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);

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);
}
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", {
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<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", {
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<void> {
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);

View File

@ -553,6 +553,7 @@ async function message_basic_tests(page: Page): Promise<void> {
{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);

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> {
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<void> {
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},

View File

@ -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) {

View File

@ -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;

View File

@ -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) {

View File

@ -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();