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

Added Twitter Bootstrap as a smart package. #84

Merged
merged 6 commits into from Apr 26, 2012

Conversation

Projects
None yet
4 participants
@fivethirty
Contributor

fivethirty commented Apr 22, 2012

Accidentally submitted this as a pull request to master the first time becasue I'm bad at reading Wikis. Sorry.

Anyhow...

Pretty straightforward except that Bootstrap includes some .png icon sprites. Had to add support for an "img" data type in Bundle.add_resource in order to handle adding them, which is probably a hack.

@adrianmaurer

This comment has been minimized.

adrianmaurer commented on 893cfaf Apr 22, 2012

Ah, I wasn't sure how to do this. Thanks! No need for my pull request ^_^

This comment has been minimized.

Owner

fivethirty replied Apr 22, 2012

I'm sure there is a cleaner way to do this (add_resource seems to be used to insert stuff into pages in all other cases) but I'm not familiar enough with the codebase to immediately see what it is.

Anyway, didn't mean to step on your pull request, sorry. Was actually going to look into adding MooTools next, but you beat me to it :)

This comment has been minimized.

adrianmaurer replied Apr 22, 2012

Ha! Yeah, I figured these were pretty quick packages to add and things I use everyday and even if it isn't the proper way I'm sure the meteor developers will see that it is an popular package to have and they will make the necessary changes.

This comment has been minimized.

Owner

fivethirty replied Apr 23, 2012

Yeah, I hope so. Bootstrap seems to sit well with the whole "write your webapp in the shortest amount of time possible" theme Meteor has going.

@jonathanKingston

This comment has been minimized.

jonathanKingston commented on a6a51ca Apr 25, 2012

This extension handler shouldn't be here, if anything it should be its own package as the twitter package would then handle all png files for the system.
Really if there is no handler for a file it should just try to serve it as static I think.

@jonathanKingston

This comment has been minimized.

jonathanKingston commented on app/lib/bundler.js in 893cfaf Apr 25, 2012

Personally I think this line should be changed to:
self.files.client[options.path] = data;
This would then just serve files up types that are not recognised as just a file... I think that would be more suitable.

This comment has been minimized.

jonathanKingston replied Apr 25, 2012

Would be great actually if the extension served files to:
/package/extensionname/filename.extension
Then allow users to override it by putting that file in their application folder.

@jonathanKingston

This comment has been minimized.

Contributor

jonathanKingston commented Apr 25, 2012

+1 for this :)

@debergalis

This comment has been minimized.

Member

debergalis commented Apr 26, 2012

This looks great! Just need one change: in production mode, the CSS is going to get minified and served from / instead of /packages/bootstrap/css. So all the links in the CSS files need to be absolute links to /packages/bootstrap/img/...

adding bootstrap-override.css to make paths to
icons absolute.  this is needed because the
bundler minifies css and serves it from the "/"
directory.
@fivethirty

This comment has been minimized.

Contributor

fivethirty commented Apr 26, 2012

editing bootstrap itself seemed really questionable to me, so i added a new css file that sets the paths to be absolute and is added after the original bootstrap files. tested by deploying to bootstrap-test.meteor.com.

@debergalis

This comment has been minimized.

Member

debergalis commented Apr 26, 2012

sold!

debergalis added a commit that referenced this pull request Apr 26, 2012

Merge pull request #84 from fivethirty/devel
Added Twitter Bootstrap as a smart package.

@debergalis debergalis merged commit 4c1f0d5 into meteor:devel Apr 26, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment