Skip to content

Conversation

@MekDrop
Copy link

@MekDrop MekDrop commented Mar 14, 2016

Laravel 5 comes with resources/assets folder. I think it's a good idea to use it instead of public folder as source so I made update.

If user wants to use resources folder instead system, he can just set use_resources var to true. Otherwise everything will stay.

I have tested for my own project and everything works. However I haven't wrote all tests needed tests to test this new functionality... I'm not a good tests writer :(

Still I hope that this my pull request will be accepted and I don't need to maintain my fork.

@fisharebest
Copy link
Owner

Laravel 5 comes with resources/assets folder. I think it's a good idea to use it

If I understand correctly:

.sass and .less files should be stored in resources/assets/
.css and .js files should be stored in public/

The laravel documentation shows files being compiled from resources/assets/less/ to public/css/.

So, I do not think it is normal to store CSS/JS files here. I have not seen other projects that do this.

Your CSS files will probably reference images and fonts. These will need to be in the public folder. So, your CSS/LESS will be full of relative paths such as url(../../public/img/icon.png).

BTW - I think your requirements could be implemented more simply, with a one-line code change. In the service provider, if you change public_path() to base_path(), then all the configuration options will be relative to the base path. You can now specify

'css_source' => 'resources/assets/css',
'js_source' => 'resources/assets/js',
'destination' => 'public/min',

@MekDrop
Copy link
Author

MekDrop commented Mar 14, 2016

Same page says that if you want to combine multiple .css, it's a good idea to store these source files in resources/assets/css. And this is the thing that I want to achieve :)

Changing public_path() to base_path() I think doesn't solve the problem. I want to place source uncompressed and unminimized files outside public path.

I think another way to solve this issue, use predefined file system from config / filesystems.php. Storage::disk('public') instead of new Filesystem(new Local(public_path()), ['visibility' => AdapterInterface::VISIBILITY_PUBLIC]) But still in that case two filesystems are needed to perform what I want to achieve...

@MekDrop
Copy link
Author

MekDrop commented Mar 19, 2016

It seems what I want to achieve is possible to do by mixing Classy-Geeks/laravel-potion with your library. Code in that case a bit ugly but it seems it works. I think I close this pull request.

@MekDrop MekDrop closed this Mar 19, 2016
@fisharebest
Copy link
Owner

Changing public_path() to base_path() I think doesn't solve the problem.

I did try this (but only briefly, as I have been ill).

It worked OK for me. What problems did you have?

The CSS URL rewriter must modify relative URLs (e.g. "../../public/img/" -> "../img/"). I think there may be difficulties if the source/destination are in different filesystems.

@MekDrop
Copy link
Author

MekDrop commented Mar 21, 2016

I hope you get well soon :) You did amazing job with this plugin. And maybe you are right here. I jumped from other framework to Laravel not long time ago so maybe I don't know something and I can't understand everything as I should. Maybe best idea to leave this for a while :)

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.

2 participants