Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removal of whitespace-only text nodes #22

Closed
dmfrancisco opened this issue Jan 22, 2018 · 1 comment
Closed

Removal of whitespace-only text nodes #22

dmfrancisco opened this issue Jan 22, 2018 · 1 comment

Comments

@dmfrancisco
Copy link
Contributor

Thank you for creating and maintaining this useful library 🙌

I noticed that whitespace was not being preserved between tags. After reading the code I realize that this is on purpose and done by this code (in the browser version):

else if (node.nodeType === NodeTypes.TEXT) {
  return node.textContent.trim() === '' ? null : unescape(node.textContent);
}

Personally I found this unexpected. For example:

convert("<span>hello</span> <span>world!</span>")

Renders to:

helloworld!

But I was expecting:

hello world!

Another example I noticed on a code block that had been syntax highlighted:

convert("<pre><span>Hello</span>\n<span>World</span></pre>")

Renders to:

HelloWorld

But I was expecting:

Hello
World

For my application I created a fork where I'm simply not trimming the whitespace. Let me know if you agree with the expectations and think this behavior should change. Thanks again 👍

@pveyes
Copy link
Owner

pveyes commented Jan 23, 2018

You're right, I forgot the exact reason why I decided to trim the whitespace for text nodes but I think it make sense to not remove it. Now that you bring it up I think I'll add pre-processing for text node (so we can still trim before rendering)

Would you create PR for this changes? I've seen your fork but I'd suggest some changes in the test: remove the whitespace caused by template literal for existing test (so the existing snapshot doesn't need to change)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants