bootstrap-ruby / bootstrap_form

Official repository of the bootstrap_form gem, a Rails form builder that makes it super easy to create beautiful-looking forms using Bootstrap 5.
MIT License
1.64k stars 352 forks source link

Errors on invalid feedback #580

Closed cseelus closed 3 years ago

cseelus commented 3 years ago

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 commented 3 years ago
1 Warning
: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 :no_entry_sign: Danger

cseelus commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

cseelus commented 3 years ago

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