search

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

Add `default_layout` so we can use it for all forms #550

Closed duleorlovic closed 4 years ago

duleorlovic commented 4 years ago

Move process_options from helper since we can not override helper. When someone wants to extend BootstrapForm::FormBuilder and use it in form_with .... builder: MyFormBuilder, before this change he could not use in this way since determination whether it is :inline :horizontal was done in helper.

bootstrap-ruby-bot commented 4 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):

  * [#550](https://github.com/bootstrap-ruby/bootstrap_form/pull/550): Add `default_layout` so we can use it for all forms - [@duleorlovic](https://github.com/duleorlovic).

Generated by :no_entry_sign: Danger

duleorlovic commented 4 years ago

The change in test is only about adding class new_user which rails adds automatically for form_for syntax if html: { class: } were not given in apply_form_for_options (please note reverse_merge and actual source for new_name class).

So I need to change only those tests that were using layout: :inline option for form_for since now we add form-inline in builder (not as html: { class: 'form-inline' } option for form_for).

If you like, I replaced those tests with newer form_with syntax and than we do not have those additional classes. I will revert last commit if form_with syntax is not needed.

I think it is safe to merge since this pull request will only add new_user class for layout: :inline forms (so form will have new_user form-inlne instead of form-inline class) and only for old form_for syntax.

duleorlovic commented 4 years ago

I reverted Use form_with syntax since is it not available on Rails < 5.1...

lcreid commented 4 years ago

I am comfortable that the change in test cases introduced by the PR is in fact the way they should be. If we break any existing users' code (which I think will be in relatively few cases), I'm comfortable defending it as a fix to code that was broken.

@duleorlovic I'll merge this now, but please confirm that we don't need to change any documentation (I don't think so, but please forgive me for double-checking.)

duleorlovic commented 4 years ago

I think example in README is enough https://github.com/bootstrap-ruby/bootstrap_form/pull/550/files#diff-04c6e90faac2675aa89e2176d2eec7d8R624