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_toolbar

Twig Templates Modified

toolbar.html.twig

CommentFileSizeAuthor
#5 move_toolbar_classes-2329789-5.patch2.09 KBlauriii
#3 move_toolbar_classes-2329789-3.patch2.09 KBlauriii
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
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Assigned: Unassigned » lauriii
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Assigned: lauriii » Unassigned
Status: Active » Needs review
FileSize
move_toolbar_classes-2329789-3.patch2.09 KB

I moved all the classes from preprocess but there's still some left in other functions. Should we handle those also in this issue?

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs review » Needs work

@lauriii what other functions?

+++ b/core/modules/toolbar/templates/toolbar.html.twig
@@ -22,11 +22,11 @@
+  <nav{{ toolbar_attributes.addClass( [ 'toolbar-bar', 'clearfix' ] ) }}>

Let's keep the inline class adding consistent please, passing multiple arguments to addClass rather than an array.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
FileSize
move_toolbar_classes-2329789-5.patch2.09 KB

Fixed that for consistency. There is still classes added e.g. in toolbar_toolbar()

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Status: Needs work » Needs review

Sending to testbot.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs review » Reviewed & tested by the community

Ah I see. I don't think we can mess with that too much, at least not in this issue. It looks like it is setting up individual toolbar items.

I re-reviewed and manually tested, this one looks good. Thanks @lauriii!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • webchick committed 08bd73b on 8.0.x 
    Issue #2329789 by lauriii: Move toolbar classes from preprocess to...

Status: Fixed » Closed (fixed)

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