Skip to content

Grouped assets example #8574

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

Merged
merged 2 commits into from
Nov 10, 2017
Merged

Grouped assets example #8574

merged 2 commits into from
Nov 10, 2017

Conversation

takman1
Copy link
Contributor

@takman1 takman1 commented Nov 1, 2017

Asset > PathPackage > Grouped assets section > example

When absolute path is provided in getUrl(), base path is ignored.

When absolute path is provided in getUrl(), base path is ignored.
@weaverryan
Copy link
Member

Hey @takman1!

If I understand it correctly, both / and without / would have the same result? So why do you want to suggest this change? In general, we suggest not including the opening / when you pass paths to the component.

@takman1
Copy link
Contributor Author

takman1 commented Nov 2, 2017

Hello Ryan,
With /... in input the base path is ignored.
But without /, it prefixes with base path.

@weaverryan
Copy link
Member

Ah, of course! You're right, and this is an important point. But there are many other times in this article when we use /logo.png. We should update all of these to logo.png. We should maybe also show at least one example of what would happen if you did use /logo.png, so that it's clear the importance of the opening /.

@takman1 could you make those changes?

@takman1
Copy link
Contributor Author

takman1 commented Nov 5, 2017

Hi @weaverryan,
I'll check all examples in this pages and suggest modifications to show the importance of the opneing /.

How alsolute paths are handeled in Package and PathPackage.
@weaverryan
Copy link
Member

This is great! Thank you @takman1!

@weaverryan weaverryan merged commit a29765d into symfony:3.3 Nov 10, 2017
weaverryan added a commit that referenced this pull request Nov 10, 2017
This PR was squashed before being merged into the 3.3 branch (closes #8574).

Discussion
----------

Grouped assets example

Asset > PathPackage > Grouped assets section > example

When absolute path is provided in getUrl(), base path is ignored.

Commits
-------

a29765d Asset Component and absolute paths
95a79fd Grouped assets example
takman1 added a commit to takman1/symfony-docs that referenced this pull request Nov 10, 2017
How to reproduce :
  - Application has a base url (/some-base-url)
  - Using "PathPackage"
  - Using RequestStackContext (3rd parameter),
  - getUrl of asset with absolute path.

Result :
  Both "base url" of application and "base path" of PathPackage are ignored.

Example :
  Application base url : /some-prefix
  $package = new PathPackage('/static/', new StaticVersionStrategy('v1.2'), new RequestStackContext($requestStack));
  echo $package->getUrl("/img.png"); // /img.png?v1.2

Check merged PR symfony#8574.

NB : Maybe there a conception problem in Asset component. I my opinion only "base path" should be ignored but "base url" should always be used to prefix the generated asset path.
javiereguiluz added a commit that referenced this pull request Jan 12, 2018
This PR was merged into the 3.3 branch.

Discussion
----------

Path package in context aware

How to reproduce :
  - Application has a base url (/some-base-url)
  - Using "PathPackage"
  - Using RequestStackContext (3rd parameter),
  - getUrl of asset with absolute path.

Result :
  Both "base url" of application and "base path" of PathPackage are ignored.

Example :
  Application base url : /some-prefix
  $package = new PathPackage('/static/', new StaticVersionStrategy('v1.2'), new RequestStackContext($requestStack));
  echo $package->getUrl("/img.png"); // /img.png?v1.2

Check merged PR #8574.

NB : Maybe there a conception problem in Asset component. I my opinion only "base path" should be ignored but "base url" should always be used to prefix the generated asset path.

Commits
-------

0dbb635 Path package in context aware
@xabbuh xabbuh added this to the 3.3 milestone Jan 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants