search

tommoor / slate-md-serializer

A Markdown serializer for the Slate editor framework
MIT License
64 stars 36 forks source link

Properly handle multi-line blockquotes #15

Closed golmansax closed 5 years ago

golmansax commented 6 years ago

Multi-line blockquotes weren't being parsed correctly so I took a stab at solving it. Not sure if I found the correct solution though.

Issue:

> Quote1
>
> Quote2

Was incorrectly creating a blockquote with text "Quote1Quote2"

tommoor commented 6 years ago

Hey @golmansax – thanks for taking a stab, if you're able to get the existing and new tests passing I'll be happy to merge a solution.

golmansax commented 6 years ago

@tommoor Was able to get this to pass CI, let me know what you think!

tommoor commented 6 years ago

The changes make sense, although I do think ideally the text inside the multiline blockquote should be split into individual paragraphs nodes.

I'm trying to get this to work correctly with https://github.com/outline/rich-markdown-editor but haven't had success thus far.

golmansax commented 6 years ago

Hm, I think you're right. Let me try again.

golmansax commented 6 years ago

@tommoor Was able to get it working so that paragraphs are nested within blockquotes. I also tested it on a live project so this time I feel better about the result. However, there's a check for obj.type === 'block-quote' that I'm not too proud of.

What do you think?

tommoor commented 6 years ago

Thanks! This works pretty well in my testing, one thing that's a bit strange is that having a list in a blockquote appears to add extra paragraphs above on serialize/deserialize – I wonder if that's related to the special case you had to add?

golmansax commented 6 years ago

Hmm let me add a test for that and debug.

I'm also working on https://github.com/tommoor/slate-md-serializer/pull/11 in conjunction because my project requires all of the empty paragraphs to be removed, so my live testing probably won't catch empty paragraph issues.

golmansax commented 6 years ago

@tommoor I added a test in 3c9c5e8 which I think describes the list in a blockquote example you mentioned. I didn't see the issue with empty paragraphs being added. See anything different between the test and the use case you were describing?

tommoor commented 6 years ago

It may have been multi-line quote specific - try creating a multiline quote with a list within.

🙏

golmansax commented 6 years ago

Added a second list-item to the quote in 64cc629 but it still has no empty paragraphs.

tommoor commented 5 years ago

Tested in all the scenarios I can think of, great bug fix – sorry for the delay in merging 😄