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

Embed bootstrap-sass (locked at version 2.3.2.2). #1799

Merged
merged 1 commit into from
Nov 12, 2013

Conversation

jcoleman
Copy link
Contributor

This is a replacement pull request for #1733 that fixes the issues with the original and passes all tests cleanly.

The big difference is that while the original embedded the scss files from bootstrap-sass, it didn't embedded the lib code that was necessary for all of that to work.

I've also namespaced the bootstrap files (they're all inside the respective rails_admin directories) so that they don't interfere with a Rails app using a newer version of bootstrap by being globally available in the root include paths.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.17%) when pulling ad9e935 on jcoleman:vendorize-bootstrap-2 into c28e715 on sferik:master.

@jcoleman
Copy link
Contributor Author

@sferik I'm not sure while Travis failed this build; I've run rake spec multiple time and gotten zero failures. The Travis build doesn't seem to show any specific failures either from what I can tell. Any thoughts?

@simple10
Copy link

@jcoleman Thanks for doing this. Saved me a lot of time. However, including bootstrap-sass in the app breaks because of a conflict with the bootstrap-sass.rb file in rails_admin.

This commit fixes the namespacing issue. Tests pass and app works.
simple10@2728c80

I'm now going to try making a Bootstrap 3 specific theme. It would be great if devs could choose between Bootstrap 2/3 by just using different themes.

@jcoleman
Copy link
Contributor Author

@simple10 Good catch! I've squashed your commit into the single commit I made so that this branch and pull request now reflects the total change in a single commit for the sake of keeping the history clean.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) when pulling 5f7f5dc on jcoleman:vendorize-bootstrap-2 into a0d04db on sferik:master.

@mshibuya
Copy link
Member

I'm afraid that we lose ability to upgrade embedded bootstrap files if we go this way.
Can't we move bootstrap files under vendor/ and have some kind of automated task which we can use to upgrade to newer bootstrap-sass, like jquery-rails does?

@caboteria
Copy link
Collaborator

Our trunk build was broken for a while so I think the build breakage may be our fault. Now that master builds cleanly I've restarted your build.

@allenwyma
Copy link

If you bootstrapped them, why not remove the bootstrap 2 dependency? I thought this commit was related to us trying to use bootstrap 3 with rails admin?

@lustremedia lustremedia mentioned this pull request Oct 30, 2013
@miclovich
Copy link

What's the status on the main branch? When will this be merged? The themefied idea seems awesome for folks to define which versions of bootstrap they require. Thanks.

mshibuya added a commit that referenced this pull request Nov 12, 2013
Embed bootstrap-sass (locked at version 2.3.2.2).
@mshibuya mshibuya merged commit dc7c76d into railsadminteam:master Nov 12, 2013
@mshibuya
Copy link
Member

Merged 🔨
Thanks so much for your work!

@jeremylynch
Copy link

I am getting the issue with bootstrap-sass using rails 3.2, I have changed my gemfile to
gem 'rails_admin', :git => 'https://github.com/sferik/rails_admin.git'

However, the master branch requires rails 4. How can I solve this issue when using rails 3.2?

@jcoleman
Copy link
Contributor Author

jcoleman commented Dec 7, 2013

@mrjlynch If you can find the commit immediately prior to the one that bumped the rails version requirement to 4 from 3.2, then you should be able to (on your local checkout of your own fork) run git checkout -b my-backported-branch COMMIT_HASH and then cherry pick the single commit referenced in this pull request. After that applies cleanly, push you your branch and lock your gem file to point to your repo and branch instead of sferik's.

@kprentiss
Copy link

Has anyone done what @jcoleman suggests here? If not, I'll give it a go and share if I get anywhere.

@sumitag
Copy link

sumitag commented Feb 17, 2014

@kprentiss I have just done what jcoleman suggested https://github.com/sumitag/rails_admin/tree/my_backported_branch

@amseledka
Copy link

@sumitag thanks! that works good, if this could be added as branch to rails_admin by @sferik , please
it would solve issues of rails3+BS3 for lots of devs

however it doesn't cover FontAwesome 4

@jcoleman jcoleman deleted the vendorize-bootstrap-2 branch June 7, 2014 03:52
@jcoleman jcoleman restored the vendorize-bootstrap-2 branch June 7, 2014 03:52
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.

None yet