Cannot customize text for radio_button_fieldset.

[misaka]

When generating a list of radio buttons for a list of objects, I'd like a way to use a way to use different text and values for each object, e.g. when listing a bunch of teams that may be assigned to a case, I'd like the value to be the team's ID and the text to be the team's name. For example, using Rails' collection_radio_buttons method, this would be analogous to:

collection_radio_buttons :team_id, teams, :id, :name

There is already a collection_select in this project, but no collection_radio_buttons, would that be the right way to implement this? Alternatively, to keep the fieldset, we could add value_method and text_method to the options of radio_button_fieldset and default them to to_s.

[misaka]

Opened a PR with a suggested way we could fix this.

[aliuk2012]

Thanks @misaka I'll take a look at the PR. I think a better solution might be to create/re-use collection_radio_buttons.

I had mocked up a spec

describe '#collection_radio_button' do
    let(:pretty_output) { HtmlBeautifier.beautify output }
    it 'outputs radio buttons wrapped in labels' do
      @location = [:ni, :isle_of_man_channel_islands, :british_abroad]
      output = builder.collection_radio_buttons :location, @location, :to_s, :to_s
      expect_equal output, [
          '<div class="form-group">',
          '<fieldset>',
          '<legend>',
          '<span class="form-label-bold">',
          'Where do you live?',
          '</span>',
          '<span class="form-hint">',
          'Select from these options because you answered you do not reside in England, Wales, or Scotland',
          '</span>',
          '</legend>',
          '<label class="block-label selection-button-radio" for="person_location_ni">',
          '<input type="radio" value="ni" name="person[location]" id="person_location_ni" />',
          'Northern Ireland',
          '</label>',
          '<label class="block-label selection-button-radio" for="person_location_isle_of_man_channel_islands">',
          '<input type="radio" value="isle_of_man_channel_islands" name="person[location]" id="person_location_isle_of_man_channel_islands" />',
          'Isle of Man or Channel Islands',
          '</label>',
          '<label class="block-label selection-button-radio" for="person_location_british_abroad">',
          '<input type="radio" value="british_abroad" name="person[location]" id="person_location_british_abroad" />',
          'I am a British citizen living abroad',
          '</label>',
          '</fieldset>',
          '</div>'
      ]
    end
end

And started to create the method

def collection_radio_buttons method, collection, value_method, text_method, options = {}, html_options = {}, *args
   content_tag :div, class: form_group_classes(method), id: form_group_id(method) do
       content_tag :fieldset, fieldset_options(method, options) do
          safe_join([
                        fieldset_legend(method),
                        radio_inputs(method, options)
                    ], "\n")
       end
   end
end

The only thing that I got stuck on was trying to work out what class the value/text method were to be able to iterate through the collection to generate the radio buttons.

[misaka]

I took a look at that too, and looking at collection_select the thing is that it doesn't use a fieldset whereas check_box_fieldset does, so I assumed that that was a design decision ... the collection_* methods don't generate a fieldset while the *_fieldset methods do. Is that not quite right? Why the duplication of collection_select and check_box_fieldset otherwise?

[misaka]

@aliuk2012 I've added a commit that adds collection_radio_buttons. But since we have to add support for value_method and text_method, it relies on the changes to radio_inputs in the previous implementation anyway. It generates exactly the same HTML as radio_button_fieldset and misses out some of the functionality of Rails' collection_radio_buttons.

TBH I'm not convinced of the value of adding this method. I think that if we're going to add collection_radio_buttons it should be a drop-in replacement for the Rails' one, re-using the existing method like collection_select does perhaps, in which case the generated HTML looks quite a bit different from what we generate.

[aliuk2012]

I took a look at that too, and looking at collection_select the thing is that it doesn't use a fieldset whereas check_boxfieldset does, so I assumed that that was a design decision ... the collection methods don't generate a fieldset while the _fieldset methods do. Is that not quite right? Why the duplication of collection_select and check_box_fieldset otherwise? collection_select shouldn't use fieldsets because its an select list

<select>
  <option value="1"> Option 1</option>
  <option value="2"> Option 2</option>
</select>

The main idea behind the various form builder methods was to try and implement them as close to the default Rails form builder as possible and to just wrap the govuk html and css classes around the elements. *_fieldset have always felt a bit weird because it doesn't follow this pattern like all the other input type methods and collection_select.

Oh I see what you mean about whether it would add value. Sorry my tests might of needed a bit of extra work. Ideally you should be able to pass whatever value/label you like to the method http://api.rubyonrails.org/classes/ActionView/Helpers/FormBuilder.html#method-i-collection_radio_buttons

output = builder.collection_radio_buttons :location, @location, :to_s, :to_i
output = builder.collection_radio_buttons :location, @location, :to_s, :to_s
output = builder.collection_radio_buttons :author_id, Author.all, :id, :name_with_initial 

I do suspect that we would be changing radio_input method or if its maybe easier to create a new one which you pass the label and value too. Not 100% sure though

[misaka]

@aliuk2012 My point is that it's hard to replicate collection_radio_buttons fully because of it's block support. I think this should be a separate piece of work ... I agree that we should replicate Rails' interface, but for the time being we have a business need to support value_method and text_method. If we don't add them now to check_box_fieldset we'll end up having to hand-code it like we've already had to do in the Correspondence Tool.