Feature: ability to pass options to the radio input fieldset #516

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants

sdhull commented Mar 5, 2011

Necessary for compatibility with jQuery mobile

Owner

justinfrench commented Apr 3, 2011

Hi, thanks! What part of jQuery-mobile needs to have html attributes attached to this specific element (as opposed to, say, the :wrapper_html we already offer)?

sdhull commented Apr 4, 2011

http://jquerymobile.com/demos/1.0a4/#docs/forms/forms-radiobuttons.html

"To make a horizontal radio button set, add the data-type="horizontal" to
the fieldset."

-Steve

On Apr 3, 2011, at 3:18 AM, justinfrench <
reply@reply.github.com>
wrote:

Hi, thanks! What part of jQuery-mobile needs to have html attributes
attached to this specific element (as opposed to, say, the :wrapper_html
we already offer)?

Reply to this email directly or view it on GitHub:
#516 (comment)

Owner

justinfrench commented Apr 5, 2011

Ok thanks! On one hand, this is easy to add in, even with a rewrite of this pull against the refactor branch. On the other hand, I can't see it being used much and we've already got so much bloat. The other thing on my mind is that I hope to make the actual markup interchangeable soon (less opinionated!), which makes :fieldset_whatever an awkward option name when others may implement with divs or whatever.

Let me think about if for a bit.

One option that comes to mind is that, given the recent commits that make it stupidly easy for you to create your own inputs, perhaps this is a candidate for that? Here's how it might look (might need a few more hooks in the Choices module, but it'll be close:

# app/inputs/horizontal_input.rb
class HorizontalRadioInput < Formtastic::Inputs::RadioInput
  def choices_wrapping_html_options
    super.merge("data-type" => "horizontal")
  end
end

# use with...
f.input :foo, :as => :horizontal_radio

Ultimately I think this is inline with my intent that there's a type of input for all your common needs (one repeatable way to do something, even if you have to make it yourself by subclassing an existing input), although you might disagree.

Keen to hear your thoughts...

Owner

justinfrench commented Apr 5, 2011

Actually, just checked, all the hooks are there for the above example to work in the refactor branch. Bundle that branch and have a play!

sdhull commented Apr 5, 2011

Yeah that sounds good. I really appreciate the time you took to look
at the pull request and think it through!

If I had known more about the refactor branch when I forked your
project, I probably just would have tried to use that. :)

So is the refactor branch ready for primetime?

-Steve

On Apr 5, 2011, at 3:18 AM, justinfrench
reply@reply.github.com
wrote:

Actually, just checked, all the hooks are there for the above example to work in the refactor branch. Bundle that branch and have a play!

Reply to this email directly or view it on GitHub:
#516 (comment)

Owner

justinfrench commented Apr 6, 2011

@sdhull yes, it's ready for local development and I'm using it in a few non-production apps I'm working on. Not 100% sure if it's ready for production apps, but I imagine your test suite can tell us more.

Definitely worth experimenting with during development at the very least. I'm about to merge into master, so I'm pretty happy.

Closing this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment