could you add an optional "options" parameter to include-css?

[jlgeering]

(include-css & styles) => (include-css options & styles) like (xhtml options & contents)?

this would allow us to write

(include-css {:media "screen, projector, print"} "/css/app.css")

instead of

[:link {:href "/css/app.css" :media "screen, projector, print" :rel "stylesheet" :type "text/css"} ]

cheers JL

[asmala]

Based on a quick look at the source, this should be fairly easy to implement. Not sure which direction James wants to take Hiccup, but something like this should do the trick:

(defn include-css
  "Include a list of external stylesheet files."
  [attr-map & styles]
  (if (map? attr-map)
    (for [style styles]
      [:link (merge {:type "text/css", :href (to-uri style), :rel "stylesheet"}
                    attr-map)])
    (apply include-css {} attr-map contents)))

Depending on how important performance is, this could also be implemented as a macro—a là xhtml in the same namespace—for compile time resolution of the (if (map? attr-map)) "dispatch".

And for backwards compatibility, it might be a good idea to add support for the degenerate case of (include-css), i.e. a call with no arguments.

[weavejester]

Or just change defn to defelem, which automatically adds the optional attr-map.

[asmala]

That wouldn't work since include-css (and include-js) can both return multiple elements. In this case the optional attributes would only be applied to the first of the returned elements.

[weavejester]

Ah, quite right. So something like defelem but which works for a list of elements.

[asmala]

In that case, the two main options seem to be:

1. Change include-css and include-css work as if they were created using defelem
2. Change defelem to add the optional attr-map to all elements in case the function body returns a seq of elements
3. Create a separate defelemseq to deal with function bodies returning element sequences

Option 1 seems rather limited and option 3 contorted so my preference would be for option 2, but that does break backwards compatibility in one edge case:

(defelem titles [& titles]
  (for [title titles]
    [:h1 title]))

(titles {:class "main-headline"} "Latest news" "Yesterday")

;With current implementation
;=> ([:h1 {:class "main-headline"} "Latest news"]
;    [:h1 "Yesterday"])

;With suggested implementation. Note attributes on second element.
;=> ([:h1 {:class "main-headline"} "Latest news"]
;    [:h1 {:class "main-headline"} "Yesterday"])

I can take a stab at a patch if you think one of the above would work. (Speaking of which, would you mind taking a look at #68? :wink:)

[asmala]

I implemented this in the 2.0 branch. Page includes now use defelem and defelem applies attrs to seqs.

[bostonaholic]

Any idea when allowing {:media "screen"} for include-css will be released?

[weavejester]

Hiccup currently isn't that far up my priority list, I'm afraid. It might be a while until I get around to completing the 2.0 branch.