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 PHPTemplate templates to Twig templates.
Remaining
- Patch needs review
Theme function name/template path | Conversion status |
---|---|
core/modules/forum/templates/forum-icon.tpl.php | converted |
core/modules/forum/templates/forum-list.tpl.php | converted, also has a #type table conversion issue: #1938906: Convert forum theme tables to table #type |
core/modules/forum/templates/forum-submitted.tpl.php | converted |
core/modules/forum/templates/forum-topic-list.tpl.php | converted, also has a #type table conversion issue: #1938906: Convert forum theme tables to table #type |
core/modules/forum/templates/forums.tpl.php | converted |
Testing steps
- Enable the Forum module.
- Navigate to admin/structure/forum/add/forum, the output should be rendered by forum-form.html.twig.
- Navigate to admin/structure/forum/add/container, the output should be rendered by forum-form.html.twig.
- Navigate to /forum. The outer container should be rendered by forums.html.twig, the list of forums rendered by forum-list.html.twig, "Last post" (n/a) rendered by forum-submitted.html.twig.
- Click "Add new forum topic" and add a test forum topic to the "General discussion" forum.
- Navigate to /forum, the topics, posts, and last post columns should be updated.
- Navigate to /forum/1. The outer container should be rendered by forums.html.twig, the list of forum topics rendered by forum-topic-list.html.twig, the forum topic icon rendered by forum-icon.html.twig. The topic submission and last reply string should be rendered by forum-submitted.html.twig.
- Test further by adding forum containers. A good test set would be a forum on the top level (not in a container), a container containing one forum, and a container containing more than one forum. Add descriptions to some of the forums and containers.
Related
#1938906: Convert forum theme tables to table #type
#1987400: forum.module - Convert theme_ functions to Twig
#1757550: [Meta] Convert core theme functions to Twig templates
Comment | File | Size | Author |
---|---|---|---|
#43 | 1898418-43-twig-forum-tpl-php.patch | 25.72 KB | joelpittet |
#43 | interdiff.txt | 1.96 KB | joelpittet |
#42 | interdiff.txt | 1.96 KB | joelpittet |
#42 | 1843740-59-views-exposed-form.patch | 9.49 KB | joelpittet |
#40 | 1898418-40.patch | 25.85 KB | star-szr |
Comments
Comment #1
c4rl CreditAttribution: c4rl commentedTagging
Comment #2
larowlanNote we're hoping much of the custom forum output will be powered by views eventually
Comment #3
joshuarussell CreditAttribution: joshuarussell commentedComment #4
disasm CreditAttribution: disasm commentedAttached is a patch
Comment #6
star-szrThanks @disasm! See #1898448-9: search.module - Convert PHPTemplate templates to Twig for a fix for the pager exceptions.
Comment #7
floydm CreditAttribution: floydm commentedA new version of the patch from #4 is attached with
$variables['pager']
add to the preprocess functions per #1898448-9: search.module - Convert PHPTemplate templates to Twig and a couple of whitespace issues fixed. Passes all forum tests in my sandbox.Comment #8
star-szrTagging.
Comment #9
star-szrI reviewed this patch over yesterday and today, working on a new patch a bit later today.
Comment #10
star-szrStill working on this, new patch will be up tomorrow.
Comment #10.0
star-szrAdd conversion summary table
Comment #10.1
star-szrAdd testing steps
Comment #11
star-szrLeaving the manual testing tag off, I've gone over the markup several times and can't see anything that has changed other than whitespace. I've added manual testing steps to the summary though.
Here's what I changed:
Comment #12
star-szrThis just removes some data types from Twig template documentation per http://drupal.org/node/1823416#docblock.
Comment #13
star-szrUpdating docs.
Comment #14
star-szrUpdated docs (forum is not a variable, the loop variable can be called anything), and updated the recursion loop hack to use @steveoliver's obviously better method from #1898432-58: node.module - Convert PHPTemplate templates to Twig :)
Comment #15
star-szrQuick tweak to use 'collection' instead of 'list' in the Twig template docs.
Comment #16
star-szrMinor tweaks and docs updates. I added a couple todos pointing to #1970960: Twig implementation does not print int(0) so we can remove the type casting once that is resolved.
I don't think these docs in forum-form.html.twig are helping anyone, removing :)
Comment #17
star-szrThis patch will need to be revised after #1975462: Allow and test for NULL and integer 0 values in Twig templates. lands to remove the type casting.
Comment #17.0
star-szrTweak test steps
Comment #18
star-szrNeeds a reroll for #1818560: Convert taxonomy entities to the new Entity Field API and #1975462: Allow and test for NULL and integer 0 values in Twig templates., I'll take care of it in the next couple days.
Comment #19
star-szrFirst the reroll for #1818560: Convert taxonomy entities to the new Entity Field API to make the patch apply again.
Comment #20
star-szrComment #21
star-szrUnassigning, this needs reviews please!
Comment #22
c4rl CreditAttribution: c4rl commentedPer #1757550-44: [Meta] Convert core theme functions to Twig templates, retitling to indicate this issue applies to templates rather than theme_ functions. See #1987400: forum.module - Convert theme_ functions to Twig for theme_ function conversion.
Comment #22.0
c4rl CreditAttribution: c4rl commentedRemove sandbox link
Comment #22.1
c4rl CreditAttribution: c4rl commentedRemove theme_ function listing
Comment #23
shanethehat CreditAttribution: shanethehat commentedComment #24
shanethehat CreditAttribution: shanethehat commentedComment #25
star-szrThanks @shanethehat :)
@steveoliver, review please!
Comment #26
star-szrTagging for profiling.
Comment #27
larowlanLooks good to me
Comment #28
steveoliver CreditAttribution: steveoliver commentedThis is almost ready. Super small nitpicks:
1. for forums templates (plural).
2. One extra linebreak needs deleted.
3. "Default theme implementation to display a status icon for a forum post."
4. Move those classes to
template_preprocess_form_icon()
5. good place to use whitespace dashes:
6. Wait, what -- @id=new?
7. Hmmm.. div @title and the invisible span? Aroo?
8. There are actually @id's all over the place in this patch.
Oh well, I guess 6, 7, and 8 can get cleaned up later.
Comment #29
jenlamptondibbs.
Comment #30
shanethehat CreditAttribution: shanethehat commented1-5 done.
Comment #31
jenlamptonWell, I was going to wait for that to pass tests, but since it's being slow, I've updated the attributes on the forum icon template here, and cleaned up the docblock a little.
Comment #33
Fabianx CreditAttribution: Fabianx commented#30:
Comment #34
c4rl CreditAttribution: c4rl commentedLooks like #31 has a bunch of issues due to inclusion of the old JSON LD code, I'll atttempt to fix.
Comment #35
c4rl CreditAttribution: c4rl commentedI think this accomplishes what was meant in #31.
Interdiff with respect to #30.
Comment #36
joelpittetJust one nitpick and a possible minor performance improvement. Cottser was saying mentioning there was a small improvement when we removed the empty variable declarations from comment module. So we may want to do that here too. Specially since we fixed #1975462: Allow and test for NULL and integer 0 values in Twig templates.
Don't think we need this and may help to remove for performance.
Same here
And here.
Please remove the space in front of {{ attributes }}
Comment #37
c4rl CreditAttribution: c4rl commentedComment #38
c4rl CreditAttribution: c4rl commentedJoel's nits from #36
Comment #40
star-szrI was wrong about initializing the variables, we need to at least initialize them unless we add if's to the template.
My mistake, sorry! See Drupal\Core\Template\TwigTemplate::getContextReference for why.
This reverts most of #38 other than the attributes tweak in the template.
Comment #42
joelpittetAh I see, here's the fix to #40. There was an accidental $$ and the way the classes were built out for the icon was not as it was originally.
Comment #43
joelpittetLet's put the right patch up for that, shall we?
Comment #44
star-szr@steveoliver, can you have another gander at this one?
Comment #45
steveoliver CreditAttribution: steveoliver commentedSince this addresses what I noticed in 1-5 of #28, I'd RTBC this after acceptable perf tests. Will get through the rest of my queue and come back and profile this if nobody gets to it before me.
Comment #46
jenlamptonprofiling.
Set up for profiling:
- change to stark theme
- enable forum module
- add 3 new forum topics
- add 3 comments to each forum topic
- build a view of one node in "full" view mode, block display
- put the block in the "content" region
Pages profiled:
- Forums page (forum) - forums, forum-list, forum-submitted
- Forum page (forum/1) - forum-icon, forum-topic-list
Comment #47
jenlamptonI'm having some crazy weirdness intermittantly with these tests, I'm not sure what is going on and could use some help debugging these results. Leaving as Needs benchmarking for now.
Forums Page (forums)
Profiler output
Profiler output
General DIscussion page (forum/1)
Profiler output
Profiler output
Comment #48
jenlamptonI just got the same results three times in a row for the second template....
Forums Page (forum)
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519b0e62696b0&...
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519b0e62696b0&...
General Discussion page (forum/1)
TEST ONE
Profiler output
Profiler output
TEST TWO
Profiler output
Profiler output
TEST THREE
">Profiler output
Profiler output
Comment #49
steveoliver CreditAttribution: steveoliver commentedThis looks great. Hey, a preprocess cleanup for performance FTW!
Comment #50
alexpott+1. Ready for #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch
Comment #51
alexpottCommitted 21da6fb and pushed to 8.x. Thanks!
Comment #52.0
(not verified) CreditAttribution: commentedRelated issues