wrapper_class is not overridden in check_box helper

[aydinkazim]

Hello,

I'm experiencing an inconsistency with the wrapper_class option when using the check_box helper in the bootstrap_form gem. For most form elements like text_field and file_field, specifying a wrapper_class will override the existing wrapper class with the provided one. However, with check_box, the specified class is appended to the existing wrapper class instead of overriding it.

Here’s an example of the code I’m using:

@f.check_box(:remove_image, wrapper_class: 'yamtar')

---

In this case, the check_box wrapper class is not overridden by 'yamtar'. Instead, it adds 'yamtar' to the existing wrapper classes. This behavior is inconsistent with other helpers like text_field and file_field, where the wrapper class is fully overridden.

Is this the intended behavior for check_box, or could it be a bug? It would be helpful if the check_box behavior matched that of the other helpers for consistency.

Thank you for your help! ❤️

[lcreid]

Thanks for reporting this, and thanks for your patience. Until recently, I haven't had any time to work on the gem.

I would also note that there's an inconsistency even with radio buttons: check boxes have mb-3 and radio buttons don't.

I think there may be other examples where sometimes we override all of the default classes, and sometimes we append them.

I would like to make them consistent, but we have to worry about backwards compatibility. We haven't really addressed that problem, so let me think about it a bit.

[lcreid]

I think the logic of not overriding form-check is because the Bootstrap mark-up would be broken without it. I don't think that's the case with most of the other input types. (Although it was true for Bootstrap 4, IIRC.) I wouldn't want to change the behaviour of wrapper_class on check boxes and radio buttons, because it's been like that a long time.

The mb-3 was introduced in #638 about three years ago. I wouldn't want to remove it outright, but I would make the case that any classes on the wrapper other than the ones required for check boxes and radio buttons would be removed before appending the custom class.

It turns out, the wrapper class for most of the other input types is mb-3 and it's removed if you specify a custom class. So I think we can say that wrapper_class should remove the mb-3 for check boxes.

What do you think, @aydinkazim (and @donv )?

[donv]

I strongly prefer to leave this version as it is, and consider introducing breaking changes to the next major version.

When we introduce the change, I would like to have two clear options: replacing all wrapper classes, and adding to the default wrapper classes. Maybe the "add" option can remove some purely cosmetic options like mb-3, not sure. Alternatively the "add" option could have logic to remove conflicting classes like "mb-2" vs. "mb-3".

[lcreid]

@aydinkazim You can try the (undocumented) option multiple: true on the check boxes. This will leave the form-check (required for Bootstrap to format a check box), but not add the mb-3. I think that will solve your issue.

[donv]

It turns out, the wrapper class for most of the other input types is mb-3 and it's removed if you specify a custom class. So I think we can say that wrapper_class should remove the mb-3 for check boxes.

@lcreid This is OK by me 👍

[lcreid]

I strongly prefer to leave this version as it is, and consider introducing breaking changes to the next major version.

The challenge is that long ago, there was a decision that the major version of bootstrap_form would match the major version of Bootstrap that it worked with. Being locked into breaking changes only when Bootstrap changes version is hampering our ability to improve the experience for developers using bootstrap_form.

I'd like to explore the idea of versioned default values, similar to, but not necessarily the same as, Rails' versioned default values. The idea would be to allow us to introduce breaking changes that developers could opt-in to. We could then issue deprecation messages for the old behaviour, but we wouldn't break existing apps.

Having worked in some code bases that had many feature flags, I'm aware that this could lead us to a bit of a mess if we don't keep the number of options to a minimum. I think the ability to improve the gem is worth the effort of managing different options.

When we introduce the change, I would like to have two clear options: replacing all wrapper classes, and adding to the default wrapper classes. Maybe the "add" option can remove some purely cosmetic options like mb-3, not sure. Alternatively the "add" option could have logic to remove conflicting classes like "mb-2" vs. "mb-3".

Those are good ideas. I think any major or breaking changes should definitely have an eye on making the behaviour of the helpers an their options more consistent and predictable.