igaster / laravel-theme

Theme support for Laravel
MIT License
520 stars 113 forks source link

Wrong URL if theme is not in public root #38

Closed billmn closed 8 years ago

billmn commented 8 years ago

Hi guys, I think there is a bug on URL generated by Theme::url() method.

I'm using a "themes" folder in the public root:

- public
   - ...
   - themes
      - theme1
         - assets
            - css
               - style.css
            - js
         - home.blade.php
         - page.blade.php
      - theme2

and I have set "themes_path" parameter like this:

'themes_path' => realpath(base_path('public/themes'))

README specify:

The path is relative to Theme Folder (NOT to public!)

So I have used this code Theme::url('assets/css/style.css') but I notice that the generated URL is wrong.

Wrong URL: /theme1/assets/css/style.css Right URL: /themes/theme1/assets/css/style.css

igaster commented 8 years ago

Hello, I assume that style.css exists in your theme and there are not any case-sensitive problems with the path... If a file is not found then this package will fallback to the public path....

I will try to replicate your setup to check it out...

billmn commented 8 years ago

@igaster Sure, the file exists and there isn't any case-sensitive problem

MarttiR commented 8 years ago

I have a similar setup, with views common to all themes in resources/views, and css and assets per-theme in public/themes/<themename>.

Indeed, assets will fail to load if themes.php is configured with

'themes_path' => realpath(base_path('public/themes'))

However, things seem to work just by leaving public out:


  'themes_path' => realpath(base_path('themes')),

...

  'themes' => [

    'default' => [
      'extends' => null,
      'views-path' => 'default',
      'asset-path' => 'themes/default',
    ],

    'black' => [
      'extends' => 'default',
      'views-path' => 'default',
      'asset-path' => 'themes/black',
    ],
  ]

Use in templates as documented, {!! Theme::url('file.name') !!} and it'll load public/themes/<themename>/file.name

billmn commented 8 years ago

Oh ok thank you

igaster commented 8 years ago

(Updated answer)

@MarttiR:

themes_path applies only to views and not to assets! You should leave it null since your views are located in laravel default folder.

Your configuration could be as follows:

  'themes_path' => null,

  'themes' => [

    'default' => [
      'extends' => null,
      'views-path' => '',
      'asset-path' => 'themes/default',
    ],

    'black' => [
      'extends' => 'default',
      'views-path' => '',
      'asset-path' => 'themes/black',
    ],
  ]

@billmn:

Can you share your configuration file? I suspect that there might be wrong setting... BTW it is a good practice to keep your views outside of the public folder so that they can not be accessed directly from a URL...

billmn commented 8 years ago

@igaster This is my configuration file:

<?php

return [

    /*
    |--------------------------------------------------------------------------
    | Switch this package on/off. Usefull for testing...
    |--------------------------------------------------------------------------
    */

    'enabled' => true,

    /*
    |--------------------------------------------------------------------------
    | File path where themes will be located.
    | Can be outside default views path EG: resources/themes
    | Leave it null if you place your themes in the default views folder
    | (as defined in config\views.php)
    |--------------------------------------------------------------------------
    */

    'themes_path' => realpath(base_path('public/themes')), // eg: realpath(base_path('resources/themes'))

    /*
    |--------------------------------------------------------------------------
    | Set behavior if an asset is not found in a Theme hierarcy.
    | Available options: THROW_EXCEPTION | LOG_ERROR | ASSUME_EXISTS | IGNORE
    |--------------------------------------------------------------------------
    */

    'asset_not_found' => 'ASSUME_EXISTS',

    /*
    |--------------------------------------------------------------------------
    | Set the Active Theme. Can be set at runtime with:
    |  Themes::set('theme-name');
    |--------------------------------------------------------------------------
    */

    'active' => 'default',

    /*
    |--------------------------------------------------------------------------
    | Define available themes. Format:
    |
    |   'theme-name' => [
    |       'extends'       => 'theme-to-extend',  // optional
    |       'views-path'    => 'path-to-views',    // defaults to: resources/views/theme-name
    |       'asset-path'    => 'path-to-assets',   // defaults to: public/theme-name
    |
    |       // you can add your own custom keys and retrieve them with Theme::config('key');
    |       'key'           => 'value',
    |   ],
    |
    |--------------------------------------------------------------------------
    */

    'themes' => [

        'default' => [
            'extends'    => null,
            'views-path' => '',
            'asset-path' => '',
        ],

        // Add your themes here...
        /*--------------[ Example Structre ]-------------
            // Recomended (all defaults) : Assets -> \public\BasicTheme , Views -> \resources\views\BasicTheme
            'BasicTheme',

            // This theme shares the views with BasicTheme but defines its own assets in \public\SomeTheme
            'SomeTheme' => [
                'views-path'    => 'BasicTheme',
            ],

            // This theme extends BasicTheme and ovverides SOME views\assets in its folders
            'AnotherTheme' => [
                'extends'   => 'BasicTheme',
            ],
        ------------------------------------------------*/
    ],

];

Yes, I know that is better keep themes outside the public folder but I would allow users to upload own themes that contains assets (css, js, ...) and this files must be public to work.

igaster commented 8 years ago

1) It is recomended to 'asset_not_found' to 'THROW_EXCEPTION' for debugging and 'LOG_ERROR' for production. It will help you spot path errors etc.

2) If you don't define a theme in the 'themes' section then the default values will apply which means that the assets will be retrieved from 'public/'. For your setup to work correctly you should create an entry for each theme. eg:


    'themes_path' => realpath(base_path('public/themes')),

    'themes' => [

        'theme1' => [
            'extends'    => null,  // can be ommited
            'views-path' => null, // defaults to 'public/themes/theme1', can be ommited
            'asset-path' => 'themes/theme1', // should be explicit defined or will point to 'public/theme1'
        ],

    // 'theme2' => ...

Also you should set somewhere your active theme. You can check your active theme with Theme::get();

Since you will need to load your themes dynamic then you can populate the configuration at runntime using Laravel config() function in your service provider.

I hope that these helps!