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

Files: 
CommentFileSizeAuthor
#10 move_forum_classes_from-2329769-10.patch2.38 KBlauriii
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,656 pass(es). View
#10 interdiff-2329769-7-10.txt1.98 KBlauriii

Comments

davidhernandez’s picture

davidhernandez’s picture

Issue tags: +FUDK
mortendk’s picture

Assigned: Unassigned » mortendk
mortendk’s picture

Assigned: mortendk » Unassigned
Status: Active » Needs work
FileSize
1.08 KB

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

mdrummond’s picture

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.

Cottser’s picture

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

Status: Needs work » Needs review
FileSize
2.37 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,259 pass(es). View

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

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

Cottser’s picture

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

Status: Needs work » Needs review
FileSize
1.98 KB
2.38 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,656 pass(es). View

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

Status: Needs review » Reviewed & tested by the community

Looks good to me.

alexpott’s picture

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.