fix(snippets): Fix unescapableCommand handling, swallow commands for snippets

.changeset/little-ears-rescue.md

@@ -0,0 +1,5 @@
+---
+"@stackoverflow/stacks-editor": patch
+---
+
+Fix the ability to move around Snippets (enter and exit with arrow keys) and swallows commands that are incompatible with snippets (e.g. creating horizontal rules)

plugins/official/stack-snippets/src/commands.ts

@@ -7,6 +7,8 @@ import {
 import { Node } from "prosemirror-model";
 import { EditorView } from "prosemirror-view";
 import { BASE_VIEW_KEY } from "../../../../src/shared/prosemirror-plugins/base-view-state";
+import { EditorState } from "prosemirror-state";
+import { caseNormalizeKeymap } from "../../../../src/shared/prosemirror-plugins/case-normalize-keymap";
 
 /** Builds a function that will update a snippet node on the up-to-date state (at time of execution) **/
 function buildUpdateDocumentCallback(view: EditorView) {
@@ -115,3 +117,30 @@ export function openSnippetModal(options?: StackSnippetOptions): MenuCommand {
         return true;
     };
 }
+
+const swallowSnippetCommand = (state: EditorState): boolean => {
+    const fromNodeType = state.selection.$from.node().type.name;
+
+    if (
+        fromNodeType === "stack_snippet" ||
+        fromNodeType === "stack_snippet_lang"
+    ) {
+        return true;
+    }
+};
+
+export const swallowedCommandList = {
+    "Mod-Enter": swallowSnippetCommand,
+    "Shift-Enter": swallowSnippetCommand,
+    "Mod-r": swallowSnippetCommand,
+};
+
+/**
+ * Snippets are comprised of a container around customized codeblocks. Some of the default behaviour for key-binds makes them behave
+ * very strangely.
+ *
+ * In these cases, we override the command to (contextually) do nothing if the current context is a snippet
+ *   This is possible because returning truthy consumes the event.
+ * **/
+export const stackSnippetCommandRedactor =
+    caseNormalizeKeymap(swallowedCommandList);

plugins/official/stack-snippets/src/stackSnippetPlugin.ts

@@ -11,7 +11,7 @@ import { EditorView } from "prosemirror-view";
 import { StackSnippetView } from "./snippet-view";
 import { StackSnippetOptions } from "./common";
 import { stackSnippetPasteHandler } from "./paste-handler";
-import { openSnippetModal } from "./commands";
+import { openSnippetModal, stackSnippetCommandRedactor } from "./commands";
 
 /**
  * Build the StackSnippet plugin using hoisted options that can be specified at runtime
@@ -30,7 +30,7 @@ export const stackSnippetPlugin: (opts?: StackSnippetOptions) => EditorPlugin =
                     return new StackSnippetView(node, view, getPos, opts);
                 },
             },
-            plugins: [stackSnippetPasteHandler],
+            plugins: [stackSnippetPasteHandler, stackSnippetCommandRedactor],
         },
         extendSchema: (schema) => {
             schema.nodes = schema.nodes.append(stackSnippetRichTextNodeSpec);

plugins/official/stack-snippets/test/commands.test.ts

@@ -8,6 +8,7 @@ import {
     snippetExternalProvider,
     validBegin,
     validEnd,
+    validJs,
     validSnippetRenderCases,
 } from "./stack-snippet-helpers";
 import { parseSnippetBlockForProsemirror } from "../src/paste-handler";
@@ -17,6 +18,14 @@ import MarkdownIt from "markdown-it";
 
 describe("commands", () => {
     const schema = buildSnippetSchema();
+    function richView(markdownInput: string, opts?: StackSnippetOptions) {
+        return new RichTextEditor(
+            document.createElement("div"),
+            markdownInput,
+            snippetExternalProvider(opts),
+            {}
+        );
+    }
 
     const whenOpenSnippetCommandCalled = (
         state: EditorState,
@@ -130,14 +139,6 @@ describe("commands", () => {
     describe("callback", () => {
         const mdit = new MarkdownIt("default", {});
         mdit.use(markdownPlugin);
-        function richView(markdownInput: string, opts?: StackSnippetOptions) {
-            return new RichTextEditor(
-                document.createElement("div"),
-                markdownInput,
-                snippetExternalProvider(opts),
-                {}
-            );
-        }
 
         const callbackTestCaseJs: string = `<!-- language: lang-js -->
 
@@ -225,4 +226,41 @@ describe("commands", () => {
             }
         );
     });
+
+    describe("redactor", () => {
+        //Note: we're testing this functionality once with a command that is universal across Macs and PC.
+        // In the pipeline this is likely using a Linux environment, in which case "Mod" means "Ctrl" too, but
+        // the main concern is on other development environments.
+        it("should swallow commands when in a Snippet context", () => {
+            const view = richView(`${validBegin}${validJs}${validEnd}`);
+            const expectedHTML = view.editorView.dom.innerHTML;
+            const event = new KeyboardEvent("keydown", {
+                ctrlKey: true,
+                key: "Enter",
+            });
+
+            view.editorView.someProp("handleKeyDown", (f) =>
+                f(view.editorView, event)
+            );
+
+            //The Dom is exactly the same - no change has occured
+            expect(view.editorView.dom.innerHTML).toBe(expectedHTML);
+        });
+
+        it("should not swallow commands when in a non-Snippet context", () => {
+            const view = richView("```javascript\nconsole.log('test');\n```");
+            const expectedHTML = view.editorView.dom.innerHTML;
+            const event = new KeyboardEvent("keydown", {
+                ctrlKey: true,
+                key: "Enter",
+            });
+
+            view.editorView.someProp("handleKeyDown", (f) =>
+                f(view.editorView, event)
+            );
+
+            //The Dom is not the same - a change has occured
+            expect(view.editorView.dom.innerHTML).not.toBe(expectedHTML);
+        });
+    });
 });

src/rich-text/commands/index.ts

@@ -433,30 +433,87 @@ export function exitInclusiveMarkCommand(
  * Ensure there's a next block to move into - Adds an additional blank paragraph block
  *  if the next node available is unselectable and there is no node afterwards that is selectable.
  * */
-export function escapeUnselectableCommand(
+export function escapeUnselectableCommandDown(
     state: EditorState,
     dispatch: (tr: Transaction) => void
 ): boolean {
     //A resolved position of the cursor. Functionally: The place we're calculating the next line for.
     const selectionEndPos = state.selection.$to;
+    const topLevelParent = selectionEndPos.node(1) || selectionEndPos.parent;
+    const isLastNode = state.doc.lastChild.eq(topLevelParent);
+    const isSelectingWholeDoc = state.doc.eq(selectionEndPos.parent);
 
-    //If you're already at the end of the document, do the default action (nothing)
-    // Note: We're checking for either the last Inline character or the last node being selected here.
-    const isLastNode = state.doc.lastChild.eq(state.selection.$to.parent);
-    const isSelectingWholeDoc = state.doc.eq(state.selection.$to.parent);
-    if (isLastNode || isSelectingWholeDoc) {
+    //If we're selecting the whole document, don't mess with the node structure
+    if (isSelectingWholeDoc) {
         return false;
     }
 
-    //Calculate the position starting at the next line in the doc (the start point to check at)
-    const findStartPos = selectionEndPos.posAtIndex(
-        selectionEndPos.indexAfter(0),
-        0
-    );
+    //If this is the last node and we're at document-level, no need to go further.
+    if (isLastNode && selectionEndPos.depth == 1) {
+        return false;
+    }
+
+    //Ensure that one of the following elements is selectable, or add a paragraph
+    if (
+        !isTextSelectableInRange(
+            selectionEndPos.after(),
+            state.doc.content.size,
+            state
+        )
+    ) {
+        insertBlankParagraph(state.doc.content.size, state, dispatch);
+    }
+
+    //No matter what, we want the default behaviour to take over from here.
+    // Either we've created a new line to edit into just in time, or there was already something for it to move to
+    return false;
+}
+
+/**
+ * Ensure there's a next block to move into - Adds an additional blank paragraph block
+ *  if the previous node available is unselectable and there is no node before that is selectable.
+ * */
+export function escapeUnselectableCommandUp(
+    state: EditorState,
+    dispatch: (tr: Transaction) => void
+): boolean {
+    //A resolved position of the cursor. Functionally: The place we're calculating the next line for.
+    const selectionBeginPos = state.selection.$to;
+    const topLevelParent =
+        selectionBeginPos.node(1) || selectionBeginPos.parent;
+    const isFirstNode = state.doc.firstChild.eq(topLevelParent);
+    const isSelectingWholeDoc = state.doc.eq(selectionBeginPos.parent);
+
+    //If we're selecting the whole document, don't mess with the node structure
+    if (isSelectingWholeDoc) {
+        return false;
+    }
 
+    //If this is the last node and we're at document-level, no need to go further.
+    if (isFirstNode && selectionBeginPos.depth == 1) {
+        return false;
+    }
+
+    //If there's not something to move into, add it now
+    if (!isTextSelectableInRange(0, selectionBeginPos.before(), state)) {
+        insertBlankParagraph(0, state, dispatch);
+    }
+
+    //No matter what, we want the default behaviour to take over from here.
+    // Either we've created a new line to edit into just in time, or there was already something for it to move to
+    return false;
+}
+
+function isTextSelectableInRange(
+    beginPos: number,
+    endPos: number,
+    state: EditorState
+): boolean {
     //Starting from the next node position down, check all the nodes for being a text block.
+    // We care whether there's at least one - not necessarily the node that's found
     let foundSelectable: boolean = false;
-    state.doc.nodesBetween(findStartPos, state.doc.content.size, (node) => {
+
+    state.doc.nodesBetween(beginPos, endPos, (node) => {
         //Already found one, no need to delve deeper.
         if (foundSelectable) return !foundSelectable;
 
@@ -470,19 +527,15 @@ export function escapeUnselectableCommand(
         return true;
     });
 
-    //If there's not something to move into, add it now
-    if (!foundSelectable) {
-        dispatch(
-            state.tr.insert(
-                state.doc.content.size,
-                state.schema.nodes.paragraph.create()
-            )
-        );
-    }
+    return foundSelectable;
+}
 
-    //No matter what, we want the default behaviour to take over from here.
-    // Either we've created a new line to edit into just in time, or there was already something for it to move to
-    return false;
+function insertBlankParagraph(
+    pos: number,
+    state: EditorState,
+    dispatch: (tr: Transaction) => void
+) {
+    dispatch(state.tr.insert(pos, state.schema.nodes.paragraph.create()));
 }
 
 export function splitCodeBlockAtStartOfDoc(

src/rich-text/editor.ts

@@ -122,10 +122,6 @@ export class RichTextEditor extends BaseView {
                     plugins: [
                         baseViewStatePlugin(this),
                         history(),
-                        ...allKeymaps(
-                            this.finalizedSchema,
-                            this.options.parserFeatures
-                        ),
                         menu,
                         richTextInputRules(
                             this.finalizedSchema,
@@ -146,6 +142,11 @@ export class RichTextEditor extends BaseView {
                         readonlyPlugin(),
                         spoilerToggle,
                         ...this.externalPluginProvider.plugins.richText,
+                        //Keymaps are executed in order and can be consuming, so we let external plugins register first
+                        ...allKeymaps(
+                            this.finalizedSchema,
+                            this.options.parserFeatures
+                        ),
                         // Paste handlers are consuming, so we let external plugins try first
                         tables,
                         richTextCodePasteHandler,

src/rich-text/key-bindings.ts

@@ -32,7 +32,8 @@ import {
     toggleList,
     splitCodeBlockAtStartOfDoc,
     exitInclusiveMarkCommand,
-    escapeUnselectableCommand,
+    escapeUnselectableCommandDown,
+    escapeUnselectableCommandUp,
     openCodeBlockLanguagePicker,
 } from "./commands";
 
@@ -92,7 +93,8 @@ export function allKeymaps(
         "Mod-'": toggleMark(schema.marks.kbd),
         // exit inline code block using the right arrow key
         "ArrowRight": exitInclusiveMarkCommand,
-        "ArrowDown": escapeUnselectableCommand,
+        "ArrowUp": escapeUnselectableCommandUp,
+        "ArrowDown": escapeUnselectableCommandDown,
     });
 
     const keymaps = [