Om: dealing with rampant empty span tags

[rabidpraxis]

I have jumped on this React bandwagon and love that you have brought over enlive to this space. One thing that has bothered me while using kioo is the insane amount of empty span tags it was producing. I poked around and found that the problem is with enlive and its parsed html output.

This is my solution to clean it up. I am a relative Clojure newbie so I'm not sure if this is the most idiomatic way to accomplish this. I also did not dig into enlive to ensure that it did not have an option for cleaning up the output.

I could not get your tests to run (n00b), so I am not sure if this passes.

Thanks!

[ckirkendall]

I had something similar when I first started but ran into some problems. I need some noodle time on this because removing white space can change the intended output to the page. Things like pre tags and css white-space: pre or pre-wrap can cause issue. Since we can't see the css I have a hard time removing the white-space. It might be possible to do this to an extent when we know the odds of it being an issue are low.

[rabidpraxis]

Humm, yes this is a good point.

It makes sense to ignore pre tags, but yeah if people are enabling whitespace via css it will do some clobbering.

Does it make sense to make it an option of component/deftemplate?

[rabidpraxis]

Technically, I don't think the spans do anything negative (maybe performance if you have a ton) they just make it difficult to weed through while inspecting.

[ckirkendall]

Jared, I like the idea. I need to think about this a bit as I want the configuration to be controlled per component/snippet/template.

Creighton

On Thu, May 8, 2014 at 12:30 AM, Jared Lobberecht notifications@github.comwrote:

This might be better handled by giving Enlive a modified TagSoup parser, with the ignorable whitespace option turned off.

https://github.com/cgrand/enlive/blob/master/src/net/cgrand/tagsoup.clj#L18

Making this configurable from Kioo would mean adding, perhaps, a :parserkey to the emit-opts argument, in kioo.core/component*.

— Reply to this email directly or view it on GitHubhttps://github.com/ckirkendall/kioo/pull/20#issuecomment-42512464 .

[andrew-nguyen]

Been following this and just encountered an issue with the extra spans when trying to integrate a third-party template. I pulled a template from wrapbootstrap and many of their CSS selectors work only on the first child. As a result, the extra spans cause a lot of funkiness. I'm currently working to "fix" the CSS to be a little more tolerant but being able to address the span issue would make it easier to integrate other themes/templates in the future.

[ckirkendall]

I am going to spend some time on this over the next week or so. My goal is to provide an options map as an additional parameter to component, snippet and template. The only option to start will be suppress whitespace spans.

[danielytics]

@ckirkendall Any update on this?

@rabidpraxis I don't think its completely harmless. I've hit similar issues like what @andrew-nguyen mentioned and its forced me to rewrite some components to not use kioo. I also seem to get a lot of Warning: Only React Components are valid for mounting. output in the js console when using kioo and it seems to be related to the spans, though I'm not 100% certain. I've been ignoring these warnings so far, but would really like to get rid of them before a proper public release of my app, if possible.

I'll have to do some digging to confirm that it is the spans and not something else, but my cursory look suggests that it is (but I'm unsure why this would be the case).

[ckirkendall]

@rabidpraxis @andrew-nguyen @danielytics This is proving to be much harder than it looks on the surface. In many many cases whitespace is significant in HTML. These empty spans are created by react to encapsulate the strings contained in a sequence of children. Kioo doesn't create the spans it just sends to react strings representing the text nodes. If I remove all white-space only strings it can drastically change the rendering because the default in HTML is to replace with a single space. The only true solution would be for react not to encapsulate white-space only strings with spans but I doubt they are going to go for that. I am open to any ideas how to improve this. I have started on a means to make it configurable but it feels very dangerous given its affects on rendering.

[andrew-nguyen]

@ckirkendall Thanks for the update and details on what's going on. I'll have to think about this a bit and try some experiments.

FYI, found this over on the reactjs google group as of a week ago:

Unfortunately there's no way to avoid the span; this is currently a technical limitation that we hope to lift sometime. If a

 tag works for you, that's great; you could also try using tags like  or  (or anything, really), depending on what styles you have.

Original question:

It seems that react js creates a "span" tag around the text (this happens even if I put the text fixed in there instead of using a variable) which messes with the look and feel, since the style applied doesn't work with that added span (fyi, this style works on the h4 and span elements related to it). I cannot modify the style, since it's being used in other applications and this project with react js is a new one.

Is there a way to avoid this extra tag? For now I replaced the with a

 tag and the look and feel is the expected one but I am not sure if this is correct.

[ckirkendall]

@andrew-nguyen @danielytics @rabidpraxis I need some help testing this. I opened up compile options to the definition of template and snippets. You can pass compile options to the compiler and I have a helper option of supress-whitespace in kioo.core. There are some gotchas that exist when exposing this. Symbol references must be fully qualified and from a clojure namespace not a clojurescript namespace because they are eval at compile time. Below is a basic test.

whitespace.html

<div>
   <span>test</span>
<div>

with whitespace

(defsnippet tmp [:div] [] {:span (content "cool")})

output

<div><span>
</span><span>cool</span>
<span>
</span>
</div>

supressing whitespace

(defsnippet tmp [:div] [] {:span (content "cool")} kioo.core/supress-whitespace)

output

<div><span>cool</span></div>

[andrew-nguyen]

@ckirkendall I haven't done any extensive testing but just added this to my project (without any other changes) and so far everything is looking much better. I'll update if I come across any issues.

[worrel]

@danielytics and other, just to clarify, the warning about "only React components are valid" is unrelated to Kioo. I get it with stock Om if I upgrade the react.js dependency to 0.10.0 (most examples & lein templates give you the 0.9.0 script). Seems FB added something (isValidComponentDescriptor) that is checking for specific methods on the prototype of the component constructor (I think), that are not there when using Om.

[danielytics]

@worrel Ah! I think I may have upgraded my React about the time I started using Kioo - that explains why I started seeing those messages.

[andrew-nguyen]

@danielytics @worrel @ckirkendall I totally missed the original comment until today but as of right now, I have been using the supress (sp?) whitespace feature and do not have any warnings or errors.

[jessesherlock]

I also just updated my templates to use supress-whitespace and it has fixed the layout problems those damn spans were causing and it hasn't caused any problems for me (except being very confused for a short time until I noticed I had corrected the typo in "suppress" when I typed it out).

Since I agree with @ckirkendall that this is a bit iffy and could cause problems I played with another approach that I think is a better solution. I replaced the call to enlive's html-resource with a wrapper that runs the file through HtmlCompressor (https://code.google.com/p/htmlcompressor/) first. No need to re-implement the corner cases of whitespace handling in HTML, it only strips the whitespace caused by template formatting and doesn't touch any whitespace your template functions added.

It seems to be working well and I can configure what type of whitespace to strip out (and I trust it handles things like pre tags and other complications properly).

You can have a look at that here: jessesherlock/kioo@9df89739fa7911f2b98c6afe95f0b22fbac72226

I think I will rip this functionality out into a library that uses enlive's get-resource multimethod and a function to wrap the name of the template instead of forking Kioo. This was more of a quick and dirty test of this approach.

I really need this functionality to work properly as I both have to have my html templates formatted in a readable way and I can't have those single space spans throwing off my layout.

Any and all suggestions are very welcome.

[andrew-nguyen]

@jessesherlock I don't have any experience with google's html compressor but just poked around their documentation a little bit. Their leading intro is:

HtmlCompressor is a small, fast and very easy to use Java library that minifies given HTML or XML source by removing extra whitespaces, comments and other unneeded characters without breaking the content structure.

That sounds like it might actually work really well. I'll keep an eye out for your library.

[ckirkendall]

@jessesherlock I will certainly take a look at this. I like the idea of something that can remove irrelevant white space across the board as long as it's smart enough to handle the edge cases.

[jessesherlock]

Looked into it deeper today:

htmlcompressor does handle all edge cases I can see but the edgiest one: the css 'white-space' property (https://developer.mozilla.org/en-US/docs/Web/CSS/white-space) can be set on any element. That can't be handled at any time other than in-browser reflow so that seems reasonable. I can't think of a use case involving setting that css property on a node that came from a template anyway.

I also looked at the kioo code more closely and realized that enlive's get-resource multimethod can't be used since it's being called inside kioo.core/component* at macro-expansion time (duh) and so only strings would work as an argument in that context.

I implemented a type and enlive's get-resource/register-resource! multimethods anyway so i could try it out in enlive templates but once I started playing with it I think a better api would be to wrap the deftemplate in a 'without-spaces' macro or have a defcompressedtemplate or something similar instead of wrapping the path argument.

I'm not clear enough on the specifics of cljs nested macros to tackle that one right now, but it's bothering me enough that I will probably pick it up again in the next few weeks and learn the ins and outs of cljs macros and how to debug them.

[jessesherlock]

@ckirkendall @andrew-nguyen Right after I typed out that last comment I realized that deftype creates a reader macro constructor and I figured that would work properly with Kioo's use of html-resource.

It does, so I cleaned that up and put it up at https://github.com/jessesherlock/enlive-ws.

It's working great for my stuff but I haven't tested it much. I don't think this is a permanent solution but it works right now (requires Kioo 1.4.1-SNAPSHOT if memory serves) and it's just a library rather than a fork.

[ckirkendall]

@jessesherlock I am incline to make the compressor resource the default resource and provide the the ability to override the resource definition if someone wants the whitespace. I feel this would give the best user experience and still provide the odd case where someone wanted the whitespace. How do others feel about that? @andrew-nguyen @danielytics @worrel @rabidpraxis

@jessesherlock thanks for working this out!

[ckirkendall]

I am closing this pull request now that white space suppression is now the default with enlive-ws.