compose-box: Insert quoted content at the cursor position.

Right now, on clicking `quote and reply` on any message, the quoted
message is always inserted at the top of compose-box irrespective of
the current cursor position. Also, after insertion of the quoted
message, the cursor is shifted at the end of the compose box.

This commit changes this behaviour to insertion of quoted message
at the current cursor position with a newline at the end of quote
and moving the cursor position to that newline after insertion.
A newline is added at the beginning of quoted message only if there
was some content already present in compose box before the previous
cursor position.

Tested on Google Chrome and Firefox browsers on Ubuntu dev environment.

Fixes: #16836
This commit is contained in:
Priyansh Garg 2020-12-31 17:23:54 +05:30 committed by Tim Abbott
parent 604f6c491c
commit 3294e8dbe0
3 changed files with 85 additions and 20 deletions

View File

@ -369,9 +369,7 @@ test("quote_and_reply", ({override}) => {
reply_type: "personal", reply_type: "personal",
}; };
$("#compose-textarea").caret = (pos) => { $("#compose-textarea").caret = noop;
assert.equal(pos, 0);
};
replaced = false; replaced = false;
expected_replacement = expected_replacement =

View File

@ -291,13 +291,13 @@ run_test("quote_and_reply", ({override}) => {
}; };
$("#compose-textarea").caret = function (arg) { $("#compose-textarea").caret = function (arg) {
if (arg === 0) {
textarea_caret_pos = 0;
return this;
}
if (arg === undefined) { if (arg === undefined) {
return textarea_caret_pos; return textarea_caret_pos;
} }
if (typeof arg === "number") {
textarea_caret_pos = arg;
return this;
}
if (typeof arg !== "string") { if (typeof arg !== "string") {
console.info(arg); console.info(arg);
throw new Error("We expected the actual code to pass in a string."); throw new Error("We expected the actual code to pass in a string.");
@ -327,15 +327,82 @@ run_test("quote_and_reply", ({override}) => {
return content; return content;
} }
function reset_test_state() {
// Reset `raw_content` property of `selected_message`.
delete selected_message.raw_content;
// Reset compose-box state.
textarea_val = "";
textarea_caret_pos = 0;
$("#compose-textarea").trigger("blur");
}
set_compose_content_with_caret("hello %there"); // "%" is used to encode/display position of focus before change set_compose_content_with_caret("hello %there"); // "%" is used to encode/display position of focus before change
compose_actions.quote_and_reply(); compose_actions.quote_and_reply();
assert.equal(get_compose_content_with_caret(), "[Quoting…]\n%hello there"); assert.equal(get_compose_content_with_caret(), "hello \n[Quoting…]\n%there");
success_function({ success_function({
raw_content: "Testing caret position", raw_content: "Testing caret position",
}); });
assert.equal( assert.equal(
get_compose_content_with_caret(), get_compose_content_with_caret(),
"translated: @_**Steve Stephenson|90** [said](https://chat.zulip.org/#narrow/stream/92-learning/topic/Tornado):\n```quote\nTesting caret position\n```\nhello there%", "hello \ntranslated: @_**Steve Stephenson|90** [said](https://chat.zulip.org/#narrow/stream/92-learning/topic/Tornado):\n```quote\nTesting caret position\n```\n%there",
);
reset_test_state();
// If the caret is initially positioned at 0, it should not
// add a newline before the quoted message.
set_compose_content_with_caret("%hello there");
compose_actions.quote_and_reply();
assert.equal(get_compose_content_with_caret(), "[Quoting…]\n%hello there");
success_function({
raw_content: "Testing with caret initially positioned at 0.",
});
assert.equal(
get_compose_content_with_caret(),
"translated: @_**Steve Stephenson|90** [said](https://chat.zulip.org/#narrow/stream/92-learning/topic/Tornado):\n```quote\nTesting with caret initially positioned at 0.\n```\n%hello there",
);
override(compose_actions, "respond_to_message", () => {
// Reset compose state to replicate the re-opening of compose-box.
textarea_val = "";
textarea_caret_pos = 0;
$("#compose-textarea").trigger("focus");
});
reset_test_state();
// If the compose-box is close, or open with no content while
// quoting a message, the quoted message should be placed
// at the beginning of compose-box.
compose_actions.quote_and_reply();
assert.equal(get_compose_content_with_caret(), "[Quoting…]\n%");
success_function({
raw_content: "Testing with compose-box closed initially.",
});
assert.equal(
get_compose_content_with_caret(),
"translated: @_**Steve Stephenson|90** [said](https://chat.zulip.org/#narrow/stream/92-learning/topic/Tornado):\n```quote\nTesting with compose-box closed initially.\n```\n%",
);
reset_test_state();
// If the compose-box is already open while quoting a message,
// but contains content like `\n\n \n` (only whitespaces and
// newlines), the compose-box should re-open and thus the quoted
// message should start from the beginning of compose-box.
set_compose_content_with_caret(" \n\n \n %");
compose_actions.quote_and_reply();
assert.equal(get_compose_content_with_caret(), "[Quoting…]\n%");
success_function({
raw_content: "Testing with compose-box containing whitespaces and newlines only.",
});
assert.equal(
get_compose_content_with_caret(),
"translated: @_**Steve Stephenson|90** [said](https://chat.zulip.org/#narrow/stream/92-learning/topic/Tornado):\n```quote\nTesting with compose-box containing whitespaces and newlines only.\n```\n%",
); );
}); });

View File

@ -458,21 +458,19 @@ export function quote_and_reply(opts) {
// are prone to glitches where you select the // are prone to glitches where you select the
// text, plus it's a complicated codepath that // text, plus it's a complicated codepath that
// can have other unintended consequences.) // can have other unintended consequences.)
//
// Note also that we always put the quoted text if (textarea.caret() !== 0) {
// above the current text, which explains us // Insert a newline before quoted message if there is
// moving the caret below. I think this is what // already some content in the compose box and quoted
// most users will want, and it's consistent with // message is not being inserted at the beginning.
// the behavior we had on FF before this change textarea.caret("\n");
// (which may have been an accident of }
// implementation). If we change this decision,
// we'll need to make `insert_syntax_and_focus`
// smarter about newlines.
textarea.caret(0);
} else { } else {
respond_to_message(opts); respond_to_message(opts);
} }
const prev_caret = textarea.caret();
compose_ui.insert_syntax_and_focus("[Quoting…]\n", textarea); compose_ui.insert_syntax_and_focus("[Quoting…]\n", textarea);
function replace_content(message) { function replace_content(message) {
@ -493,6 +491,8 @@ export function quote_and_reply(opts) {
content += `${fence}quote\n${message.raw_content}\n${fence}`; content += `${fence}quote\n${message.raw_content}\n${fence}`;
compose_ui.replace_syntax("[Quoting…]", content, textarea); compose_ui.replace_syntax("[Quoting…]", content, textarea);
compose_ui.autosize_textarea($("#compose-textarea")); compose_ui.autosize_textarea($("#compose-textarea"));
// Update textarea caret to point to the new line after quoted message.
textarea.caret(prev_caret + content.length + 1);
} }
if (message && message.raw_content) { if (message && message.raw_content) {