taxonomy.module - Convert PHPTemplate templates to Twig

From sandbox issue: #1760990: Twig: convert core/modules/taxonomy/taxonomy-term.tpl.php

commit attribution goes to pixelmord and bstoppel

This looks good to me, just rolling a new patch to fix the @see references. The missing parens for these have now been fixed in the Twig sandbox.

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -552,7 +553,7 @@ function taxonomy_term_view_multiple(array $terms, $view_mode = 'full', $langcod
- * Process variables for taxonomy-term.tpl.php.
+ * Preprocess variables for taxonomy term templates.

Can this be "Preprocess variables for taxonomy-term.html.twig.", or is there a reason for referring to multiple templates?

We decided not to refer to the template by file name so that the preprocess function would not be Twig-specific, and could also be used for the PHPTemplate sticklers. I agree though, that it should not be plural. How about: Preprocess variables for the taxonomy term template. That seems more appropriate.

This looks very close to RTBC to me.

I would just mention that we sneak'ed in a change from $label -> $term_name. ;-)

Leaving for others to review, then RTBC.

A few more changes:
- updated preprocess docs to match new standards
- removed references to render_var and PHP variables in twig template docs
- removed references to data types in twig template docs
- double checked list of available variables in template - and corrected
- printed title_attributes and content_attributes in the template, so that if someone adds any ever, they will work.
- changed the variable name to url from term_url
- changed the variable name back to title (from name/label) for consistency with other templates, and title_prefix and title_suffix
- also printed title_prefix and title_suffix.

I'd also like to add that there is no where in core where we can see a taxonomy term full view mode that's not the term listing page (yet). I tested a patch at [#1881606 ] to see if I could use views to see one, but no luck yet. This will work for now, but we should retest after we can see term entities elsewhere.

Add commit message to summary

add steps to test

added related

Thanks @eamen, can you (or someone else) try the patch at #1881606: Use a derivative to integrate all entities as row plugins and see if we can get a taxonomy term to appear someplace other than on the taxonomy/term/N page, so we can test that template output too?

Once that is done (and if it works) please change the status to RTBC!

Tweaking commit message

Tried that #1881606, couldn't get past the views taxonomy term edit screen, posted a bug. Your twig conversion looks good except for a very minor trailing whitespace. (if you are using Sublime Text Editor, enable the TrailingSpaces Package, it's amazing:)

Noticed the form tables aren't in here, that's a good way to go. Here is the related #type=>table conversion for those if anybody is looking:
#1938932: Remove taxonomy_overview_terms() and taxonomy_overview_vocabularies() theme functions in favour of table #type

blocked

Add conversion summary table

Linkify, update blocker

The twig coding standards indicate that there should not be an "other variables" section: http://drupal.org/node/1823416

@jastraat - great catch! Feel free to reroll with that update and remove the trailing whitespace here:

+++ b/core/modules/taxonomy/templates/taxonomy-term.html.twigundefined
@@ -0,0 +1,52 @@
+ *   Use 'content' to print them all, or print a subset such as ¶

Re-rolled with changes from #11.

I diffed the two patches, changes look good. Thanks @jastraat!

This is looking really good, thanks everyone. Found some minor documentation tweaks and then I think this is ready to go:

1. +++ b/core/modules/taxonomy/taxonomy.moduleundefined
   @@ -529,17 +530,33 @@ function taxonomy_term_view_multiple(array $terms, $view_mode = 'full', $langcod
   + * Prepares variables for the taxonomy term template.
   

   Per #1913208: [policy] Standardize template preprocess function documentation this should end in "templates", so maybe:

   Prepares variables for taxonomy term templates.

2. +++ b/core/modules/taxonomy/templates/taxonomy-term.html.twigundefined
   @@ -0,0 +1,50 @@
   + * Default theme implementation to display a term.
   

   I think this should say "taxonomy term", even though the .tpl.php didn't.

3. +++ b/core/modules/taxonomy/templates/taxonomy-term.html.twigundefined
   @@ -0,0 +1,50 @@
   + * - url: Direct url of the current term.
   

   URL is an acronym so should be all caps.

4. +++ b/core/modules/taxonomy/templates/taxonomy-term.html.twigundefined
   @@ -0,0 +1,50 @@
   + * - content_attributes: HTML attributes for the title element.
   + *    It includes the 'class' information, which includes:
   + *   - taxonomy-term: The current template type, i.e., "theming hook".
   + *   - vocabulary-[vocabulary-name]: The vocabulary to which the term belongs to.
   + *     For example, if the term is a "Tag" it would result in "vocabulary-tag".
   

   The second level should be indented by one more space to line up with "It includes…"

5. +++ b/core/modules/taxonomy/templates/taxonomy-term.html.twigundefined
   @@ -0,0 +1,50 @@
   + * - is_front: Flags true when presented in the front page.
   + * - logged_in: Flags true when the current user is a logged-in member.
   + * - is_admin: Flags true when the current user is an administrator.
   

   I think we can probably leave out is_admin, is_front, and logged_in since those are available to every template, see _template_preprocess_default_variables().

Oops, status.

i will take it.

Documentation tweaks recommended on comment #14 done.

Changes look good, two minor nitpicks:

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -526,11 +526,12 @@ function taxonomy_term_view(Term $term, $view_mode = 'full', $langcode = NULL) {
+ * Prepares variables for the taxonomy term templates.

We should remove the word "the" now that it's plural.

+++ b/core/modules/taxonomy/templates/taxonomy-term.html.twigundefined
@@ -18,18 +18,15 @@
+ *    - vocabulary-[vocabulary-name]: The vocabulary to which the term belongs to.
+ *      For example, if the term is a "Tag" it would result in "vocabulary-tag".

The first line here is too long (actually, it was too long before as well but I didn't notice). Please wrap "to." onto the next line.

Edit: The phrasing is awkward anyway, how about "The vocabulary that this term belongs to." - then no need to wrap :)

Documentation tweaks recommended on comment #18 done.

Patch #19 fixed.

Update blocker again

I just manually tested the patch again, works well. I also compared output when removing the if statement from both the Twig and PHPTemplate files to make sure the linked title was output consistently.

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -525,21 +526,38 @@ function taxonomy_term_view(Term $term, $view_mode = 'full', $langcode = NULL) {
 function taxonomy_term_view_multiple(array $terms, $view_mode = 'full', $langcode = NULL) {
+

If we can just remove this extra line added in taxonomy_term_view_multiple(), I'd say this is ready to go!

tweaks recommended on comment #22 done.

Thanks @johnnygamba! After reviewing again I found another small update:

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -529,17 +530,33 @@ function taxonomy_term_view_multiple(array $terms, $view_mode = 'full', $langcod
+ *   - is_admin: Flags true when the current user is an administrator.
+ *   - logged_in: Flags true when the current user is a logged-in member.
+ *   - is_front: Flags true when presented in the front page.

I didn't notice these before, let's remove these 3 lines from the preprocess docblock. These variables are not passed in from the calling code, rather they are added in template_preprocess().

Hi @Cottser, no problem, here it is.

Thanks again @johnnygamba. I looked over this thoroughly and found one more bit of documentation that's a bit confusing and can be improved, this is in the preprocess function:

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -529,17 +530,30 @@ function taxonomy_term_view_multiple(array $terms, $view_mode = 'full', $langcod
+ *   - elements: An array of elements to display in view mode.

The wording here is confusing and I think we should be more clear about what this variable is and what it contains. Something like this could work:

 *   - elements: An associative array containing the taxonomy term and any
 *     fields attached to the term. Properties used:
 *     - #term: The term object.
 *     - #view_mode: The current view mode for this taxonomy term, e.g.
 *       'full' or 'teaser'.

Made the changes recommended in #26.

@myke - thank you! The changes look good, but taxonomy-term.html.twig is missing from your patch.

I have rerolled the above patch to include taxonomy-term.html.twig and new documentation. Also removed a few references to zebra and id variables in the documentation as per #1968322: Remove unused $id and $zebra variables from templates hence my interdiff relates to that change.

Update commit message

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -529,17 +530,31 @@ function taxonomy_term_view_multiple(array $terms, $view_mode = 'full', $langcod
-  $variables['term'] = $variables['elements']['#term'];
-  $term = $variables['term'];
+  $term = $variables['term'] = $variables['elements']['#term'];

Is there a reason for this change?

I'm not sure if it's official but I thought that we're usually not doing multiple assignments on a single line like this...

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -529,17 +530,31 @@ function taxonomy_term_view_multiple(array $terms, $view_mode = 'full', $langcod
-  $variables['label'] = check_plain($term->label());
-  $variables['page']      = $variables['view_mode'] == 'full' && taxonomy_term_is_page($term);
+  $variables['title'] = check_plain($term->label());

Hm, this was discussed before in another issue and possibly also here. The thing is that all entities now have a label. And terms have a name. But neither terms nor entities in general have a "title". So I'm not sure this makes sense to rename, at least not in such an issue that should just convert and not actually change things like this?

What about user.tpl.php, for example? Users also have a name and not a title (a title in that context would IMHO be something totally different).

@Berdir, after giving it some thought I actually agree with you on both points. Let's leave those both alone for now in this conversion.

Add API changes section for our variable tweak

Remove API changes

Implemented feedback from #30. This patch also removes title_prefix, title_suffix, title_attributes, and content_attributes from the Twig template since they weren't in the PHPTemplate version at all. If someone can explain their inclusion here we can roll this back.

I'll test it

Oops, my bad. Thought you were still looking for manual testers.

Removed title_prefix and title_suffix docs from the preprocess as well. These are standard variables but aren't used in this preprocess/template - see _template_preprocess_default_variables().

This is the theme layer, and most people who are building things here will see two things, 1) the Drupal user interface (which does not call any of these things a label) and 2) the code in the template file. The names of things in those two places need to match in all cases.

If someone changes the User Interface so the title on a node, a term, or a user becomes a "label", at that point we can talk about changing the variable name in the matching template file. If we keep coming up with arbitrary names for things that people won't recognize and can't relate to anything they've already seen then Drupal continues to be a huge WTF.

In preprocess (where we are using PHP, and we can expect that people will know what an entity is, and that a node, a user, a term, etc is actually one of these things) we can use the weird-drupal-entity-language, and wrap it nicely in a name that makes sense to people in templates.

Looking at the term UI, the correct name for the variable here is Name. I'll reroll to reflect that change.

okay, attached patch renames title/label to name, adds a comment that says why, removes some whitespace (linebreaks) in the template, and removes one line from the docblock in the template file (since globally added vars don't need to be defined in every template).

Giving her another go!

Update commit message

+ * - taxonomy-term: The current template type, i.e., "theming hook".

Don't need a comma after i.e.

+ *   - vocabulary-[vocabulary-name]: The vocabulary that this term belongs to.
+ *     For example, if the term is a "Tag" it would result in "vocabulary-tag".

This explanation seems to be mixing the concepts of a human readable name "Tag" with what is implied to be the machine name "tag" - in general though there could be a lot of differences beyond capitalisation between the human/machine names for a vocabulary.

+ * - term: Full term object.

"object" is a PHP data type so we shouldn't use it in Twig templates, unless we're using "object" to mean something else in a Twig template?

+ *   Use 'content' to print them all, or print a subset such as
+ *   'content.field_example'. Use {% hide(content.field_example) %} to
+ *   temporarily suppress the printing of a given element.

This is different to the way we're documenting properties of variables in Twig templates elsewhere, normally we just have an indented list of the available properties. Obviously this will be dynamic in this case, since they're fields that are configured per-vocab so it isn't so easy to just come up with a list but it's not clear to me from the docs how you'd access the "description".

I also noticed that while there are manual testing steps in the summary (amazing!) there is no tag for manual testing on the issue, so we need somebody to apply the patch, follow the steps and post the before/after markup.

I'll work on the reroll and tweaks.

First the reroll.

This should address most of #38 and #39 and I made a couple other docs tweaks. Feedback welcome :)

I compared the markup on the taxonomy term page via visual diff and DaisyDiff, it matches exactly.

So you don't have to take my word for it:

Before:

    <div id="taxonomy-term-1" class="taxonomy-term vocabulary-foo-bar-bazes">

  
  <div class="content">
    <div class="taxonomy-term-description"><p>Term goes here.</p></div>  </div>

</div>

After:

    <div id="taxonomy-term-1" class="taxonomy-term vocabulary-foo-bar-bazes">
    <div class="content">
    <div class="taxonomy-term-description"><p>Term goes here.</p></div>
  </div>
</div>

#42: 1898460-42.patch queued for re-testing.

#42: 1898460-42.patch queued for re-testing.

I will reroll.

Rerolled for #1966704: In-place editing for taxonomy terms and custom blocks (added title_prefix and title_suffix in the taxonomy-term template).

rm sand

I will provide a patch with only the .tpl.php conversions

eh.. seems to this module only has tpl.php templates then there is nothing to do here :-p

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -9,6 +9,7 @@
+use Drupal\Core\Template\Attribute;

@@ -492,6 +505,7 @@ function template_preprocess_taxonomy_term(&$variables) {
+  $variables['attributes'] = new Attribute($variables['attributes']);

Instantiating the Attribute object in preprocess is not needed now that #1982024: Lazy-load Attribute objects later in the rendering process only if needed is in, so we should remove both of these lines.

#53 done

Tagging for profiling.

Scenario: Using Stark, devel generate 1 term, 100 articles, and set site front page to taxonomy/term/. Comparing 8.x (3b1f85aa3) to #54.

=== 8.x..8.x compared (5199baba8e9fa..5199bd009a07e):

ct  : 77,473|77,473|0|0.0%
wt  : 620,231|623,142|2,911|0.5%
cpu : 616,038|616,039|1|0.0%
mu  : 7,099,588|7,099,588|0|0.0%
pmu : 7,190,276|7,190,276|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=5199baba8e9fa&...

=== 8.x..taxonomy-1898460-54 compared (5199baba8e9fa..5199bd928acb2):

ct  : 77,473|77,560|87|0.1%
wt  : 620,231|623,616|3,385|0.5%
cpu : 616,038|620,039|4,001|0.6%
mu  : 7,099,588|7,128,372|28,784|0.4%
pmu : 7,190,276|7,218,884|28,608|0.4%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=5199baba8e9fa&...

Nice work, thanks Ed!

+1. Ready for #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch

Revise summary

Committed 8f7dc88 and pushed to 8.x. Thanks!