paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-04 14:03:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

message_edit: Add message edit local echo.

Browse Source 
Updates the message editing process to do a local 'echo'.

On slow connections, now there is visual confirmation of the edit,
similar to when sending messages.  The contains_backend_only_syntax
logic and check are the same as there.

We showing "(SAVING)" until the edit is completed, and on successful
edit, the word "(EDITED)" appears.  There's likely useful future work
to do on making the animation experience nicer.

Substantially rewritten by tabbott to better handle corner cases and
communicate more clearly about what's happening.

Fixes: #3530.
This commit is contained in:
Jack Tiggleman
2019-04-22 14:13:23 -04:00
committed by Tim Abbott
parent f0fd812cc5
commit 1682d75ea8
7 changed files with 148 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
docs/subsystems/sending-messages.md
View File

@@ -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

6
frontend_tests/casper_tests/08-edit.js
View File

@@ -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');
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");

37
static/js/echo.js
View File

@@ -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;
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) {

74
static/js/message_edit.js
View File

@@ -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();
}

3
static/js/message_events.js
View File

@@ -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);

15
static/js/message_list_view.js
View File

@@ -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;
}
},

6
static/templates/edited_notice.hbs
View File

@@ -1,3 +1,9 @@
{{#if msg/local_edit_timestamp}}
<div class="message_edit_notice auto-select" title="{{#tr this}}Edited (__last_edit_timestr__){{/tr}}">
({{t "SAVING" }})
</div>
{{else}}
<div class="message_edit_notice auto-select" title="{{#tr this}}Edited (__last_edit_timestr__){{/tr}}">
({{t "EDITED" }})
</div>
{{/if}}