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

Ability to provide html attributes to fieldset legends #89

Closed zheileman closed 6 years ago

zheileman commented 7 years ago

This PR will add the ability to propagate html attributes down to the legend inner element (span at the moment) for the radio_button_fieldset and check_box_fieldset methods.

This solves an accessibility issue we were facing: legends need to exist for screen reader users, but we wanted them to be visually hidden for non screen reader users, and this was not possible with the current version of the gem.

Now, you can provide an optional hash as in these examples:

f.radio_button_fieldset :case_type, choices: [...],
  legend_options: {class: 'visuallyhidden'}

f.check_box_fieldset :waste_transport, [...],
  legend_options: {class: 'visuallyhidden'}

And the legend element will produce the following markup:

<legend>
  <span class="form-label-bold visuallyhidden">What is your appeal about?</span>
</legend>

Or, when errors found:

<legend>
  <span class="form-label-bold visuallyhidden">What is your appeal about?</span>
  <span class="error-message">Select what your appeal is about</span>
</legend>

Note any class or classes in legend_options will append to the current span class form-label-bold.

aliuk2012 commented 6 years ago

@zheileman and I had a chat about this PR and covered the following points:

I accept this PR in principle but what I think should happen is that we should "extend" the existing css class list and not completely remove it. For example for labels and text fields we have a set_labels_classes and set_fields_classes (https://github.com/ministryofjustice/govuk_elements_form_builder/blob/master/lib/govuk_elements_form_builder/form_builder.rb#L87-L98). This functionality allows devs to adds any additional css classes to those elements. We should allow the same for fieldset legends. So in this use case that @zheileman provided the markup would return something like class="forms-label-bold visuallyhidden". The legend would still be hidden but it means thats devs do not have to remember to add form-label-bold to the class list if they want to add custom css class names.

I also toyed with the idea of having some sort of option flag like "hide_legend" option in the method call but @zheileman and I agreed that this might not be the best approach.

Pros for introducing hidden flag:

Cons for introducing hidden flag: