ministryofjustice / govuk_elements_form_builder

Form builder helper methods to develop GOV.UK elements styled applications in Ruby on Rails
https://govuk-elements-rails-guide.herokuapp.com/
MIT License
6 stars 14 forks source link

Change convention to localise radio input choices #67

Open lostie opened 7 years ago

lostie commented 7 years ago

Solves https://github.com/ministryofjustice/govuk_elements_form_builder/issues/63

This allows us to be able to configure both the locale for the attribute label and the labels for each of the choices, e.g:

en:
  helpers:
    label:
      person:
        location: Location Label
        location_choices:
          ni: Northern Ireland
          isle_of_man_channel_islands: Isle of Man or Channel Islands
          british_abroad: I am a British citizen living abroad

Whereas before it was not possible to do this as the location key was being used to define the list of choices, so we could not define the actual label for the location attribute itself.

aliuk2012 commented 7 years ago

@robmckinnon @misaka @alan @ministryofjustice/ruby Could someone review this PR please?

jsugarman commented 7 years ago

LGTM 👍

csutter commented 7 years ago

It's a breaking change for existing apps isn't it? In that case can we hold off a sec? Happy to have a closer look on Monday when I'm back in the office.

lostie commented 7 years ago

@csutter Yes, it will be a breaking change for the existent apps, but we can flag this in the versioning (bump the major version, which I haven't done yet, intentionally).

Also, the breaking changes will happen only if you actively update the gem in the applications.

I'm happy to get it reviewed on Monday, regardless ;)

robmckinnon commented 7 years ago

The gem already lets you set a radio button question as the fieldset legend text, as documented here: https://govuk-elements-rails-guide.herokuapp.com/form-elements#form-stacked-radio-buttons

screen shot 2016-11-18 at 21 22 10

screen shot 2016-11-18 at 21 29 26

@lostie Can you use the fieldset legend to set the question for a radio button fieldset, instead of the approach in this PR?

lostie commented 7 years ago

@robmckinnon I was already using that (as described).

This issue appeared once I got an error in the radio button fieldset (e.g maybe an unusual case, but none of the options was selected by default).

The error section, as it stands, doesn't display the name of the field correctly.

csutter commented 7 years ago

@lostie I see what you mean. Usually we override the error helper to get more granular control over the message like so:

activemodel:
    errors:
      models:
        challenged_decision_form:
          attributes:
            challenged_decision:
              inclusion: Select whether you have challenged the decision with HMRC first

(c.f. https://github.com/ministryofjustice/tax-tribunals-datacapture/blob/master/config/locales/cost.yml#L165)

This does require you to set errors.format to exclude the field name though:

en:
  errors:
    format: "%{message}"

... which means that you will need to manually specify all error messages for every field across the app.

I think it would be good for us to have a think about whether that's still the best way of doing this. /cc @robmckinnon @aliuk2012