Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
path_to_theme()
and $theme_path
from D7 have no equivalent in Drupal 8.
Proposed resolution
Add an active_theme_path twig function.
Remaining tasks
User interface changes
API changes
The new twig function, active_theme_path.
Beta phase evaluation
Issue category | Task because it is adding a twig function |
---|---|
Issue priority | Major because tstoekler marked in comment #10 |
Disruption | Minimal disruption as it is a getter method. Possible disruption if changes upstream (Twig) |
Comment | File | Size | Author |
---|---|---|---|
#48 | add_an-2416857-48.patch | 3.11 KB | cilefen |
#39 | add_an-2416857-39.patch | 3.24 KB | lauriii |
| |||
#31 | add-twig-active-theme-path-2416857-2.patch | 3.28 KB | evgeny.chernyavskiy |
#28 | Screen Shot 2015-09-25 at 12.32.26.png | 18.01 KB | Anonymous (not verified) |
#26 | add-twig-active-theme-path-2416857.patch | 2.63 KB | evgeny.chernyavskiy |
Comments
Comment #1
cilefen CreditAttribution: cilefen commentedComment #2
cilefen CreditAttribution: cilefen commentedComment #3
cilefen CreditAttribution: cilefen commentedComment #4
chr.fritschComment #6
star-szrComment from #2416831-6: Add an active_theme twig function applies here too I think.
Patch from #3 still applies, for the record :)
Comment #7
cilefen CreditAttribution: cilefen commentedPostponed on #2416831: Add an active_theme twig function for the service injection.
Comment #8
star-szrBut the tests aren't postponed :) someone else can be working on those in the meantime.
Comment #9
cilefen CreditAttribution: cilefen commentedComment #10
tstoecklerHit this today, as well. And was about to roll the exact same patch. ... And this one comes with tests also. Awesome. Looks good to me.
Also marking major per #2168231: Twig Functions needed in templates.
Comment #12
cilefen CreditAttribution: cilefen commentedreroll
Comment #15
star-szrI think this is blocked on #2416831: Add an active_theme twig function for injecting the theme manager.
Comment #16
tstoecklerOK, the parent is in now. Now this just needs to be changed to use the injected theme manager.
Comment #17
cilefen CreditAttribution: cilefen commentedComment #18
zauravComment #19
heddnComment #20
heddnRemoving tags.
Comment #21
lauriiiComment #22
cilefen CreditAttribution: cilefen commentedDraft CR. There being no further Novice tasks, I removed that tag.
Comment #23
cilefen CreditAttribution: cilefen commentedComment #24
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedThis needs another reroll. So tagging novice (instructions).
Comment #25
evgeny.chernyavskiy CreditAttribution: evgeny.chernyavskiy as a volunteer commentedComment #26
evgeny.chernyavskiy CreditAttribution: evgeny.chernyavskiy as a volunteer commentedAttaching a re-roll of https://www.drupal.org/node/2416857#comment-9976363
Comment #27
evgeny.chernyavskiy CreditAttribution: evgeny.chernyavskiy as a volunteer commentedSmall conflict probably due to an extra space/new line. Marking as "Needs review".
Comment #28
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedOk great! I tested manually using an inline template render array, to check that this actually does what we expect:
We have test coverage and a CR.
Epic nitpick. This newline is not needed. I guess this can be removed on commit?
Comment #29
evgeny.chernyavskiy CreditAttribution: evgeny.chernyavskiy as a volunteer commented@pjonckiere,
Doh, missed that one. Not sure what you mean by removing on commit though?
Comment #30
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedIf you can make a new patch with that line removed, that would be great and I'll RTBC again. But I didn't want to reject the patch just because of that line, so I RTBC'd anyway and a branch maintainer could remove that line on commit. The former is preferred though.
Comment #31
evgeny.chernyavskiy CreditAttribution: evgeny.chernyavskiy as a volunteer commentedRe-rolling my previous patch from https://www.drupal.org/node/2416857#comment-10370239 with amendments per https://www.drupal.org/node/2416857#comment-10370595. Also updated documentation for a couple methods to include the missing @throws declarations.
Comment #32
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedLooks good to me! Back to RTBC.
ps: changing status to "needs review" is sufficient, there is no need to tag them
Comment #34
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedFailed to set field driver to mysql
This looks like something unrelated to this patch, so retesting.
Comment #36
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedComment #38
cilefen CreditAttribution: cilefen commentedComment #39
lauriiiComment #40
lauriiiThis change shouldn't be in the patch. Commiter please remove this change :)
Comment #41
catchDoes this get resolved into the compiled templates or is it done when the template is rendered?
I think it's the latter, but we really don't want the theme path compiled with the templates - moving directory would break it then.
Comment #42
catchComment #43
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commented#41: It is determined dynamically on run-time.
Comment #46
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedComment #47
cilefen CreditAttribution: cilefen commentedComment #48
cilefen CreditAttribution: cilefen commentedoops
Comment #49
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedComment #50
alexpottThese changes are totally out-of-scope. Removed.
Committed 7594f7e and pushed to 8.0.x. Thanks!
Comment #53
Wim LeersAdding this was a grave mistake. See #2611246: [PP-1] Document the recommended method of creating file URLs to theme assets (e.g. images).
Comment #54
thijsv CreditAttribution: thijsv commentedSo it is smarter to not use this anymore and wait for the stream wrappers solution? I mean, is it possible that this active_theme function will disappear in the future?
Comment #55
bradjones1I agree with Wim, here - this function doesn't return a path relative to the base path, and you may not necessarily have the base path in your twig template context. Stream wrappers are a much better solution.
But, since this is committed to core, that ship has (mostly) sailed. What's the process to get this marked as deprecated for 9.0?
Comment #56
cilefen CreditAttribution: cilefen commentedOpen an issue unless there is already one.