Move validation errors for check boxes and radio buttons into last div.form-check.#448
Conversation
mattbrictson
left a comment
mattbrictson
left a comment
There was a problem hiding this comment.
This looks great. Thanks for introducing the UPGRADE doc in this PR; it should really help users of the gem.
I am going to resolve the conflicts and merge this in.
| # </div> | ||
| # HTML | ||
| # assert_equivalent_xml expected, output | ||
| # end |
There was a problem hiding this comment.
I'm going to remove this. It the comment served well for the review of this PR, but I would rather not keep comment-out code in the codebase. I'll move the reasoning for removing this into a commit comment instead.
mattbrictson
force-pushed
the
419-check-radio-errors-d
branch
from
6d34088 to
9d73dd5
Compare
This test case was removed as it was something that's too difficult to
support with Bootstrap 4.
test 'form_group renders the "error" class and message corrrectly when object is invalid' do
@user.email = nil
assert @user.invalid?
output = @builder.form_group :email do
%{<p class="form-control-static">Bar</p>}.html_safe
end
expected = <<-HTML.strip_heredoc
<div class="form-group">
<p class="form-control-static">Bar</p>
<div class="invalid-feedback">can't be blank, is too short (minimum is 5 characters)</div>
</div>
HTML
assert_equivalent_xml expected, output
end
In its place, we've recommended a workaround in the UPGRADE-4.0 doc,
which is to output the error message manually. This workaround has a
corresponding test to verify its correctness.
|
@lcreid after rebasing this PR on the latest See test failure here: https://travis-ci.org/bootstrap-ruby/bootstrap_form/jobs/384966902#L913 |
|
@mattbrictson the correction you made to the test case is correct. I know the tests were all passing when I originally submitted the PR. My git-fu is not good enough for me to figure out how the test could have been passing when I originally submitted the pull request. (Actually, I have an idea how this happened, but I'll submit this comment and follow up later.) The correction you made to the test case is in fact a change to the expected output that is a result of this PR. In other words, this PR changes the expected output, due to the way Bootstrap 4 works. As I said, the code is all good the way it is right now. |
|
Great! Thanks for double-checking 👍 |
|
FWIW: PR #454 fixed the validation error for horizontal forms. I must have broken the test case again somehow, although I still can't explain why Travis didn't complain until @mattbrictson did the final merge. At any rate, it's all right now. |
|
Does this solve inline radio buttons? The formatting is wonky when you use error_message on the last radio.
|
3 participants
Bootstrap 4 validation errors for groups of check boxes or radio buttons are supposed to be inside the
<div class="form-check">of the last check box or radio button in the group. See: https://getbootstrap.com/docs/4.0/components/forms/#supported-elements.This is the third variation on a solution to this issue
form_groupthat rendered text other than frombootstrap_formhelpers, but was never completely successfulform_groupblocks rendering arbitrary text. This PR is heavily based on the approach used in PR Move errors inside input-group and form-group. #422, although it's actually a reimplementationSee #436 (comment) and the comments that follow it for why we decided to take the approach in this PR.
This PR also includes a UPGRADE-4.0.md document describing breaking changes and work-arounds that result from this PR.
Fixes #418.