From 627634a76ea6520dfdc393f775e10efd34fa91f0 Mon Sep 17 00:00:00 2001 From: Beowulf Date: Fri, 17 Jan 2025 18:22:43 +0100 Subject: [PATCH 1/2] Prevent prefix continuation if currently a text expander popup is open This fixes that mentions and emoji autocompletion was broken in e.g. a list, because the list handling take presidency over the text expansion. (cherry picked from commit 276ef10dd5a1167a8bcc20599197f89ff7f9b8a4) --- tests/e2e/markdown-editor.test.e2e.ts | 26 +++++++++++++++++++ .../js/features/comp/ComboMarkdownEditor.js | 2 ++ 2 files changed, 28 insertions(+) diff --git a/tests/e2e/markdown-editor.test.e2e.ts b/tests/e2e/markdown-editor.test.e2e.ts index 762113d563..1e30b8d3b9 100644 --- a/tests/e2e/markdown-editor.test.e2e.ts +++ b/tests/e2e/markdown-editor.test.e2e.ts @@ -224,3 +224,29 @@ test('markdown insert table', async ({page}) => { await expect(textarea).toHaveValue('| Header | Header |\n|---------|---------|\n| Content | Content |\n| Content | Content |\n| Content | Content |\n'); await save_visual(page); }); + +test('text expander has higher prio then prefix continuation', async ({page}) => { + const response = await page.goto('/user2/repo1/issues/new'); + expect(response?.status()).toBe(200); + + const textarea = page.locator('textarea[name=content]'); + const initText = `* first`; + await textarea.fill(initText); + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.indexOf('rst'), it.value.indexOf('rst'))); + await textarea.press('End'); + + // Test emoji completion + await textarea.press('Enter'); + await textarea.pressSequentially(':smile_c'); + await textarea.press('Enter'); + await expect(textarea).toHaveValue(`* first\n* 😸`); + + // Test username completion + await textarea.press('Enter'); + await textarea.pressSequentially('@user'); + await textarea.press('Enter'); + await expect(textarea).toHaveValue(`* first\n* 😸\n* @user2 `); + + await textarea.press('Enter'); + await expect(textarea).toHaveValue(`* first\n* 😸\n* @user2 \n* `); +}); diff --git a/web_src/js/features/comp/ComboMarkdownEditor.js b/web_src/js/features/comp/ComboMarkdownEditor.js index 8ae5defa47..707101190c 100644 --- a/web_src/js/features/comp/ComboMarkdownEditor.js +++ b/web_src/js/features/comp/ComboMarkdownEditor.js @@ -99,6 +99,8 @@ class ComboMarkdownEditor { e.target._shiftDown = true; } if (e.key === 'Enter' && !e.shiftKey && !e.ctrlKey && !e.altKey) { + // Prevent special line break handling if currently a text expander popup is open + if (this.textarea.hasAttribute('aria-expanded')) return; if (!this.breakLine()) return; // Nothing changed, let the default handler work. this.options?.onContentChanged?.(this, e); e.preventDefault(); From 348e0e1face01e952b59bdefc28ac48fdcf07d19 Mon Sep 17 00:00:00 2001 From: Beowulf Date: Fri, 17 Jan 2025 18:42:42 +0100 Subject: [PATCH 2/2] Leave list/quote expanison with double enter When editing a list or similar syntax elements, pressing enter starts a new line with the line introducer (e.g. `- ` for a plain list). But currently it's uncomfortable when someone wants to leave the list. Pressing enter again simply adds more and more lines with the prefix. With this change the list is terminated if enter is pressed on a line which contains the introducer but nothing else. This behavior is known from other markdown editors like the on used by GitLab or GitHub. Additionally I changed the regex for detecting a prefix. - Why: With the change you can add a single whitespace at the end if you want to keep an "empty" line. So if you want to write: ``` - First - - Third ``` You just need to add a whitespace in the second line to prevent that the prefix will be removed. - Changes in detail: - ordered bullet list prefix detection: nothing changed - todo list and unordered list prefix detection: have been split up: - todo list: Changed that only 1 to 4 whitespaces can be between the list char (`-`,`*`,`+`) and the checkbox (`[ ]`,`[x]`) - Why? If more then 4 spaces are between the list char and the checkbox, this is no longer detected as a prefix for a todo item based on the markdown standard. Due to the amount of spaces it is instead parsed as code. - unordered list: The prefix now needs to have exactly one space after the list char (`-`,`*`,`+`). More spaces will not be taken into account for detecting the prefix. - quote prefix detection: nothing changed The current e2e-tests where simplified and duplicated tests where removed. Test cases for the new functionality where added. (cherry picked from commit 7ea62c5ce475db81f2930a569c98190a52e6cae6) --- tests/e2e/markdown-editor.test.e2e.ts | 51 +++++++++---------- .../js/features/comp/ComboMarkdownEditor.js | 22 ++++++-- 2 files changed, 43 insertions(+), 30 deletions(-) diff --git a/tests/e2e/markdown-editor.test.e2e.ts b/tests/e2e/markdown-editor.test.e2e.ts index 1e30b8d3b9..35e9de2ea6 100644 --- a/tests/e2e/markdown-editor.test.e2e.ts +++ b/tests/e2e/markdown-editor.test.e2e.ts @@ -109,7 +109,7 @@ test('markdown indentation', async ({page}) => { }); test('markdown list continuation', async ({page}) => { - const initText = `* first\n* second\n* third\n* last`; + const initText = `* first\n* second`; const response = await page.goto('/user2/repo1/issues/new'); expect(response?.status()).toBe(200); @@ -119,25 +119,20 @@ test('markdown list continuation', async ({page}) => { const indent = page.locator('button[data-md-action="indent"]'); await textarea.fill(initText); - // Test continuation of '* ' prefix - await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.indexOf('cond'), it.value.indexOf('cond'))); + // Test continuation of ' * ' prefix + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.indexOf('rst'), it.value.indexOf('rst'))); + await indent.click(); await textarea.press('End'); await textarea.press('Enter'); - await textarea.pressSequentially('middle'); - await expect(textarea).toHaveValue(`* first\n* second\n* middle\n* third\n* last`); - - // Test continuation of ' * ' prefix - await indent.click(); - await textarea.press('Enter'); await textarea.pressSequentially('muddle'); - await expect(textarea).toHaveValue(`* first\n* second\n${tab}* middle\n${tab}* muddle\n* third\n* last`); + await expect(textarea).toHaveValue(`${tab}* first\n${tab}* muddle\n* second`); // Test breaking in the middle of a line await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.lastIndexOf('ddle'), it.value.lastIndexOf('ddle'))); await textarea.pressSequentially('tate'); await textarea.press('Enter'); await textarea.pressSequentially('me'); - await expect(textarea).toHaveValue(`* first\n* second\n${tab}* middle\n${tab}* mutate\n${tab}* meddle\n* third\n* last`); + await expect(textarea).toHaveValue(`${tab}* first\n${tab}* mutate\n${tab}* meddle\n* second`); // Test not triggering when Shift held await textarea.fill(initText); @@ -145,35 +140,36 @@ test('markdown list continuation', async ({page}) => { await textarea.press('Shift+Enter'); await textarea.press('Enter'); await textarea.pressSequentially('...but not least'); - await expect(textarea).toHaveValue(`* first\n* second\n* third\n* last\n\n...but not least`); + await expect(textarea).toHaveValue(`* first\n* second\n\n...but not least`); // Test continuation of ordered list - await textarea.fill(`1. one\n2. two`); + await textarea.fill(`1. one`); await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.length, it.value.length)); await textarea.press('Enter'); + await textarea.pressSequentially(' '); + await textarea.press('Enter'); await textarea.pressSequentially('three'); - await expect(textarea).toHaveValue(`1. one\n2. two\n3. three`); + await textarea.press('Enter'); + await textarea.press('Enter'); + await expect(textarea).toHaveValue(`1. one\n2. \n3. three\n\n`); // Test continuation of alternative ordered list syntax - await textarea.fill(`1) one\n2) two`); + await textarea.fill(`1) one`); await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.length, it.value.length)); await textarea.press('Enter'); + await textarea.pressSequentially(' '); + await textarea.press('Enter'); await textarea.pressSequentially('three'); - await expect(textarea).toHaveValue(`1) one\n2) two\n3) three`); - - // Test continuation of blockquote - await textarea.fill(`> knowledge is power`); - await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.length, it.value.length)); await textarea.press('Enter'); - await textarea.pressSequentially('france is bacon'); - await expect(textarea).toHaveValue(`> knowledge is power\n> france is bacon`); + await textarea.press('Enter'); + await expect(textarea).toHaveValue(`1) one\n2) \n3) three\n\n`); // Test continuation of checklists - await textarea.fill(`- [ ] have a problem\n- [x] create a solution`); + await textarea.fill(`- [ ]have a problem\n- [x]create a solution`); await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.length, it.value.length)); await textarea.press('Enter'); await textarea.pressSequentially('write a test'); - await expect(textarea).toHaveValue(`- [ ] have a problem\n- [x] create a solution\n- [ ] write a test`); + await expect(textarea).toHaveValue(`- [ ]have a problem\n- [x]create a solution\n- [ ]write a test`); // Test all conceivable syntax (except ordered lists) const prefixes = [ @@ -189,7 +185,6 @@ test('markdown list continuation', async ({page}) => { '> ', '> > ', '- [ ] ', - '- [ ]', // This does seem to render, so allow. '* [ ] ', '+ [ ] ', ]; @@ -197,8 +192,12 @@ test('markdown list continuation', async ({page}) => { await textarea.fill(`${prefix}one`); await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.length, it.value.length)); await textarea.press('Enter'); + await textarea.pressSequentially(' '); + await textarea.press('Enter'); await textarea.pressSequentially('two'); - await expect(textarea).toHaveValue(`${prefix}one\n${prefix}two`); + await textarea.press('Enter'); + await textarea.press('Enter'); + await expect(textarea).toHaveValue(`${prefix}one\n${prefix} \n${prefix}two\n\n`); } }); diff --git a/web_src/js/features/comp/ComboMarkdownEditor.js b/web_src/js/features/comp/ComboMarkdownEditor.js index 707101190c..89a252f6f3 100644 --- a/web_src/js/features/comp/ComboMarkdownEditor.js +++ b/web_src/js/features/comp/ComboMarkdownEditor.js @@ -409,13 +409,27 @@ class ComboMarkdownEditor { // Find the beginning of the current line. const lineStart = Math.max(0, value.lastIndexOf('\n', start - 1) + 1); // Find the end and extract the line. - const lineEnd = value.indexOf('\n', start); - const line = value.slice(lineStart, lineEnd === -1 ? value.length : lineEnd); + const nextLF = value.indexOf('\n', start); + const lineEnd = nextLF === -1 ? value.length : nextLF; + const line = value.slice(lineStart, lineEnd); // Match any whitespace at the start + any repeatable prefix + exactly one space after. - const prefix = line.match(/^\s*((\d+)[.)]\s|[-*+]\s+(\[[ x]\]\s?)?|(>\s+)+)?/); + const prefix = line.match(/^\s*((\d+)[.)]\s|[-*+]\s{1,4}\[[ x]\]\s?|[-*+]\s|(>\s?)+)?/); // Defer to browser if we can't do anything more useful, or if the cursor is inside the prefix. - if (!prefix || !prefix[0].length || lineStart + prefix[0].length > start) return false; + if (!prefix) return false; + const prefixLength = prefix[0].length; + if (!prefixLength || lineStart + prefixLength > start) return false; + // If the prefix is just indentation (which should always be an even number of spaces or tabs), check if a single whitespace is added to the end of the line. + // If this is the case do not leave the indentation and continue with the prefix. + if ((prefixLength % 2 === 1 && /^ +$/.test(prefix[0])) || /^\t+ $/.test(prefix[0])) { + prefix[0] = prefix[0].slice(0, prefixLength - 1); + } else if (prefixLength === lineEnd - lineStart) { + this.textarea.setSelectionRange(lineStart, lineEnd); + if (!document.execCommand('insertText', false, '\n')) { + this.textarea.setRangeText('\n'); + } + return true; + } // Insert newline + prefix. let text = `\n${prefix[0]}`;