-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Proposed a new article about using pure PHP libraries with Assetic #5166
Conversation
|
||
{% stylesheets filter="scssphp" output="css/app.css" | ||
"assets/scss/bootstrap.scss" | ||
"assets/scss/font-awesome.scss" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a reference about managing FontAwesome using Bower here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's a good idea, because this article is about using PHP only libraries. But once the Bower article written by @wouterj is published, we can indeed add a link to it in the introduction of this article (where we say "JS is the best for this"). I think taht would make a lot of sense. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for adding a note. My point is that it should be clear that managing (minimize, compile) web assets with PHP is possible, but checking in "assets/scss/font-awesome.scss" in the VCS is not needed if a package manager like Bower is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 interesting point, yeah maybe a # this is required when done PHP exclusive compilation of assets
As mentioned in #5166 (comment) What about compiling a file that is managed by a package manager? The example I'd like to be clear is: the scss files from FontAwesome can be managed using |
@rvanlaak let me explain the purpose of this article: the idea is to explain how to do the usual web asset stuff WITHOUT using any JavaScript tool or technology (Bower, Gulp, Grunt, ...). This article shows how can you achieve everything with Assetic and two PHP libraries while setting up everything in just a few minutes. So the FontAwesome files, as long as Bootstrap and jQuery files, should be committed to the repository and stored in the |
i think this article should include packages such as robloach component or other bundles that bring in third party stuff. The path should be adaptable to point to those things without needing to persist the assets into the repository which is a very bad practice. |
@cordoval I'm afraid that adding JavaScript stuff or third-party bundles into this article can't be possible. The intro of the article says: "using JavaScript for doing this stuff is the best way to do it" and provides a link to that solution. But if you can't or don't want to use JavaScript, then you have this article which explains the simple and pragmatic way to do it using only PHP. |
I think you are not understanding what i am saying. Let me rephrase it and reiterate it to make it plain. I mentioned @RobLoach's component only in the sense to bring css in, it has php packages that bring assets and js as well. Notice there is no js involved here. Repeating for emphasis here, they are not the js method to bring assets in. These are purely php. That said I hope is clear up to this point. I was never talking about js way of bringing assets here, which I am afraid you misunderstood. That said, there is no need then to persist these things into the repository, or at least there is no need to persist them all. It can be done right with php as well, or the most right way with only php. |
@cordoval I understand you know. Thanks for explaining it in detail. However, I still think it's not necessary. In this case, I don't consider a bad practice to persist assets in the repository. If your application is complex, use Bower and JS-stuff to continuosly update those assets. But if you application is simple enough, it's better to not update them continuously: things can break and you don't have time/money to solve those problems. So it's better to add those assets to the repo and treat them like regular assets, without updating them continuously. Anyway, we could add a very short note about the existence of pure PHP tools to manage assets similarly to JS-stuff. |
I meant to put just a note, not even a full paragraph, to avoid saying this is the only way as there are the ways i just explained. |
@cordoval I agree. We'll do that. |
👍 for not using any JS tooling in the article, but following a PHP approach. Did get that, but did want to discuss the gray areas where developers could make a decision to use a slight other approach. The use of bower is only one example of that ;) I don't see any other areas atm, so adding a note/link to @wouterj 's Bower article is fine ;) |
@rvanlaak i think @javiereguiluz understood what i meant. PHP approach is not just one approach is manyfold. I think this article is saying this is the best way, but if you are going to do it plainly bare bones here are the ways people approach it. |
@@ -0,0 +1,7 @@ | |||
Form |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be Front-End
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will be done by my PR and can be removed from this PR, to avoid merge conflicts
|
||
The official Symfony Best Practices recommend to use Assetic to | ||
:doc:`manage web assets </best_practices/web-assets>`, unless you are | ||
comfortable with JavaScript-based frontend tools. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
front-end
|
||
.. code-block:: html+jinja | ||
|
||
<!DOCTYPE html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably a good idea to add a filename comment to the top of the template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated it to display {# app/Resources/views/base.html.twig #}
Is that the common practice in the Symfony docs when referring to the base/layout template?
This article overlaps in a way with the articles from the Assetic cookbook section. We should find a way not to document thing more than once. |
Assetic includes a lot of ready-to-use filters but it doesn't include their | ||
associated libraries. Therefore, before enabling the filters used in this article | ||
you must install two libraries. Open a command console, browse to your project | ||
directory and execute the following commands: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's very important to not make this article just a step-by-step solution for one problem (Sass and minifying), but make it ready to use for other libraries as well.
This mostly means that we should try to not say "Do this", but instead explain what "this" is and how we've come up with "this". In this case, it means explaining how one can find these package names. How does one know it has to install exactly these packages?
Imo, as this is about Assetic, we should move this one to |
@wouterj I think you are right and this article should be moved back to the Assetic cookbook section instead of adding it to the new Frontend section. I've just changed it. Thanks. |
This PR seems to be finished. I've addressed every comment and implemented every suggestion. Can we label it as "finished"? Thanks! |
looks good 👍 |
…ith Assetic (javiereguiluz) This PR was merged into the 2.3 branch. Discussion ---------- Proposed a new article about using pure PHP libraries with Assetic | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes | Applies to | all | Fixed tickets | - In #5159 @wouterj proposes to create a new Cookbook section dedicated to frontend articles. I think it's a great idea and in this pull request I propose an article showing how to use pure PHP libraries to combine, compile and minimize CSS, SCSS and JS files. Commits ------- f5c4d93 Fixed an indentation problem 4b8b3fb Moved the article back to the Assetic section 07087dd Avoid the ugly double back slashes 85e6a54 Added more configuration formats bc6ffbe Rewords and code improvements 3ea9c86 Fixed some typos and grammar issues 1f6e16c Minor fixes cc5b630 Reworded some wrong statements 044cd73 The file extension is not needed 14d0346 Fixed the Twig lexer name 798b5b5 Added the missin index file 86da338 Proposed a new article about using pure PHP libraries with Assetic
Merged! Nice job making this short and concise - that's the best part (and a good reason why someone might opt for the pure PHP solutions). I made some final minor tweaks at sha: bf18f16 Thanks! |
* 2.8: (122 commits) moving options below basic usage changing a few orders of sections - mostly to move overridden after the main options Rewrite note a bit fix capitalization tweak headline to be consistent with 2.6 [Contributing] [Standards] Added entry for identical comparison Wrap the table creation inside the class extending Command, so users know where the comes. They can use it as standalone when needed [Serializer] Array Denormalization [Cookbook][Assetic] complete a sentence Fixing "Undefined method" error on Symfony 2.7 [#5293] Tweak to link to the article about parent bundles Minor tweak to remove FOSUserBundle example and replace with a link to the doc about this overriding 3rd party bundles Adjust line wrapping Add formatting for file and method names Update load_balancer_reverse_proxy.rst removing upgrading.rst - this has already been moved into several documents in its own section [#5166] Minor tweaks Revert "Changed dump() to var_dump()" Changed dump() to var_dump() ...
In #5159 @wouterj proposes to create a new Cookbook section dedicated to frontend articles. I think it's a great idea and in this pull request I propose an article showing how to use pure PHP libraries to combine, compile and minimize CSS, SCSS and JS files.