Move classes out of the preprocess functions and into the Twig templates. Use the addClass() attribute method to add classes in the template. Use the clean_class filter to filter class names, if necessary. Maintain all existing functionality and ensure all existing class names are still in the markup, even ones that are inherited.

See the following issues for more detailed examples:
#2217731: Move field classes out of preprocess and into templates
#2254153: Move node classes out of preprocess and into templates

See this change record for information about using the addClass() method:
https://www.drupal.org/node/2315471

See this change record for more information about the phase 1 process of moving class from preprocess to templates:
https://www.drupal.org/node/2325067

Preprocess Functions Modified

template_preprocess_taxonomy_term

Twig Templates Modified

taxonomy-term.html.twig

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

davidhernandez’s picture

Issue tags: +FUDK
lauriii’s picture

Assigned: Unassigned » lauriii
lauriii’s picture

Assigned: lauriii » Unassigned
Status: Active » Needs review
FileSize
1.32 KB
ckrina’s picture

Status: Needs review » Reviewed & tested by the community

The patch applies cleanly and the taxonomy terms mantain all the classes in the markup after it.
So moving it to RTBC.

lauriii’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.57 KB

I added comment about the new variable vocabulary_name to twig.

star-szr’s picture

+++ b/core/modules/taxonomy/taxonomy.module
@@ -320,10 +320,7 @@ function template_preprocess_taxonomy_term(&$variables) {
+  $variables['vocabulary_name'] = $term->bundle();

I don't have a strong objection but just curious why term.bundle wasn't used directly in the Twig template.

Looks good though :)

lauriii’s picture

That is true! I didn't notice that the term object is being passed for the template but its perfect!

RainbowArray’s picture

This last patch lost the comment about the variable vocabulary_name.

RainbowArray’s picture

Status: Needs review » Needs work
lauriii’s picture

Status: Needs work » Needs review

We don't have it as a variable anymore because we are using term.bundle. Putting back to needs review.

star-szr’s picture

+++ b/core/modules/taxonomy/templates/taxonomy-term.html.twig
@@ -29,7 +29,13 @@
+<div id="taxonomy-term-{{ term.id }}"{{ attributes.addClass(classes) }}>

Perhaps out of scope (it was like this before…) but the attributes should probably be |without('id') otherwise we could end up with two ID attributes.

And it could be cleaned up nicer with #2325517: Add methods for adding/removing attributes (not classes) on Attribute objects. Otherwise, looks great to me.

lauriii’s picture

FileSize
1.34 KB

That is a good thing to fix. Now we cannot get double id attributes.

RainbowArray’s picture

Status: Needs review » Reviewed & tested by the community

Awesome. Great work laurii!

davidhernandez’s picture

Should bundle be added to the comment under term: The taxonomy term entity, including:?

RainbowArray’s picture

Term bundle isn't something we're new we're adding with this patch. So not sure.

davidhernandez’s picture

No, not a new term, but I also wouldn't assume themers know what that means. I really only bring it up for completion sake, because the comment block has a term section. (Probably because term.id was already used.)

* - term: The taxonomy term entity, including:
 *   - id: The ID of the taxonomy term.
star-szr’s picture

Status: Reviewed & tested by the community » Needs work

Good catch, let's add it.

lauriii’s picture

Status: Needs work » Needs review
FileSize
1.65 KB

That is a good catch! I would also assume that term.bundle isn't obvious for all of the themers.

RainbowArray’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

Laurii, could you make sure to post interdiffs when you post patches? That makes it easier to see what has changed from patch to patch. Thanks for this fix!

lauriii’s picture

Yeah sure! Just tought its a bit overhead for small patch like this :)

star-szr’s picture

Manually tested to confirm, the order of the classes changed and that's it.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed cdad8d7 and pushed to 8.0.x. Thanks!

  • alexpott committed cdad8d7 on 8.0.x
    Issue #2329775 by lauriii | davidhernandez: Move taxonomy term classes...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.