Skip to content

Conversation

@lcreid
Copy link
Contributor

@lcreid lcreid commented Sep 11, 2017

This PR adds a new helper bootstrap_form_with to support the Rails 5.1 form_with helper (#326). I consider this PR to be more of a "proof of concept", so I hope I get lots of comments.

There are a number of new test files. Out of an abundance of caution, I chose to duplicate many of the existing tests for bootstrap_form_for and bootstrap_form_tag, but using bootstrap_form_with. In addition, for the new tests I wanted to test everything with and without specifying a DOM id as an argument to the field helpers. (The Rails helpers when used with form_with don't generate the default DOM ids like they do under form_for.)

In addition to the automated tests, I've integrated this into an application I'm building. The application is working, except for the problem mentioned in the next paragraph.

I implemented a subclass BootstrapForm::FormBuilderFormWith of the existing BootstrapForm::FormBuilder class to handle some of the new functionality. The application that I'm using as a "proof of concept" for bootstrap_form_with uses a custom form builder that was implemented on top of BootstrapForm::FormBuilder. The way I implemented the subclass is clumsy when a developer needs to further subclass the BootstrapForm form builder. I'm open to suggestions as to a better way. In the meantime, I'll try some other approaches.

desheikh and others added 19 commits February 15, 2017 14:44
)

* Support :prepend and :append for the `select` helper
Fixes bootstrap-ruby#147

* Bootstrap 4 has renamed `control-label` to `form-control-label`
The main changes were around the fact that `form_with` doesn't
automatically add class and id attributes in many places that the
other form builders did.

Added a `for:` to the label options if an ID was specified for
the input element, so that the label would get the correct value
for the `for` attribute.

Sub-classed the form builder to override methods to make `form_with`
work without the default Rails-generated DOM IDs.
The `bootstrap_form_with` helper uses Ruby 2+ syntax. I put a test
for the Rails version, so the helper is only parse if running
Rails 5.1+.
@lcreid
Copy link
Contributor Author

lcreid commented Sep 21, 2017

This latest update removes the need for the BootstrapForm::FormBuilderFormWith class mentioned above. I believe this will cause fewer problems for people who want to sub-class BootstrapForm::FormBuilder.

@lcreid
Copy link
Contributor Author

lcreid commented Oct 8, 2017

I have a branch in my own fork that back ports form_with support to Bootstrap 3. Do you want me to submit a PR against master?

Remove confusing comment.

Remove some more unnecessary diffs.
@lcreid
Copy link
Contributor Author

lcreid commented Oct 8, 2017

Sorry, folks. That last commit may have caused more problems than it solved. Let me look at this some more...

@lcreid
Copy link
Contributor Author

lcreid commented Oct 9, 2017

I think somewhere along the way master and bootstrap-4 branches diverged in a few formatting matters, mostly around whether single or double-quotes were used for strings. I believe that my last commit to this PR makes the two branches more consistent. If someone wants me to change this back, I'd be happy to. Just let me know.

@lcreid
Copy link
Contributor Author

lcreid commented Jan 8, 2018

@mattbrictson @GBH Is there a way to change this PR to merge to master, or should I just close this and submit a new PR against master?

@GBH
Copy link
Contributor

GBH commented Jan 8, 2018

I feel this PR is quite outdated. Probably easier to start a new one form master branch.

@lcreid
Copy link
Contributor Author

lcreid commented Jan 9, 2018

Closing this and opening a new PR #369 against master, per issue #361.

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.

5 participants