Skip to content

Commit

Permalink
Change markdown parsing when pasting plain text to be more conservati…
Browse files Browse the repository at this point in the history 
…ve in the document editor (#7768)
  • Loading branch information
@emmatown @dcousens
emmatown authored and dcousens committed Aug 4, 2022
1 parent 0114841 commit 6cb00be
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 69 deletions.
5 changes: 5 additions & 0 deletions .changeset/fast-gorillas-build.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-6/fields-document': patch
---

Fixes pasting plain text in the document editor removing markdown link definition and usages
1 change: 0 additions & 1 deletion packages/fields-document/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
"io-ts-excess": "^1.0.1",
"is-hotkey": "^0.2.0",
"match-sorter": "^6.3.1",
"mdast-util-definitions": "^4.0.0",
"mdast-util-from-markdown": "^0.8.5",
"mdast-util-gfm-autolink-literal": "^0.1.3",
"mdast-util-gfm-strikethrough": "^0.2.3",
Expand Down
115 changes: 104 additions & 11 deletions packages/fields-document/src/DocumentEditor/pasting/markdown.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/** @jest-environment jsdom */
/** @jsxRuntime classic */
/** @jsx jsx */
import { Node } from 'slate';
import { makeEditor, jsx } from '../tests/utils';
import { MyDataTransfer } from './data-transfer';

Expand Down Expand Up @@ -247,20 +248,23 @@ there is a break before this
</text>
</paragraph>
<paragraph>
<text />
<link
@@isInline={true}
href="http://keystonejs.com/link-reference"
>
<text>
Link reference
</text>
</link>
<text />
<text>
[Link reference][1]
</text>
</paragraph>
<paragraph>
<text>
![Image reference](http://keystonejs.com/image-reference)
![Image reference][2]
</text>
</paragraph>
<paragraph>
<text>
[1]: http://keystonejs.com/link-reference
</text>
</paragraph>
<paragraph>
<text>
[2]: http://keystonejs.com/image-reference
<cursor />
</text>
</paragraph>
Expand Down Expand Up @@ -345,3 +349,92 @@ test('a link nested inside bold', () => {
</editor>
`);
});

// this is written like this rather than a snapshot because the snapshot
// formatting creates html escapes(which is nice for the formatting)
// this test shows ensures that the snapshot formatting is not buggy
// and we're not showing html escapes to users or something
test('html in inline content is just written', () => {
const input = `a<code>blah</code>b`;
expect(Node.string(deserializeMarkdown(input))).toEqual(input);
});

test('html in complex inline content', () => {
expect(deserializeMarkdown(`__content [link<code>blah</code>](https://keystonejs.com) content__`))
.toMatchInlineSnapshot(`
<editor
marks={
Object {
"bold": true,
}
}
>
<paragraph>
<text
bold={true}
>
content
</text>
<link
@@isInline={true}
href="https://keystonejs.com"
>
<text
bold={true}
>
link&lt;code&gt;blah&lt;/code&gt;
</text>
</link>
<text
bold={true}
>
content
<cursor />
</text>
</paragraph>
</editor>
`);
});

// the difference between a delightful "oh, nice! the editor did the formatting i wanted"
// and "UGH!! the editor just removed some of the content i wanted" can be really subtle
// and while we want the delightful experiences, avoiding the bad experiences is _more important_

// so even though we could parse link references & definitions in some cases we don't because it feels a bit too magical
// also note that so the workaround of "paste into some plain text place, copy it from there"
// like html pasting doesn't exist here since this is parsing _from_ plain text
// so erring on the side of "don't be too smart" is better
test('link and image references and images are left alone', () => {
const input = `[Link reference][1]
![Image reference][2]
![Image](http://keystonejs.com/image)
[1]: http://keystonejs.com/link-reference
[2]: http://keystonejs.com/image-reference`;

expect(
deserializeMarkdown(input)
.children.map(node => Node.string(node))
.join('\n\n')
).toEqual(input);
});

// ideally, we would probably convert the mark here, but like the comment on the previous test says,
// it being not perfect is fine, as long as it doesn't make things _worse_
test('marks in image tags are converted', () => {
const input = `![Image **blah**](https://keystonejs.com/image)`;

expect(deserializeMarkdown(input)).toMatchInlineSnapshot(`
<editor>
<paragraph>
<text>
![Image **blah**](https://keystonejs.com/image)
<cursor />
</text>
</paragraph>
</editor>
`);
});
60 changes: 19 additions & 41 deletions packages/fields-document/src/DocumentEditor/pasting/markdown.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import autoLinkLiteralMarkdownSyntax from 'micromark-extension-gfm-autolink-lite
// @ts-ignore
import gfmStrikethroughFromMarkdownExtension from 'mdast-util-gfm-strikethrough/from-markdown';
import gfmStrikethroughMarkdownSyntax from 'micromark-extension-gfm-strikethrough';
import definitions from 'mdast-util-definitions';
import { Descendant } from 'slate';
import { getTextNodeForCurrentlyActiveMarks, addMarkToChildren } from './utils';

Expand All @@ -17,22 +16,19 @@ const markdownConfig = {

export function deserializeMarkdown(markdown: string) {
const root = mdASTUtilFromMarkdown(markdown, markdownConfig);
const getDefinition = definitions(root);
let nodes = root.children;
if (nodes.length === 1 && nodes[0].type === 'paragraph') {
nodes = nodes[0].children;
}
return deserializeChildren(nodes, getDefinition);
return deserializeChildren(nodes, markdown);
}

type GetDefinition = ReturnType<typeof definitions>;

type MDNode = ReturnType<typeof mdASTUtilFromMarkdown>['children'][number];

function deserializeChildren(nodes: MDNode[], getDefinition: GetDefinition) {
function deserializeChildren(nodes: MDNode[], input: string) {
const outputNodes: Descendant[] = [];
for (const node of nodes) {
const result = deserializeMarkdownNode(node, getDefinition);
const result = deserializeMarkdownNode(node, input);
if (result.length) {
outputNodes.push(...result);
}
Expand All @@ -43,64 +39,45 @@ function deserializeChildren(nodes: MDNode[], getDefinition: GetDefinition) {
return outputNodes;
}

function deserializeMarkdownNode(node: MDNode, getDefinition: GetDefinition): Descendant[] {
function deserializeMarkdownNode(node: MDNode, input: string): Descendant[] {
switch (node.type) {
case 'blockquote': {
return [{ type: 'blockquote', children: deserializeChildren(node.children, getDefinition) }];
}
case 'linkReference': {
return [
{
type: 'link',
href: getDefinition(node.identifier)?.url || '',
children: deserializeChildren(node.children, getDefinition),
},
];
return [{ type: 'blockquote', children: deserializeChildren(node.children, input) }];
}
case 'link': {
return [
{
type: 'link',
href: node.url,
children: deserializeChildren(node.children, getDefinition),
children: deserializeChildren(node.children, input),
},
];
}
case 'code': {
return [{ type: 'code', children: [{ text: node.value }] }];
}
case 'paragraph': {
return [{ type: 'paragraph', children: deserializeChildren(node.children, getDefinition) }];
return [{ type: 'paragraph', children: deserializeChildren(node.children, input) }];
}
case 'heading': {
return [
{
type: 'heading',
level: node.depth,
children: deserializeChildren(node.children, getDefinition),
children: deserializeChildren(node.children, input),
},
];
}
case 'list': {
return [
{
type: node.ordered ? 'ordered-list' : 'unordered-list',
children: deserializeChildren(node.children, getDefinition),
children: deserializeChildren(node.children, input),
},
];
}
case 'imageReference': {
return [
getTextNodeForCurrentlyActiveMarks(
`![${node.alt || ''}](${getDefinition(node.identifier)?.url || ''})`
),
];
}
case 'image': {
return [getTextNodeForCurrentlyActiveMarks(`![${node.alt || ''}](${node.url})`)];
}
case 'listItem': {
return [{ type: 'list-item', children: deserializeChildren(node.children, getDefinition) }];
return [{ type: 'list-item', children: deserializeChildren(node.children, input) }];
}
case 'thematicBreak': {
return [{ type: 'divider', children: [{ text: '' }] }];
Expand All @@ -109,28 +86,29 @@ function deserializeMarkdownNode(node: MDNode, getDefinition: GetDefinition): De
return [getTextNodeForCurrentlyActiveMarks('\n')];
}
case 'delete': {
return addMarkToChildren('strikethrough', () =>
deserializeChildren(node.children, getDefinition)
);
return addMarkToChildren('strikethrough', () => deserializeChildren(node.children, input));
}
case 'strong': {
return addMarkToChildren('bold', () => deserializeChildren(node.children, getDefinition));
return addMarkToChildren('bold', () => deserializeChildren(node.children, input));
}
case 'emphasis': {
return addMarkToChildren('italic', () => deserializeChildren(node.children, getDefinition));
return addMarkToChildren('italic', () => deserializeChildren(node.children, input));
}
case 'inlineCode': {
return addMarkToChildren('code', () => [getTextNodeForCurrentlyActiveMarks(node.value)]);
}
// while it would be nice if we parsed the html here
// while it might be nice if we parsed the html here
// it's a bit more complicated than just parsing the html
// because an html node might just be an opening/closing node
// but we just have an opening/closing node here
// not the opening and closing and children
case 'html':
case 'text': {
return [getTextNodeForCurrentlyActiveMarks(node.value)];
}
}
return [];
return [
getTextNodeForCurrentlyActiveMarks(
input.slice(node.position!.start.offset, node.position!.end.offset)
),
];
}
16 changes: 0 additions & 16 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9750,13 +9750,6 @@ mdast-util-compact@^1.0.0:
dependencies:
unist-util-visit "^1.1.0"

mdast-util-definitions@^4.0.0:
version "4.0.0"
resolved "https://registry.yarnpkg.com/mdast-util-definitions/-/mdast-util-definitions-4.0.0.tgz#c5c1a84db799173b4dcf7643cda999e440c24db2"
integrity sha512-k8AJ6aNnUkB7IE+5azR9h81O5EQ/cTDXtWdMq9Kk5KcEW/8ritU5CeLg/9HhOC++nALHBlaogJ5jz0Ybk3kPMQ==
dependencies:
unist-util-visit "^2.0.0"

mdast-util-definitions@^5.0.0:
version "5.1.1"
resolved "https://registry.yarnpkg.com/mdast-util-definitions/-/mdast-util-definitions-5.1.1.tgz#2c1d684b28e53f84938bb06317944bee8efa79db"
Expand Down Expand Up @@ -13915,15 +13908,6 @@ unist-util-visit@^1.0.0, unist-util-visit@^1.1.0:
dependencies:
unist-util-visit-parents "^2.0.0"

unist-util-visit@^2.0.0:
version "2.0.3"
resolved "https://registry.yarnpkg.com/unist-util-visit/-/unist-util-visit-2.0.3.tgz#c3703893146df47203bb8a9795af47d7b971208c"
integrity sha512-iJ4/RczbJMkD0712mGktuGpm/U4By4FfDonL7N/9tATGIF4imikjOuagyMY53tnZq3NP6BcmlrHhEKAfGWjh7Q==
dependencies:
"@types/unist" "^2.0.0"
unist-util-is "^4.0.0"
unist-util-visit-parents "^3.0.0"

unist-util-visit@^4.0.0:
version "4.1.0"
resolved "https://registry.yarnpkg.com/unist-util-visit/-/unist-util-visit-4.1.0.tgz#f41e407a9e94da31594e6b1c9811c51ab0b3d8f5"
Expand Down

0 comments on commit 6cb00be

Please sign in to comment.