[2.7] Update Twig docs for asset features #5574
Conversation
| @@ -1027,46 +1027,12 @@ assets won't be cached when deployed. For example, ``/images/logo.png`` might | |||
| look like ``/images/logo.png?v2``. For more information, see the :ref:`ref-framework-assets-version` | |||
| configuration option. | |||
|
|
|||
| .. _`book-templating-version-by-asset`: | |||
There was a problem hiding this comment.
Does it make sense to add a warning that this is not supported? Otherwise, people who use old links might get lost as the anchor wouldn't match anymore.
There was a problem hiding this comment.
Do you mean adding an admonition about this in the docs? I don't like that, it'll put completely unrelated text in the documentation (also, anyone linking to this currently wouldn't read the warning anyways)
There was a problem hiding this comment.
Honestly, I cannot remember anymore what I had in mind back then.
wouterj
force-pushed
the
javiereguiluz-update_twig_templates
branch
2 times, most recently
from
2515ef9 to
7f38440
Compare
wouterj
force-pushed
the
javiereguiluz-update_twig_templates
branch
from
7f38440 to
b327da7
Compare
|
Rebased, ready to merge imo |
|
👍 |
| Returns the absolute URL for the given absolute path. This is useful to convert | ||
| an existing path: | ||
| Returns the absolute URL from the passed relative path. Assume the following | ||
| code was used to render ``http://example.com/sub/list.html``: |
There was a problem hiding this comment.
This is confusing me a little bit. Are we saying basically "Assume your Symfony installation is at http://example.com/sub`? What I mean is: are we trying to show that if you don't include /, then you should pass paths relative to your document root (e.g. the web dir) and that absolute_url will figure out if your installation is under a sub-directory and correct? If so, this is exactly how asset() works now - I'm just verifying that I'm not missing something. I think the fact that it corrects the subdirectory is not that important: it's just something that it should do. What's important is that we recommend (first) that you don't use the /, and that the path you should use is relative to your document root (e.g. web/).
There was a problem hiding this comment.
I've re-read your code, and it makes sense now. I'd re-word the second sentence:
For example, assume you're on the following page in your app
``http://example.com/products/hover-board``I think the .html in the URL confused me for some reason too :). Obviously, if you change to /products, the code below will need to change too
|
@wouterj What is the state here? |
|
|
||
| .. code-block:: jinja | ||
|
|
||
| {{ absolute_url(asset(path)) }} | ||
| {{ absolute_url('/human.txt') }} |
There was a problem hiding this comment.
we could also add the current URL here:
{# assume the current URL is http://example.com/products/hover-board #}There was a problem hiding this comment.
Doing it twice seems a bit duplicated to me. I've not included it in the PR, but feel free to make this change during merging if you feel it makes things better.
|
I clarified some comments. After we do those, I'll be 👍 |
wouterj
force-pushed
the
javiereguiluz-update_twig_templates
branch
from
b327da7 to
0cd7e6c
Compare
|
Thanks @weaverryan for the review. I've rebased and updated this PR now. |
|
Thank you Wouter. |
…z, WouterJ) This PR was merged into the 2.7 branch. Discussion ---------- [2.7] Update Twig docs for asset features Finishes #5171 | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | 2.7+ | Fixed tickets | #4982 (partially) Commits ------- 0cd7e6c Correctly document new twig functions bc18ff1 Updated Twig template to take into account asset() function changes
Finishes #5171