search

yogthos / json-html

Provide EDN/JSON and get a DOM node with a human representation of the data
MIT License
162 stars 15 forks source link

render-map broken for (not only) numeric keys #2

Closed ul closed 9 years ago

ul commented 9 years ago

This line is fine for keywords and strings, but bad for other cases https://github.com/yogthos/json-html/blob/master/src-cljs/json_html/core.cljs#L35

yogthos commented 9 years ago

ah good catch, I committed a fix and I'll push it out later today

yogthos commented 9 years ago

Ok, 0.2.4 should fix the issue. I now create a keyword from the key before calling name, so it should work for pretty much anything now. :)

ul commented 9 years ago

oh, one more problem with name — namespacing is lost! (name :x/y) => "y"

yogthos commented 9 years ago

Not sure what can be done about that one unfortunately.

ul commented 9 years ago

I don't insist, your library is great with existing functionality, but is it principal moment to not render namespaces? I think, namespaced keywords are quite common use-case. E.g., I use json-html to render current Reagent app state in debug panel, and it has a number of namespaced keys introduced by some system (in sense of Stuart Sierra's system) components (I prefer to keep almost everything in system-wide atom if it will not impact performance). Is smth. like (->> k keyword ((juxt namespace name)) (remove nil?) (clojure.string/join "/")) overcomplication for json-html goals?

yogthos commented 9 years ago

Ah that's a good suggestion, just pushed out a new version 0.2.5 with the improvement.

ul commented 9 years ago

Thank you! I hope this solution will be useful not only for me.