diff --git a/src/components/Widgets/Markdown/MarkdownControl/VisualEditor/__tests__/__snapshots__/parser.spec.js.snap b/src/components/Widgets/Markdown/MarkdownControl/VisualEditor/__tests__/__snapshots__/parser.spec.js.snap index 725d69b9..81948733 100644 --- a/src/components/Widgets/Markdown/MarkdownControl/VisualEditor/__tests__/__snapshots__/parser.spec.js.snap +++ b/src/components/Widgets/Markdown/MarkdownControl/VisualEditor/__tests__/__snapshots__/parser.spec.js.snap @@ -1308,12 +1308,16 @@ Object { "text": "blue moon", }, Object { - "data": Object { - "isBreak": true, - }, - "kind": "text", - "text": " + "kind": "inline", + "nodes": Array [ + Object { + "data": undefined, + "kind": "text", + "text": " ", + }, + ], + "type": "break", }, Object { "data": undefined, diff --git a/src/components/Widgets/Markdown/MarkdownControl/VisualEditor/plugins.js b/src/components/Widgets/Markdown/MarkdownControl/VisualEditor/plugins.js index efb7e029..6418e0d1 100644 --- a/src/components/Widgets/Markdown/MarkdownControl/VisualEditor/plugins.js +++ b/src/components/Widgets/Markdown/MarkdownControl/VisualEditor/plugins.js @@ -1,3 +1,4 @@ +import { Text, Inline } from 'slate'; import SlateSoftBreak from 'slate-soft-break'; import EditList from 'slate-edit-list'; import EditTable from 'slate-edit-table'; @@ -21,7 +22,13 @@ const SoftBreak = (options = {}) => ({ return unwrapped.insertBlock(defaultBlock).apply(); } - return state.transform().insertText('\n').apply(); + const textNode = Text.createFromString('\n'); + const breakNode = Inline.create({ type: 'break', nodes: [ textNode ] }); + return state.transform() + .insertInline(breakNode) + .insertText('') + .collapseToStartOfNextText() + .apply(); } }); @@ -32,7 +39,7 @@ const SoftBreakOpts = { export const SoftBreakConfigured = SoftBreak(SoftBreakOpts); -export const ParagraphSoftBreakConfigured = SlateSoftBreak({ onlyIn: ['paragraph'], shift: true }); +export const ParagraphSoftBreakConfigured = SoftBreak({ onlyIn: ['paragraph'], shift: true }); const BreakToDefaultBlock = ({ onlyIn = [], defaultBlock = 'paragraph' }) => ({ onKeyDown(e, data, state) { diff --git a/src/components/Widgets/Markdown/serializers/__tests__/remarkStripTrailingBreaks.spec.js b/src/components/Widgets/Markdown/serializers/__tests__/remarkStripTrailingBreaks.spec.js new file mode 100644 index 00000000..a7b0500f --- /dev/null +++ b/src/components/Widgets/Markdown/serializers/__tests__/remarkStripTrailingBreaks.spec.js @@ -0,0 +1,24 @@ +import unified from 'unified'; +import u from 'unist-builder'; +import remarkStripTrailingBreaks from '../remarkStripTrailingBreaks'; + +const process = children => { + const tree = u('root', children); + const strippedMdast = unified() + .use(remarkStripTrailingBreaks) + .runSync(tree); + + return strippedMdast.children; +}; + +describe('remarkStripTrailingBreaks', () => { + it('should remove trailing breaks at the end of a block', () => { + expect(process([u('break')])).toEqual([]); + expect(process([u('break'), u('text', '\n \n')])).toEqual([u('text', '\n \n')]); + expect(process([u('text', 'a'), u('break')])).toEqual([u('text', 'a')]); + }); + + it('should not remove trailing breaks that are not at the end of a block', () => { + expect(process([u('break'), u('text', 'a')])).toEqual([u('break'), u('text', 'a')]); + }); +}); diff --git a/src/components/Widgets/Markdown/serializers/index.js b/src/components/Widgets/Markdown/serializers/index.js index 2d1de14a..19e2a0e9 100644 --- a/src/components/Widgets/Markdown/serializers/index.js +++ b/src/components/Widgets/Markdown/serializers/index.js @@ -16,7 +16,8 @@ import remarkToSlate from './remarkSlate'; import remarkSquashReferences from './remarkSquashReferences'; import remarkImagesToText from './remarkImagesToText'; import remarkShortcodes from './remarkShortcodes'; -import remarkEscapeMarkdownEntities from './remarkEscapeMarkdownEntities' +import remarkEscapeMarkdownEntities from './remarkEscapeMarkdownEntities'; +import remarkStripTrailingBreaks from './remarkStripTrailingBreaks'; import slateToRemark from './slateRemark'; import registry from '../../../../lib/registry'; @@ -127,6 +128,7 @@ export const remarkToMarkdown = obj => { */ const escapedMdast = unified() .use(remarkEscapeMarkdownEntities) + .use(remarkStripTrailingBreaks) .runSync(mdast); const markdown = unified() diff --git a/src/components/Widgets/Markdown/serializers/remarkSlate.js b/src/components/Widgets/Markdown/serializers/remarkSlate.js index 4779ff99..7f403089 100644 --- a/src/components/Widgets/Markdown/serializers/remarkSlate.js +++ b/src/components/Widgets/Markdown/serializers/remarkSlate.js @@ -300,7 +300,8 @@ function convertNode(node, nodes) { * line breaks within a text node. */ case 'break': { - return createText('\n', { isBreak: true }); + const textNode = createText('\n'); + return createInline('break', {}, [ textNode ]); } /** diff --git a/src/components/Widgets/Markdown/serializers/remarkStripTrailingBreaks.js b/src/components/Widgets/Markdown/serializers/remarkStripTrailingBreaks.js new file mode 100644 index 00000000..67a37c3b --- /dev/null +++ b/src/components/Widgets/Markdown/serializers/remarkStripTrailingBreaks.js @@ -0,0 +1,56 @@ +import mdastToString from 'mdast-util-to-string'; + +/** + * Removes break nodes that are at the end of a block. + * + * When a trailing double space or backslash is encountered at the end of a + * markdown block, Remark will interpret the character(s) literally, as only + * break entities followed by text qualify as breaks. A manually created MDAST, + * however, may have such entities, and users of visual editors shouldn't see + * these artifacts in resulting markdown. + */ +export default function remarkStripTrailingBreaks() { + const transform = node => { + if (node.children) { + node.children = node.children + .map((child, idx, children) => { + + /** + * Only touch break nodes. Convert all subsequent nodes to their text + * value and exclude the break node if no non-whitespace characters + * are found. + */ + if (child.type === 'break') { + const subsequentNodes = children.slice(idx + 1); + + /** + * Create a small MDAST so that mdastToString can process all + * siblings as children of one node rather than making multiple + * calls. + */ + const fragment = { type: 'root', children: subsequentNodes }; + const subsequentText = mdastToString(fragment); + return subsequentText.trim() ? child : null; + } + + /** + * Always return the child if not a break. + */ + return child; + }) + + /** + * Because some break nodes may be excluded, we filter out the resulting + * null values. + */ + .filter(child => child) + + /** + * Recurse through the MDAST by transforming each individual child node. + */ + .map(transform); + } + return node; + }; + return transform; +}; diff --git a/src/components/Widgets/Markdown/serializers/slateRemark.js b/src/components/Widgets/Markdown/serializers/slateRemark.js index 522a9b7a..7812dfac 100644 --- a/src/components/Widgets/Markdown/serializers/slateRemark.js +++ b/src/components/Widgets/Markdown/serializers/slateRemark.js @@ -21,6 +21,7 @@ const typeMap = { 'table': 'table', 'table-row': 'tableRow', 'table-cell': 'tableCell', + 'break': 'break', 'thematic-break': 'thematicBreak', 'link': 'link', 'image': 'image', @@ -113,6 +114,16 @@ function combineTextAndInline(nodes) { return acc; } + /** + * Break nodes contain a single child text node with a newline character + * for visual purposes in the editor, but Remark break nodes have no + * children, so we remove the child node here. + */ + if (node.type === 'break') { + acc.push({ kind: 'inline', type: 'break' }); + return acc; + } + /** * Convert remaining inline nodes to standalone text nodes with ranges. */ @@ -216,14 +227,6 @@ function wrapTextWithMarks(textNode, markTypes) { * replaced with multiple MDAST nodes, so the resulting array must be flattened. */ function convertTextNode(node) { - /** - * Translate soft breaks, which are just newline escape sequences. We track - * them with an `isBreak` boolean in the node data. - */ - if (get(node.data, 'isBreak')) { - return u('break'); - } - /** * If the Slate text node has a "ranges" property, translate the Slate AST to * a nested MDAST structure. Otherwise, just return an equivalent MDAST text @@ -454,11 +457,11 @@ function convertNode(node, children, shortcodePlugins) { } /** - * Thematic Breaks + * Breaks * - * Thematic breaks don't have children. We parse them separately for - * clarity. + * Breaks don't have children. We parse them separately for clarity. */ + case 'break': case 'thematic-break': { return u(typeMap[node.type]); }