Reimplement editor Tab handling with accessibility safeguards (#6813)

The primary goal is to balance having the editor work as expected by developers (with Tab key affecting indentation) while also not impeding keyboard navigation.

* Tab indents, Shift+Tab unindents, but only when that indent would be valid. E.g. moving existing list items down or up one level.
* Indenting a selection always works.
* When an "invalid" indent is attempted, nothing happens and a toast is shown with a hint to press again to leave the editor.
* Attempting the same action again allows the textarea lose focus by allowing the browser's default key handler.
* Pressing Esc also loses focus immediately.
* No tab handling happens until the text editor has been interacted with (other than just having been focused).
* Changing indentation in block quotes adds or removes quote levels instead.

Screenshot of the toast being shown:
a6287d29-4ce0-4977-aae8-ef1aff2ac89f

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6813
Reviewed-by: Otto <otto@codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Danko Aleksejevs <danko@very.lv>
Co-committed-by: Danko Aleksejevs <danko@very.lv>

web_src/js/features/comp/ComboMarkdownEditor.js

@@ -2,13 +2,13 @@ import '@github/markdown-toolbar-element';
 import '@github/text-expander-element';
 import $ from 'jquery';
 import {attachTribute} from '../tribute.js';
-import {hideElem, showElem, autosize, isElemVisible, replaceTextareaSelection} from '../../utils/dom.js';
+import {autosize, hideElem, isElemVisible, replaceTextareaSelection, showElem} from '../../utils/dom.js';
 import {initEasyMDEPaste, initTextareaPaste} from './Paste.js';
 import {handleGlobalEnterQuickSubmit} from './QuickSubmit.js';
 import {renderPreviewPanelContent} from '../repo-editor.js';
 import {easyMDEToolbarActions} from './EasyMDEToolbarActions.js';
 import {initTextExpander} from './TextExpander.js';
-import {showErrorToast} from '../../modules/toast.js';
+import {showErrorToast, showHintToast} from '../../modules/toast.js';
 import {POST} from '../../modules/fetch.js';
 
 let elementIdCounter = 0;
@@ -35,6 +35,9 @@ export function validateTextareaNonEmpty(textarea) {
   return true;
 }
 
+// Matches the beginning of a line containing leading whitespace and possibly valid list or block quote prefix
+const listPrefixRegex = /^\s*((\d+)[.)]\s|[-*+]\s{1,4}\[[ x]\]\s?|[-*+]\s|(>\s?)+)?/;
+
 class ComboMarkdownEditor {
   constructor(container, options = {}) {
     container._giteaComboMarkdownEditor = this;
@@ -88,24 +91,62 @@ class ComboMarkdownEditor {
       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.indentSelection(false, false);
     });
     this.textareaMarkdownToolbar.querySelector('button[data-md-action="unindent"]')?.addEventListener('click', () => {
-      this.indentSelection(true);
+      this.indentSelection(true, false);
     });
     this.textareaMarkdownToolbar.querySelector('button[data-md-action="new-table"]')?.setAttribute('data-modal', `div[data-markdown-table-modal-id="${elementIdCounter}"]`);
     this.textareaMarkdownToolbar.querySelector('button[data-md-action="new-link"]')?.setAttribute('data-modal', `div[data-markdown-link-modal-id="${elementIdCounter}"]`);
 
+    // Track whether any actual input or pointer action was made after focusing, and only intercept Tab presses after that.
+    this.tabEnabled = false;
+    // This tracks whether last Tab action was ignored, and if it immediately happens *again*, lose focus.
+    this.ignoredTabAction = false;
+    this.ignoredTabToast = null;
+
+    this.textarea.addEventListener('focus', () => {
+      this.tabEnabled = false;
+      this.ignoredTabAction = false;
+    });
+    this.textarea.addEventListener('pointerup', () => {
+      // Assume if a pointer is used then Tab handling is a bit less of an issue.
+      this.tabEnabled = true;
+    });
     this.textarea.addEventListener('keydown', (e) => {
       if (e.shiftKey) {
         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;
+
+      // Prevent special keyboard handling if currently a text expander popup is open
+      if (this.textarea.hasAttribute('aria-expanded')) return;
+
+      const noModifiers = !e.shiftKey && !e.ctrlKey && !e.altKey;
+      if (e.key === 'Escape') {
+        // Explicitly lose focus and reenable tab navigation.
+        e.target.blur();
+        this.tabEnabled = false;
+      } else if (e.key === 'Tab' && this.tabEnabled && !e.altKey && !e.ctrlKey) {
+        if (this.indentSelection(e.shiftKey, true)) {
+          this.options?.onContentChanged?.(this, e);
+          e.preventDefault();
+          this.activateTabHandling();
+        } else if (!this.ignoredTabAction) {
+          e.preventDefault();
+          this.ignoredTabAction = true;
+          this.ignoredTabToast?.hideToast();
+          this.ignoredTabToast = showHintToast(
+            this.container.dataset[e.shiftKey ? 'shiftTabHint' : 'tabHint'],
+            {gravity: 'bottom', useHtmlBody: true},
+          );
+          this.ignoredTabToast.toastElement.role = 'alert';
+        }
+      } else if (e.key === 'Enter' && noModifiers) {
         if (!this.breakLine()) return; // Nothing changed, let the default handler work.
         this.options?.onContentChanged?.(this, e);
         e.preventDefault();
+      } else if (noModifiers) {
+        this.activateTabHandling();
       }
     });
     this.textarea.addEventListener('keyup', (e) => {
@@ -142,6 +183,15 @@ class ComboMarkdownEditor {
     }
   }
 
+  activateTabHandling() {
+    this.tabEnabled = true;
+    this.ignoredTabAction = false;
+    if (this.ignoredTabToast) {
+      this.ignoredTabToast.hideToast();
+      this.ignoredTabToast = null;
+    }
+  }
+
   setupDropzone() {
     const dropzoneParentContainer = this.container.getAttribute('data-dropzone-parent-container');
     if (dropzoneParentContainer) {
@@ -403,13 +453,15 @@ class ComboMarkdownEditor {
     }
   }
 
-  indentSelection(unindent) {
+  // Indent all lines that are included in the selection, partially or whole, while preserving the original selection at the end.
+  indentSelection(unindent, validOnly) {
     // Indent with 4 spaces, unindent 4 spaces or fewer or a lost tab.
     const indentPrefix = '    ';
-    const unindentRegex = /^( {1,4}|\t)/;
+    const unindentRegex = /^( {1,4}|\t|> {0,4})/;
+    const indentLevel = / {4}|\t|> /g;
 
-    // Indent all lines that are included in the selection, partially or whole, while preserving the original selection at the end.
-    const lines = this.textarea.value.split('\n');
+    const value = this.textarea.value;
+    const lines = value.split('\n');
     const changedLines = [];
     // The current selection or cursor position.
     const [start, end] = [this.textarea.selectionStart, this.textarea.selectionEnd];
@@ -419,31 +471,66 @@ class ComboMarkdownEditor {
     let [newStart, newEnd] = [start, end];
     // The start and end position of the current line (where end points to the newline or EOF)
     let [lineStart, lineEnd] = [0, 0];
+    // Index of the first line included in the selection (or containing the cursor)
+    let firstLineIdx = 0;
 
-    for (const line of lines) {
+    // Find all the lines in selection beforehand so we know the full set before we start changing.
+    const linePositions = [];
+    for (const [i, line] of lines.entries()) {
       lineEnd = lineStart + line.length + 1;
       if (lineEnd <= start) {
         lineStart = lineEnd;
         continue;
       }
+      linePositions.push([lineStart, line]);
+      if (start >= lineStart && start < lineEnd) {
+        firstLineIdx = i;
+        editStart = lineStart;
+      }
+      editEnd = lineEnd - 1;
+      if (lineEnd >= end) break;
+      lineStart = lineEnd;
+    }
 
-      const updated = unindent ? line.replace(unindentRegex, '') : indentPrefix + line;
+    // Block quotes need to be nested/unnested instead of whitespace added/removed. However, only do this if the *whole* selection is in a quote.
+    const isQuote = linePositions.every(([_, line]) => line[0] === '>');
+
+    const line = lines[firstLineIdx];
+    // If there's no indent to remove, do nothing
+    if (unindent && start === end && !unindentRegex.test(line)) {
+      return false;
+    }
+
+    // If there is no selection and this is an ambiguous command (Tab handling), only (un)indent if already in a code/list.
+    if (!unindent && validOnly && start === end) {
+      // Check there's any indentation or prefix at all.
+      const match = line.match(listPrefixRegex);
+      if (!match || !match[0].length) return false;
+      // Check that the line isn't already indented in relation to parent.
+      const levels = line.match(indentLevel)?.length ?? 0;
+      const parentLevels = !firstLineIdx ? 0 : lines[firstLineIdx - 1].match(indentLevel)?.length ?? 0;
+      // Quotes can *begin* multiple levels in, so just allow whatever for now.
+      if (levels - parentLevels > 0 && !isQuote) return false;
+    }
+
+    // Apply indentation changes to lines.
+    for (const [i, [lineStart, line]] of linePositions.entries()) {
+      const updated = isQuote ?
+        (unindent ? line.replace(/^>\s{0,4}>/, '>') : `> ${line}`) :
+        (unindent ? line.replace(unindentRegex, '') : indentPrefix + line);
       changedLines.push(updated);
       const move = updated.length - line.length;
-
-      if (start >= lineStart && start < lineEnd) {
-        editStart = lineStart;
-        newStart = Math.max(start + move, lineStart);
-      }
-
+      if (i === 0) newStart = Math.max(start + move, lineStart);
       newEnd += move;
-      editEnd = lineEnd - 1;
-      lineStart = lineEnd;
-      if (lineStart > end) break;
     }
 
     // Update changed lines whole.
     const text = changedLines.join('\n');
+    if (text === value.slice(editStart, editEnd)) {
+      // Nothing changed, likely due to Shift+Tab when no indents are left.
+      return false;
+    }
+
     this.textarea.focus();
     this.textarea.setSelectionRange(editStart, editEnd);
     if (!document.execCommand('insertText', false, text)) {
@@ -454,6 +541,8 @@ class ComboMarkdownEditor {
 
     // Set selection to (effectively) be the same as before.
     this.textarea.setSelectionRange(newStart, Math.max(newStart, newEnd));
+
+    return true;
   }
 
   breakLine() {
@@ -470,7 +559,7 @@ class ComboMarkdownEditor {
     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{1,4}\[[ x]\]\s?|[-*+]\s|(>\s?)+)?/);
+    const prefix = line.match(listPrefixRegex);
 
     // Defer to browser if we can't do anything more useful, or if the cursor is inside the prefix.
     if (!prefix) return false;
@@ -489,14 +578,20 @@ class ComboMarkdownEditor {
     }
 
     // Insert newline + prefix.
-    let text = `\n${prefix[0]}`;
+    let text = `${prefix[0]}`;
     // Increment a number if present. (perhaps detecting repeating 1. and not doing that then would be a good idea)
     const num = text.match(/\d+/);
     if (num) text = text.replace(num[0], Number(num[0]) + 1);
     text = text.replace('[x]', '[ ]');
 
-    if (!document.execCommand('insertText', false, text)) {
-      this.textarea.setRangeText(text);
+    // Split the newline and prefix addition in two, so that it's two separate undo entries in Firefox
+    // Chrome seems to bundle everything together more aggressively, even with prior text input.
+    if (document.execCommand('insertText', false, '\n')) {
+      setTimeout(() => {
+        document.execCommand('insertText', false, text);
+      }, 1);
+    } else {
+      this.textarea.setRangeText(`\n${text}`);
     }
 
     return true;