Skip to content

[WIP] Bootstrap v4#315

Merged
mattbrictson merged 10 commits intomasterfrom
bootstrap-v4
Jan 7, 2018
Merged

[WIP] Bootstrap v4#315
mattbrictson merged 10 commits intomasterfrom
bootstrap-v4

Conversation

@mattbrictson
Copy link
Copy Markdown
Contributor

This is our official PR that consolidates all changes needed to support Bootstrap v4.

This PR is a work in progress. It may or may not be merged, depending on whether we decide to publish this as a separate version or even spin it off as a separate gem. If you have ideas or feedback on these logistical topics, please join the discussion here: #268

If you would like to try this preliminary Bootstrap v4 support in your Rails project, use the following Gemfile entry:

gem "bootstrap_form", git: "https://github.com/bootstrap-ruby/rails-bootstrap-forms.git", branch: "bootstrap-v4"

If you would like to contribute to this branch (i.e. fix bugs or provide wrappers for additional v4 features), please open your own PR and use rails-bootstrap-forms:bootstrap-v4 as the base branch. If and when your PR is reviewed and merged, it will become part of this larger PR.

Thanks for your help!

@GBH
Copy link
Copy Markdown
Contributor

GBH commented Jan 7, 2018

This PR probably should be closed as it's not correct for Boostrap4 beta3

Let's close this and promote bootstrap-v4 to master so we can start working on it.

@mattbrictson
Copy link
Copy Markdown
Contributor Author

@GBH good finds. Should we still merge this branch into master as originally planned, and then address these issues from there?

@mattbrictson
Copy link
Copy Markdown
Contributor Author

Rebased against latest master

@GBH
Copy link
Copy Markdown
Contributor

GBH commented Jan 7, 2018

@mattbrictson I think it's fine to merge. Let's just promote everything v4 to master so new PRs can be made.

@mattbrictson
Copy link
Copy Markdown
Contributor Author

@GBH great, thanks! 👍

@lcreid, @donv, @desheikh: any concerns before I merge this?

Copy link
Copy Markdown
Contributor

@lcreid lcreid left a comment

Choose a reason for hiding this comment

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

I (and I'm sure many other people) have been using the bootstrap-4 branch for some time. We should merge this and fix up the small differences between Bootstrap 4 when this PR was done, and Bootstrap 4 as it is now (e.g. has-danger).

Comment thread README.md
```html
<div class="form-group has-error">
<label class="control-label" for="user_email">Email</label>
<div class="form-group has-danger">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As @GBH pointed out, has-danger isn't in Bootstrap 4 anymore. We should either fix the PR, or merge and remember to fix. Whatever is easier.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I vote for merge and fix later or this is going to be sitting here for weeks.


def error_class
"has-error"
"has-danger"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TODO: Fix this class. I think we want is-invalid now, but correct me if I'm wrong.

@mattbrictson
Copy link
Copy Markdown
Contributor Author

Thanks @lcreid, I added a reminder to #361 to deal with has_danger, form-control-feedback, and form-control-label. I'll merge this now.

@mattbrictson mattbrictson merged commit 810a901 into master Jan 7, 2018
@lcreid lcreid deleted the bootstrap-v4 branch October 29, 2025 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants