FEATURE: Smarter list editing in DEditor (#27563)

This commit introduces behaviour similar to sites
like GitHub, Notion, and others where, if you
are already typing a list and press enter in the composer,
we continue the list on the next line.

Then, if you press enter again on the next line with
an empty list item, we remove that item on that last line.

This works with the following list types:

* star bullet
- dash bullet
* [] star and dash bullet with checkbox
1. numbered

This also works if you are in the middle of a list, and
with indented sub-lists.

With the numbered lists, we continue with the next number
in the sequence, and if you start a new line in the middle
of the list, we renumber the rest of the list.

app/assets/javascripts/discourse/app/components/d-editor.js

@@ -326,6 +326,7 @@ export default Component.extend(TextareaTextManipulation, {
     this._itsatrap.bind(`${PLATFORM_KEY_MODIFIER}+shift+.`, () =>
       this.send("insertCurrentTime")
     );
+    this._itsatrap.bind("enter", () => this.maybeContinueList(), "keyup");
 
     // disable clicking on links in the preview
     this.element

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

@@ -5,6 +5,7 @@ import { isEmpty } from "@ember/utils";
 import { generateLinkifyFunction } from "discourse/lib/text";
 import toMarkdown from "discourse/lib/to-markdown";
 import {
+  caretPosition,
   clipboardHelpers,
   determinePostReplaceSelection,
 } from "discourse/lib/utilities";
@@ -15,6 +16,9 @@ import I18n from "discourse-i18n";
 const INDENT_DIRECTION_LEFT = "left";
 const INDENT_DIRECTION_RIGHT = "right";
 
+// Supports '- ', '* ', '1. ', '- [ ]', '- [x]', `* [ ] `, `* [x] `, '1. [ ] ', '1. [x] '
+const LIST_REGEXP = /^(\s*)([*-]|(\d+)\.)\s(\[[\sx]\]\s)?/;
+
 const OP = {
   NONE: 0,
   REMOVED: 1,
@@ -492,6 +496,128 @@ export default Mixin.create({
     return str;
   },
 
+  _updateListNumbers(text, currentNumber) {
+    return text
+      .split("\n")
+      .map((line) => {
+        if (line.replace(/^\s+/, "").startsWith(`${currentNumber}.`)) {
+          const result = line.replace(
+            `${currentNumber}`,
+            `${currentNumber + 1}`
+          );
+          currentNumber += 1;
+          return result;
+        }
+        return line;
+      })
+      .join("\n");
+  },
+
+  @bind
+  maybeContinueList() {
+    const offset = caretPosition(this._textarea);
+    const text = this._textarea.value;
+    const lines = text.substring(0, offset).split("\n");
+
+    // Only continue if the previous line was a list item.
+    const previousLine = lines[lines.length - 2];
+    const match = previousLine?.match(LIST_REGEXP);
+    if (!match) {
+      return;
+    }
+
+    const listPrefix = match[0];
+    const indentationLevel = match[1];
+    const bullet = match[2];
+    const hasCheckbox = Boolean(match[4]);
+    const numericBullet = parseInt(match[3], 10);
+    const isNumericBullet = !isNaN(numericBullet);
+    const newBullet = isNumericBullet ? `${numericBullet + 1}.` : bullet;
+    let newPrefix = `${newBullet} ${hasCheckbox ? "[ ] " : ""}`;
+
+    // Do not append list item if there already is one on this line.
+    let currentLineEnd = text.indexOf("\n", offset);
+    if (currentLineEnd < 0) {
+      currentLineEnd = text.length;
+    }
+    const currentLine = text.substring(offset, currentLineEnd);
+    if (currentLine.startsWith(newPrefix)) {
+      newPrefix = "";
+    }
+
+    /*
+      Autocomplete list element on next line if current line has list element containing text.
+      or there's text on the line after the cursor (|):
+
+      - | some text
+
+      Becomes:
+
+      -
+      - | some text
+
+      And
+
+      - some text|
+
+      Becomes:
+
+      - some text
+      - |
+    */
+    const shouldAutocomplete =
+      previousLine.replace(listPrefix, "").trim().length > 0 ||
+      currentLine.trim().length > 0;
+
+    if (shouldAutocomplete) {
+      let autocompletePrefix = `${indentationLevel}${newPrefix}`;
+      let autocompletePostfix = text.substring(offset);
+      const autocompletePrefixLength = autocompletePrefix.length;
+
+      /*
+        For numeric items, we have to also replace the rest of the
+        numbered items in the list with their new values. Cursor is |.
+
+        1. foo|
+        2. bar
+
+        Becomes
+
+        1. foo
+        2.
+        3. bar
+      */
+      if (isNumericBullet && !text.substring(offset).match(/^\s*$/g)) {
+        autocompletePostfix = this._updateListNumbers(
+          text.substring(offset),
+          numericBullet + 1
+        );
+        autocompletePrefix += autocompletePostfix;
+
+        this.replaceText(
+          text.substring(offset, offset + autocompletePrefix.length),
+          autocompletePrefix,
+          {
+            skipNewSelection: true,
+          }
+        );
+      } else {
+        this._insertAt(offset, offset, autocompletePrefix);
+      }
+
+      this.selectText(offset + autocompletePrefixLength, 0);
+    } else {
+      // Clear the new autocompleted list item if there is no other text.
+      const offsetWithoutPrefix = offset - `\n${listPrefix}`.length;
+      this.replaceText(
+        text,
+        text.substring(0, offsetWithoutPrefix) + text.substring(offset),
+        { skipNewSelection: true }
+      );
+      this.selectText(offsetWithoutPrefix, 0);
+    }
+  },
+
   @bind
   indentSelection(direction) {
     if (![INDENT_DIRECTION_LEFT, INDENT_DIRECTION_RIGHT].includes(direction)) {
@@ -502,10 +628,12 @@ export default Mixin.create({
     const { lineVal } = selected;
     let value = selected.value;
 
-    // Perhaps this is a bit simplistic, but it is a fairly reliable
-    // guess to say whether we are indenting with tabs or spaces. for
-    // example some programming languages prefer tabs, others prefer
-    // spaces, and for the cases with no tabs it's safer to use spaces
+    /*
+      Perhaps this is a bit simplistic, but it is a fairly reliable
+      guess to say whether we are indenting with tabs or spaces. for
+      example some programming languages prefer tabs, others prefer
+      spaces, and for the cases with no tabs it's safer to use spaces
+    */
     let indentationSteps, indentationChar;
     let linesStartingWithTabCount = value.match(/^\t/gm)?.length || 0;
     let linesStartingWithSpaceCount = value.match(/^ /gm)?.length || 0;
@@ -517,24 +645,26 @@ export default Mixin.create({
       indentationSteps = 2;
     }
 
-    // We want to include all the spaces on the selected line as
-    // well, no matter where the cursor begins on the first line,
-    // because we want to indent those too. * is the cursor/selection
-    // and . are spaces:
-    //
-    // BEFORE               AFTER
-    //
-    //     *                *
-    // ....text here        ....text here
-    // ....some more text   ....some more text
-    //                  *                    *
-    //
-    // BEFORE               AFTER
-    //
-    //  *                   *
-    // ....text here        ....text here
-    // ....some more text   ....some more text
-    //                  *                    *
+    /*
+      We want to include all the spaces on the selected line as
+      well, no matter where the cursor begins on the first line,
+      because we want to indent those too. * is the cursor/selection
+      and . are spaces:
+
+      BEFORE               AFTER
+
+          *                *
+      ....text here        ....text here
+      ....some more text   ....some more text
+                       *                    *
+
+      BEFORE               AFTER
+
+       *                   *
+      ....text here        ....text here
+      ....some more text   ....some more text
+                       *                    *
+    */
     const indentationRegexp = new RegExp(`^${indentationChar}+`);
     const lineStartsWithIndentationChar = lineVal.match(indentationRegexp);
     const indentationCharsBeforeSelection = value.match(indentationRegexp);

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

@@ -1,8 +1,16 @@
 import { next } from "@ember/runloop";
-import { click, fillIn, focus, render, settled } from "@ember/test-helpers";
+import {
+  click,
+  fillIn,
+  focus,
+  render,
+  settled,
+  triggerKeyEvent,
+} from "@ember/test-helpers";
 import { hbs } from "ember-cli-htmlbars";
 import { module, test } from "qunit";
 import { withPluginApi } from "discourse/lib/plugin-api";
+import { setCaretPosition } from "discourse/lib/utilities";
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
 import formatTextWithSelection from "discourse/tests/helpers/d-editor-helper";
 import {
@@ -975,6 +983,78 @@ third line`
     }
   );
 
+  testCase(
+    "smart lists - pressing enter on a line with a list item starting with * creates a list item on the next line",
+    async function (assert, textarea) {
+      const initialValue = "* first item in list\n";
+      this.set("value", initialValue);
+      setCaretPosition(textarea, initialValue.length);
+      await triggerKeyEvent(textarea, "keyup", "Enter");
+      assert.strictEqual(this.value, initialValue + "* ");
+    }
+  );
+
+  testCase(
+    "smart lists - pressing enter on a line with a list item starting with - creates a list item on the next line",
+    async function (assert, textarea) {
+      const initialValue = "- first item in list\n";
+      this.set("value", initialValue);
+      setCaretPosition(textarea, initialValue.length);
+      await triggerKeyEvent(textarea, "keyup", "Enter");
+      assert.strictEqual(this.value, initialValue + "- ");
+    }
+  );
+
+  testCase(
+    "smart lists - pressing enter on a line with a list item starting with a number (e.g. 1.) in a list creates a list item on the next line with an auto-incremented number",
+    async function (assert, textarea) {
+      const initialValue = "1. first item in list\n";
+      this.set("value", initialValue);
+      setCaretPosition(textarea, initialValue.length);
+      await triggerKeyEvent(textarea, "keyup", "Enter");
+      assert.strictEqual(this.value, initialValue + "2. ");
+    }
+  );
+
+  testCase(
+    "smart lists - pressing enter inside a list inserts a new list item on the next line",
+    async function (assert, textarea) {
+      const initialValue = "* first item in list\n\n* second item in list";
+      this.set("value", initialValue);
+      setCaretPosition(textarea, 21);
+      await triggerKeyEvent(textarea, "keyup", "Enter");
+      assert.strictEqual(
+        this.value,
+        "* first item in list\n* \n* second item in list"
+      );
+    }
+  );
+
+  testCase(
+    "smart lists - pressing enter inside a list with numbers inserts a new list item on the next line and renumbers the rest of the list",
+    async function (assert, textarea) {
+      const initialValue = "1. first item in list\n\n2. second item in list";
+      this.set("value", initialValue);
+      setCaretPosition(textarea, 22);
+      await triggerKeyEvent(textarea, "keyup", "Enter");
+      assert.strictEqual(
+        this.value,
+        "1. first item in list\n2. \n3. second item in list"
+      );
+    }
+  );
+
+  testCase(
+    "smart lists - pressing enter again on an empty list item removes the list item",
+    async function (assert, textarea) {
+      const initialValue = "* first item in list with empty line\n* \n";
+      this.set("value", initialValue);
+      setCaretPosition(textarea, initialValue.length);
+      await triggerKeyEvent(textarea, "keyup", "Enter");
+      assert.strictEqual(this.value, "* first item in list with empty line\n");
+    }
+  );
+
   (() => {
     // Tests to check cursor/selection after replace-text event.
     const BEFORE = "red green blue";