Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Task
Convert theme_ functions to Twig templates.
Steps to Test
- function
node_add_list
-- view source at/node/add
- function
custom_block_add_list
-- view source at/block/add
Must add a custom block type so as to see the list
[admin/structure/block/block-content] - function
admin_block_content
-- view source at/admin/structure/display-modes/view/add
- function seven_preprocess_tablesort_indicator
/admin/reports/dblog
Remaining
- theme_ function -> Twig conversions (and preprocess function additions or changes) moved over from the existing conversion issue
Patch review
Theme function name | Conversion status |
---|---|
seven_admin_block_content | converted |
seven_custom_block_add_list | converted |
seven_node_add_list | converted |
seven_tablesort_indicator | converted |
Comment | File | Size | Author |
---|---|---|---|
#178 | interdiff.txt | 882 bytes | joelpittet |
#178 | 1987424-twig-seven-theme-178.patch | 9.78 KB | joelpittet |
#171 | interdiff.txt | 4.82 KB | star-szr |
#171 | interdiff-3-label-to-title.txt | 1.05 KB | star-szr |
#171 | interdiff-2-block_content.txt | 2.12 KB | star-szr |
Comments
Comment #0.0
star-szrUpdated issue summary.
Comment #1
c4rl CreditAttribution: c4rl commentedPer #1757550-44: [Meta] Convert core theme functions to Twig templates, retitling to indicate this issue applies to theme_ functions rather than templates. See #1938848: seven.theme - Convert PHPTemplate templates to Twig for template conversion.
Comment #1.0
c4rl CreditAttribution: c4rl commentedUpdated issue summary.
Comment #2
jenlamptonhere's the conversion of only the theme function overrides.
Comment #3
TommyK CreditAttribution: TommyK commented#2: twig-convert_seven_theme_funcs-1987424-2.patch queued for re-testing.
Comment #4
TommyK CreditAttribution: TommyK commentedI found that the description of the content type and the custom block is missing from the lists. I will add this in an upcoming patch.
Comment #5
TommyK CreditAttribution: TommyK commentedAdds description keys to the arrays for custom blocks and content types.
Comment #6
Pierre Paul Lefebvre CreditAttribution: Pierre Paul Lefebvre commentedI've set the default theme to Seven. Created a node and promoted it to frontpage.
Run 519fc53e46a65 uploaded successfully for drupal-perf-drupalcon.
Run 519fc817b51ae uploaded successfully for drupal-perf-drupalcon.
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519fc53e46a65&...
Run 519fc53e46a65 uploaded successfully for drupal-perf-drupalcon.
Run 519fc84e17238 uploaded successfully for drupal-perf-drupalcon.
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519fc53e46a65&...
Comment #7
ruplFunctional review went great. Steps:
/node/add
, saw list as expected/node/add
, saw message as expected<script>
tag), no problems thereI can't find an example of the 'administrative block' template, but the format is similar to the other twig files that it seems good to go.
Comment #8
sarahjean CreditAttribution: sarahjean commentedI was able to apply the patch, I didn't test install.
I can verify the message was on
node/add
, as well asblock/add
. The message was also present for a custom block type.Comment #9
OpenChimp CreditAttribution: OpenChimp commentedComment #10
OpenChimp CreditAttribution: OpenChimp commentedprofiling not run correctly. Needs to be redone
Comment #12
tlattimore CreditAttribution: tlattimore commented#5: twig-convert_seven_theme_funcs-1987424-5.patch queued for re-testing.
Comment #14
star-szr#5: twig-convert_seven_theme_funcs-1987424-5.patch queued for re-testing.
Comment #15
tlattimore CreditAttribution: tlattimore commentedI'll work on profiling this.
Comment #16
tlattimore CreditAttribution: tlattimore commentedHere is some of profiling results.
Scenario:
node/add
as the frontpageComment #17
tlattimore CreditAttribution: tlattimore commentedRemoving needs profiling tag. We'll see what others think about the profiling results.
Comment #18
joelpittetMinor nitpick re #5
Should be for admin-block-content.html.twig. like the rest.
Comment #19
eromero1 CreditAttribution: eromero1 commenteddibs
Comment #20
sbudker1 CreditAttribution: sbudker1 commenteddibS!
Comment #21
eromero1 CreditAttribution: eromero1 commenteddibs
Comment #22
sbudker1 CreditAttribution: sbudker1 commenteddibs!
Comment #23
eromero1 CreditAttribution: eromero1 commenteddibs!!
Comment #24
sbudker1 CreditAttribution: sbudker1 commenteddibs!
Comment #25
eromero1 CreditAttribution: eromero1 commenteddibs
Comment #26
sbudker1 CreditAttribution: sbudker1 commentedComment #27
eromero1 CreditAttribution: eromero1 commentedI tried to reroll the patch, but I got an error
Warning: Missing argument 3 for template_preprocess(), called in /drupal/core/includes/theme.inc on line 1130 and defined in template_preprocess() (line 2474 of core/includes/theme.inc).
Comment #28
star-szr@eromero1 - good catch, I ran across that myself last night. That is an unrelated issue, here is the issue for that bug:
#2030457: Warning: Missing argument 3 for template_preprocess() for theme functions overridden by templates
So if you rerolled and that was the only error you got, please post your reroll! It's almost always easier to work with a newly rerolled patch even if it's not perfect.
Comment #29
Carolyn CreditAttribution: Carolyn commentedReroll of #5. This needs work so that the arrows show up on table headers.
Comment #30
Carolyn CreditAttribution: Carolyn commentedComment #31
joelpittetCarolyn could you do another re-roll it looks like all the $vars to $variables got reverted in in #29
Comment #32
joelpittetThese can be removed too.
Comment #33
drupalninja99 CreditAttribution: drupalninja99 commented-Rerolled patch
-Changed $vars to $variables
-Removed references to @see template_preprocess()
Comment #34
drupalninja99 CreditAttribution: drupalninja99 commentedHold on, patch is wrong, let me take another crack at it.
Comment #35
drupalninja99 CreditAttribution: drupalninja99 commented-Fixed the re-roll.
Comment #36
drupalninja99 CreditAttribution: drupalninja99 commentedComment #37
Les Limremoved tag.
Comment #38
Les LimEDIT: blergh, issue queue fail.
Comment #39
LewisNyman#35: seven-1987424-35.patch queued for re-testing.
Comment #40
LewisNymanThe latest patch is also trying to add ie.css back in the theme. It also removes the CSS from the install page.
Comment #41
star-szrComment #42
jenlamptontrying again.
Comment #43
siccababes CreditAttribution: siccababes commentedIf test bot turns green, then I think this can be considered RTBC. After applying the patch, I made seven the default theme. I played around a little and everything looked fine.
Comment #44
LewisNymanThis patch still removes css from preprocess_install_page, apart from that we're good to go.
Comment #44.0
LewisNymanRevise summary
Comment #45
star-szrThis also needs a reroll.
Comment #46
utilum CreditAttribution: utilum commentedComment #47
Gaelan CreditAttribution: Gaelan commentedRerolled!
Comment #48
star-szrThanks @Gaelan. @oshelach had just assigned it yesterday, but can still work on #44. Seems to me like we shouldn't be changing seven_preprocess_install_page() at all in this patch.
Comment #49
utilum CreditAttribution: utilum commentedComment #50
star-szrThanks @oshelach :) This needs a reroll again. A couple of these might be dependent on other issues getting in, #1987406: node.module - Convert theme_ functions to Twig for node-add-list and #1987410: [meta] system.module - Convert theme_ functions to Twig for admin-block-content.
Based on the work being done on #2049207: [Follow up] Replace .tpl.php with .html.twig in documentation, these should probably be formatted like "Implements hook_preprocess_HOOK for ____ templates."
Comment #51
utilum CreditAttribution: utilum commentedComment #52
utilum CreditAttribution: utilum commentedUm.... lemme just fix those function description comments.
Comment #53
utilum CreditAttribution: utilum commentedComment #54
utilum CreditAttribution: utilum commentedSpotted a couple additional documentation comments to modify, I hope this isn't off the mark.
Comment #55
utilum CreditAttribution: utilum commentedComment #56
utilum CreditAttribution: utilum commentedOops. Here's the actual patch.
Comment #58
utilum CreditAttribution: utilum commentedSorry, white space.
Comment #59
utilum CreditAttribution: utilum commentedComment #61
utilum CreditAttribution: utilum commentedComment #62
rteijeiro CreditAttribution: rteijeiro commentedCode seems ok and patch applies well. Let go for RTBC?
Comment #63
star-szrThanks @oshelach! I don't think this is quite ready yet…
No hyphens or underscores in these descriptions please. You should be able to copy descriptions from the existing ones in core or see the latest patch in #2049207: [Follow up] Replace .tpl.php with .html.twig in documentation for more examples.
This is good, just capitalize HTML please.
This is premature I think since it's still a theme function - might need to be moved to this issue: #1987410: [meta] system.module - Convert theme_ functions to Twig.
Unless it works fine like this too :)
Comment #64
joelpittetThis looks like someone may have forgot to convert the 'You have not created any content types yet. '... bit from node_add_list.
Can someone manually test this?
Comment #65
rteijeiro CreditAttribution: rteijeiro commented@joelpittet: The message is still there. It seems to be in the template that makes sense and it's better ;)
Comment #66
joelpittet@rteijeiro so the steps to reproduce:
Remove content types
Go to /node/add
Screenshot of what it should say after the content types are removed. This needs test with the patch:)
Comment #67
rteijeiro CreditAttribution: rteijeiro commentedYes, you are right! Just confused the page.
Re-created the patch after discussing with you ;)
Added core/themes/seven/templates/node-add-list.html.twig template missed from #2 patch.
Comment #68
rteijeiro CreditAttribution: rteijeiro commentedForget Cottser comments in #63.
This is the good patch and I hope final one :P
Comment #69
joelpittetIt looks like the twig templates got lost in #42 So we'll need to make sure those get added back in from #35
Thanks @rteijeiro for adding in the node-add-list.html.twig
These two need to be added as well.
+++ b/core/themes/seven/templates/admin-block-content.html.twig
+++ b/core/themes/seven/templates/custom-block-add-list.html.twig
Comment #70
rteijeiro CreditAttribution: rteijeiro commentedOk, I will try to fix this issue today ;)
Comment #71
rteijeiro CreditAttribution: rteijeiro commentedSorry for the delay. Here is the patch with the two templates from comment #35.
Comment #73
joelpittetThanks for the patch and adding those twig files back in @rteijeiro. One of the exceptions/fails is related to no {{ compact }} variable passed by preprocess, check #35 again in the preprocess and compare to see where those got lost.
Comment #74
rteijeiro CreditAttribution: rteijeiro commentedLet's see if now passes the test. It seems that seven_preprocess_admin_block_content function was not created :(
Comment #76
joelpittet#74: 1987424-74.patch queued for re-testing.
Comment #78
joelpittetDoubl ++ on this node-add-list.html.twig
Most likely why it's failing.
Also I think this is missing an tablesort-indicator.html.twig. It may not be needed but the width/height of the image may need changing from the system version? Just need a markup check on that one.
Comment #79
joelpittetWhoops. Needs work, see #78
Comment #80
rteijeiro CreditAttribution: rteijeiro commentedRe-rolled!
Comment #82
rteijeiro CreditAttribution: rteijeiro commentedIt seems that for seven theme the test is using:
$this->assertLinkByHref(url('block/add/foo', $options));
But for bartik and startk themes is using:
$this->clickLink('foo');
I will try to fix it tomorrow. Today my simpletest module is not working and I don't know why :(
Comment #83
rteijeiro CreditAttribution: rteijeiro commentedNew re-roll.
Comment #84
robmc CreditAttribution: robmc commentedRe-rolled
Comment #85
rteijeiro CreditAttribution: rteijeiro commentedComment #87
rteijeiro CreditAttribution: rteijeiro commentedPatch for the failing test. It seems to be something weird in $options of
$this->assertLinkByHref(url('block/add/foo', $options));
Comment #89
rteijeiro CreditAttribution: rteijeiro commentedSorry for the crappy patch in #87. It seems I uploaded the interdiff :(
It seems that there is no
theme
parameter forblock/add
so that's because I removed $options in$this->assertLinkByHref(url('block/add/foo', $options));
Hope now patch it's right.
Comment #90
joelpittetSome nitpicks and regression stuff after another review.
@rteijeiro thanks for the new patch.
I think we need to keep that... but I could be wrong. It looks like it makes sure the block is added to the right theme.
check_plain and filter_xss_admin have been replaced with String::checkPlain() and Xss::filterAdmin
This this needs be the \Drupal::l() above for the route to stay in tact.
This would be nicer as a trans block.
Comment #91
rteijeiro CreditAttribution: rteijeiro commentedIt seems that tests fail now in a different place. Trying to figure out how to fix it.
Comment #93
joelpittetSorry @rteijeiro that custom_block_add_list is confusing because we changed it from using $variables['types'] to using ['content'] which is what types is originally built from. This had all sorts of things done in it. So I moved the query params up to seven's url.
The trans block was a bit trickier but the purpose is you don't need a giant string any longer, the trick is that only simple variables can be used inside so I had to set the url above to use it inside.
Comment #93.0
joelpittetUpdate conversion table
Comment #94
skt CreditAttribution: skt commentedI got part-way through comparing before & after....
For function
node_add_list
, no output markup change.Before:
After:
For function
custom_block_add_list
, no output markup change.Before:
After:
For
admin_block_content
, only change was an added space in the ul tag:<ul class="admin-list ">
Before:
After:
Comment #94.0
skt CreditAttribution: skt commentedAdd steps to test
Comment #95
star-szrThat's awesome, thanks a million @skt!
I think there are a few ways to solve the minor markup discrepancy.
1. Change 'compact' to ' compact' and remove the space between admin-list and {{ compact }}.
2. Build out the attributes array/object in preprocess and then do
<ul{{ attributes }}>
.Looking at what is happening over in #1987410: [meta] system.module - Convert theme_ functions to Twig (code snippet from that issue below) and in general I think we should do 2 and also add a @todo to remove the code in the Seven preprocess once #1987410: [meta] system.module - Convert theme_ functions to Twig is resolved.
Comment #95.0
star-szrAdded steps to test info
Comment #96
joelpittet93: 1987424-93-twig-seven-theme.patch queued for re-testing.
Comment #97
star-szrTagging for reroll.
Comment #98
longwaveRerolled.
Comment #99
joelpittet@longwave nice work on the re-roll, any chance you want to grab the items from #95?
Comment #100
longwaveFollowed the suggestion in #95, removed a blank comment line, and fixed a function name reference.
template_preprocess_admin_block_content() and template_preprocess_node_add_list() don't actually exist yet, though they will after #1987410: [meta] system.module - Convert theme_ functions to Twig and #1987406: node.module - Convert theme_ functions to Twig get in.
Comment #101
joelpittetI think this image template got into this patch by accident. It doesn't look to be part of the seven theme.
Comment #102
longwaveRerolled without image.html.twig.
Comment #103
joelpittetI found that the preprocess_tablesort_indicator is not working:( when I was doing manual testing. The image doesn't change to the seven image. All 3 other templates work perfectly and output matches.
Comment #104
joelpittetOk here is a fix to #103 and #102. Removed the image.html.twig that got in there by accident and fixed the tablesort indicator output.
Comment #105
joelpittetHere's 4 theme's markup comparisons:
Comment #106
joelpittetComment #107
star-szrThanks for all the work here @joelpittet!
We can rework this and make the tablesort override lean and mean now that #2152261: Clean up for tablesort-indicator.html.twig is in!
Comment #108
joelpittetThis of course won't work... but it should. There should be some globally accessible variables in the twig context and maybe a few more functions like 'drupal_get_path' or something of it's kin.
Comment #109
joelpittetFYI, re #108 even though It passed... theme_path is not a real variable... yet.
Comment #110
star-szrTagging for reroll.
Comment #111
InternetDevels CreditAttribution: InternetDevels commentedHere is the reroll. I'm not sure about preprocess I've added, maybe someone may suggest better solution.
Comment #112
joelpittet@InternetDevels what did you add, could you provide an interdiff for the changes you made from #108?
Comment #113
star-szrComment #114
InternetDevels CreditAttribution: InternetDevels commentedIt's a bit difficult to create interdiff between an applicable and a non-applicable patches, but I hope that attached file may help you to see what changes I've added.
Comment #115
joelpittet@interdevels sorry, wasn't clear there. Don't worry about interdiffs on rerolls. Just for your changes. It helps see clearly what you changed from the previous patch. I know it's tricky if your changes are right after a reroll though if you are using branches in it helps to commit and post the reroll first then commit and post the patch + interdiff. I could have miss read your post when you said your not sure about stuff you added. So if that was part of the conflict resolution I apologize but maybe you can point out that section of code?
Comment #116
star-szrTagging for reroll.
Comment #117
IshaDakota CreditAttribution: IshaDakota commentedrerolled.
Comment #118
joelpittetSome merge markers got into the re-roll. Could you give it another crack?
Comment #119
longwaveComment #120
star-szrTagging for reroll. Thanks everyone for keeping this one up to date!
Comment #121
Sutharsan CreditAttribution: Sutharsan commentedPatch #119 rerolled.
Comment #122
joelpittetThanks for the re-roll here are some notes we should take care of to avoid any confusion or pushback.
'url' or 'href' may be a better key to not confuse people who have a conceptual concept of 'path'
I'd rather do this in the preprocess and as a route because we'll very likely change these paths all to routes and depreciate/change the url() function in twig.
I know I put this url/theme_path stuff in here but can we move that back to preprocess because again we may be changing url() to routes and hopefully coming up with a nicer way to get a global theme_path variable or function into twig. @see [2168231]
Comment #123
star-szrAdding the issue referenced in #3 above as a related issue.
Comment #124
JeroenTMade changes as suggested by joelpittet.
Patch attached.
Comment #125
joelpittetCouple more things, hope this goes green.
So we don't get in trouble by the routing police, the url() call can become:
\Drupal::url('node.type_add');
Have been told that url() will likely break things for multilingual by prefixing the file path with /fr/ or something. So we should be using file_create_url() here.
Comment #126
JeroenTI made the routing police happy again.
Comment #127
joelpittetThanks for the routing policing fix:)
Any chance you can confirm this actually works? I'd assume you need to still use the drupal_get_path('theme', 'seven') prefix in there, but I could be wrong.
Comment #128
longwaveThe drupal_get_path() call is still needed; without it, the tablesort indicators do not display properly.
Comment #130
joelpittetlol,
Comment #131
joelpittet128: 1987424-twig-seven-theme-128.patch queued for re-testing.
Comment #132
willieseabrook CreditAttribution: willieseabrook commentedComment #133
star-szrTagging for reroll.
Comment #134
mark.labrecqueComment #135
mark.labrecqueComment #136
mark.labrecqueComment #138
mark.labrecqueRe-rolled, but this fails at the Drupal installation test. Need to do a trace to determine the issue here.
Comment #139
joelpittetThanks for the re-roll @mark.labrecque, here are a couple notes though in general the re-roll looks great. I think it's the duplicate use statement.
This is now a duplicate, likely due to the patch getting in that did the remainder of these. Remove one 'use' statement.
Just noticed this else is not to code standards. The word else should start on the next line as it did before.
Comment #140
mark.labrecqueThanks for spotting that @joelpittet. I will try that.
Comment #141
mark.labrecqueComment #142
mark.labrecqueLet's try this...
Comment #143
mark.labrecqueComment #144
joelpittetAwesome could you post the interdiff as well? Docs here https://drupal.org/documentation/git/interdiff
Or the bottom of this little reference. http://pittet.ca/drupal/sprint/patch
Comment #145
joelpittetSomehow a an extra space got added to the end of this line. You can make your editor strip whitespace from the end of lines.
If you are using sublime add these two lines to your settings:
"trim_trailing_white_space_on_save": true,
"ensure_newline_at_eof_on_save": true,
Comment #146
JeroenTMade changes as suggested by joelpittet.
Patch attached.
Comment #147
JeroenTMarking as needs review.
Comment #148
JeroenTAnd this is the right interdiff.
Comment #149
LewisNymanComment #150
a-fro CreditAttribution: a-fro commentedSorry to bring up the routing police again, but shouldn't this be changed to the route name?
Otherwise, the code looks good. I'm also attaching a before/after diff on the themed output, which looked fine to me.
Comment #151
a-fro CreditAttribution: a-fro commentedI went ahead and made the change so we can try and get this through at DrupalCon.
Comment #152
a-fro CreditAttribution: a-fro commentedComment #153
a-fro CreditAttribution: a-fro commentedComment #154
a-fro CreditAttribution: a-fro commentedComment #155
a-fro CreditAttribution: a-fro commentedI ran a before/after diff on the output again after applying the last patch. I'm attaching the results.
What you'll see is that the only difference (other than the twig debug output) is that the tablesort indicator is now missing
@joelpittet suggests that we note it and ask @scor if that is an issue.
Also, we noted that there is a difference between how system.module and seven are rendering the tablesort indicator.
The implementation of the twig template at core/modules/system/templates/tablesort-indicator.html.twig might benefit from the approach taken here in seven.
Comment #156
mark.labrecqueComment #157
scor CreditAttribution: scor commentedDon't worry about typeof="foaf:Image", it's printed as a side effect of the way we render images. It's perfectly fine if it is no longer there, since those images are just from the presentation layer (not the actual data being displayed).
Any reason why this one is not at the third person? ("Adds ...")
Comment #158
a-fro CreditAttribution: a-fro commentedAfter talking with @cottser about @scor's documentation feedback, we decided it was clearer to remove the line altogether and update the description. Attaching an interdiff and the new patch.
Edited: ugh, forgot the commas. Patch coming momentarily.
Comment #159
a-fro CreditAttribution: a-fro commentedTrying again, with oxford commas ;) Think this is rtbc.
Comment #160
okami CreditAttribution: okami commentedComment #161
a-fro CreditAttribution: a-fro commentedExtra files removed. Sorry about that @cottser!
Comment #162
joelpittet@a-fro Thanks for the doc fixes and manual testing.
This is RTBC. No profiling needed as they are all admin ui facing.
Comment #163
star-szrKicking this back because the docs are not up to par and there is a @todo that can be addressed. Thanks @a-fro and everyone who has worked on this one so far :) we are close!
Hang on, theme_admin_block() has been converted to twig. Let's see what can be done with this.
Completely out of scope, please remove this hunk from the patch :) thanks!
'docblock' refers to path, template uses 'url'.
Remove these
@ingroup themeable
per #2226185: Remove @ingroup themeable from core theme Twig template docblocks please :)It looks like from the template and preprocess that only url, label, and description variables are actually available inside each block type.
Each item in types contains url, label, and description (as per the preprocess docs). This should be updated.
The add_content_type_url variable should be documented.
Comment #164
JeroenTComment #165
JeroenT1.
I was unable to delete this preprocess function completely because the url is in link_path and the twig file expects it to be in url.
2. Fixed
3. Fixed
4. Fixed
5. Fixed
6. Fixed
7. Fixed
Comment #166
JeroenTForgot patch..
Comment #167
JeroenTComment #168
star-szrThanks @JeroenT, the changes look great! This needs a reroll now though.
Comment #169
lokapujyaReroll.
Comment #170
joelpittetAwesome thanks @lokapujya and @Cottser, this is back to RTBC as per #168
Comment #171
star-szrUpdating the patch with a few changes here, I didn't say RTBC in 168 just that the changes looked good :)
I made 3 sets of changes, with an interdiff for each (or I made one interdiff with all changes if you prefer). 2 and 3 came from another round of manual testing where I found a couple templates were pretty broken.
Comment #172
lokapujyado we need to set the title variable in the preprocess function?
Comment #173
star-szrTitle is passed in by the calling code, for example:
Comment #174
lokapujyaadmin/structure/block/custom-blocks/types => admin/structure/block/block-content
Comment #175
lokapujyaComment #176
joelpittet@Cottser Ah sorry, I did a deductive RTBC. RTBC in #162 +- good on doc changes in #163- #168 + reroll = Back to RTBC.
Thanks for changing Path to URL, that is a bit clearer as path has special meaning to drupal.
Comment #177
hass CreditAttribution: hass commentedThere is a translatable string, but it cannot translated. Aside from this, isn't there no way of using a theme_image in twig?
Edit: i was not aware of trans, but the
<p>
need to be moved outside the translatable string.Comment #178
joelpittet@hass, as far as I know, you can't build renderable arrays in twig(maybe you can with
{{ {'#theme': 'image', '#uri': 'path/to/file.jpg', ...} }}
, though I've not tried). At the moment I see no need to do this though. If that is a need it be opened up into a followup issue, please? There are a number of twig filter/function related issues, though we really need to meta those up together...Thanks you for the catch of the
p
tag on the inside. The trans tag in twig is an easier way to write translatable strings with long or multi-line copy.Comment #179
alexpottCommitted bec1967 and pushed to 8.x. Thanks!
Comment #181
pwolanin CreditAttribution: pwolanin commentedWe might need to revert part of this - hard-coding the A tag contents for the system admin block is the wrong approach. We should have blocked that part on: #2073811: Add a url generator twig extension