preserveNewlines off by one newline about markdown-draft-js HOT 13 CLOSED

Thanks! Tests would be really awesome too... The newline preservation behaviour is a bit of a tricky one, lots of edge cases, I also have this issue:

#108

Question for you: Draft by default does a "hard" newline when you press return, so a brand new paragraph, although you can optionally insert a soft newline. I'm wondering if the behaviour you're describing is expected given draft's default hard vs soft newline behaviour, but I'm not 100% sure as I haven't dug in to your solution or the issue too deeply yet, just read your note 😳

from markdown-draft-js.

I think I'll write new tests to help confirm if my issue is real.

Hopefully I don't have to modify soft vs hard new lines. I worry that I don't understand the subtleties around changing that. Hard newlines are <p> and soft newlines are <br>?

from markdown-draft-js.

I ran my new tests on master without my changes, and they failed in the same way I described in my first post. I think this means it's not just me–that my issue is legitimate. Take a look at my new committed tests af777bd. What do you think?

Here's what it looks like when my new tests fail:

dan@ubuntu:~/markdown-draft-js$ npm test

> [email protected] test /home/dan/markdown-draft-js
> karma start --single-run

Deprecated private createPreprocessor() API
15 01 2020 14:26:15.867:INFO [framework.browserify]: bundle built
15 01 2020 14:26:15.873:INFO [karma-server]: Karma v4.3.0 server started at http://0.0.0.0:9876/
15 01 2020 14:26:15.874:INFO [launcher]: Launching browsers Chrome with concurrency unlimited
15 01 2020 14:26:15.879:INFO [launcher]: Starting browser Chrome
15 01 2020 14:26:16.748:INFO [Chrome 79.0.3945 (Linux 0.0.0)]: Connected on socket IlbwJ8IX6d9aV89wAAAA with id 52201770
Chrome 79.0.3945 (Linux 0.0.0) markdownToDraft renders unstyled blank lines correctly FAILED
  Error: Expected 'c' to equal ''.
      at <Jasmine>
      at UserContext.<anonymous> (/tmp/test/markdown-to-draft.spec.js:39:45 <- /tmp/76393500ce65b93ee2cb2ca479625b23.browserify.js:7752:45)
      at <Jasmine>
  Error: Expected '' to equal 'c'.
      at <Jasmine>
      at UserContext.<anonymous> (/tmp/test/markdown-to-draft.spec.js:41:45 <- /tmp/76393500ce65b93ee2cb2ca479625b23.browserify.js:7754:45)
      at <Jasmine>
  Error: Expected 'd' to equal ''.
      at <Jasmine>
      at UserContext.<anonymous> (/tmp/test/markdown-to-draft.spec.js:43:45 <- /tmp/76393500ce65b93ee2cb2ca479625b23.browserify.js:7756:45)
      at <Jasmine>
  TypeError: Cannot read property 'text' of undefined
      at <Jasmine>
      at UserContext.<anonymous> (/tmp/test/markdown-to-draft.spec.js:45:39 <- /tmp/76393500ce65b93ee2cb2ca479625b23.browserify.js:7758:39)
      at <Jasmine>
Chrome 79.0.3945 (Linux 0.0.0) draftToMarkdown whitespace handles unstyled blank lines FAILED
  Expected 'a
  
  b
  
  
  c
  
  
  
  d' to equal 'a
  b
  
  c
  
  
  d'.
      at <Jasmine>
      at UserContext.<anonymous> (/tmp/test/draft-to-markdown.spec.js:67:24 <- /tmp/76393500ce65b93ee2cb2ca479625b23.browserify.js:6228:24)
      at <Jasmine>
Chrome 79.0.3945 (Linux 0.0.0): Executed 98 of 99 (2 FAILED) (skipped 1) (0.255 secs / 0.094 secs)
TOTAL: 2 FAILED, 96 SUCCESS
npm ERR! Test failed.  See above for more details.

Thanks a lot for the help,
Daniel

from markdown-draft-js.

Hi Rosey!

I hope you and your family are well!! No rush (family is more important) but we are waiting to hear if you have feedback or want to merge this PR (#112).

All the best!
Thanks again for this wonderful package!

from markdown-draft-js.

I'm sorry :) we've all been down with the flu this past week and are still on the recovery! Tomorrow is doctors appointments but I'll try to find some time later this week when the toddler is in daycare to sit down and look at this 🙂

from markdown-draft-js.

Get well soon! 😊

from markdown-draft-js.

Ugh I'm actually back at the hospital again so depending on how things go it may be a longer wait :(

from markdown-draft-js.

Sorry things have been very busy around here. I'm just taking a quick look at the PR now but I'll probably have to finish looking a bit later. I think it looks good and thanks for writing tests! I just want to check your branch out locally and confirm that it still works correctly in cases where there's a blockquote followed by a paragraph, just because the comment in the code about "styled followed by unstyled" makes me wonder if in some cases an extra newline was necessary to avoid things like block quotes persisting, eg:

> Test
Hello

renders as

Test
Hello

and so if we were converting we'd want it to be

> Test

Hello

To ensure blockquote is escaped.

Anyway I'll confirm when I have a chance and write a test for this case if one doesn't exist!

from markdown-draft-js.

from markdown-draft-js.

Closing as the related PR has been merged 🙂

from markdown-draft-js.

@danielsnider I released your fix under 2.2.0 🙂 👏 🎉

from markdown-draft-js.

Thank you! 🎉🎉

from markdown-draft-js.

@Rosey It was a pleasure to help. Thanks again for this great project.

from markdown-draft-js.