Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

message-editing: Make default "Move messages" form context-dependent. 

In the previous menu for moving messages, the default option was
"Move this and all following messages." However, this default choice
was not always aligned with user intentions, particularly when moving
the first or last message in a topic. In such cases, the desired
behavior often corresponds to "Move all messages in this topic" for the
first message and "Move only this message" for the last message.

To address this, we have updated the default options as follows:

1. **When moving the first message in a topic:** The default option is
now "Move all messages in this topic." This change better represents
the user's intention when moving the initial message in a topic.

2. **When moving the last message in a topic:** The default option has
been adjusted to "Move only this message." This change ensures that
users can easily move the last message without affecting other messages
in the topic.

These changes are designed to enhance the user experience and
facilitate the management of topics, especially when users follow or
unmute topics.

Fixes: #27298.
This commit is contained in:
Sayam Samal 2023-10-23 03:06:26 +05:30 committed by Tim Abbott
parent 49263ba69f
commit 5c82a923a9
3 changed files with 159 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
web/src/message_edit.js
View File

@ -1337,3 +1337,39 @@ export function with_first_message_id(stream_id, topic_name, success_cb, error_c
error: error_cb,
});
}
export function is_message_oldest_or_newest(
stream_id,
topic_name,
message_id,
success_callback,
error_callback,
) {
const data = {
anchor: message_id,
num_before: 1,
num_after: 1,
narrow: JSON.stringify([
{operator: "stream", operand: stream_id},
{operator: "topic", operand: topic_name},
]),
};
channel.get({
url: "/json/messages",
data,
success(data) {
let is_oldest = true;
let is_newest = true;
for (const message of data.messages) {
if (message.id < message_id) {
is_oldest = false;
} else if (message.id > message_id) {
is_newest = false;
}
}
success_callback(is_oldest, is_newest);
},
error: error_callback,
});
}

121
web/src/stream_popover.js
View File

@ -14,6 +14,7 @@ import * as dropdown_widget from "./dropdown_widget";
import * as hash_util from "./hash_util";
import {$t, $t_html} from "./i18n";
import * as message_edit from "./message_edit";
import * as message_lists from "./message_lists";
import * as popover_menus from "./popover_menus";
import {left_sidebar_tippy_options} from "./popover_menus";
import * as settings_data from "./settings_data";
@ -27,6 +28,7 @@ import * as sub_store from "./sub_store";
import * as ui_report from "./ui_report";
import * as ui_util from "./ui_util";
import * as unread_ops from "./unread_ops";
import * as util from "./util";
// In this module, we manage stream popovers
// that pop up from the left sidebar.
let stream_popover_instance = null;
@ -207,7 +209,119 @@ function build_stream_popover(opts) {
});
}
export function build_move_topic_to_stream_popover(
async function get_message_placement_from_server(
current_stream_id,
topic_name,
current_message_id,
) {
return new Promise((resolve) => {
message_edit.is_message_oldest_or_newest(
current_stream_id,
topic_name,
current_message_id,
(is_oldest, is_newest) => {
if (is_oldest) {
resolve("first");
} else if (is_newest) {
resolve("last");
} else {
resolve("intermediate");
}
},
);
});
}
async function get_message_placement_in_conversation(
current_stream_id,
topic_name,
current_message_id,
) {
// First we check if the placement of the message can be determined
// in the current message list. This allows us to avoid a server call
// in most cases.
if (message_lists.current.data.filter.supports_collapsing_recipients()) {
// Next we check if we are in a conversation view. If we are
// in a conversation view, we check if the message is the
// first or the last message in the current view. If not, we
// can conclude that the message is an intermediate message.
//
// It's safe to assume message_lists.current.data is non-empty, because
// current_message_id must be present in it.
if (message_lists.current.data.filter.is_conversation_view()) {
if (
message_lists.current.data.fetch_status.has_found_oldest() &&
message_lists.current.data.first().id === current_message_id
) {
return "first";
} else if (
message_lists.current.data.fetch_status.has_found_newest() &&
message_lists.current.data.last().id === current_message_id
) {
return "last";
}
return "intermediate";
}
// If we are not in a conversation view, but still know
// the view contains the entire conversation, we check if
// we can find the adjacent messages in the current view
// through which we can determine if the message is an
// intermediate message or not.
const msg_list = message_lists.current.data.all_messages();
let found_newer_matching_message = false;
let found_older_matching_message = false;
const current_dict = {
stream_id: current_stream_id,
topic: topic_name,
};
for (let i = msg_list.length - 1; i >= 0; i -= 1) {
const message = msg_list[i];
const message_dict = {
stream_id: message.stream_id,
topic: message.topic,
};
if (util.same_stream_and_topic(current_dict, message_dict)) {
if (message.id > current_message_id) {
found_newer_matching_message = true;
} else if (message.id < current_message_id) {
found_older_matching_message = true;
}
if (found_newer_matching_message && found_older_matching_message) {
return "intermediate";
}
}
}
if (
message_lists.current.data.fetch_status.has_found_newest() &&
!found_newer_matching_message
) {
return "last";
}
if (
message_lists.current.data.fetch_status.has_found_oldest() &&
!found_older_matching_message
) {
return "first";
}
}
// In case we are unable to determine the placement of the message
// in the current message list, we make a server call to determine
// the placement.
return await get_message_placement_from_server(
current_stream_id,
topic_name,
current_message_id,
);
}
export async function build_move_topic_to_stream_popover(
current_stream_id,
topic_name,
only_topic_edit,
@ -260,6 +374,11 @@ export function build_move_topic_to_stream_popover(
const move_limit_buffer = 5;
args.disable_topic_input = !message_edit.is_topic_editable(message, move_limit_buffer);
disable_stream_input = !message_edit.is_stream_editable(message, move_limit_buffer);
args.message_placement = await get_message_placement_in_conversation(
current_stream_id,
topic_name,
message.id,
);
}
function get_params_from_form() {

6
web/templates/move_topic_to_stream.hbs
View File

@ -17,9 +17,9 @@
<input name="current_stream_id" type="hidden" value="{{current_stream_id}}" />
{{#if from_message_actions_popover}}
<select class="message_edit_topic_propagate modal_select bootstrap-focus-style">
<option value="change_one"> {{t "Move only this message" }}</option>
<option selected="selected" value="change_later"> {{t "Move this and all following messages in this topic" }}</option>
<option value="change_all"> {{t "Move all messages in this topic" }}</option>
<option value="change_one" {{#if (eq message_placement "last")}}selected{{/if}}> {{t "Move only this message" }}</option>
<option value="change_later" {{#if (eq message_placement "intermediate")}}selected{{/if}}> {{t "Move this and all following messages in this topic" }}</option>
<option value="change_all" {{#if (eq message_placement "first")}}selected{{/if}}> {{t "Move all messages in this topic" }}</option>
</select>
{{/if}}
<div class="topic_move_breadcrumb_messages">