diff --git a/.changeset/little-ears-rescue.md b/.changeset/little-ears-rescue.md new file mode 100644 index 00000000..8c5ac5cf --- /dev/null +++ b/.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) diff --git a/plugins/official/stack-snippets/src/commands.ts b/plugins/official/stack-snippets/src/commands.ts index d7c32e7b..327639f1 100644 --- a/plugins/official/stack-snippets/src/commands.ts +++ b/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); diff --git a/plugins/official/stack-snippets/src/stackSnippetPlugin.ts b/plugins/official/stack-snippets/src/stackSnippetPlugin.ts index bcbf3d4f..ba9d3a8f 100644 --- a/plugins/official/stack-snippets/src/stackSnippetPlugin.ts +++ b/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); diff --git a/plugins/official/stack-snippets/test/commands.test.ts b/plugins/official/stack-snippets/test/commands.test.ts index 7b0e00f1..3fff649a 100644 --- a/plugins/official/stack-snippets/test/commands.test.ts +++ b/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 = ` @@ -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); + }); + }); }); diff --git a/src/rich-text/commands/index.ts b/src/rich-text/commands/index.ts index d762fa7f..c6ff819b 100644 --- a/src/rich-text/commands/index.ts +++ b/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( diff --git a/src/rich-text/editor.ts b/src/rich-text/editor.ts index 96cd934e..c0342218 100644 --- a/src/rich-text/editor.ts +++ b/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, diff --git a/src/rich-text/key-bindings.ts b/src/rich-text/key-bindings.ts index 326c3318..9d1cdc0a 100644 --- a/src/rich-text/key-bindings.ts +++ b/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 = [ diff --git a/test/rich-text/commands/index.test.ts b/test/rich-text/commands/index.test.ts index 1bcf0bff..a053251c 100644 --- a/test/rich-text/commands/index.test.ts +++ b/test/rich-text/commands/index.test.ts @@ -5,7 +5,8 @@ import { Transaction, } from "prosemirror-state"; import { - escapeUnselectableCommand, + escapeUnselectableCommandDown, + escapeUnselectableCommandUp, exitInclusiveMarkCommand, insertRichTextHorizontalRuleCommand, toggleHeadingLevel, @@ -787,7 +788,7 @@ describe("commands", () => { }); }); - describe("escapeUnselectableCommand", () => { + describe("escapeUnselectableCommandDown", () => { const whenEscapeUnselectableCommandCalled = ( state: EditorState, shouldMatchTrans?: (tr: Transaction) => boolean @@ -799,7 +800,10 @@ describe("commands", () => { dispatchTr = tr; }; - const result = escapeUnselectableCommand(state, captureDispatch); + const result = escapeUnselectableCommandDown( + state, + captureDispatch + ); return { result, @@ -918,4 +922,126 @@ describe("commands", () => { expect(matchedTransaction).toBe(true); }); }); + + describe("escapeUnselectableCommandUp", () => { + const whenEscapeUnselectableCommandCalled = ( + state: EditorState, + shouldMatchTrans?: (tr: Transaction) => boolean + ) => { + let dispatchCalled = false; + let dispatchTr: Transaction = null; + const captureDispatch = (tr: Transaction) => { + dispatchCalled = true; + dispatchTr = tr; + }; + + const result = escapeUnselectableCommandUp(state, captureDispatch); + + return { + result, + dispatchCalled, + matchedTransaction: shouldMatchTrans + ? shouldMatchTrans(dispatchTr) + : null, + }; + }; + + it("should do nothing if first node in the document is selected", () => { + //Selection is the only line in the document, therefore the end. + let state = createState( + "Here's a paragraph - a text block mind you", + [] + ); + state = state.apply( + state.tr.setSelection(NodeSelection.create(state.doc, 0)) + ); + + const { result, dispatchCalled } = + whenEscapeUnselectableCommandCalled(state); + + expect(result).toBe(false); + expect(dispatchCalled).toBe(false); + }); + + it("should do nothing if first inline text in the document is selected", () => { + //Selection is the only line in the document, therefore the beginning. + let state = createState( + "Here's a paragraph - a text block mind you", + [] + ); + state = state.apply( + state.tr.setSelection(TextSelection.create(state.doc, 0, 0)) + ); + + const { result, dispatchCalled } = + whenEscapeUnselectableCommandCalled(state); + + expect(result).toBe(false); + expect(dispatchCalled).toBe(false); + }); + + it("should not alter the document if there is a preceding textblock node", () => { + let state = createState( + "Here's a paragraph - a text block mind you", + [] + ); + const firstNodePosEnd = state.doc.lastChild.firstChild.nodeSize + 1; + state = state.apply( + state.tr.insert( + 0, + parseHtmlToDoc("This is another node, wild!", false) + ) + ); + state = state.apply( + state.tr.setSelection( + TextSelection.create( + state.doc, + firstNodePosEnd - 3, + firstNodePosEnd + ) + ) + ); + + const { result, dispatchCalled } = + whenEscapeUnselectableCommandCalled(state); + + expect(result).toBe(false); + expect(dispatchCalled).toBe(false); + }); + + it("should add a paragraph block if there are no preceding textblock nodes", () => { + let state = EditorState.create({ + doc: parseHtmlToDoc( + "
Header 1Header 2
onetwo
", + false + ), + schema: testRichTextSchema, + plugins: [], + }); + + const selection = TextSelection.create(state.doc, 4); + expect(selection.$to.parent.textContent).toBe("Header 1"); + expect(selection.$from.parent.textContent).toBe("Header 1"); + state = state.apply(state.tr.setSelection(selection)); + + const { result, dispatchCalled, matchedTransaction } = + whenEscapeUnselectableCommandCalled(state, (tr) => { + if (tr.steps.length !== 1) return false; + + const step = tr.steps[0] as ReplaceStep; + if (step.slice === undefined) return false; + + if (step.slice.content.childCount !== 1) return false; + if (step.slice.content.firstChild.type.name !== "paragraph") + return false; + if (step.slice.content.firstChild.textContent != "") + return false; + return true; + }); + + expect(result).toBe(false); + expect(dispatchCalled).toBe(true); + expect(matchedTransaction).toBe(true); + }); + }); });