mirror of https://github.com/zulip/zulip.git
echo: Remove checks on current message list.
Originally, we only wanted to do local echo in the current view. However, now that we're looking at navigating the user to the conversation that they send a new message to, it's going to be quite common that we immediately visit a destination different from the current view, where local echo in that different view would be valuable. The most interesting block was added inaf188205cb
/ #8989. But in6637f2dbb7
, the key logic for checking `msg_list.data.fetch_status.has_found_newest` was duplicated in the `add_new_messages` code path, which critically also updates `update_expected_max_message_id` and thus may close a race with fetching message history for a view we're being navigated to, where the locally echoed message might fail to appear at all. This change does come with a slight regression: If we are looking at a search view where the filter is one that we cannot apply locally, a newly sent message will now be locally echoed (returning the compose box for drafting another) even though it cannot be displayed in the current view, which means that the message will not appear in either the compose box or the current view for the brief period before we get a reply from the server in this scenario. This is a minor detail, likely not worth troubling ourselves over. Given our intent to experiment with navigating the user out of the search view in this scenario, this is likely not important. Co-authored-by: Tim Abbott <tabbott@zulip.com>
This commit is contained in:
parent
a27fae16df
commit
4efcc33dc2
|
@ -10,7 +10,6 @@ import * as markdown from "./markdown";
|
||||||
import * as message_lists from "./message_lists";
|
import * as message_lists from "./message_lists";
|
||||||
import * as message_live_update from "./message_live_update";
|
import * as message_live_update from "./message_live_update";
|
||||||
import * as message_store from "./message_store";
|
import * as message_store from "./message_store";
|
||||||
import * as narrow_state from "./narrow_state";
|
|
||||||
import * as people from "./people";
|
import * as people from "./people";
|
||||||
import * as pm_list from "./pm_list";
|
import * as pm_list from "./pm_list";
|
||||||
import * as recent_view_data from "./recent_view_data";
|
import * as recent_view_data from "./recent_view_data";
|
||||||
|
@ -210,35 +209,29 @@ export function is_slash_command(content) {
|
||||||
}
|
}
|
||||||
|
|
||||||
export function try_deliver_locally(message_request, insert_new_messages) {
|
export function try_deliver_locally(message_request, insert_new_messages) {
|
||||||
if (message_lists.current === undefined) {
|
// Checks if the message request can be locally echoed, and if so,
|
||||||
return undefined;
|
// adds a local echoed copy of the message to appropriate message lists.
|
||||||
}
|
//
|
||||||
|
// Returns the message object, or undefined if it cannot be
|
||||||
|
// echoed; in that case, the compose box will remain in the
|
||||||
|
// sending state rather than being cleared to allow composing a
|
||||||
|
// next message.
|
||||||
|
//
|
||||||
|
// Notably, this algorithm will allow locally echoing a message in
|
||||||
|
// cases where we are currently looking at a search view where
|
||||||
|
// `!filter.can_apply_locally(message)`; so it is possible for a
|
||||||
|
// message to be locally echoed but not appear in the current
|
||||||
|
// view; this is useful to ensure it will be visible in other
|
||||||
|
// views that we might navigate to before we get a response from
|
||||||
|
// the server.
|
||||||
if (markdown.contains_backend_only_syntax(message_request.content)) {
|
if (markdown.contains_backend_only_syntax(message_request.content)) {
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (narrow_state.filter() !== undefined && !narrow_state.filter().can_apply_locally(true)) {
|
|
||||||
return undefined;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (is_slash_command(message_request.content)) {
|
if (is_slash_command(message_request.content)) {
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!message_lists.current.data.fetch_status.has_found_newest()) {
|
|
||||||
// If the current message list doesn't yet have the latest
|
|
||||||
// messages before the one we just sent, local echo would make
|
|
||||||
// it appear as though there were no messages between what we
|
|
||||||
// have and the new message we just sent, when in fact we're
|
|
||||||
// in the process of fetching those from the server. In this
|
|
||||||
// case, it's correct to skip local echo; we'll get the
|
|
||||||
// message we just sent placed appropriately when we get it
|
|
||||||
// from either server_events or message_fetch.
|
|
||||||
blueslip.info("Skipping local echo until newest messages get loaded.");
|
|
||||||
return undefined;
|
|
||||||
}
|
|
||||||
|
|
||||||
const local_id_float = local_message.get_next_id_float();
|
const local_id_float = local_message.get_next_id_float();
|
||||||
|
|
||||||
if (!local_id_float) {
|
if (!local_id_float) {
|
||||||
|
|
Loading…
Reference in New Issue