-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Integration Bootstrap3 #864
Conversation
Migrating step:
OK |
@@ -50,7 +50,7 @@ def with_full_error_for(object, *args) | |||
end | |||
|
|||
test 'error should be able to pass html options' do | |||
with_error_for @user, :name, id: 'error', class: 'yay' | |||
with_error_for(@user, :name, id: 'error', class: 'yay') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't reallly required for the PR is it? You should remove this change as it will make it easier to merge.
def options_classes(klass, opts) | ||
# chech if use bootstrap3 stylesheet | ||
# replace default class with options[:class] | ||
if SimpleForm.default_wrapper == :bootstrap3 && opts[:class] && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately we can't and will not make Simple Form code dependent of twitter bootstrap. All the bootstrap configuration must be in the application code, not Simple Form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, It's ugly
but in bootstrap 3, you must control input size on div(bootstrap2 on input), so if we config a default class eg: col-lg-2
now, if you add class on input, it will use the large size not the newer.
I do not find a better way to handle this case. Do you have any advice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is the case so we should not add anything to Simple Form and let the user configure the class when calling the input
method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, It mean user must config every input.
It looks so bad for me. I like default config and user can use outbox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion it is the right and the best path. Simple Form can't and
won't be coupled with bootstrap. It is a independent project is the
bootstrap configurations are only a facility.
We have to give generic configurations like the wrappers API and the input
options.
If we have to change Simple Form every time a new bootstrap version is
released I sincerely prefer to remove all the bootstrap support.
On Aug 24, 2013 10:31 PM, "soffolk" notifications@github.com wrote:
In lib/simple_form/wrappers/many.rb:
@@ -68,6 +68,19 @@ def html_options(options)
def html_classes(input, options)
@defaults[:class].dup
end
+
# check wrappers is bootstrap3
# maybe better to if use bootstrap3 wrappers
def options_classes(klass, opts)
# chech if use bootstrap3 stylesheet
# replace default class with options[:class]
if SimpleForm.default_wrapper == :bootstrap3 && opts[:class] &&
Oh, It mean user must config every input.
It look so bad for me. I like default config and user can use outbox
—
Reply to this email directly or view it on GitHubhttps://github.com//pull/864/files#r5967279
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right! I think I can create a new repo to integration bootstrap 3 with simple_form
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed. I'm pretty sure that with the current
features is possible to use simple form with Twitter Bootstrap 3.
On Aug 24, 2013 10:49 PM, "soffolk" notifications@github.com wrote:
In lib/simple_form/wrappers/many.rb:
@@ -68,6 +68,19 @@ def html_options(options)
def html_classes(input, options)
@defaults[:class].dup
end
+
# check wrappers is bootstrap3
# maybe better to if use bootstrap3 wrappers
def options_classes(klass, opts)
# chech if use bootstrap3 stylesheet
# replace default class with options[:class]
if SimpleForm.default_wrapper == :bootstrap3 && opts[:class] &&
You are right! I think I can create a new repo to integration bootstrap 3
with simple_form—
Reply to this email directly or view it on GitHubhttps://github.com//pull/864/files#r5967304
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I must research the simple_form source code deeply for integration more nice. or someone can do it
|
||
```erb | ||
<%= simple_form_for @user do |f| %> | ||
<%= f.input :username, controls_html: {class: 'col-lg-5'} %> # => <div class='col-lg-4'><input class='string .../></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be class='col-lg-5'
instead class='col-lg-4'
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my typo!
Because I do not find the way to integration bootstrap3 without check bootstrap3 in simple_form codebase, it'll not be merged!
If you want to use it, may be checkout my integration_bootstrap3 or simple_form_bootstrap3
This is the initializer I'm using for Bootstrap 3 https://gist.github.com/adamico/6387890, more details at http://stabco.tumblr.com/post/59760641051/simple-form-bootstrap3-integration |
Nice @adamico! 👍 |
hey @adamico , how you solved the inline checkbox issue? |
Do we need to be using Rails 4 to implement the @adamico solution? In my gemfile I have: When I run bundle update simple_form I get: Bundler could not find compatible versions for gem "actionpack":
|
@caarlos0 , I updated my blog post with a working solution for inline check boxes and radio buttons : http://stabco.tumblr.com/post/59760641051/simple-form-bootstrap3-integration |
Nice! I'll try it later. Thank you very much! |
Can't get it ro work with simple form 2.1.0. it says config.input_class is undefined :S |
@andrezimpel try the master branch. |
@caarlos0 do i have to run a new (other) generator? |
not that I can recall.. |
@caarlos0 thanks, but I still get the no method error "undefined method `input_class=' for SimpleForm:Module" with the current master. |
@andrezimpel did you put this in your Gemfile?
|
Naw, my fault. I just did bundle update. Now i just have to figure out the activemodel version stuff, since I'm using rails 3.2. Thank you very much for your help @caarlos0! |
It happens 🍺 Nice to hear that. Cheers @andrezimpel |
@andrezimpel Could you please report back if you figure out how to get it working with rails 3.2? |
@MrHubble Yep. I did it this way:
I haven't ran into problems until now with that. 😃 |
@andrezimpel So far so good, using your forked version w Rails4/Bootstrap3 no problem until now too... until a definitive solution is pulled out... |
@andrezimpel your initializer gave me problems because the inputs (StringInput, TextInput and others) end up conflicting with the StringInput and TextInput of Formtastic used by active_admin |
@mattangriffel I'm have the same issue with conflicting simple_form/formtastic when overriding inputs. How did you resolve your issue? |
@mattangriffel I'm not using simple firm with formtastic. I take a look at in a couple of hours. |
Simple Form doesn't support Bootstrap 3 yet, I'm working on it at heartcombo/simple_form-bootstrap#28. Closing this is favor of that issue |
Here's a quick way to fix the checkbox issue whilst we wait for Rafael to implement Bootstrap3: https://github.com/wtfiwtz/simple_form_bootstrap3/commits/master |
@hraynaud I just replaced the entire top section (
(which adds the form-control class to every simple_form input field) and then constructed the checkbox manually to get around it having the form-control class when created with simple_form:
|
@mattangriffel : I found a solution to your problem: Instead of creating top-level objects |
Integration Bootstrap3
You need not to change any codebase to migration from bootstrap 2 to bootstrap 3