weavejester / hiccup

Fast library for rendering HTML in Clojure
http://weavejester.github.io/hiccup
Eclipse Public License 1.0
2.68k stars 174 forks source link

Allow setting arbitrary properties on the html5 root element #71

Closed algernon closed 11 years ago

algernon commented 11 years ago

Allow setting arbitrary properties on the html5 root element

Update: Reworked the patch so that it doesn't use :root-options anymore, but dissocs :xml? instead. Update#2: Another rework, uses defelem for xhtml-tags

algernon commented 11 years ago

This is the fix for #70

weavejester commented 11 years ago

I'd prefer the attributes be merged together. i.e. instead of:

(html5 {:lang "en" :root-options {:prefix "blah"}} ...)

I'd prefer:

(html5 {:lang "en" :prefix "blah"} ...)
algernon commented 11 years ago

I thought about that too, but dismissed it due to :xml? - but I suppose I could just dissoc that. I'll update the patch shortly.

(The reason I don't want to special case :prefix, is because in xml mode, one would need "xml:og" "blah" instead of :prefix "og: blah")

weavejester commented 11 years ago

Since attributes can't include "?" it should be safe just to dissoc :xml?.

This also fits in a little better with :lang, which is essentially an attribute with some special additions for XML.

weavejester commented 11 years ago

Instead of having a new xhtml-tag-with-options function, we could change xhtml-tag to use defelem so that it has an optional map of attributes. This would be more in line with other functions in Hiccup.

algernon commented 11 years ago

I didn't want to change xhtml-tag because it's not a private function, and perhaps someone somewhere relies on it not having options. I don't mind updating the patch to transform xhtml-tag instead, though.

weavejester commented 11 years ago

That's a good policy to have in general, but defelem adds an optional attribute map, by checking to see if the first argument to the function is a map. Because xhtml-tag takes a string (lang) as it's first argument, this shouldn't impact any existing use of the function.

semperos commented 11 years ago

I'm a little confused. Using hiccup 1.0.2, should this:

(html5 {:foo "bar"} [:body])

...produce this?

<!DOCTYPE html>\n<html><body></body></html>

I was expecting to see:

<!DOCTYPE html>\n<html foo="bar"><body></body></html>

...but I get what you see in the first HTML snippet above. Am I using this function incorrectly?

algernon commented 11 years ago

The feature did not make it into 1.0.2, but was merged shortly after that version was tagged and released, as far as I remember. You are using the functionality correctly, but 1.0.2 does not support it, sadly.

semperos commented 11 years ago

Understood, thanks for the response. Didn't think to check where it fell relative to the last release, since it's been a few months.

For anyone stumbling on this thread needing to accomplish this now, here's a simple workaround:

(html
  (:html5 doctype)
  [:html {:foo "bar"}
    [:head ,,,]
    [:body ,,,]])
weavejester commented 11 years ago

1.0.3 should have been released a while ago, but it looks like it slipped off my todo list. I've now release 1.0.3 that should work as expected.

semperos commented 11 years ago

Great, thank you @weavejester , it works as expected in 1.0.3.