From 47d1703b67bf07a2fc0f08fa4ff8422c294f9e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 11 Mar 2024 17:35:50 +0100 Subject: [PATCH] FIX: code "block" detection before showing autocomplete (#26023) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **TL;DR:** Refactor autocomplete to use async markdown parsing for code block detection. Previously, the `inCodeBlock` function in `discourse/app/lib/utilities.js` used regular expressions to determine if a given position in the text was inside a code block. This approach had some limitations and could lead to incorrect behavior in certain edge cases. This commit refactors `inCodeBlock` to use a more robust algorithm that leverages Discourse's markdown parsing library. The new approach works as follows: 1. Check if the text contains any code block markers using a regular expression. If not, return `false` since the cursor can't be in a code block. 1. If potential code blocks exist, find a unique marker character that doesn't appear in the text. 1. Insert the unique marker character into the text at the cursor position. 1. Parse the modified text using Discourse's markdown parser, which converts the markdown into a tree of tokens. 1. Traverse the token tree to find the token that contains the unique marker character. 1. Check if the token's type is one of the types representing code blocks ("code_inline", "code_block", or "fence"). If so, return `true`, indicating that the cursor is inside a code block. Otherwise, return `false`. This algorithm provides a more accurate way to determine the cursor's position in relation to code blocks, accounting for the various ways code blocks can be represented in markdown. To accommodate this change, the autocomplete `triggerRule` option is now an async function. The autocomplete logic in `composer-editor.js`, `d-editor.js`, and `hashtag-autocomplete.js` has been updated to handle the async nature of `inCodeBlock`. Additionally, many of the tests have been refactored to handle async behavior. The test helpers now simulate typing and autocomplete selection in a more realistic, step-by-step manner. This should make the tests more robust and reflective of real-world usage. This is a significant refactor that touches multiple parts of the codebase, but it should lead to more accurate and reliable autocomplete behavior, especially when dealing with code blocks in the editor. > Written by an 🤖 LLM. Edited by a 🧑‍💻 human. --- .../app/components/composer-editor.js | 4 +- .../discourse/app/components/d-editor.js | 8 +- .../discourse/app/lib/autocomplete.js | 69 +++-- .../discourse/app/lib/hashtag-autocomplete.js | 11 +- .../discourse/app/lib/utilities.js | 62 ++-- .../composer-editor-mentions-test.js | 63 ++-- .../discourse/tests/acceptance/emoji-test.js | 21 +- .../acceptance/hashtag-autocomplete-test.js | 10 +- .../discourse/tests/helpers/component-test.js | 3 - .../discourse/tests/helpers/qunit-helpers.js | 33 ++- .../tests/unit/lib/autocomplete-test.js | 274 ++++++------------ .../tests/unit/lib/utilities-test.js | 34 +-- .../user-status-on-mentions-test.js | 4 +- 13 files changed, 239 insertions(+), 357 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index 9c27f80aaf9..544f7f87dda 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -241,8 +241,8 @@ export default Component.extend(ComposerUploadUppy, { key: "@", transformComplete: (v) => v.username || v.name, afterComplete: this._afterMentionComplete, - triggerRule: (textarea) => - !inCodeBlock(textarea.value, caretPosition(textarea)), + triggerRule: async (textarea) => + !(await inCodeBlock(textarea.value, caretPosition(textarea))), onClose: destroyUserStatuses, }); } diff --git a/app/assets/javascripts/discourse/app/components/d-editor.js b/app/assets/javascripts/discourse/app/components/d-editor.js index 87e5e150998..34b913810e0 100644 --- a/app/assets/javascripts/discourse/app/components/d-editor.js +++ b/app/assets/javascripts/discourse/app/components/d-editor.js @@ -532,10 +532,6 @@ export default Component.extend(TextareaTextManipulation, { }, onKeyUp: (text, cp) => { - if (inCodeBlock(text, cp)) { - return false; - } - const matches = /(?:^|[\s.\?,@\/#!%&*;:\[\]{}=\-_()])(:(?!:).?[\w-]*:?(?!:)(?:t\d?)?:?) ?$/gi.exec( text.substring(0, cp) @@ -639,8 +635,8 @@ export default Component.extend(TextareaTextManipulation, { }); }, - triggerRule: (textarea) => - !inCodeBlock(textarea.value, caretPosition(textarea)), + triggerRule: async (textarea) => + !(await inCodeBlock(textarea.value, caretPosition(textarea))), }); }, diff --git a/app/assets/javascripts/discourse/app/lib/autocomplete.js b/app/assets/javascripts/discourse/app/lib/autocomplete.js index 1cadf7d5761..43424f57929 100644 --- a/app/assets/javascripts/discourse/app/lib/autocomplete.js +++ b/app/assets/javascripts/discourse/app/lib/autocomplete.js @@ -17,8 +17,9 @@ import discourseLater from "discourse-common/lib/later"; export const SKIP = "skip"; export const CANCELLED_STATUS = "__CANCELLED"; -const allowedLettersRegex = /[\s\t\[\{\(\/]/; -let _autoCompletePopper; + +const ALLOWED_LETTERS_REGEXP = /[\s[{(/]/; +let _autoCompletePopper, _inputTimeout; const keys = { backSpace: 8, @@ -46,15 +47,13 @@ const keys = { z: 90, }; -let inputTimeout; - export default function (options) { if (this.length === 0) { return; } if (options === "destroy" || options.updateData) { - cancel(inputTimeout); + cancel(_inputTimeout); this[0].removeEventListener("keydown", handleKeyDown); this[0].removeEventListener("keyup", handleKeyUp); @@ -242,7 +241,7 @@ export default function (options) { // the time autocomplete was first displayed and the time of completion // Specifically this may happen due to uploads which inject a placeholder // which is later replaced with a different length string. - let pos = guessCompletePosition({ completeTerm: true }); + let pos = await guessCompletePosition({ completeTerm: true }); if ( pos.completeStart !== undefined && @@ -374,26 +373,31 @@ export default function (options) { } else { selectedOption = -1; } - ul.find("li").click(function ({ originalEvent }) { + ul.find("li").click(async function ({ originalEvent }) { + // this is required to prevent the default behaviour when clicking on a tag + originalEvent.preventDefault(); + selectedOption = ul.find("li").index(this); // hack for Gboard, see meta.discourse.org/t/-/187009/24 if (autocompleteOptions == null) { const opts = { ...options, _gboard_hack_force_lookup: true }; - const forcedAutocompleteOptions = dataSource(prevTerm, opts); - forcedAutocompleteOptions?.then((data) => { + const data = await dataSource(prevTerm, opts); + if (data) { updateAutoComplete(data); - completeTerm(autocompleteOptions[selectedOption], originalEvent); + await completeTerm( + autocompleteOptions[selectedOption], + originalEvent + ); if (!options.single) { me.focus(); } - }); + } } else { - completeTerm(autocompleteOptions[selectedOption], originalEvent); + await completeTerm(autocompleteOptions[selectedOption], originalEvent); if (!options.single) { me.focus(); } } - return false; }); if (options.appendSelector) { @@ -537,19 +541,20 @@ export default function (options) { closeAutocomplete(); }); - function checkTriggerRule(opts) { - return options.triggerRule ? options.triggerRule(me[0], opts) : true; + async function checkTriggerRule(opts) { + const shouldTrigger = await options.triggerRule?.(me[0], opts); + return shouldTrigger ?? true; } - function handleKeyUp(e) { + async function handleKeyUp(e) { if (options.debounced) { discourseDebounce(this, performAutocomplete, e, INPUT_DELAY); } else { - performAutocomplete(e); + await performAutocomplete(e); } } - function performAutocomplete(e) { + async function performAutocomplete(e) { if ([keys.esc, keys.enter].includes(e.which)) { return true; } @@ -572,9 +577,10 @@ export default function (options) { if (completeStart === null && cp > 0) { if (key === options.key) { let prevChar = me.val().charAt(cp - 2); + const shouldTrigger = await checkTriggerRule(); if ( - checkTriggerRule() && - (!prevChar || allowedLettersRegex.test(prevChar)) + shouldTrigger && + (!prevChar || ALLOWED_LETTERS_REGEXP.test(prevChar)) ) { completeStart = cp - 1; updateAutoComplete(dataSource("", options)); @@ -586,13 +592,12 @@ export default function (options) { } } - function guessCompletePosition(opts) { + async function guessCompletePosition(opts) { let prev, stopFound, term; let prevIsGood = true; let element = me[0]; - let backSpace = opts && opts.backSpace; - let completeTermOption = opts && opts.completeTerm; - + let backSpace = opts?.backSpace; + let completeTermOption = opts?.completeTerm; let caretPos = caretPosition(element); if (backSpace) { @@ -612,15 +617,15 @@ export default function (options) { if (stopFound) { prev = element.value[caretPos - 1]; + const shouldTrigger = await checkTriggerRule({ backSpace }); if ( - checkTriggerRule({ backSpace }) && - (prev === undefined || allowedLettersRegex.test(prev)) + shouldTrigger && + (prev === undefined || ALLOWED_LETTERS_REGEXP.test(prev)) ) { start = caretPos; term = element.value.substring(caretPos + 1, initialCaretPos); end = caretPos + term.length; - break; } } @@ -633,7 +638,7 @@ export default function (options) { return { completeStart: start, completeEnd: end, term }; } - function handleKeyDown(e) { + async function handleKeyDown(e) { let i, term, total, userToComplete; let cp; @@ -644,8 +649,8 @@ export default function (options) { if (options.allowAny) { // saves us wiring up a change event as well - cancel(inputTimeout); - inputTimeout = discourseLater(function () { + cancel(_inputTimeout); + _inputTimeout = discourseLater(() => { if (inputSelectedItems.length === 0) { inputSelectedItems.push(""); } @@ -669,7 +674,7 @@ export default function (options) { } if (completeStart === null && e.which === keys.backSpace && options.key) { - let position = guessCompletePosition({ backSpace: true }); + let position = await guessCompletePosition({ backSpace: true }); completeStart = position.completeStart; if (position.completeEnd) { @@ -716,7 +721,7 @@ export default function (options) { selectedOption >= 0 && (userToComplete = autocompleteOptions[selectedOption]) ) { - completeTerm(userToComplete, e); + await completeTerm(userToComplete, e); } else { // We're cancelling it, really. return true; diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js index f36ba31a83b..d37b140cf6d 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js @@ -140,12 +140,8 @@ export function setupHashtagAutocomplete( ); } -export function hashtagTriggerRule(textarea) { - if (inCodeBlock(textarea.value, caretPosition(textarea))) { - return false; - } - - return true; +export async function hashtagTriggerRule(textarea) { + return !(await inCodeBlock(textarea.value, caretPosition(textarea))); } function _setup( @@ -168,7 +164,8 @@ function _setup( } return _searchGeneric(term, siteSettings, contextualHashtagConfiguration); }, - triggerRule: (textarea, opts) => hashtagTriggerRule(textarea, opts), + triggerRule: async (textarea, opts) => + await hashtagTriggerRule(textarea, opts), }); } diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js index fc69a453740..d85a1a3ce17 100644 --- a/app/assets/javascripts/discourse/app/lib/utilities.js +++ b/app/assets/javascripts/discourse/app/lib/utilities.js @@ -1,5 +1,6 @@ import Handlebars from "handlebars"; import $ from "jquery"; +import { parseAsync } from "discourse/lib/text"; import toMarkdown from "discourse/lib/to-markdown"; import { capabilities } from "discourse/services/capabilities"; import * as AvatarUtils from "discourse-common/lib/avatar-utils"; @@ -421,34 +422,45 @@ export function postRNWebviewMessage(prop, value) { } } -const CODE_BLOCKS_REGEX = - /^( |\t).*|`[^`]+`|^```[^]*?^```|\[code\][^]*?\[\/code\]/gm; -//| ^ | ^ | ^ | ^ | -// | | | | -// | | | code blocks between [code] -// | | | -// | | +--- code blocks between three backticks -// | | -// | +----- inline code between backticks -// | -// +------- paragraphs starting with 2 spaces or tab - -const OPEN_CODE_BLOCKS_REGEX = /^( |\t).*|`[^`]+|^```[^]*?|\[code\][^]*?/gm; - -export function inCodeBlock(text, pos) { - let end = 0; - for (const match of text.matchAll(CODE_BLOCKS_REGEX)) { - end = match.index + match[0].length; - if (match.index <= pos && pos <= end) { - return true; +function pickMarker(text) { + // Uses the private use area (U+E000 to U+F8FF) to find a character that + // is not present in the text. This character will be used as a marker in + // place of the caret. + for (let code = 0xe000; code <= 0xf8ff; ++code) { + const char = String.fromCharCode(code); + if (!text.includes(char)) { + return char; } } + return null; +} - // Character at position `pos` can be in a code block that is unfinished. - // To check this case, we look for any open code blocks after the last closed - // code block. - const lastOpenBlock = text.slice(end).search(OPEN_CODE_BLOCKS_REGEX); - return lastOpenBlock !== -1 && pos >= end + lastOpenBlock; +function findToken(tokens, marker, level = 0) { + if (level > 50) { + return null; + } + const token = tokens.find((t) => (t.content ?? "").includes(marker)); + return token?.children ? findToken(token.children, marker, level + 1) : token; +} + +const CODE_MARKERS_REGEX = / |```|~~~|(? { - if (clock) { - clock.restore(); - } - }); + needs.hooks.afterEach(() => clock?.restore()); needs.pretender((server, helper) => { server.get("/u/search/users", () => { @@ -65,11 +61,12 @@ acceptance("Composer - editor mentions", function (needs) { await visit("/"); await click("#create-topic"); - await emulateAutocomplete(".d-editor-input", "abc @u"); - await click(".autocomplete.ac-user .selected"); + const editor = query(".d-editor-input"); + + await simulateKeys(editor, "abc @u\r"); assert.strictEqual( - query(".d-editor-input").value, + editor.value, "abc @user ", "should replace mention correctly" ); @@ -78,21 +75,13 @@ acceptance("Composer - editor mentions", function (needs) { test("selecting user mentions after deleting characters", async function (assert) { await visit("/"); await click("#create-topic"); - await fillIn(".d-editor-input", "abc @user a"); - // Emulate user typing `@` and `u` in the editor - await triggerKeyEvent(".d-editor-input", "keydown", "Backspace"); - await fillIn(".d-editor-input", "abc @user "); - await triggerKeyEvent(".d-editor-input", "keyup", "Backspace"); + const editor = query(".d-editor-input"); - await triggerKeyEvent(".d-editor-input", "keydown", "Backspace"); - await fillIn(".d-editor-input", "abc @user"); - await triggerKeyEvent(".d-editor-input", "keyup", "Backspace"); - - await click(".autocomplete.ac-user .selected"); + await simulateKeys(editor, "abc @user a\b\b\r"); assert.strictEqual( - query(".d-editor-input").value, + editor.value, "abc @user ", "should replace mention correctly" ); @@ -102,25 +91,14 @@ acceptance("Composer - editor mentions", function (needs) { await visit("/"); await click("#create-topic"); - // Emulate user pressing backspace in the editor const editor = query(".d-editor-input"); - await fillIn(".d-editor-input", "abc @user 123"); + + await simulateKeys(editor, "abc @user 123"); await setCaretPosition(editor, 9); - - await triggerKeyEvent(".d-editor-input", "keydown", "Backspace"); - await fillIn(".d-editor-input", "abc @use 123"); - await triggerKeyEvent(".d-editor-input", "keyup", "Backspace"); - await setCaretPosition(editor, 8); - - await triggerKeyEvent(".d-editor-input", "keydown", "Backspace"); - await fillIn(".d-editor-input", "abc @us 123"); - await triggerKeyEvent(".d-editor-input", "keyup", "Backspace"); - await setCaretPosition(editor, 7); - - await click(".autocomplete.ac-user .selected"); + await simulateKeys(editor, "\b\b\r"); assert.strictEqual( - query(".d-editor-input").value, + editor.value, "abc @user 123", "should replace mention correctly" ); @@ -134,12 +112,15 @@ acceptance("Composer - editor mentions", function (needs) { await visit("/"); await click("#create-topic"); - await emulateAutocomplete(".d-editor-input", "@u"); + const editor = query(".d-editor-input"); + + await simulateKeys(editor, "@u"); assert.ok( exists(`.autocomplete .emoji[alt='${status.emoji}']`), "status emoji is shown" ); + assert.equal( query( ".autocomplete .user-status-message-description" @@ -153,14 +134,16 @@ acceptance("Composer - editor mentions", function (needs) { await visit("/"); await click("#create-topic"); - await emulateAutocomplete(".d-editor-input", "abc @u"); + const editor = query(".d-editor-input"); + + await simulateKeys(editor, "abc @u"); assert.deepEqual( [...queryAll(".ac-user .username")].map((e) => e.innerText), ["user", "user2", "user_group", "foo"] ); - await emulateAutocomplete(".d-editor-input", "abc @f"); + await simulateKeys(editor, "\bf"); assert.deepEqual( [...queryAll(".ac-user .username")].map((e) => e.innerText), diff --git a/app/assets/javascripts/discourse/tests/acceptance/emoji-test.js b/app/assets/javascripts/discourse/tests/acceptance/emoji-test.js index 3daeffd43ef..0addc91713b 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/emoji-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/emoji-test.js @@ -1,4 +1,4 @@ -import { click, fillIn, triggerKeyEvent, visit } from "@ember/test-helpers"; +import { click, visit } from "@ember/test-helpers"; import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; import { test } from "qunit"; import { @@ -6,6 +6,8 @@ import { exists, normalizeHtml, query, + simulateKey, + simulateKeys, visible, } from "discourse/tests/helpers/qunit-helpers"; @@ -16,12 +18,13 @@ acceptance("Emoji", function (needs) { await visit("/t/internationalization-localization/280"); await click("#topic-footer-buttons .btn.create"); - await fillIn(".d-editor-input", "this is an emoji :blonde_woman:"); + await simulateKeys(query(".d-editor-input"), "a :blonde_wo\t"); + assert.ok(visible(".d-editor-preview")); assert.strictEqual( normalizeHtml(query(".d-editor-preview").innerHTML.trim()), normalizeHtml( - `

this is an emoji :blonde_woman:

` + `

a :blonde_woman:

` ) ); }); @@ -30,13 +33,13 @@ acceptance("Emoji", function (needs) { await visit("/t/internationalization-localization/280"); await click("#topic-footer-buttons .btn.create"); - await fillIn(".d-editor-input", "this is an emoji :blonde_woman:t5:"); + await simulateKeys(query(".d-editor-input"), "a :blonde_woman:t5:"); assert.ok(visible(".d-editor-preview")); assert.strictEqual( normalizeHtml(query(".d-editor-preview").innerHTML.trim()), normalizeHtml( - `

this is an emoji :blonde_woman:t5:

` + `

a :blonde_woman:t5:

` ) ); }); @@ -49,13 +52,13 @@ acceptance("Emoji", function (needs) { await visit("/t/internationalization-localization/280"); await click("#topic-footer-buttons .btn.create"); - await fillIn(".d-editor-input", ":s"); - await triggerKeyEvent(".d-editor-input", "keyup", "ArrowDown"); // ensures a keyup is triggered + const editor = query(".d-editor-input"); + + await simulateKeys(editor, ":s"); assert.notOk(exists(".autocomplete.ac-emoji")); - await fillIn(".d-editor-input", ":sw"); - await triggerKeyEvent(".d-editor-input", "keyup", "ArrowDown"); // ensures a keyup is triggered + await simulateKey(editor, "w"); assert.ok(exists(".autocomplete.ac-emoji")); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/hashtag-autocomplete-test.js b/app/assets/javascripts/discourse/tests/acceptance/hashtag-autocomplete-test.js index 42268677be2..88c2b46b454 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/hashtag-autocomplete-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/hashtag-autocomplete-test.js @@ -2,14 +2,13 @@ import { click, visit } from "@ember/test-helpers"; import { test } from "qunit"; import { acceptance, - emulateAutocomplete, + query, + simulateKeys, } from "discourse/tests/helpers/qunit-helpers"; acceptance("#hashtag autocompletion in composer", function (needs) { needs.user(); - needs.settings({ - tagging_enabled: true, - }); + needs.settings({ tagging_enabled: true }); needs.pretender((server, helper) => { server.get("/hashtags", () => { return helper.response({ @@ -56,8 +55,7 @@ acceptance("#hashtag autocompletion in composer", function (needs) { test(":emoji: unescape in autocomplete search results", async function (assert) { await visit("/t/internationalization-localization/280"); await click("#topic-footer-buttons .btn.create"); - - await emulateAutocomplete(".d-editor-input", "abc #o"); + await simulateKeys(query(".d-editor-input"), "abc #o"); assert.dom(".hashtag-autocomplete__option").exists({ count: 3 }); assert diff --git a/app/assets/javascripts/discourse/tests/helpers/component-test.js b/app/assets/javascripts/discourse/tests/helpers/component-test.js index 77a88f45d76..98899f28cda 100644 --- a/app/assets/javascripts/discourse/tests/helpers/component-test.js +++ b/app/assets/javascripts/discourse/tests/helpers/component-test.js @@ -1,6 +1,5 @@ import { render } from "@ember/test-helpers"; import { setupRenderingTest as emberSetupRenderingTest } from "ember-qunit"; -import $ from "jquery"; import QUnit, { test } from "qunit"; import { autoLoadModules } from "discourse/instance-initializers/auto-load-modules"; import { AUTO_GROUPS } from "discourse/lib/constants"; @@ -48,8 +47,6 @@ export function setupRenderingTest(hooks) { autoLoadModules(this.owner, this.registry); this.owner.lookup("service:store"); - - $.fn.autocomplete = function () {}; }); } diff --git a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js index 35788564694..6876ef53a4c 100644 --- a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js +++ b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js @@ -1,9 +1,9 @@ import { run } from "@ember/runloop"; import { - fillIn, getApplication, settled, triggerKeyEvent, + typeIn, } from "@ember/test-helpers"; import { isEmpty } from "@ember/utils"; import { setupApplicationTest } from "ember-qunit"; @@ -610,14 +610,31 @@ export async function paste(element, text, otherClipboardData = {}) { return e; } -export async function emulateAutocomplete(inputSelector, text) { - await triggerKeyEvent(inputSelector, "keydown", "Backspace"); - await fillIn(inputSelector, `${text} `); - await triggerKeyEvent(inputSelector, "keyup", "Backspace"); +export async function simulateKey(element, key) { + if (key === "\b") { + await triggerKeyEvent(element, "keydown", "Backspace"); - await triggerKeyEvent(inputSelector, "keydown", "Backspace"); - await fillIn(inputSelector, text); - await triggerKeyEvent(inputSelector, "keyup", "Backspace"); + const pos = element.selectionStart; + element.value = element.value.slice(0, pos - 1) + element.value.slice(pos); + element.selectionStart = pos - 1; + element.selectionEnd = pos - 1; + + await triggerKeyEvent(element, "keyup", "Backspace"); + } else if (key === "\t") { + await triggerKeyEvent(element, "keydown", "Tab"); + await triggerKeyEvent(element, "keyup", "Tab"); + } else if (key === "\r") { + await triggerKeyEvent(element, "keydown", "Enter"); + await triggerKeyEvent(element, "keyup", "Enter"); + } else { + await typeIn(element, key); + } +} + +export async function simulateKeys(element, keys) { + for (let key of keys) { + await simulateKey(element, key); + } } // The order of attributes can vary in different browsers. When comparing diff --git a/app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js b/app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js index 5b1f2b7c938..96e674f6792 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js @@ -2,261 +2,153 @@ import { setupTest } from "ember-qunit"; import { compile } from "handlebars"; import $ from "jquery"; import { module, test } from "qunit"; -import autocomplete from "discourse/lib/autocomplete"; +import { setCaretPosition } from "discourse/lib/utilities"; +import { + simulateKey, + simulateKeys, +} from "discourse/tests/helpers/qunit-helpers"; module("Unit | Utility | autocomplete", function (hooks) { setupTest(hooks); - let elements = []; - function textArea(value) { - let element = document.createElement("TEXTAREA"); - element.value = value; - document.getElementById("ember-testing").appendChild(element); - elements.push(element); - return element; + let _element; + + const template = compile( + ` +
+ +
+ `.trim() + ); + + function textArea() { + _element = document.createElement("TEXTAREA"); + document.getElementById("ember-testing").appendChild(_element); + return _element; } - function cleanup() { - elements.forEach((e) => { - e.remove(); - autocomplete.call($(e), { cancel: true }); - autocomplete.call($(e), "destroy"); - }); - elements = []; - } - - hooks.afterEach(function () { - cleanup(); + hooks.afterEach(() => { + if (!_element) { + return; + } + const $e = $(_element); + $e.autocomplete({ cancel: true }); + $e.autocomplete("destroy"); + _element.remove(); }); - function simulateKey(element, key) { - let keyCode = key.charCodeAt(0); - - let bubbled = false; - let trackBubble = function () { - bubbled = true; - }; - - element.addEventListener("keydown", trackBubble); - - let keyboardEvent = new KeyboardEvent("keydown", { - key, - keyCode, - which: keyCode, - }); - - element.dispatchEvent(keyboardEvent); - - element.removeEventListener("keydown", trackBubble); - - if (bubbled) { - let pos = element.selectionStart; - let value = element.value; - // backspace - if (key === "\b") { - element.value = value.slice(0, pos - 1) + value.slice(pos); - element.selectionStart = pos - 1; - element.selectionEnd = pos - 1; - } else { - element.value = value.slice(0, pos) + key + value.slice(pos); - element.selectionStart = pos + 1; - element.selectionEnd = pos + 1; - } - } - - element.dispatchEvent( - new KeyboardEvent("keyup", { key, keyCode, which: keyCode }) - ); - } - test("Autocomplete can complete really short terms correctly", async function (assert) { - let element = textArea(""); - let $element = $(element); + const element = textArea(); - autocomplete.call($element, { + $(element).autocomplete({ key: ":", - transformComplete: () => "sad:", + template, + transformComplete: (e) => e.slice(1), dataSource: () => [":sad:"], - template: compile(`
- -
`), }); - simulateKey(element, "a"); - simulateKey(element, " "); + await simulateKeys(element, "a :)\r"); - simulateKey(element, ":"); - simulateKey(element, ")"); - simulateKey(element, "\r"); - - let sleep = (millisecs) => - new Promise((promise) => setTimeout(promise, millisecs)); - // completeTerm awaits transformComplete - // we need to wait for it to be done - // Note: this is somewhat questionable given that when people - // press ENTER on an autocomplete they do not want to be beholden - // to an async function. - let inputEquals = async function (value) { - let count = 3000; - while (count > 0 && element.value !== value) { - count -= 1; - await sleep(1); - } - }; - - await inputEquals("a :sad: "); assert.strictEqual(element.value, "a :sad: "); assert.strictEqual(element.selectionStart, 8); assert.strictEqual(element.selectionEnd, 8); }); - test("Autocomplete can account for cursor drift correctly", function (assert) { - let element = textArea(""); - let $element = $(element); + test("Autocomplete can account for cursor drift correctly", async function (assert) { + const element = textArea(); + const db = ["test1", "test2"]; - autocomplete.call($element, { + $(element).autocomplete({ key: "@", - dataSource: (term) => - ["test1", "test2"].filter((word) => word.includes(term)), - template: compile(`
- -
`), + template, + dataSource: (term) => db.filter((word) => word.includes(term)), }); - simulateKey(element, "@"); - simulateKey(element, "\r"); + await simulateKeys(element, "@\r"); assert.strictEqual(element.value, "@test1 "); assert.strictEqual(element.selectionStart, 7); assert.strictEqual(element.selectionEnd, 7); - simulateKey(element, "@"); - simulateKey(element, "2"); - simulateKey(element, "\r"); + await simulateKeys(element, "@2\r"); assert.strictEqual(element.value, "@test1 @test2 "); assert.strictEqual(element.selectionStart, 14); assert.strictEqual(element.selectionEnd, 14); - element.selectionStart = 6; - element.selectionEnd = 6; + await setCaretPosition(element, 6); + await simulateKeys(element, "\b\b"); - simulateKey(element, "\b"); - simulateKey(element, "\b"); - simulateKey(element, "\r"); + assert.strictEqual(element.value, "@tes @test2 "); + + await simulateKey(element, "\r"); assert.strictEqual(element.value, "@test1 @test2 "); assert.strictEqual(element.selectionStart, 7); assert.strictEqual(element.selectionEnd, 7); - // lets see that deleting last space triggers autocomplete - element.selectionStart = element.value.length; - element.selectionEnd = element.value.length; - simulateKey(element, "\b"); - let list = document.querySelectorAll("#ac-testing ul li"); - assert.strictEqual(list.length, 1); + // ensures that deleting last space triggers autocomplete + await setCaretPosition(element, element.value.length); + await simulateKey(element, "\b"); - simulateKey(element, "\b"); - list = document.querySelectorAll("#ac-testing ul li"); - assert.strictEqual(list.length, 2); + assert.dom("#ac-testing ul li").exists({ count: 1 }); + + await simulateKey(element, "\b"); + + assert.dom("#ac-testing ul li").exists({ count: 2 }); // close autocomplete - simulateKey(element, "\r"); + await simulateKey(element, "\r"); // does not trigger by mistake at the start element.value = "test"; - element.selectionStart = element.value.length; - element.selectionEnd = element.value.length; - simulateKey(element, "\b"); - list = document.querySelectorAll("#ac-testing ul li"); - assert.strictEqual(list.length, 0); + await setCaretPosition(element, element.value.length); + await simulateKey(element, "\b"); + + assert.dom("#ac-testing ul li").exists({ count: 0 }); }); - test("Autocomplete can handle spaces", function (assert) { - let element = textArea(""); - let $element = $(element); + test("Autocomplete can handle spaces", async function (assert) { + const element = textArea(); + const db = [ + { username: "jd", name: "jane dale" }, + { username: "jb", name: "jack black" }, + ]; - autocomplete.call($element, { + $(element).autocomplete({ key: "@", + template, dataSource: (term) => - [ - { username: "jd", name: "jane dale" }, - { username: "jb", name: "jack black" }, - ] - .filter((user) => { - return user.username.includes(term) || user.name.includes(term); - }) + db + .filter( + (user) => user.username.includes(term) || user.name.includes(term) + ) .map((user) => user.username), - template: compile(`
- -
`), }); - simulateKey(element, "@"); - simulateKey(element, "j"); - simulateKey(element, "a"); - simulateKey(element, "n"); - simulateKey(element, "e"); - simulateKey(element, " "); - simulateKey(element, "d"); - simulateKey(element, "\r"); + await simulateKeys(element, "@jane d\r"); assert.strictEqual(element.value, "@jd "); }); - test("Autocomplete can render on @", function (assert) { - let element = textArea("@"); - let $element = $(element); + test("Autocomplete can render on @", async function (assert) { + const element = textArea(); - autocomplete.call($element, { + $(element).autocomplete({ key: "@", + template, dataSource: () => ["test1", "test2"], - template: compile(`
- -
`), }); - element.dispatchEvent(new KeyboardEvent("keydown", { key: "@" })); - element.dispatchEvent(new KeyboardEvent("keyup", { key: "@" })); + await simulateKey(element, "@"); - let list = document.querySelectorAll("#ac-testing ul li"); - assert.strictEqual(list.length, 2); - - let selected = document.querySelectorAll("#ac-testing ul li a.selected"); - assert.strictEqual(selected.length, 1); - assert.strictEqual(selected[0].innerText, "test1"); + assert.dom("#ac-testing ul li").exists({ count: 2 }); + assert.dom("#ac-testing li a.selected").exists({ count: 1 }); + assert.dom("#ac-testing li a.selected").hasText("test1"); }); }); diff --git a/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js b/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js index 4019f393d80..bd65cdc2b04 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js @@ -219,34 +219,16 @@ module("Unit | Utilities", function (hooks) { ); }); - test("inCodeBlock", function (assert) { - const texts = [ - // CLOSED CODE BLOCKS: - "000\n\n 111\n\n000", - "000 `111` 000", - "000\n```\n111\n```\n000", - "000\n[code]111[/code]\n000", - // OPEN CODE BLOCKS: - "000\n\n 111", - "000 `111", - "000\n```\n111", - "000\n[code]111", - // COMPLEX TEST: - "000\n\n```\n111\n```\n\n000\n\n`111 111`\n\n000\n\n[code]\n111\n[/code]\n\n 111\n\t111\n\n000`111", - // INDENTED OPEN CODE BLOCKS: - // - Using tab - "000\n\t```111\n\t111\n\t111```\n000", - // - Using spaces - `000\n \`\`\`111\n 111\n 111\`\`\`\n000`, - ]; + test("inCodeBlock", async function (assert) { + const text = + "000\n\n```\n111\n```\n\n000\n\n`111 111`\n\n000\n\n[code]\n111\n[/code]\n\n 111\n\t111\n\n000`000"; - texts.forEach((text) => { - for (let i = 0; i < text.length; ++i) { - if (text[i] === "0" || text[i] === "1") { - assert.strictEqual(inCodeBlock(text, i), text[i] === "1"); - } + for (let i = 0; i < text.length; ++i) { + if (text[i] === "0" || text[i] === "1") { + let inCode = await inCodeBlock(text, i); + assert.strictEqual(inCode, text[i] === "1"); } - }); + } }); test("mergeSortedLists", function (assert) { diff --git a/plugins/chat/test/javascripts/acceptance/user-status-on-mentions-test.js b/plugins/chat/test/javascripts/acceptance/user-status-on-mentions-test.js index b3156fd7969..257af7c073c 100644 --- a/plugins/chat/test/javascripts/acceptance/user-status-on-mentions-test.js +++ b/plugins/chat/test/javascripts/acceptance/user-status-on-mentions-test.js @@ -3,10 +3,10 @@ import { skip, test } from "qunit"; import pretender, { response } from "discourse/tests/helpers/create-pretender"; import { acceptance, - emulateAutocomplete, loggedInUser, publishToMessageBus, query, + simulateKeys, } from "discourse/tests/helpers/qunit-helpers"; acceptance("Chat | User status on mentions", function (needs) { @@ -321,7 +321,7 @@ acceptance("Chat | User status on mentions", function (needs) { } async function typeWithAutocompleteAndSend(text) { - await emulateAutocomplete(".chat-composer__input", text); + await simulateKeys(query(".chat-composer__input"), text); await click(".autocomplete.ac-user .selected"); await click(".chat-composer-button.-send"); }