search

dillonkearns / elm-review-html-to-elm

Turn HTML into Elm. With support for elm-tailwind-modules.
http://html-to-elm.com/
BSD 3-Clause "New" or "Revised" License
95 stars 9 forks source link

Text nodes should retain whitespace #16

Closed cllns closed 1 year ago

cllns commented 2 years ago

Amazing and helpful tool!

I ran into a small issue when copying HTML from TailwindUI into elm-to-html.com

The issue is that text nodes are run through String.trim. This makes for cleaner strings, but whitespace has semantic meaning in text nodes, which is lost.

(I could see 'fixing' this possibly generating very long trailing spaces and new lines, so maybe any whitespace could be collapsed into a single space? Not sure if that's actually tenable.)

Here is the original/expected HTML, followed by the HTML generated by Elm, after having been passed through elm-to-html.com

-<p>Faucibus commodo massa rhoncus, volutpat. <strong>Dignissim</strong> sed <strong>eget risus enim</strong>.</p>
+<p>Faucibus commodo massa rhoncus, volutpat.<strong>Dignissim</strong>sed<strong>eget risus enim</strong></p>

Expected:

image

Actual:

image
dillonkearns commented 1 year ago

Hey! Thank you for reporting the issue and finding the relevant code.

                trimmed : String
                trimmed =
                    String.trim textBody
                        |> Regex.replace
                            (Regex.fromString "\\s+"
                                |> Maybe.withDefault Regex.never
                            )
                            (\_ -> " ")

This is the current implementation (as you linked to). I wonder if the regex replace is sufficient and maybe just removing the String.trim would handle it correctly?

dillonkearns commented 1 year ago

Okay, I think this might address the issues. Here's a preview of the deployed site that you can play around with, any thoughts?

https://deploy-preview-20--html-to-elm-tailwind.netlify.app/

The one case that comes to mind that I'm not sure about is if whitespace needs to be preserved within nodes that have nothing but whitespace or if it is safe to remove all whitespace within whitespace-only nodes. Otherwise it's seeming pretty good.

cllns commented 1 year ago

Thanks @dillonkearns. Sorry I didn't look at it in a timely manner but the preview link looks great

dillonkearns commented 1 year ago

No problem, thanks for taking a look! It's live now on the site and new package release. Thanks again for reporting the issue! 🙏