lucasefe / themes_for_rails

Theme Support for Rails 3
This very same page :)
MIT License
308 stars 102 forks source link

base_theme_image_path is not defined #43

Closed neerajsingh0101 closed 12 years ago

neerajsingh0101 commented 12 years ago

I am getting error that base_theme_image_path is not defined when I use

 current_theme_image_path

Indeed this method is used but it is not defined.

https://github.com/lucasefe/themes_for_rails/blob/master/lib/themes_for_rails/url_helpers.rb#L19

lucasefe commented 12 years ago

Hey, can you please tell me where are you using this? COntroller, View... which part? And a code snippet would be great too. Thanks.

neerajsingh0101 commented 12 years ago

I am using it in view. I was able to get around this issue by following commit

https://github.com/neerajdotname/themes_for_rails/commit/45d2c17d51dbafdcb37dd345c48406a1d8656b29

lucasefe commented 12 years ago

Are you sure this is not working with the current version?

neerajsingh0101 commented 12 years ago

themes_for_rails has a hard dependency on Rails 3.0.11. I am not able to test it.

This is the error I am getting

Bundler could not find compatible versions for gem "rails":
  In Gemfile:
    themes_for_rails (>= 0) ruby depends on
      rails (= 3.0.11) ruby

    rails (3.2.1)
lucasefe commented 12 years ago

Sorry. Fixed... Try again, please.

neerajsingh0101 commented 12 years ago

It is still not working. I invited to my project. When the homepage comes up then look at the footer. With the forked version I see the logo of credit cards. With your master version I do not see the credit card images.

lucasefe commented 12 years ago

OK, as gar as I can see you are putting your images (and the rest of assets) on app/assets, and themes/..../assets, so what we need is something different. We need to allow changing the assets prefix. I am thinking about something like this:

ThemesForRails.config.assets_prefix # => ""

ThemesForRails.config.assets_prefix = "assets/"

And then, when you do something like this (in haml):

= theme_image_path 'image.png' 

You'll get:

"/themes/default/assets/images/image.png'

What do you think?

lucasefe commented 12 years ago

Ok, I did some changes. You can specify a new option named assets_prefix, that will apply to every themes, but only regarding static assets, and not the views.

ThemesForRails.config.assets_prefix = "assets" (no slashes). 

On the branch named refactoring you can try it.

https://github.com/lucasefe/themes_for_rails/tree/refactoring

On that same branch I am planning on doing a big refactoring, because the project has a lot of repetition so far, and it's kind of hard maintain it.

Let me know what do you think.

neerajsingh0101 commented 12 years ago

I have all themes related assets under the theme directory. However /admin does not use any theme. So for them I use standard app/assets. I am testing refactoring branch.

neerajsingh0101 commented 12 years ago

Changed the route from

 match "#{theme_dir}/:theme(/:assets_prefix)/images/*asset" => 'themes_for_rails/assets#images',
        :as => :base_theme_image,
        :constraints => { :theme => /.*/ }

to

 match "#{theme_dir}/:theme(/:assets_prefix)/images/*asset" => 'themes_for_rails/assets#images',
        :as => :base_theme_image,
        :constraints => { :theme => /\w*/ }

Let me know when refactoring is done and I will test out this branch.

lucasefe commented 12 years ago

Since I was expecting to support themes names with dots, I had to modify it a little bit more. Regex are not my thing but it ended up like this:

match "#{theme_dir}/:theme(/:assets_prefix)/images/*asset" => 'themes_for_rails/assets#images', 
  :as => :base_theme_image, :constraints => { :theme => /[\w\.]*/ } 

Already pushed. Give it a go!

neerajsingh0101 commented 12 years ago

sounds good. I will give it a try.

lucasefe commented 12 years ago

@neerajdotname I just released a new ThemesForRails gem version, compatible with the rails assets pipeline... It supports new configuration options. Give it a try if you want. I dropped the assets_prefix because I really don't like it as a solution. This solution is more flexible.

Just letting you know.

Cheers.

neerajsingh0101 commented 12 years ago

@lucasefe I will check out the new release tonight and will provide you my feedback. Thanks for taking care of this.