Remove tab key handling in markdown editor, add toolbar buttons instead, re #4072 #4142 (#4263)

We haven't decided much (to my knowledge), and I've been using the main branch in production (as one does) and found out even I myself rely on Tab sometimes working to move focus and have been caught off guard by it indenting lines instead.

So this removes Tab handling and instead adds two new buttons to the toolbar. The indentation logic is unchanged (other than now focusing the textarea during button handling, to ensure execCommand works, and thus undo history is preserved).

I'm not sure which terminology to use in tooltips. Could also add keyboard shortcuts for the whole toolbar eventually, but as is this is hopefully an better solution to the problems I previously created than un-merging the whole thing :)

<img width="414" alt="Screenshot with two new buttons" src="/attachments/b7af3aa4-a195-48d1-be0a-1559f25dce8e">

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4263
Reviewed-by: Otto <otto@codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Danko Aleksejevs <danko@very.lv>
Co-committed-by: Danko Aleksejevs <danko@very.lv>

options/locale/locale_en-US.ini

@@ -213,6 +213,8 @@ buttons.ref.tooltip = Reference an issue or pull request
 buttons.switch_to_legacy.tooltip = Use the legacy editor instead
 buttons.enable_monospace_font = Enable monospace font
 buttons.disable_monospace_font = Disable monospace font
+buttons.indent.tooltip = Nest items by one level
+buttons.unindent.tooltip = Unnest items by one level
 
 [filter]
 string.asc = A - Z

release-notes/8.0.0/feat/4072.md

@@ -1,7 +1,5 @@
-Added handling of Tab and Enter keys to the new Markdown editor, in line with what standalone text editors usually do. This is mostly focused on quickly writing and rearranging lists.
-
-- Pressing Tab prepending 4 spaces to the line under cursor, or all the lines in the selection.
-- Pressing Shift+Tab removes up to 4 spaces.
-- Pressing Enter repeats any indentation and a "repeatable" prefix (list or blockquote) from the current line.
-- To avoid interfering with keyboard navigation, the Tab presses are only handled once there has been some other interaction with the element after focusing.
-- Pressing Escape removes focus from the editor and resumes default Tab navigation.
+- Added Enter key handling to the new Markdown editor ([#4072](https://codeberg.org/forgejo/forgejo/pulls/4072)):
+  - Pressing Enter while in a list, quote or code block will copy the prefix to the new line.
+  - Ordered list index will be increased for the new line, and task list "checkbox" will be unchecked.
+- Added indent/unindent function for a line or selection.
+  - Currently available as toolbar buttons ([#4263](https://codeberg.org/forgejo/forgejo/pulls/4263))

templates/shared/combomarkdowneditor.tmpl

@@ -35,6 +35,8 @@ Template Attributes:
 				<md-unordered-list class="markdown-toolbar-button" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.list.unordered.tooltip"}}">{{svg "octicon-list-unordered"}}</md-unordered-list>
 				<md-ordered-list class="markdown-toolbar-button" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.list.ordered.tooltip"}}">{{svg "octicon-list-ordered"}}</md-ordered-list>
 				<md-task-list class="markdown-toolbar-button" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.list.task.tooltip"}}">{{svg "octicon-tasklist"}}</md-task-list>
+				<button type="button" class="markdown-toolbar-button" data-md-button data-md-action="unindent" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.unindent.tooltip"}}">{{svg "octicon-arrow-left"}}</button>
+				<button type="button" class="markdown-toolbar-button" data-md-button data-md-action="indent" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.indent.tooltip"}}">{{svg "octicon-arrow-right"}}</button>
 			</div>
 			<div class="markdown-toolbar-group">
 				<md-mention class="markdown-toolbar-button" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.mention.tooltip"}}">{{svg "octicon-mention"}}</md-mention>

tests/e2e/markdown-editor.test.e2e.js

@@ -17,66 +17,65 @@ test('Test markdown indentation', async ({browser}, workerInfo) => {
 
   const textarea = page.locator('textarea[name=content]');
   const tab = '    ';
+  const indent = page.locator('button[data-md-action="indent"]');
+  const unindent = page.locator('button[data-md-action="unindent"]');
   await textarea.fill(initText);
   await textarea.click(); // Tab handling is disabled until pointer event or input.
 
   // Indent, then unindent first line
+  await textarea.focus();
   await textarea.evaluate((it) => it.setSelectionRange(0, 0));
-  await textarea.press('Tab');
+  await indent.click();
   await expect(textarea).toHaveValue(`${tab}* first\n* second\n* third\n* last`);
-  await textarea.press('Shift+Tab');
+  await unindent.click();
   await expect(textarea).toHaveValue(initText);
 
   // Indent second line while somewhere inside of it
+  await textarea.focus();
   await textarea.press('ArrowDown');
   await textarea.press('ArrowRight');
   await textarea.press('ArrowRight');
-  await textarea.press('Tab');
+  await indent.click();
   await expect(textarea).toHaveValue(`* first\n${tab}* second\n* third\n* last`);
 
   // Subsequently, select a chunk of 2nd and 3rd line and indent both, preserving the cursor position in relation to text
+  await textarea.focus();
   await textarea.evaluate((it) => it.setSelectionRange(it.value.indexOf('cond'), it.value.indexOf('hird')));
-  await textarea.press('Tab');
+  await indent.click();
   const lines23 = `* first\n${tab}${tab}* second\n${tab}* third\n* last`;
   await expect(textarea).toHaveValue(lines23);
   await expect(textarea).toHaveJSProperty('selectionStart', lines23.indexOf('cond'));
   await expect(textarea).toHaveJSProperty('selectionEnd', lines23.indexOf('hird'));
 
   // Then unindent twice, erasing all indents.
-  await textarea.press('Shift+Tab');
+  await unindent.click();
   await expect(textarea).toHaveValue(`* first\n${tab}* second\n* third\n* last`);
-  await textarea.press('Shift+Tab');
+  await unindent.click();
   await expect(textarea).toHaveValue(initText);
 
   // Indent and unindent with cursor at the end of the line
+  await textarea.focus();
   await textarea.evaluate((it) => it.setSelectionRange(it.value.indexOf('cond'), it.value.indexOf('cond')));
   await textarea.press('End');
-  await textarea.press('Tab');
+  await indent.click();
   await expect(textarea).toHaveValue(`* first\n${tab}* second\n* third\n* last`);
-  await textarea.press('Shift+Tab');
+  await unindent.click();
   await expect(textarea).toHaveValue(initText);
 
-  // Ensure textarea is blurred on Esc, and does not intercept Tab before input
-  await textarea.press('Escape');
-  await expect(textarea).not.toBeFocused();
-  await textarea.focus();
-  await textarea.press('Tab');
-  await expect(textarea).toHaveValue(initText);
-  await expect(textarea).not.toBeFocused(); // because tab worked as normal
-
   // Check that Tab does work after input
   await textarea.focus();
   await textarea.evaluate((it) => it.setSelectionRange(it.value.length, it.value.length));
   await textarea.press('Shift+Enter'); // Avoid triggering the prefix continuation feature
   await textarea.pressSequentially('* least');
-  await textarea.press('Tab');
+  await indent.click();
   await expect(textarea).toHaveValue(`* first\n* second\n* third\n* last\n${tab}* least`);
 
   // Check that partial indents are cleared
+  await textarea.focus();
   await textarea.fill(initText);
   await textarea.evaluate((it) => it.setSelectionRange(it.value.indexOf('* second'), it.value.indexOf('* second')));
   await textarea.pressSequentially('  ');
-  await textarea.press('Shift+Tab');
+  await unindent.click();
   await expect(textarea).toHaveValue(initText);
 });
 
@@ -91,6 +90,7 @@ test('Test markdown list continuation', async ({browser}, workerInfo) => {
 
   const textarea = page.locator('textarea[name=content]');
   const tab = '    ';
+  const indent = page.locator('button[data-md-action="indent"]');
   await textarea.fill(initText);
 
   // Test continuation of '* ' prefix
@@ -101,7 +101,7 @@ test('Test markdown list continuation', async ({browser}, workerInfo) => {
   await expect(textarea).toHaveValue(`* first\n* second\n* middle\n* third\n* last`);
 
   // Test continuation of '    * ' prefix
-  await textarea.press('Tab');
+  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`);

web_src/css/editor/combomarkdowneditor.css

@@ -27,6 +27,7 @@
   padding: 5px;
   cursor: pointer;
   color: var(--color-text);
+  line-height: 20px;
 }
 
 .combo-markdown-editor .markdown-toolbar-button:hover {

web_src/js/features/comp/ComboMarkdownEditor.js

@@ -83,36 +83,21 @@ class ComboMarkdownEditor {
       // the editor usually is in a form, so the buttons should have "type=button", avoiding conflicting with the form's submit.
       if (el.nodeName === 'BUTTON' && !el.getAttribute('type')) el.setAttribute('type', 'button');
     }
+    this.textareaMarkdownToolbar.querySelector('button[data-md-action="indent"]')?.addEventListener('click', () => {
+      this.indentSelection(false);
+    });
+    this.textareaMarkdownToolbar.querySelector('button[data-md-action="unindent"]')?.addEventListener('click', () => {
+      this.indentSelection(true);
+    });
 
-    // Track whether any actual input or pointer action was made after focusing, and only intercept Tab presses after that.
-    this.interceptTab = false;
-    this.textarea.addEventListener('focus', () => {
-      this.interceptTab = false;
-    });
-    this.textarea.addEventListener('pointerup', () => {
-      // Assume if a pointer is used then Tab handling is a bit less of an issue.
-      this.interceptTab = true;
-    });
     this.textarea.addEventListener('keydown', (e) => {
       if (e.shiftKey) {
         e.target._shiftDown = true;
       }
-      const unmodified = !e.shiftKey && !e.ctrlKey && !e.altKey;
-      if (e.key === 'Escape') {
-        // Explicitly lose focus and reenable tab navigation.
-        e.target.blur();
-        this.interceptTab = false;
-      } else if (e.key === 'Tab' && this.interceptTab && !e.altKey && !e.ctrlKey) {
-        this.indentSelection(e.shiftKey);
+      if (e.key === 'Enter' && !e.shiftKey && !e.ctrlKey && !e.altKey) {
+        if (!this.breakLine()) return; // Nothing changed, let the default handler work.
         this.options?.onContentChanged?.(this, e);
         e.preventDefault();
-      } else if (e.key === 'Enter' && unmodified) {
-        if (this.breakLine()) {
-          this.options?.onContentChanged?.(this, e);
-          e.preventDefault();
-        }
-      } else {
-        this.interceptTab ||= unmodified;
       }
     });
     this.textarea.addEventListener('keyup', (e) => {
@@ -355,6 +340,7 @@ class ComboMarkdownEditor {
 
     // Update changed lines whole.
     const text = changedLines.join('\n');
+    this.textarea.focus();
     this.textarea.setSelectionRange(editStart, editEnd);
     if (!document.execCommand('insertText', false, text)) {
       // execCommand is deprecated, but setRangeText (and any other direct value modifications) erases the native undo history.