paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-23 04:52:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

message_edit: Allow only content edit in message_edit_form.

Browse Source 
We now allow only content edit in message_edit_form which can
be opened by pencil icon in the message row, "Edit message"
option in popover and by using e hotkey.

As a result of this change, we also do not show topic and stream
edit options when using "View source" options.

We would instead support changing stream and topic from the modal
which will be opened from the "Move message" option in message
actions popover.
This commit is contained in:
Sahil Batra
2022-10-01 17:20:44 +05:30
committed by Tim Abbott
parent 0b67cf0071
commit 0fc19732bc
3 changed files with 31 additions and 261 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
frontend_tests/puppeteer_tests/edit.ts
View File

@@ -20,10 +20,9 @@ async function trigger_edit_last_message(page: Page): Promise<void> {
await page.waitForSelector(".message_edit_content", {visible: true});
}
async function edit_stream_message(page: Page, topic: string, content: string): Promise<void> {
async function edit_stream_message(page: Page, content: string): Promise<void> {
await trigger_edit_last_message(page);
await common.clear_and_type(page, ".message_edit_topic", topic);
await common.clear_and_type(page, ".message_edit_content", content);
await page.click(".message_edit_save");
@@ -37,7 +36,7 @@ async function test_stream_message_edit(page: Page): Promise<void> {
content: "test editing",
});
await edit_stream_message(page, "edits", "test edited");
await edit_stream_message(page, "test edited");
await common.check_messages_sent(page, "zhome", [["Verona > edits", ["test edited"]]]);
}
@@ -61,7 +60,7 @@ async function test_edit_message_with_slash_me(page: Page): Promise<void> {
)} and normalize-space()="Desdemona"]`,
);
await edit_stream_message(page, "edits", "/me test edited a message with me");
await edit_stream_message(page, "/me test edited a message with me");
await page.waitForSelector(
`xpath/${last_message_xpath}//*[${common.has_class_x(

250
static/js/message_edit.js
View File

@@ -15,9 +15,7 @@ import * as composebox_typeahead from "./composebox_typeahead";
import * as condense from "./condense";
import * as confirm_dialog from "./confirm_dialog";
import * as dialog_widget from "./dialog_widget";
import {DropdownListWidget} from "./dropdown_list_widget";
import * as echo from "./echo";
import * as giphy from "./giphy";
import {$t, $t_html} from "./i18n";
import * as loading from "./loading";
import * as markdown from "./markdown";
@@ -28,9 +26,7 @@ import {page_params} from "./page_params";
import * as resize from "./resize";
import * as rows from "./rows";
import * as settings_data from "./settings_data";
import * as stream_bar from "./stream_bar";
import * as stream_data from "./stream_data";
import * as sub_store from "./sub_store";
import * as ui_report from "./ui_report";
import * as upload from "./upload";
import * as util from "./util";
@@ -401,7 +397,6 @@ export function get_available_streams_for_moving_messages(current_stream_id) {
}
function edit_message($row, raw_content) {
let stream_widget;
$row.find(".message_reactions").hide();
condense.hide_message_expander($row);
condense.hide_message_condenser($row);
@@ -426,43 +421,16 @@ function edit_message($row, raw_content) {
file_upload_enabled = true;
}
const is_stream_editable =
message.is_stream && settings_data.user_can_move_messages_between_streams();
const is_editable =
editability === editability_types.TOPIC_ONLY ||
editability === editability_types.FULL ||
is_stream_editable;
const is_content_editable = editability === editability_types.FULL;
const is_editable = editability === editability_types.FULL;
// current message's stream has been already been added and selected in Handlebars
const available_streams = is_stream_editable
? get_available_streams_for_moving_messages(message.stream_id)
: null;
const select_move_stream_widget_name = `select_move_stream_${message.id}`;
const opts = {
widget_name: select_move_stream_widget_name,
data: available_streams,
default_text: $t({defaultMessage: "No streams"}),
include_current_item: true,
value: message.stream_id,
on_update: set_propagate_selector_display,
};
const $form = $(
render_message_edit_form({
is_stream: message.type === "stream",
message_id: message.id,
is_editable,
is_content_editable,
topic: message.topic,
content: raw_content,
file_upload_enabled,
minutes_to_edit: Math.floor(page_params.realm_message_content_edit_limit_seconds / 60),
is_stream_editable,
select_move_stream_widget_name,
notify_new_thread: notify_new_thread_default,
notify_old_thread: notify_old_thread_default,
giphy_enabled: giphy.is_giphy_enabled(),
max_message_length: page_params.max_message_length,
}),
);
@@ -478,82 +446,37 @@ function edit_message($row, raw_content) {
.toggle(compose.compute_show_video_chat_button());
upload.feature_check($(`#edit_form_${CSS.escape(rows.id($row))} .compose_upload_file`));
const $stream_header_colorblock = $row.find(".stream_header_colorblock");
const $message_edit_content = $row.find("textarea.message_edit_content");
const $message_edit_topic = $row.find("input.message_edit_topic");
const $message_edit_topic_propagate = $row.find("select.message_edit_topic_propagate");
const $message_edit_breadcrumb_messages = $row.find("div.message_edit_breadcrumb_messages");
const $message_edit_countdown_timer = $row.find(".message_edit_countdown_timer");
const $copy_message = $row.find(".copy_message");
// One might expect us to initially show the message move
// propagation select UI if and only if the user has permission to
// edit the topic. However, in the common case that a user sent
// the message themselves and thus has permission to edit the
// content, this dropdown can feel like clutter, so we don't show
// it until the stream/topic has been changed in that case.
//
// So we show this widget initially if and only if the stream or
// topic is editable, but the content is not.
$message_edit_topic_propagate.toggle(is_editable && !is_content_editable);
if (is_stream_editable) {
stream_widget = new DropdownListWidget(opts);
stream_widget.setup();
}
stream_bar.decorate(message.stream, $stream_header_colorblock, false);
switch (editability) {
case editability_types.NO:
$message_edit_content.attr("readonly", "readonly");
$message_edit_topic.attr("readonly", "readonly");
create_copy_to_clipboard_handler($row, $copy_message[0], message.id);
break;
case editability_types.NO_LONGER:
// You can currently only reach this state in non-streams. If that
// changes (e.g. if we stop allowing topics to be modified forever
// in streams), then we'll need to disable
// row.find('input.message_edit_topic') as well.
$message_edit_content.attr("readonly", "readonly");
create_copy_to_clipboard_handler($row, $copy_message[0], message.id);
break;
case editability_types.TOPIC_ONLY:
$message_edit_content.attr("readonly", "readonly");
// Hint why you can edit the topic but not the message content
create_copy_to_clipboard_handler($row, $copy_message[0], message.id);
break;
case editability_types.FULL: {
$copy_message.remove();
const edit_id = `#edit_form_${CSS.escape(rows.id($row))} .message_edit_content`;
const listeners = resize.watch_manual_resize(edit_id);
if (listeners) {
currently_editing_messages.get(rows.id($row)).listeners = listeners;
}
composebox_typeahead.initialize_compose_typeahead(edit_id);
compose_ui.handle_keyup(null, $(edit_id).expectOne());
$(edit_id).on("keydown", function (event) {
compose_ui.handle_keydown(event, $(this).expectOne());
});
$(edit_id).on("keyup", function (event) {
compose_ui.handle_keyup(event, $(this).expectOne());
});
break;
if (editability !== editability_types.FULL) {
$message_edit_content.attr("readonly", "readonly");
create_copy_to_clipboard_handler($row, $copy_message[0], message.id);
} else {
$copy_message.remove();
const edit_id = `#edit_form_${CSS.escape(rows.id($row))} .message_edit_content`;
const listeners = resize.watch_manual_resize(edit_id);
if (listeners) {
currently_editing_messages.get(rows.id($row)).listeners = listeners;
}
composebox_typeahead.initialize_compose_typeahead(edit_id);
compose_ui.handle_keyup(null, $(edit_id).expectOne());
$(edit_id).on("keydown", function (event) {
compose_ui.handle_keydown(event, $(this).expectOne());
});
$(edit_id).on("keyup", function (event) {
compose_ui.handle_keyup(event, $(this).expectOne());
});
}
// Add tooltip
// Add tooltip and timer
if (
editability === editability_types.FULL &&
page_params.realm_message_content_edit_limit_seconds > 0
) {
$row.find(".message-edit-timer").show();
}
// add timer
if (
editability === editability_types.FULL &&
page_params.realm_message_content_edit_limit_seconds > 0
) {
// Give them at least 10 seconds.
// If you change this number also change edit_limit_buffer in
// zerver.actions.message_edit.check_update_message
@@ -575,11 +498,6 @@ function edit_message($row, raw_content) {
if (seconds_left <= 0) {
clearInterval(countdown_timer);
$message_edit_content.prop("readonly", "readonly");
if (message.type === "stream") {
$message_edit_topic.prop("readonly", "readonly");
$message_edit_topic_propagate.hide();
$message_edit_breadcrumb_messages.hide();
}
// We don't go directly to a "TOPIC_ONLY" type state (with an active Save button),
// since it isn't clear what to do with the half-finished edit. It's nice to keep
// the half-finished edit around so that they can copy-paste it, but we don't want
@@ -594,13 +512,6 @@ function edit_message($row, raw_content) {
if (!is_editable) {
$row.find(".message_edit_close").trigger("focus");
} else if (message.type === "stream" && message.topic === compose.empty_topic_placeholder()) {
$message_edit_topic.val("");
$message_edit_topic.trigger("focus");
} else if (editability === editability_types.TOPIC_ONLY) {
$row.find(".message_edit_topic").trigger("focus");
} else if (editability !== editability_types.FULL && is_stream_editable) {
$row.find(".select_stream_setting .dropdown-toggle").trigger("focus");
} else {
$message_edit_content.trigger("focus");
// Put cursor at end of input.
@@ -616,55 +527,6 @@ function edit_message($row, raw_content) {
edit_obj.scrolled_by = scroll_by;
message_viewport.scrollTop(message_viewport.scrollTop() + scroll_by);
const original_stream_id = message.stream_id;
const original_topic = message.topic;
// Change the `stream_header_colorblock` when clicked on any dropdown item.
function update_stream_header_colorblock() {
// Stop the execution if stream_widget is undefined.
if (!stream_widget) {
return;
}
const stream_name = stream_data.maybe_get_stream_name(
Number.parseInt(stream_widget.value(), 10),
);
stream_bar.decorate(stream_name, $stream_header_colorblock, false);
}
function set_propagate_selector_display() {
update_stream_header_colorblock();
const new_topic = $message_edit_topic.val();
const new_stream_id = is_stream_editable
? Number.parseInt(stream_widget.value(), 10)
: null;
const is_topic_edited = new_topic !== original_topic && new_topic !== "";
const is_stream_edited = is_stream_editable ? new_stream_id !== original_stream_id : false;
$message_edit_topic_propagate.toggle(
is_topic_edited || is_stream_edited || $message_edit_topic_propagate.is(":visible"),
);
$message_edit_breadcrumb_messages.toggle(is_stream_edited);
if (is_stream_edited) {
/* Reinitialize the typeahead component with content for the new stream. */
const new_stream_name = sub_store.get(new_stream_id).name;
$message_edit_topic.data("typeahead").unlisten();
composebox_typeahead.initialize_topic_edit_typeahead(
$message_edit_topic,
new_stream_name,
true,
);
}
}
if (!message.locally_echoed) {
$message_edit_topic.on("keyup", () => {
set_propagate_selector_display();
});
}
composebox_typeahead.initialize_topic_edit_typeahead($message_edit_topic, message.stream, true);
}
function start_edit_maintaining_scroll($row, content) {
@@ -877,52 +739,25 @@ export function save_message_row_edit($row) {
let changed = false;
let edit_locally_echoed = false;
let content_changed = false;
let new_content;
const old_content = message.raw_content;
let topic_changed = false;
let new_topic;
const old_topic = message.topic;
let stream_changed = false;
let new_stream_id;
const old_stream_id = message.stream_id;
show_message_edit_spinner($row);
const $edit_content_input = $row.find(".message_edit_content");
const can_edit_content = $edit_content_input.attr("readonly") !== "readonly";
if (can_edit_content) {
new_content = $edit_content_input.val();
content_changed = old_content !== new_content;
changed = content_changed;
}
const $edit_topic_input = $row.find(".message_edit_topic");
const can_edit_topic = message.is_stream && $edit_topic_input.attr("readonly") !== "readonly";
if (can_edit_topic) {
new_topic = $edit_topic_input.val();
topic_changed = new_topic !== old_topic && new_topic.trim() !== "";
}
const can_edit_stream =
message.is_stream && settings_data.user_can_move_messages_between_streams();
if (can_edit_stream) {
const $edit_stream_input = $(`#id_select_move_stream_${message_id}`);
new_stream_id = Number.parseInt($edit_stream_input.data("value"), 10);
stream_changed = new_stream_id !== old_stream_id;
changed = old_content !== new_content;
}
// Editing a not-yet-acked message (because the original send attempt failed)
// just results in the in-memory message being changed
if (message.locally_echoed) {
if (new_content !== message.raw_content || topic_changed || stream_changed) {
if (new_content !== message.raw_content) {
// `edit_locally` handles the case where `new_topic/new_stream_id` is undefined
echo.edit_locally(message, {
raw_content: new_content,
new_topic,
new_stream_id,
});
$row = message_lists.current.get_row(message_id);
}
@@ -930,50 +765,17 @@ export function save_message_row_edit($row) {
return;
}
const request = {message_id: message.id};
if (topic_changed || stream_changed) {
const selected_topic_propagation =
$row.find("select.message_edit_topic_propagate").val() || "change_later";
const send_notification_to_old_thread = $row
.find(".send_notification_to_old_thread")
.is(":checked");
const send_notification_to_new_thread = $row
.find(".send_notification_to_new_thread")
.is(":checked");
request.propagate_mode = selected_topic_propagation;
request.send_notification_to_old_thread = send_notification_to_old_thread;
request.send_notification_to_new_thread = send_notification_to_new_thread;
notify_old_thread_default = send_notification_to_old_thread;
notify_new_thread_default = send_notification_to_new_thread;
changed = true;
}
if (topic_changed) {
request.topic = new_topic;
}
if (stream_changed) {
request.stream_id = new_stream_id;
}
if (content_changed) {
request.content = new_content;
}
if (!changed) {
// If they didn't change anything, just cancel it.
end_message_row_edit($row);
return;
}
if (
changed &&
!topic_changed &&
!stream_changed &&
!markdown.contains_backend_only_syntax(new_content)
) {
// If the topic isn't changed, and the new message content
// could have been locally echoed, than we can locally echo
// the edit.
const request = {message_id: message.id, content: new_content};
if (!markdown.contains_backend_only_syntax(new_content)) {
// If the new message content could have been locally echoed,
// than we can locally echo the edit.
currently_echoing_messages.set(message_id, {
raw_content: new_content,
orig_content: message.content,

35
static/templates/message_edit_form.hbs
View File

@@ -5,37 +5,6 @@
<span class="send-status-close">&times;</span>
<span class="error-msg"></span>
</div>
{{#if is_stream}}
<div class="edit-controls">
<div class="message_edit_header">
<div class="stream_header_colorblock" {{#unless is_stream_editable}}style="display:none"{{/unless}}></div>
<div class="select_stream_setting" {{#unless is_stream_editable}}style="display:none"{{/unless}}>
{{> settings/dropdown_list_widget
widget_name=select_move_stream_widget_name
list_placeholder=(t 'Filter streams')}}
</div>
<i class="fa fa-angle-right" aria-hidden="true" {{#unless is_stream_editable}}style="display:none"{{/unless}}></i>
<input type="text" placeholder="{{topic}}" value="{{topic}}" class="message_edit_topic" id="message_edit_topic" autocomplete="off" />
</div>
<select class='message_edit_topic_propagate' style='display:none;'>
<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>
</select>
<div class="message_edit_breadcrumb_messages" style='display:none;'>
<label class="checkbox">
<input class="send_notification_to_new_thread" name="send_notification_to_new_thread" type="checkbox" {{#if notify_new_thread}}checked="checked"{{/if}} />
<span></span>
{{t "Send automated notice to new topic" }}
</label>
<label class="checkbox">
<input class="send_notification_to_old_thread" name="send_notification_to_old_thread" type="checkbox" {{#if notify_old_thread}}checked="checked"{{/if}} />
<span></span>
{{t "Send automated notice to old topic" }}
</label>
</div>
</div>
{{/if}}
<div class="edit-controls">
{{> copy_message_button message_id=this.message_id}}
<textarea class="message_edit_content" maxlength="{{ max_message_length }}">{{content}}</textarea>
@@ -57,7 +26,7 @@
<div class="btn-wrapper inline-block">
<button type="button" class="button small rounded message_edit_cancel">{{t "Cancel" }}</button>
</div>
{{#if is_content_editable}}
{{#if is_editable}}
<div class="message-edit-feature-group">
{{> compose_control_buttons }}
</div>
@@ -65,7 +34,7 @@
{{else}}
<button type="button" class="button small rounded message_edit_close">{{t "Close" }}</button>
{{/if}}
{{#if is_content_editable}}
{{#if is_editable}}
<div class="message-edit-timer">
<span class="message_edit_countdown_timer"></span>
<span>