search

weavejester / hiccup

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

New elem: radio-buttons, radio-button version of drop-down #48

Closed myguidingstar-zz closed 12 years ago

myguidingstar-zz commented 12 years ago

radio-buttons share args with drop-down

weavejester commented 12 years ago

I'm not so sure about this function. The name radio-buttons could be confused with radio-button, and it doesn't represent a single element, as all the other functions in the namespace do.

myguidingstar-zz commented 12 years ago

I agree that I didn't think carefully about the function name. Will "radio-group" do? Anyway, the function is important because people think the same way about radios and dropdowns.

weavejester commented 12 years ago

"radio-group" is better, but don't open up a new pull request for it, instead just update the existing branch.

There are a few other problems that need to be solved before this can be merged (I am warming to the idea, though):

  1. Don't use defelem if you're not defining an element.
  2. Use label instead of p to label the radio buttons.
  3. make-name is already called by radio-button, so you're effectively calling it twice

This also needs a unit test to prevent regressions.

myguidingstar-zz commented 12 years ago

ok I will close this pull request and use the other.

weavejester commented 12 years ago

Please use this pull request, as it contains the history of the conversation.

weavejester commented 12 years ago

I've taken a look through the code and I can't think of a good way to group together the radio buttons with their respective labels in a way that's generic enough to include in the core library.

In Bootstrap, for instance, controls and labels are grouped together by divs with the "control-group" class, but this obviously isn't going to be universal.

I'm not certain there's a way of implementing this function in a way that is both simple and generic.