Errors on invalid feedback

[cseelus]

Follow up on issue 579.

Took a little while, since I first wanted the project we use bootstrap_form with, to use Bootstrap 5 in a new development branch. So far the transition went smoother than the upgrade from 3 to 4, although it is far from complete.

The bootstrap-5 branch of bootstrap_form works surprisingly well already.

[bootstrap-ruby-bot]

	1 Warning
⚠️	Please update CHANGELOG.md with a description of your changes. If this PR is not a user-facing change (e.g. just refactoring), you can disregard this.

Here's an example of a CHANGELOG.md entry (place it immediately under the * Your contribution here! line):

* [#580](https://github.com/bootstrap-ruby/bootstrap_form/pull/580): Errors on invalid feedback - [@cseelus](https://github.com/cseelus).

Generated by 🚫 Danger

[lcreid]

This looks good so far. Could you please add some documentation in UPGRADE-5.0.md to describe this change and perhaps tell people what they would have to do if they can't or don't want to change their CSS from the previous version? Thanks in advance.

[cseelus]

I would add the following text to the bottom of UPGRADE-5.0.md:

---

Different behavior for errors_on helper

The errors_on helper now wraps the error message in a CSS class invalid-feedback, instead of alert and alert-danger, as before.

This will display the error as any other Bootstrap inline form error, instead of displaying it as an Bootstrap alert.

---

Could you elaborate on what you mean by "tell people what they would have to do if they can't or don't want to change their CSS from the previous version"? Then I will add this also.
As far as I understand it, this should work without any extra CSS (as with < 5.0).

[lcreid]

Could you elaborate on what you mean by "tell people what they would have to do if they can't or don't want to change their CSS from the previous version"? Then I will add this also.
As far as I understand it, this should work without any extra CSS (as with < 5.0).

I'm thinking of the (unlikely) case where someone might have deliberately styled the CSS classes differently. Then they would have to do something to get the same results as before. And if they can't do anything about it, perhaps they should be able to. Maybe we could add a class option to errors_on, where they can specify a class or classes that override the default invalid-feedback.

It may seem like an unlikely case, but I think it's good practice in general to give people a way to keep using our gem,. Another solution might be to document that if the new behaviour of errors_on is unacceptable, they can hard-code the HTML and CSS in place of using errors_on, or write their own method that replaces it. But I think adding a class argument to the method (and documenting it) feels like not too much effort. Let me know if I'm wrong.

What you wrote above is just perfect for what I originally asked for.

Thanks again for taking the time to contribute to bootstrap_form.

[cseelus]

So you mean an option like class: 'foo', that will just override '.invalid-feedback'?

Regarding the custom helper idea you asked for, I played around with the idea for a bit:

  def custom_errors_on(form, name, options = {})
    return if form.object.errors[name].blank?

    tag.div(class: 'alert alert-danger') do
      # hidden input tag is needed for the 'invalid-feedback' to be shown
      concat tag.input(type: 'hidden', class: 'is-invalid')
      concat form.errors_on(name, options)
    end
  end

[lcreid]

So you mean an option like class: 'foo', that will just override '.invalid-feedback'?

Yeah, basically something like that. BTW, I hope I'm not frustrating you, but I really don't have the cycles to think through the details, so I like to year what you propose. But, yeah, class: 'foo' is probably all we need here. (And see my comment at the end.)

Regarding the custom helper idea you asked for, I played around with the idea for a bit:

  def custom_errors_on(form, name, options = {})
    return if form.object.errors[name].blank?

    tag.div(class: 'alert alert-danger') do
      # hidden input tag is needed for the 'invalid-feedback' to be shown
      concat tag.input(type: 'hidden', class: 'is-invalid')
      concat form.errors_on(name, options)
    end
  end

Thanks for doing the work to play with this. It seems to me like adding the class option to errors_on, plus documentation, isn't very much work compared to having to tell people how to write their own method to override ours. Maybe we could just add the class option? What do you think?

[cseelus]

I've added a custom class option for this helper.

[lcreid]

Thanks. This looks good.

Could you please add a simple test case to ensure the option generates the HTML you expect? Your change is simple and I'm sure it works, but we need the test to ensure that we don't break the behaviour in the future if, for example, we do some major refactoring or clean-up of the code.

We need to tell people about the new option in the README. There's a short section for errors_on. Just mention the option and perhaps give a quick example (maybe grab the test case).

Thanks again for taking the time to make this PR. We're almost there.

[cseelus]

Now that the approach seems to be ok, I added tests and more info about this change to the docs and into the changelog.

[lcreid]

Looks great! I'll resolve the merge conflict and merge it. Thanks again for all your work on this.

[lcreid]

Nice. Clear and to the point. I like it.

[lcreid]

Again. Very clear.

[lcreid]

👍