Skip to content
This repository was archived by the owner on May 5, 2020. It is now read-only.

Allow value and text method options to radio_button_fieldset (#82) #83

Conversation

misaka
Copy link
Contributor

@misaka misaka commented Mar 27, 2017

NOTE: There are two changes here, one changes the existing radio_buttons_fieldset and the other adds a collection_radio_buttons method. See the discussion happening in issue #82, my proposal is that we just make the change to radio_buttons_fieldset for now and drop collection_radio_buttons until we can do it properly.

When generating a radio-button fieldset, there are times when we want to specify
how to get the form value or label text for the choices passed in, the same way
collection_radio_buttons does in Rails. For example, when assigning a team to
a person we may want to use the team's DB ID as the value and the team's name as
the radio button text. This change adds these options to
radio_button_fieldset (adding them to other places, such as
check_box_fieldset, is an exercise for later).

This addresses issue #82.

…yofjustice#82)

When generating a radio-button fieldset, there are times when we want to specify
how to get the form value or label text for the choices passed in, the same way
`collection_radio_buttons` does in Rails. For example, when assigning a team to
a person we may want to use the team's DB ID as the value and the team's name as
the radio button text. This change adds these options to
`radio_button_fieldset` (adding them to other places, such as
`check_box_fieldset`, is an exercise for later).
@misaka misaka force-pushed the add-value-and-text-methods-to-radio-button-fieldsets branch from c9696a2 to a5f6280 Compare March 27, 2017 22:03
@misaka misaka requested a review from stephenrichards March 29, 2017 09:15
@misaka misaka changed the title Allow value and text method options to radio_button_fieldset (#82) WIP: Allow value and text method options to radio_button_fieldset (#82) Mar 29, 2017
@misaka
Copy link
Contributor Author

misaka commented Mar 29, 2017

@aliuk2012 Just to check, this PR has both changes in it, the addition of value_method and text_method to radio_buttons_fieldset AND the addition of a new method collection_radio_buttons. Are you approving the addition of both of these?

@aliuk2012
Copy link
Contributor

Could you strip out the collection_radio_button method and move that into its own branch? If we log an issue and reference that branch we can then look at adding that method as part of another feature.

Copy link
Contributor

@stephenrichards stephenrichards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, lets get radio_button_fieldset merged, and put the collection_radio_buttons into a separate PR for further discussion.

@misaka misaka force-pushed the add-value-and-text-methods-to-radio-button-fieldsets branch from 9d8ffdf to a5f6280 Compare March 29, 2017 15:00
@misaka
Copy link
Contributor Author

misaka commented Mar 29, 2017

Commit removed, merging.

@misaka misaka changed the title WIP: Allow value and text method options to radio_button_fieldset (#82) Allow value and text method options to radio_button_fieldset (#82) Mar 29, 2017
@misaka misaka merged commit f1b36b8 into ministryofjustice:master Mar 29, 2017
@misaka misaka deleted the add-value-and-text-methods-to-radio-button-fieldsets branch March 29, 2017 15:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants