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_forums
template_preprocess_forum_icon

Twig Templates Modified

forums.html.twig
forum-icon.html.twig

CommentFileSizeAuthor
#10 move_forum_classes_from-2329769-10.patch2.38 KBlauriii
#10 interdiff-2329769-7-10.txt1.98 KBlauriii
#7 move_forum_classes_from-2329769-7.patch2.37 KBlauriii
#4 2329769-forum.diff1.08 KBmortendk

Comments

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Parent issue: » #2322163: [meta] Consensus Banana Phase 1, move CSS classes from preprocess to twig templates.
davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Issue tags: +FUDK
mortendk’s picture

mortendk commented
Assigned: Unassigned » mortendk
mortendk’s picture

mortendk commented
Assigned: mortendk » Unassigned
Status: Active » Needs work
StatusFileSize
new 2329769-forum.diff1.08 KB

fixed the icon
needs some love for the table, think we should fix the tables first

rainbowarray’s picture

rainbowarray
she/her
Primary language English
Location Twin Cities, Minnesota
commented

This looks good. Think we'll need a .patch file for testbot to work?

The icon_status_class thing is kind of weird. We haven't had _class on the other variables we're passing to the template. That is how the variable is written in preprocess though I guess.

star-szr’s picture

star-szr
he/him
Primary language English
commented

Yeah it's a bit weird, icon_status_class is all built in preprocess and as it is doesn't need to go through clean_class either. But it's too complex for a ternary and it looks like we'd have to pass a ton of weird variables to the template to build the class entirely in the template.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Needs review
StatusFileSize
new move_forum_classes_from-2329769-7.patch2.37 KB

I changed icon_status_class to icon_status since icon_status_class didn't make sense to me. I also removed clean_class from it.

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented

Are we excluding the forums change from this, or is there a good way to tackle that table code?

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs review » Needs work

I don't think there is much we can do at this point about the table, there isn't really a template for it at this time. 

  1. +++ b/core/modules/forum/forum.module
@@ -694,26 +694,25 @@ function template_preprocess_forum_icon(&$variables) {
-    $icon_status_class = 'sticky';
+    $icon_status = 'sticky';
...
+  $variables['icon_status'] = $icon_status;

    While we're making this change why not just do $variables['icon_status'] directly, and get rid of the assignment at the end? 

  2. +++ b/core/modules/forum/templates/forum-icon.html.twig
@@ -11,13 +11,20 @@
+ * - icon_status: Points which status icon should be used.

    Maybe 'Indicates' instead of 'Points' here?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2329769-7-10.txt1.98 KB
new move_forum_classes_from-2329769-10.patch2.38 KB

I agree with @Cottser since there's no easy way to move classes from the table to templates since there isn't one.

mikl’s picture

mikl
Location Møn
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

alexpott’s picture

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

Committed a5d36ef and pushed to 8.0.x. Thanks!

  • alexpott committed a5d36ef on 8.0.x 
    Issue #2329769 by lauriii, mortendk | davidhernandez: Move forum classes...

Status: Fixed » Closed (fixed)

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