Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move errors inside input-group and form-group. #434

Merged
merged 24 commits into from
Feb 23, 2018

Conversation

lcreid
Copy link
Contributor

@lcreid lcreid commented Feb 19, 2018

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.)

This PR is a second attempt to address this issue. It's mutually exclusive with PR #422, the first attempt.

The changes introduced by this PR can be summarized as:

  • The code to handle the :append and :prepend options is now called from inside form_group
  • For the helpers that didn't call prepend_and_append_input, a method was created to strip the :append and :prepend options before the form_group is called. This is kind of ugly, but if issue :append and :prepend not applied to collection selects. #428 is resolved by deciding that :append and :prepend should be available to all or almost all helpers, the method for stripping options will be less of an issue
  • Tests were changed to move the help and error text inside the enclosing div

in PR #422, there was one test in bootstrap_form_group_test.rb that was commented out. The approach in this PR does not comment out that test. With this approach, users' code should work without change in all cases.

Fixes #417.
Fixes #395. This PR should cover the last issues around the changed format for check boxes and radio buttons.

@bootstrap-ruby-bot
Copy link

bootstrap-ruby-bot commented Feb 19, 2018

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

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

Generated by 🚫 Danger

Copy link
Contributor

@mattbrictson mattbrictson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! ✨

I'll merge #433 first, then check back here to make sure there aren't any conflicts.

@mattbrictson mattbrictson merged commit 3eaa1ac into bootstrap-ruby:master Feb 23, 2018
@lcreid lcreid deleted the 417-input-group-b branch February 25, 2018 19:29
lcreid added a commit to lcreid/rails-bootstrap-forms that referenced this pull request Jun 4, 2018
* Test case for bootstrap-ruby#417.

* Undoing mess created on wrong branch.

* Remove test that accidentally leaked in from another branch.

* Working but needs cleanup bootstrap-ruby#417.

* Partially cleaned up.
Added method for error and help and optionally prepend and append.

* Remove unneeded method and parameters.

* Test selects error.

* Test selects error.

* time_zone_select works for new error placement.

* Add tests for date, time, datetime selects.

* Add tests for date, time, datetime selects.

* date, time, datetime tests pass with old implementation.

* Date, time, and datetime for new implementation.

* Tests for file and collection selects.

* File and collection selects with new implementation.

* Keep tests but revert lib to try another approach.

* form_group tests passing.

* Clean up and add back commented test.

* checkbox and radio button tests still have old behaviour.

* Clean up and block append and prepend.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

invalid-feedback should be inside input-group [v4] Rendering of checkboxes and radios changed
3 participants