Skip to content

Moved web assets to web/ and implemented a pure PHP compression solution for CSS and JS files #33

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

Closed
wants to merge 7 commits into from

Conversation

javiereguiluz
Copy link
Member

This fixes #5

"%kernel.root_dir%/Resources/assets/js/bootstrap-3.3.4.min.js"
"%kernel.root_dir%/Resources/assets/js/highlight.pack.js" %}
{# uncomment the following lines to combine and minimize JavaScript assets with Assetic
{% javascripts filter="jsqueeze" output="js/app.js"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be ?jsqueeze to apply the minification only in non-debug mode

@javiereguiluz
Copy link
Member Author

I'm not sure about merging this change. I'd love to hear first the opinions of @weaverryan and @wouterj about it. They both know this topic well and they seem to have strong opinions about it (and definitely more qualified than mine!)

@stof
Copy link
Member

stof commented Apr 20, 2015

@weaverryan ping as there was a typo when mentioning you previously

@wouterj any opinion on this ?

@wouterj
Copy link
Member

wouterj commented May 11, 2015

  • I don't like depending on ports, they never are as good as the original ones. If it works with the code in the lib, I'm fine with using it here (although I don't believe we should recommend it).
  • Imo, sass/scss files are not public files, they shouldn't be accessed. That's why I vote to put these in app/Resources/public or the like, at least outside of web/. This is also following what the best practices guide says.

@javiereguiluz
Copy link
Member Author

This PR is ready for the final review:

  • The assets' sources are moved back to app/Resources/assets/
  • The compiled assets are web/css/app.css and web/js/app.js
  • The fonts have been moved to web/fonts/ to not duplicate them
  • The SCSS is now compiled and the CSS are minified with a PHP library (Leafo\ScssPhp)
  • The JS are minified with a PHP library (JsSqueeze)

The result is that:

  • The application still looks nice without configuring anything or executing any command.
  • If someone uncomments Assetic blocks, everything should still work because we use PHP-only solutions.

@stof
Copy link
Member

stof commented May 29, 2015

@javiereguiluz it would be great to provide a way to recompile the assets for contributors (using assetic:dump won't work as the Assetic blocks are commented), and explaining it in the contributing file.

@javiereguiluz
Copy link
Member Author

@stof I think that's a very nice idea! Thanks. I've added a note about contributing in 62417ca

@stof
Copy link
Member

stof commented May 29, 2015

@javiereguiluz I suggest putting this in CONTRIBUTING.md instead, to follow the github conventions

@javiereguiluz
Copy link
Member Author

@stof much better indeed. Done in a88b360. Thanks.

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.

Why are assets in app/Resources/ ?
3 participants