diff --git a/docs/subsystems/sending-messages.md b/docs/subsystems/sending-messages.md index 5fed53b512..089571ea9b 100644 --- a/docs/subsystems/sending-messages.md +++ b/docs/subsystems/sending-messages.md @@ -190,6 +190,15 @@ messages. primarily done in `exports.process_from_server` in `static/js/echo.js`. +### Local echo in message editing + +Zulip also supports local echo in the message editing code path for +edits to just the content of a message. The approach is analogous +(using `markdown.contains_backend_only_syntax`, etc.)), except we +don't need any of the `local_id` tracking logic, because the message +already has a permanent message id; as a result, the whole +implementation was under 150 lines of code. + ## Putting it all together This section just has a brief review of the sequence of steps all in diff --git a/frontend_tests/casper_tests/08-edit.js b/frontend_tests/casper_tests/08-edit.js index 438d3c412c..2cc7957b04 100644 --- a/frontend_tests/casper_tests/08-edit.js +++ b/frontend_tests/casper_tests/08-edit.js @@ -1,5 +1,8 @@ var common = require('../casper_lib/common.js'); +casper.options.verbose = true; +casper.options.logLevel = "debug"; + common.start_and_log_in(); function then_edit_last_message() { @@ -100,8 +103,9 @@ casper.then(function () { // 37 is left arrow key code casper.then(function () { casper.test.assertNotVisible('form.message_edit_form', 'Message edit box not visible'); - - common.keypress(37); + casper.waitUntilVisible(".message_edit_notice", function () { + common.keypress(37); + }); casper.waitUntilVisible(".message_edit_content", function () { var fieldVal = common.get_form_field_value('.message_edit_content'); casper.test.assertEquals(fieldVal, "test edited pm", "Opened editing last own message"); diff --git a/static/js/echo.js b/static/js/echo.js index 511c818378..91190b5ada 100644 --- a/static/js/echo.js +++ b/static/js/echo.js @@ -135,6 +135,14 @@ exports.try_deliver_locally = function try_deliver_locally(message_request) { }; exports.edit_locally = function edit_locally(message, request) { + // Responsible for doing the rendering work of locally editing the + // content ofa message. This is used in several code paths: + // * Editing a message where a message was locally echoed but + // it got an error back from the server + // * Locally echoing any content-only edits to fully sent messages + // * Restoring the original content should the server return an + // error after having locally echoed content-only messages. + // The details of what should be changed are encoded in the request. const raw_content = request.raw_content; const message_content_edited = raw_content !== undefined && message.raw_content !== raw_content; @@ -156,10 +164,37 @@ exports.edit_locally = function edit_locally(message, request) { if (message_content_edited) { message.raw_content = raw_content; - markdown.apply_markdown(message); + if (request.content !== undefined) { + // This happens in the code path where message editing + // failed and we're trying to undo the local echo. We use + // the saved content and flags rather than rendering; this + // is important in case + // markdown.contains_backend_only_syntax(message) is true. + message.content = request.content; + message.mentioned = request.mentioned; + message.mentioned_me_directly = request.mentioned_me_directly; + message.alerted = request.alerted; + } else { + // Otherwise, we markdown-render the message; this resets + // all flags, so we need to restore those flags that are + // properties of how the user has interacted with the + // message, and not its rendering. + markdown.apply_markdown(message); + if (request.starred !== undefined) { + message.starred = request.starred; + } + if (request.historical !== undefined) { + message.historical = request.historical; + } + if (request.collapsed !== undefined) { + message.collapsed = request.collapsed; + } + } } - // We don't handle unread counts since local messages must be sent by us + // We don't have logic to adjust unread counts, because message + // reaching this code path must either have been sent by us or the + // topic isn't being edited, so unread counts can't have changed. home_msg_list.view.rerender_messages([message]); if (current_msg_list === message_list.narrowed) { diff --git a/static/js/message_edit.js b/static/js/message_edit.js index 89fdaf683d..a6253d734c 100644 --- a/static/js/message_edit.js +++ b/static/js/message_edit.js @@ -4,6 +4,7 @@ var render_topic_edit_form = require('../templates/topic_edit_form.hbs'); var currently_editing_messages = {}; var currently_deleting_messages = []; +var currently_echoing_messages = {}; var editability_types = { NO: 1, @@ -77,6 +78,10 @@ function get_editability(message, edit_limit_seconds_buffer) { return editability_types.FULL; } + if (currently_echoing_messages[message.id]) { + return editability_types.NO; + } + var now = new XDate(); if (page_params.realm_message_content_edit_limit_seconds + edit_limit_seconds_buffer + now.diffSeconds(message.timestamp * 1000) > 0 && message.sent_by_me) { @@ -461,6 +466,7 @@ exports.save = function (row, from_topic_edited_only) { } var message = current_msg_list.get(message_id); var changed = false; + var edit_locally_echoed = false; var new_content = row.find(".message_edit_content").val(); var topic_changed = false; @@ -508,15 +514,83 @@ exports.save = function (row, from_topic_edited_only) { exports.end(row); return; } + + if (changed && !topic_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. + currently_echoing_messages[message_id] = { + raw_content: new_content, + orig_content: message.content, + orig_raw_content: message.raw_content, + + // Store flags that are about user interaction with the + // message so that echo.edit_locally() can restore these + // flags. + starred: message.starred, + historical: message.historical, + collapsed: message.collapsed, + + // These flags are rendering artifacts we'll want if the + // edit fails and we need to revert to the original + // rendering of the message. + alerted: message.alerted, + mentioned: message.mentioned, + mentioned_me_directly: message.mentioned, + }; + edit_locally_echoed = true; + + // Settings these attributes causes a "SAVING" notice to + // briefly appear where "EDITED" would normally appear until + // the message is acknowledged by the server. + message.local_edit_timestamp = Math.round(new Date().getTime() / 1000); + + echo.edit_locally(message, currently_echoing_messages[message_id]); + + row = current_msg_list.get_row(message_id); + message_edit.end(row); + } + channel.patch({ url: '/json/messages/' + message.id, data: request, success: function () { var spinner = row.find(".topic_edit_spinner"); loading.destroy_indicator(spinner); + + if (edit_locally_echoed) { + delete message.local_edit_timestamp; + delete currently_echoing_messages[message_id]; + } }, error: function (xhr) { if (msg_list === current_msg_list) { + message_id = rows.id(row); + + if (edit_locally_echoed) { + var echoed_message = message_store.get(message_id); + var echo_data = currently_echoing_messages[message_id]; + + delete echoed_message.local_edit_timestamp; + delete currently_echoing_messages[message_id]; + + // Restore the original content. + echo.edit_locally(echoed_message, { + content: echo_data.orig_content, + raw_content: echo_data.orig_raw_content, + mentioned: echo_data.mentioned, + mentioned_me_directly: echo_data.mentioned_me_directly, + alerted: echo_data.alerted, + }); + + row = current_msg_list.get_row(message_id); + if (!message_edit.is_editing(message_id)) { + // Return to the message editing open UI state. + start_edit_maintaining_scroll(row, echo_data.orig_raw_content); + } + } + var message = channel.xhr_error_message(i18n.t("Error saving edit"), xhr); row.find(".edit_error").text(message).show(); } diff --git a/static/js/message_events.js b/static/js/message_events.js index 0dc88ff2a1..53f6cca0b6 100644 --- a/static/js/message_events.js +++ b/static/js/message_events.js @@ -110,6 +110,9 @@ exports.update_messages = function update_messages(events) { if (msg === undefined) { return; } + + delete msg.local_edit_timestamp; + msgs_to_rerender.push(msg); message_store.update_booleans(msg, event.flags); diff --git a/static/js/message_list_view.js b/static/js/message_list_view.js index cbd4e16e94..67a9342bc8 100644 --- a/static/js/message_list_view.js +++ b/static/js/message_list_view.js @@ -195,8 +195,14 @@ MessageListView.prototype = { _RENDER_THRESHOLD: 50, _get_msg_timestring: function (message_container) { - if (message_container.msg.last_edit_timestamp !== undefined) { - const last_edit_time = new XDate(message_container.msg.last_edit_timestamp * 1000); + let last_edit_timestamp; + if (message_container.msg.local_edit_timestamp !== undefined) { + last_edit_timestamp = message_container.msg.local_edit_timestamp; + } else { + last_edit_timestamp = message_container.msg.last_edit_timestamp; + } + if (last_edit_timestamp !== undefined) { + const last_edit_time = new XDate(last_edit_timestamp * 1000); const today = new XDate(); return timerender.render_date(last_edit_time, undefined, today)[0].textContent + " at " + timerender.stringify_time(last_edit_time); @@ -219,6 +225,11 @@ MessageListView.prototype = { message_container.edited_in_left_col = !include_sender; message_container.edited_alongside_sender = include_sender && !status_message; message_container.edited_status_msg = include_sender && status_message; + } else { + delete message_container.last_edit_timestr; + message_container.edited_in_left_col = false; + message_container.edited_alongside_sender = false; + message_container.edited_status_msg = false; } }, diff --git a/static/templates/edited_notice.hbs b/static/templates/edited_notice.hbs index 4a2319aafc..054b8888ce 100644 --- a/static/templates/edited_notice.hbs +++ b/static/templates/edited_notice.hbs @@ -1,3 +1,9 @@ +{{#if msg/local_edit_timestamp}} +
+ ({{t "SAVING" }}) +
+{{else}}
({{t "EDITED" }})
+{{/if}}