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_field__node__title() to a Twig template.
This was added in #1988612: Apply formatters and widgets to rendered entity base fields, starting with node.title but now theme_field() is gone so we shouldn't be overriding it anymore.
Remaining tasks
- Patch
- Patch review
- Manual testing
Steps to test
- Create a node.
- Visit the homepage (default homepage view). The node title of the teaser should be rendered via field--node--title.html.twig.
Comment | File | Size | Author |
---|---|---|---|
#7 | interdiff.txt | 1.47 KB | joelpittet |
#7 | 2231505-7-twig-theme_field__node__title.patch | 3.02 KB | joelpittet |
Comments
Comment #1
baisongPatch attached. This is my first twig conversion patch, so please let me know what might be wrong!
Comment #2
star-szrThanks @baisong, this looks like a great start!
We should refer to field.html.twig instead of the now-defunct theme_field() function.
This could be something like "List of all the field items."
No space between
<span
and {{ attributes }} here, the Attribute object automatically inserts whitespace as needed so that we don't end up with things like<span >
:)Comment #3
baisongThanks, Cottser! Incorporated feedback.
Comment #6
baisongIt looks like it failed on these 2 assertions in LocalePathTest.php:
Line 143
Line 145
Trying to debug through the test locally...
Comment #7
joelpittetThose are just whitespace errors because of the newline at the end of the twig file.
That xpath looks suspicious as it says .= when there is also a span element in the a tag. This changes that.
@baisong thanks for pointing out the error messages and putting together this patch! I need this for another issue I'm working on:)
#2229435: Clean up the way attributes are printed in field.html.twig
Comment #8
star-szrThanks for the quick fix @joelpittet and thanks again @baisong for working on this!
The markup is identical here and this unblocks #2229435: Clean up the way attributes are printed in field.html.twig and #2226805: Remove unneeded code from template_preprocess_field(). I think this is ready to go.
Before:
After (including debug output):
Comment #9
star-szrFilling in testing steps in the issue summary.
Comment #11
star-szr7: 2231505-7-twig-theme_field__node__title.patch queued for re-testing.
Comment #12
star-szrLooks like a random test fail.
Comment #13
webchickHm. This is used in lots of places, so let's please get some profiling on this change.
Comment #14
star-szrAgreed, I can profile this.
Comment #15
star-szrProfiling scenario: 50 nodes in teaser mode on the default frontpage view (adjusted to display 50 nodes).
Total: 50 uses of this template.
The numbers look fine to me, keep in mind that this page isn't actually this slow, I had to disable render cache to actually get a comparison: https://gist.github.com/Cottser/10997441
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=5350e128cfd2b&...
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=5350e128cfd2b&...
Here's a run from 8.x with render cache enabled:
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?extra=8.x..8.x&sour...
(wall time 685,129 microsecs)
Comment #16
webchickOh shoot, I'm so sorry! Thought I committed this one already. :(
Committed and pushed to 8.x. Thanks!
Comment #18
joelpittetThank you!