Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DEV: Remove jquery from textarea-manipulation, improve undo handling (#17050) 

This commit removes many uses of `this._$textarea`, and also switches us to use `document.execCommand("insertText")` for the majority of manipulations. This means that the browser undo history will be preserved when doing things like pasting rich html, using bold/italic shortcuts, etc.

These manipulations are already extensively tested. This commit extends a few of the tests to verify the undo behavior.

There are still a few cases (e.g. replacing upload placeholders with true URLs) where we don't necessarily want to bring the composer into focus. In those cases, the old history-breaking behavior remains for now.
This commit is contained in:
David Taylor 2022-06-10 01:42:50 +01:00 committed by GitHub
parent c054a47d9a
commit 75d9c16156
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 111 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
app/assets/javascripts/discourse/app/mixins/textarea-text-manipulation.js
View File

@ -112,13 +112,12 @@ export default Mixin.create({
this._textarea.selectionStart = from;
this._textarea.selectionEnd = from + length;
this._$textarea.trigger("change");
if (opts.scroll) {
const oldScrollPos = this._$textarea.scrollTop();
const oldScrollPos = this._textarea.scrollTop;
if (!this.capabilities.isIOS) {
this._$textarea.focus();
this._textarea.focus();
}
this._$textarea.scrollTop(oldScrollPos);
this._textarea.scrollTop = oldScrollPos;
}
});
},
@ -178,7 +177,7 @@ export default Mixin.create({
const [hval, hlen] = getHead(head);
const example = I18n.t(`composer.${exampleKey}`);
this.set("value", `${pre}${hval}${example}${tail}${post}`);
this._insertAt(sel.start, sel.end, `${hval}${example}${tail}`);
this.selectText(pre.length + hlen, example.length);
} else if (opts && !opts.multiline) {
let [hval, hlen] = getHead(head);
@ -190,13 +189,11 @@ export default Mixin.create({
}
if (pre.slice(-hlen) === hval && post.slice(0, tail.length) === tail) {
this.set(
"value",
`${pre.slice(0, -hlen)}${sel.value}${post.slice(tail.length)}`
);
// Already wrapped in the surround. Remove it.
this._insertAt(sel.start - hlen, sel.end + tail.length, sel.value);
this.selectText(sel.start - hlen, sel.value.length);
} else {
this.set("value", `${pre}${hval}${sel.value}${tail}${post}`);
this._insertAt(sel.start, sel.end, `${hval}${sel.value}${tail}`);
this.selectText(sel.start + hlen, sel.value.length);
}
} else {
@ -208,10 +205,8 @@ export default Mixin.create({
pre.slice(-tlen) === tail &&
post.slice(0, hlen) === hval
) {
this.set(
"value",
`${pre.slice(0, -hlen)}${sel.value}${post.slice(tlen)}`
);
// Already wrapped in the surround. Remove it.
this._insertAt(sel.start - hlen, sel.end + tlen, sel.value);
this.selectText(sel.start - hlen, sel.value.length);
} else {
const contents = this._getMultilineContents(
@ -224,7 +219,7 @@ export default Mixin.create({
opts
);
this.set("value", `${pre}${contents}${post}`);
this._insertAt(sel.start, sel.end, contents);
if (lines.length === 1 && tlen > 0) {
this.selectText(sel.start + hlen, sel.value.length);
} else {
@ -280,27 +275,31 @@ export default Mixin.create({
return;
}
let pre = sel.pre;
let post = sel.value + sel.post;
let start = sel.start;
let end = sel.end;
if (pre.length > 0) {
pre = pre.replace(/\n*$/, "\n\n");
const newLinesBeforeSelection = sel.pre?.match(/\n*$/)?.[0]?.length;
if (newLinesBeforeSelection) {
start -= newLinesBeforeSelection;
}
if (post.length > 0) {
post = post.replace(/^\n*/, "\n\n");
if (sel.pre.length > 0) {
text = `\n\n${text}`;
}
const newLinesAfterSelection = sel.post?.match(/^\n*/)?.[0]?.length;
if (newLinesAfterSelection) {
end += newLinesAfterSelection;
}
if (sel.post.length > 0) {
text = `${text}\n\n`;
} else {
post = "\n";
text = `${text}\n`;
}
const value = pre + text + post;
this.set("value", value);
this._$textarea.val(value);
this._$textarea.prop("selectionStart", (pre + text).length + 2);
this._$textarea.prop("selectionEnd", (pre + text).length + 2);
this._insertAt(start, end, text);
this._textarea.setSelectionRange(start + text.length, start + text.length);
schedule("afterRender", this, this.focusTextArea);
},
@ -318,16 +317,16 @@ export default Mixin.create({
}
}
const insert = `${sel.pre}${text}`;
const value = `${insert}${sel.post}`;
this.set("value", value);
this._$textarea.val(value);
this._$textarea.prop("selectionStart", insert.length);
this._$textarea.prop("selectionEnd", insert.length);
next(() => this._$textarea.trigger("change"));
this._insertAt(sel.start, sel.end, text);
this.focusTextArea();
},
_insertAt(start, end, text) {
this._textarea.setSelectionRange(start, end);
this._textarea.focus();
document.execCommand("insertText", false, text);
},
extractTable(text) {
if (text.endsWith("\n")) {
text = text.substring(0, text.length - 1);

76
app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js
View File

@ -12,6 +12,7 @@ import { click, fillIn, settled, visit } from "@ember/test-helpers";
import I18n from "I18n";
import { skip, test } from "qunit";
import { Promise } from "rsvp";
import { isLegacyEmber } from "discourse-common/config/environment";
function pretender(server, helper) {
server.post("/uploads/lookup-urls", () => {
@ -73,10 +74,12 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
});
appEvents.on("composer:upload-started", () => {
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n"
);
if (!isLegacyEmber()) {
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n"
);
}
});
const image = createFile("avatar.png");
@ -149,11 +152,13 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
uploadStarted++;
if (uploadStarted === 2) {
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n[Uploading: avatar2.png...]()\n",
"it should show the upload placeholders when the upload starts"
);
if (!isLegacyEmber()) {
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n[Uploading: avatar2.png...]()\n",
"it should show the upload placeholders when the upload starts"
);
}
}
});
appEvents.on("composer:uploads-cancelled", () => {
@ -181,10 +186,13 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
const done = assert.async();
appEvents.on("composer:upload-started", () => {
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n"
);
if (!isLegacyEmber()) {
// Event handling is different in legacy - the text hasn't been inserted when this event fires
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n"
);
}
});
appEvents.on("composer:all-uploads-complete", () => {
@ -211,10 +219,13 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
const done = assert.async();
appEvents.on("composer:upload-started", () => {
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n Text after the image."
);
if (!isLegacyEmber()) {
// Event handling is different in legacy - the text hasn't been inserted when this event fires
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n Text after the image."
);
}
});
appEvents.on("composer:all-uploads-complete", () => {
@ -244,10 +255,13 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
const done = assert.async();
appEvents.on("composer:upload-started", () => {
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n Text after the image."
);
if (!isLegacyEmber()) {
// Event handling is different in legacy - the text hasn't been inserted when this event fires
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n Text after the image."
);
}
});
appEvents.on("composer:all-uploads-complete", () => {
@ -269,10 +283,12 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
const done = assert.async();
appEvents.on("composer:upload-started", () => {
assert.strictEqual(
query(".d-editor-input").value,
"[Uploading: avatar.png...]()\n"
);
if (!isLegacyEmber()) {
assert.strictEqual(
query(".d-editor-input").value,
"[Uploading: avatar.png...]()\n"
);
}
});
appEvents.on("composer:all-uploads-complete", () => {
@ -295,10 +311,12 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
const done = assert.async();
appEvents.on("composer:upload-started", () => {
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n"
);
if (!isLegacyEmber()) {
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n[Uploading: avatar.png...]()\n"
);
}
});
appEvents.on("composer:all-uploads-complete", () => {

28
app/assets/javascripts/discourse/tests/integration/components/d-editor-test.js
View File

@ -172,6 +172,22 @@ discourseModule("Integration | Component | d-editor", function (hooks) {
assert.strictEqual(textarea.selectionEnd, 11);
});
testCase(
"bold button maintains undo history",
async function (assert, textarea) {
textarea.selectionStart = 6;
textarea.selectionEnd = 11;
await click("button.bold");
assert.strictEqual(this.value, "hello **world**.");
assert.strictEqual(textarea.selectionStart, 8);
assert.strictEqual(textarea.selectionEnd, 13);
document.execCommand("undo");
assert.strictEqual(this.value, "hello world.");
}
);
testCase(
`bold with a multiline selection`,
async function (assert, textarea) {
@ -472,6 +488,9 @@ third line`
assert.strictEqual(textarea.selectionStart, 27);
assert.strictEqual(textarea.selectionEnd, 27);
document.execCommand("undo");
assert.strictEqual(this.value, "first line\nsecond line\nthird line");
},
});
@ -509,6 +528,9 @@ third line`
await click("button.blockquote");
assert.strictEqual(this.value, "one\n\n\n> \n> two");
document.execCommand("undo");
assert.strictEqual(this.value, "one\n\n\n\ntwo");
},
});
@ -834,6 +856,9 @@ third line`
let element = query(".d-editor");
await paste(element, "\ta\tb\n1\t2\t3");
assert.strictEqual(this.value, "||a|b|\n|---|---|---|\n|1|2|3|\n");
document.execCommand("undo");
assert.strictEqual(this.value, "");
},
});
@ -863,6 +888,9 @@ third line`
"See [discourse](https://www.discourse.org/) in action"
);
assert.strictEqual(event.defaultPrevented, true);
document.execCommand("undo");
assert.strictEqual(this.value, "See discourse in action");
}
);