Errors on helper uses weird class names

[cseelus]

The errors_on helper outputs a div with alert and alert-danger classes, typically used for Rails flash messages.

Why not use the invalid-feedback class name, like with other form errors?

Could do a PR, if there is a consensus on this change, alternatively add an option to change those class names.

[lcreid]

Thanks for taking the time to create an issue. You make an good point. I don't use errors_on myself, so I hadn't really considered it.

On of the challenges we have is maintaining backwards compatibility for all the existing applications that use this gem. I'd be happy to take a PR, but I'd like to hear how we can use the consisten classes, without breaking existing applications. Perhaps a configuration option, but I'll leave the thinking up to you.

I suggest you throw some questions of ideas in a reply here so we can discuss them before you do a PR, but I'm sure we can some up with something so you can prepare a PR.

Thanks again for your suggestion and interest in bootstrap_form.

[cseelus]

To keep it backwards compatible (within the current major version?), yes I too think the only way is to add another configuration option (besides the hide_attribute_name one).

My suggestion would be to call it inline (e.g. inline: true) in reference to the default inline error message.

So to get the default inline error output, one would have to use it like this:

<%= f.errors_on :tasks, hide_attribute_name: true, inline: true %>

Calling that option inline might be a bit redundant, as the docs state this helper is for custom inline errors, so another option would be to call it feedback, in reference to the default inline errors.

[lcreid]

Thanks for your patience with my slow reply. And thanks for the suggestions. I'm now thinking that we probably just completely overlooked the errors_on method when Bootstrap 4 changed its error classes. I still don't think we should break Bootstrap 4 applications that use bootstrap_form, but we should make the default consistent for Bootstrap 5.

I was thinking more like a global configuration option you could set to change the classes for errors_on. Something like what was done in #562 . That way you don't have to specify it on each use of errors_on. Then, for Bootstrap 5, we can make the default classes for errors_on to be the same as all the other error cases.

What do you think?

[cseelus]

Ah no problem, we all have lots of stuff to do.

I like your suggestion to use the default error classes for errors_on with Bootstrap 5: Should I open a PR for that?

I'm just not sure if implementing a global config option for errors_on with Bootstrap 4 is really meaningful at this point in time, with the final release of Bootstrap 5 imminent and me seemingly being the first one to even notice that inconsistency.

[lcreid]

If you're cool with doing this only for Bootstrap 5, so am I. Go ahead with the PR for Bootstrap 5. Note there's already a branch bootstrap-5, so make sure you do your PR against that branch. We might still consider a configuration option to allow people to revert to the old classes, but we can do that in another PR. The new ("correct") class names should be the default for errors_on.

Thanks again for your help!

[donv]

Looks like this was resolved in #580 .