Move errors inside input-group and form-group.

[lcreid]

It looks like Bootstrap 4 specifies that errors should be wrapped inside the input or form group (see twbs/bootstrap#25020 and https://getbootstrap.com/docs/4.0/components/forms/#validation). This PR moves the error (and help) text inside the input-group and form-group.

(Based on the examples in the Bootstrap docs, the error and help text doesn't have to be inside an input or form group, but they do have to be within the same container as the input or select, and label (if any) of the control.)

The changes introduced by this PR can be summarized as:

• Since the majority of helpers called a method (prepend_and_append_input) to handle the :append and :prepend options, the error and help code was moved from form_group to prepend_and_append_input
• For the helpers that didn't call prepend_and_append_input, an "extract method" was done on prepend_and_append_input to separate out the code that makes the control, and optionally the error and help text (control_error_help). The helpers were modified to call control_error_help
• Help and error messages were added to check_box and radio_button methods, as they fit neither of the above models
• Tests were changed to move the help and error text inside the enclosing div

I couldn't see another way to move the error text inside the input group, without major changes to form_group and form_group_builder.

Because in effect the error handling is pushed down to the individual helpers, it's not longer possible to get an error message from form_group when the contents of the block that form_group calls don't include a call to a bootstrap_form helper. There is one test in bootstrap_form_group_test.rb that has a form_group around a p tag, and checks for the error message. This test has been commented out in this PR.

NOTE: This PR contains the tests in PR #424, because the tests were needed to ensure this PR didn't introduce regressions.

Fixes #417.
Fixes #418.

[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):

* [#422](https://github.com/bootstrap-ruby/bootstrap_form/pull/422): Move errors inside input-group. - [@lcreid](https://github.com/lcreid).

Generated by 🚫 Danger

[GBH]

I would render @builder.text_field :email, prepend: "foo", append: "bar" instead of doing it though form_helper. Less html that way. All tests should just call helpers directly.

[lcreid]

That's how I started, but then it renders the default Rails error markup. @builder is missing the bootstrap_form override of the default Rails error rendering. I was going to enter an issue to change the way we make the builder, so it would render properly without the extra HTML, which I agree is a unnecessary and can be a pain.

[GBH]

You can put code used for error suppression (https://github.com/bootstrap-ruby/bootstrap_form/blob/master/lib/bootstrap_form/helper.rb#L45) into setup and teardown for tests that need it. You'll want to extract all tests that deal with error rendering into a separate file though.

[lcreid]

That's a good suggestion. In the spirit of limiting the scope of each PR, I continued to just tolerate a few extra lines of HTML in each test, but we should revisit the suggestion for easier error testing in another PR.

[lcreid]

The latest push gives better test coverage, so the PR is ready for review.

[mattbrictson]

It took me a while to fully understand the issues being addressed here. Thanks for linking to the bug reports; that helped. This PR looks like a reasonable solution to the problems.

I am concerned that form_group no longer outputs errors. This will probably trip up a lot of people who were using form_group to wrap custom controls.

Would it be fair to say that people could get the old behavior by using form_group plus control_error_help?

[mattbrictson]

Is this FIXME still valid? If so, could you clarify what additional cases need to be handled here?

[mattbrictson]

Also, do static_control and custom_control lose their ability to display errors as a result of this PR?

[lcreid]

Thanks for the feedback, @mattbrictson . I'm going to take another look at how this could be done without having to move the error reporting out of form_group. There are two challenges that I was running in to:

• The way the radio buttons and check boxes are used, in some cases stand-alone and in some cases as a group inside a single form group
• The new concept of the input group, and the fact that the errors have to be wrapped inside the input group

Feel free to ask more questions or give suggestions. I'll make another branch locally and see if I can find a way to keep the error reporting behaviour more consistent with the old way, while still supporting the new requirements. If I figure something out, I'll push a different PR so you can see which makes more sense.

[mattbrictson]

Closed in favor of #434