gravityblast / web-app-theme

A simple theme for web apps
Other
2.45k stars 387 forks source link

V3.1.0 - Updated to use the rails 3.1 assets pipeline #41

Closed tscolari closed 9 years ago

tscolari commented 12 years ago

I rewrote most of the gem's structure. All assets (stylesheets, images) were moved to the app/assets/, and they no longer need to be copied to the rails application. Updated the theme generator to create a web_app_theme.css manifest, instead of copying all the css, that will add the selected theme to the file. Also created a new assets generator that will copy theme's files to application, just for customizations or rails <=3.1 compatibility. I also rewrote the cucumber steps to use aruba.

rgabo commented 12 years ago

@tscolari, there are still issues with relative paths in the theme CSS to fonts and images as Sprockets will pull them all into assets/application.css, whereas the fonts are relative to web-app-theme/themes/default. One easy fix is to pull in web_app_theme separately from the gem assets:

// Include web-app-theme separately because of relative paths
= stylesheet_link_tag 'web-app-theme/base', 'web-app-theme/themes/default/style'
= stylesheet_link_tag 'application'
= javascript_include_tag 'application'
= csrf_meta_tags

application.html.haml

tscolari commented 12 years ago

And what if we use absolute paths in the css files? The big point of using the gem's assets is that the css files can be updated with bundle, without regenerating anything

rgabo commented 12 years ago

Yes, having the assets in the gem plays perfectly to the strengths of the Rails 3.1 asset pipeline. The problem is that the CSS's that are in web-app-theme expect other assets to be in the same relative place. This will either break by default (assets get flattened out by the Asset Pipeline) or when using ?debug_assets=1 (assets stay where they were).

Even if pulling in web-app-theme separately as I've shown in my previous comment, some assets will inadvertently be placed wrong, e.g. generating CRUD views will use image_tag 'icons/tick.png', whereas even if those assets are under icons, an image_tag 'tick.png' is enough and oddly, image_tag 'icons/tick.png does not work with 3.1.0.rc6, even though the documentation states that it should.

It will not be trivial to maintain both Rails 2 compatibility and getting the most out of Rails 3 (while trying not to touch the themes themselves) and I'm still getting myself familiar with the Asset Pipeline. Another option that is only available in Rails 3 is to use asset_data_uri to embed the asset into the CSS (getting rid of any path issues), but of course this needs ERb preprocessing.

I suggest we wait for the final 3.1 release and work with the asset pipeline based on what you've already done. When the final release hits, it'll be easier to figure out the best way and we can regroup.

tscolari commented 12 years ago

Yes, I agree. I'll do more research about the assets pipeline while rails 3.1 isn't final.

In my opinion, I think that sprockets should fix relative paths when generating a css from a manifest, otherwise, it will be tricky to build complex schemes.

rgabo commented 12 years ago

Sprockets allow you to use style.css.erb instead of style.css and there, you can easily call <%= asset_path 'image.png' %> or <%= asset_data_uri 'logo.png' %>. See the Edge Guides for the asset pipeline.

I don't think they would want to go down the path of rewriting people's asset's, even if it's 'just' the relative paths. I think the templating (erb) path is cleaner.

tscolari commented 12 years ago

I made some changes, I would like your opinion. I changed the css to templates as your said, using the asset_path method before images and fonts. This make it works in rails 3.1.

On rails 3.0 and bellow, it must generate/copy the assets anyway. So instead of copying, when the version is < 3.1.0, the generator overwrites the asset_path method, and use the css template to generate a raw css, moving all assets references to /stylesheets/. Also the images and everything else is copied to the public/stylesheets folder, as it does in the current version.

So when using 3.0 for example, the web_app_theme:theme generator will call the assets generator also. And the assets_generator will do this templating with the css.

On the rails 3.1 and above, it will not call the assets generator. And the assets generator will only copy the css.erb file (no parsing), that sprockets will serve.

Basically the generator will behave different for each version, and copy files to each version's default location.

It's not a clean fix, but this seems to work for both versions.

rgabo commented 12 years ago

@tscolari, awesome that you've already updated, I'll check it out tomorrow. I do want to hear from @pilu of course on what direction he wants to take the project. Clearly, it should be a solution that takes advantage of the Asset Pipeline and I think this pull request has come closest to that. Let's iterate a bit more and hear what @pilu has to say.

nathanvda commented 12 years ago

Good work. This solution indeed uses the asset pipeline better than my solution does. Nice work. We now have 4 pull requests for rails 3.1 compatibility, with slightly different approaches. To me it seems we better join forces, instead of all hacking away on our own solution.

rgabo commented 12 years ago

@nathanvda, I think we all agree, but its still not crystal clear what the right path is. The asset pipeline has a quite different approach that makes it trickier to maintain backwards compatibility. I like the idea of web_app_theme precompiling the css.erb files for earlier versions, similar to what Rails 3.1 would do.

nathanvda commented 12 years ago

@rgabo no, I agree, to me the right path is not clear at all. But this fork/pull seems to be the best approach up until now. Precompiling the css files using erb seems like a valid approach. But inside another fork, the backward compatibility was dropped entirely, and maybe that is a feasible approach as well.

tscolari commented 12 years ago

I agree too. Fell free to add commits and suggestions!

I must do more testing, but I think it has, at least it should have, the same behavior that the 0.8.0 version when using in the rails 3.0 and bellow. The differences are that assets are been referred by absolute path (but the path is the same) and that the images ["avatar", "icons/*"] now reside inside a web-app-theme folder.

I should add more tests, some for different rails versions.