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_filter_guidelines
template_preprocess_text_format_wrapper
template_preprocess_filter_tips

Twig Templates Modified

filter-guidelines.html.twig
text-format-wrapper.html.twig
filter-tips.html.twig

CommentFileSizeAuthor
#11 interdiff.txt1.88 KBstar-szr
#11 2329837-11.patch4.8 KBstar-szr
#7 interdiff.txt504 bytesstar-szr
#7 2329837-7.patch4.79 KBstar-szr
#5 interdiff.txt1.08 KBstar-szr
#5 2329837-5.patch4.66 KBstar-szr
#3 2329837-3.patch4.67 KBstar-szr
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez commented
Issue tags: +FUDK
star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Assigned: Unassigned » star-szr

Going to try this one.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Assigned: star-szr » Unassigned
Status: Active » Needs review
FileSize
2329837-3.patch4.67 KB

This is going to be blocked on #2330731: Attribute::addClass() adds empty class because there are single classes added with a ternary. But other than that, reviews please!

I used the |replace filter in filter-tips.html.twig to preserve the pre-patch classes which contain underscores (can be seen by navigating to /filter/tips).

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez commented

Why use wrapper_classes on tip and tip_classes on item? Wouldn't tip_classes, item_classes be better? Also, on using description_class, is the aria attribute accessible at the template?

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
FileSize
2329837-5.patch4.66 KB
interdiff.txt1.08 KB

Thanks for the review! The aria-describedby gets unset, so it's not accessible through the template. tip_classes and item_classes does make a lot more sense, here's that.

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez commented
Status: Needs review » Needs work

We don't need this line in template_preprocess_filter_tips since long is no longer checked in preprocess, correct?

$long = $variables['long'];

Otherwise, everything works fine. I manually tested and saw all the same classes before and after.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs work » Needs review
FileSize
2329837-7.patch4.79 KB
interdiff.txt504 bytes

Nice catch, thanks @davidhernandez!

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/filter/filter.module
@@ -752,9 +749,10 @@ function template_preprocess_filter_guidelines(&$variables) {
+  $variables['description_class'] = FALSE;
...
   if (isset($variables['attributes']['aria-describedby'])) {
-    $variables['attributes']['class'][] = 'description';
+    $variables['description_class'] = TRUE;

+++ b/core/modules/filter/templates/text-format-wrapper.html.twig
@@ -7,6 +7,8 @@
+ * - description_class: Flag for whether or not to add a class to the
+ *   description container.

@@ -16,6 +18,11 @@
+      set classes = [
+        description_class ? 'description',
+      ]

Do we need description_class - can't we just check if attributes.aria-describedby is set in the twig template?

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs work » Reviewed & tested by the community 
+++ b/core/modules/filter/filter.module
@@ -752,9 +749,10 @@ function template_preprocess_filter_guidelines(&$variables) {
   if (isset($variables['attributes']['aria-describedby'])) {
-    $variables['attributes']['class'][] = 'description';
+    $variables['description_class'] = TRUE;
     $variables['attributes']['id'] = $variables['attributes']['aria-describedby'];
     // Remove aria-describedby attribute as it shouldn't be visible here.
     unset($variables['attributes']['aria-describedby']);

It's not available in the template because it is unset after being checked in the preprocess. It's basically being moved to the ID attribute. Here is the issue: #2126761: The body field summary textarea indicates it has a description with aria-describedby attribute, but the DOM id value points to a non-existent node.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
FileSize
2329837-11.patch4.8 KB
interdiff.txt1.88 KB

Better variable name c/o @alexpott :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 364c388 and pushed to 8.0.x. Thanks!

  • alexpott committed 364c388 on 8.0.x 
    Issue #2329837 by Cottser | davidhernandez: Move filter classes from...

Status: Fixed » Closed (fixed)

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