Follow-up to #2152261: Clean up for tablesort-indicator.html.twig

Problem/Motivation

Fix the urls created for the image assets.

url() will produce /fr-ca/ or /drupal/ prefixed urls, I should have used file_create_url() for this but that doesn't exist in twig yet.

I've got a proposal started for that over at #2168231: Twig Functions needed in templates.
In the mean time I'd like to revert part of this so that it's working.
And the best solution for this template would be #2189729: Factor out tablesort-indicator.html.twig IMO.

Proposed resolution

Make the core template use the same pattern of building the asset file url in preprocess and pass it to the template.

Remaining tasks

User interface changes

API changes

Files: 
CommentFileSizeAuthor
#2 interdiff.txt2.26 KBjoelpittet
#2 2310311-fix-tablesort-indicator-urls-2.patch2.12 KBjoelpittet
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,418 pass(es). View
#1 2310311-1-sort-indicator-url-fix.patch2.06 KBjoelpittet
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,974 pass(es). View

Comments

joelpittet’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
2.06 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,974 pass(es). View

Here's the patch

joelpittet’s picture

Issue summary: View changes
FileSize
2.12 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,418 pass(es). View
2.26 KB

Some doc fixes from @Cottser in #2073811: Add a url generator twig extension

Cottser’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Quick fix

Looks great, thanks @joelpittet!

dawehner’s picture

Seriously what is the point of this issue ... doesn't this just cause additional reroll work?
I would argue that this patch should add a file_create_url() twig extension instead.

joelpittet’s picture

It's to fix my mistake and hopefully be a warning to others who use url() instead of file_create_url().

dawehner’s picture

Yeah sure, let's get this in, unless the other one is faster.

joelpittet’s picture

Dibs on re-roll for the url generator issue if this gets in first;)

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

#2073811: Add a url generator twig extension is in. Shall we make this issue about adding a file_create_url() twig extension instead?

joelpittet’s picture

Status: Needs work » Closed (duplicate)

Debating using this as a file_create_url() because we have an issue discussing it over #2168231: Twig Functions needed in templates so for now closing it as a duplicate and may re-open and possibly consider calling that one a [META]

Thank you @alexpott

Cottser’s picture

I think we're covered by #2308187: Provide a twig extension for file_create_url so yes this can be closed as a dupe.