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

Files: 
CommentFileSizeAuthor
#11 interdiff.txt1.88 KBCottser
#11 2329837-11.patch4.8 KBCottser
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,100 pass(es). View
#7 interdiff.txt504 bytesCottser
#7 2329837-7.patch4.79 KBCottser
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,066 pass(es). View
#5 interdiff.txt1.08 KBCottser
#5 2329837-5.patch4.66 KBCottser
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,605 pass(es). View

Comments

davidhernandez’s picture

Issue tags: +FUDK
Cottser’s picture

Assigned: Unassigned » Cottser

Going to try this one.

Cottser’s picture

Assigned: Cottser » Unassigned
Status: Active » Needs review
FileSize
4.67 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,607 pass(es). View

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

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?

Cottser’s picture

FileSize
4.66 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,605 pass(es). View
1.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

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.

Cottser’s picture

Status: Needs work » Needs review
FileSize
4.79 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,066 pass(es). View
504 bytes

Nice catch, thanks @davidhernandez!

davidhernandez’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

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?

Cottser’s picture

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.

Cottser’s picture

FileSize
4.8 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,100 pass(es). View
1.88 KB

Better variable name c/o @alexpott :)

alexpott’s picture

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.