From 0fc19732bc57c04551d68d1bc62a526b04d3c2dc Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Sat, 1 Oct 2022 17:20:44 +0530 Subject: [PATCH] message_edit: Allow only content edit in message_edit_form. 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. --- frontend_tests/puppeteer_tests/edit.ts | 7 +- static/js/message_edit.js | 250 +++---------------------- static/templates/message_edit_form.hbs | 35 +--- 3 files changed, 31 insertions(+), 261 deletions(-) diff --git a/frontend_tests/puppeteer_tests/edit.ts b/frontend_tests/puppeteer_tests/edit.ts index 947b20e4f1..482d1019bd 100644 --- a/frontend_tests/puppeteer_tests/edit.ts +++ b/frontend_tests/puppeteer_tests/edit.ts @@ -20,10 +20,9 @@ async function trigger_edit_last_message(page: Page): Promise { await page.waitForSelector(".message_edit_content", {visible: true}); } -async function edit_stream_message(page: Page, topic: string, content: string): Promise { +async function edit_stream_message(page: Page, content: string): Promise { 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 { 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 { )} 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( diff --git a/static/js/message_edit.js b/static/js/message_edit.js index 7e66b15be7..9d37c4a917 100644 --- a/static/js/message_edit.js +++ b/static/js/message_edit.js @@ -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, diff --git a/static/templates/message_edit_form.hbs b/static/templates/message_edit_form.hbs index ff0abdc294..695c9e6de3 100644 --- a/static/templates/message_edit_form.hbs +++ b/static/templates/message_edit_form.hbs @@ -5,37 +5,6 @@ × - {{#if is_stream}} -
-
-
-
- {{> settings/dropdown_list_widget - widget_name=select_move_stream_widget_name - list_placeholder=(t 'Filter streams')}} -
- - -
- - -
- {{/if}}
{{> copy_message_button message_id=this.message_id}} @@ -57,7 +26,7 @@
- {{#if is_content_editable}} + {{#if is_editable}}
{{> compose_control_buttons }}
@@ -65,7 +34,7 @@ {{else}} {{/if}} - {{#if is_content_editable}} + {{#if is_editable}}