Append custom custom classes to labels #397
Conversation
Before, custom classes passed in the `label` hash would override any existing classes. For example: ```erb <%= form_with model: @model do |f| %> <%= f.govuk_text_area :details, label: { text: "Some details", class: "special-class" } %> <% end %> ``` would output: ```html <!-- form markup --!> <label for="model-details-field" class="special-class"> Some details </label> <!-- form markup --!> ``` Taking into consideration how the rest of the library behaves, this behvaiour was unexpected and meant users would need to explicitly provide default label classes. This change makes use of the `HTMLAttributes` and `HTMLClasses` traits so that label elements build classes as usual —except any custom classes are appended. For example: ```erb <%= form_with model: @model do |f| %> <%= f.govuk_text_area :details, label: { text: "Some details", class: "special-class" } %> <% end %> ``` would now output: ```html <!-- form markup --!> <label for="model-details-field" class="govuk-label special-class"> Some details </label> <!-- form markup --!? ``` This change in behaviour will affect users of the gem who are customising label classes but _don't_ want default classes to be applied. I'm not sure if that is something we need to worry about though, since the form builder is intended to build GOV.UK (or other brand) forms.
👷 Deploy Preview for govuk-form-builder processing.
|
|
Hey @peteryates I'm lucky enough to be back using this wonderful gem, and I found myself banging my head just now trying to figure out why all the default classes were being overriden 😆. Is there a reason the existing behaviour was the way it was? I'd also be interested to know how this might impact other users. Let me know what you think, and if you need anything from me! |
|
I think I just missed it 😞 Amazed nobody's raised a bug in all that time. Will properly review and get this merged over the weekend. Thanks for the fix, glad to have you back! |
Continuing from the prevoius commit, this change makes the additional classes passed in via the hint, legend and caption hashes add to the default rather than overwrite it. This makes it more consistent with the behaviour in the rest of the library.
|
Perfect, thanks @cpjmcquillan. While reviewing I noticed that legends, captions and hints suffered from the same problem, so I followed up the commit with one that extends your implementation to them too. |
|
Nice one @peteryates, thanks. I'm not sure what your plans are for releasing this. Do you want me to test this branch on our application to check for any regressions? I'm happy to just fire it off into the world and see what happens though 😆 |
|
I'll try and get a release out tomorrow, should be able to find a bit of time. |
Before, custom classes passed in the
labelhash would override any existing classes.For example:
would output:
Taking into consideration how the rest of the library behaves, this behvaiour was unexpected and meant users would need to explicitly provide default label classes.
This change makes use of the
HTMLAttributesandHTMLClassestraits so that label elements build classes as usual —except any custom classes are appended.For example:
would now output:
This change in behaviour will affect users of the gem who are customising label classes but don't want default classes to be applied.
I'm not sure if that is something we need to worry about though, since the form builder is intended to build GOV.UK (or other brand) forms.