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_container

Twig Templates Modified

container.html.twig

Files: 
CommentFileSizeAuthor
#9 interdiff-2329759-6-9.txt691 bytesSutharsan
#9 drupal-container-classes-2329759-9.patch1.55 KBSutharsan
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,614 pass(es). View
#6 drupal-container-classes-2329759-6.patch1.63 KBSutharsan
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,595 pass(es), 16 fail(s), and 0 exception(s). View

Comments

davidhernandez’s picture

Issue tags: +FUDK
Sutharsan’s picture

Assigned: Unassigned » Sutharsan
Sutharsan’s picture

Assigned: Sutharsan » Unassigned
Status: Active » Needs review
FileSize
914 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,559 pass(es), 49 fail(s), and 30 exception(s). View

template_preprocess_container adds one static class. No comments to change in either the preprocess or the twig template.

Cottser’s picture

Status: Needs review » Needs work

Thanks @Sutharsan! I think we need to create a variable in preprocess to use in the Twig template to set this class conditionally. Otherwise the form-wrapper class will be added unnecessarily.

The last submitted patch, 3: drupal-container-classes-2329759-3.patch, failed testing.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
1.63 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,595 pass(es), 16 fail(s), and 0 exception(s). View
1.7 KB

Hmm, did I really overlook that :/
Not 100% sure about the name 'has_parent'. But it matches with FAPI #array_parents

mdrummond’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/templates/container.html.twig
@@ -9,10 +9,16 @@
+{% if has_parent %}
+  <div{{ attributes.addClass('form-wrapper') }}>{{ children }}</div>
+{% else %}
+  <div{{ attributes }}>{{ children }}</div>
+{% endif %}

The method we've preferred for setting conditional classes is a ternary operator in a set classes block at the top. So something like this:

has_parent ? 'form-wrapper'

You can look at the meta for an example of how this is done.

The advantage of this approach is that it isolates the variation to just the class name. Otherwise if somebody wants to change the HTML, they have to change it in two places.

Thanks for the patches!

The last submitted patch, 6: drupal-container-classes-2329759-6.patch, failed testing.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
1.55 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,614 pass(es). View
691 bytes

#7 comment included.

However, .... this will fail some tests because <div{{ attributes.addClass() }}>results in <div class="">. This is because addClass assumes existing classes.

Sutharsan’s picture

I created #2330731: Attribute::addClass() adds empty class to fix the empty class problem.

Status: Needs review » Needs work

The last submitted patch, 9: drupal-container-classes-2329759-9.patch, failed testing.

Cottser’s picture

Great, this looks pretty good to go after that gets committed. The only thing is according to our guidelines in #2322163: [meta] Consensus Banana Phase 1, move CSS classes from preprocess to twig templates. since this is using a ternary it should be in a set tag, but we can discuss that as well.

davidhernandez’s picture

Cottser’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 9: drupal-container-classes-2329759-9.patch, failed testing.

Cottser’s picture

Status: Needs work » Postponed
Related issues: +#2336355: Refactor Attribute rendering so that class="" is not printed

For anyone following along at home, we are now blocked on #2336355: Refactor Attribute rendering so that class="" is not printed.

davidhernandez’s picture

Cottser’s picture

Status: Needs review » Reviewed & tested by the community

Good to go now.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 542980f on 8.0.x
    Issue #2329759 by Sutharsan: Move container classes from preprocess to...

Status: Fixed » Closed (fixed)

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

Leagnus’s picture

How can i get views name (or any other views attributes) in container.html.twig to set individual class for its wrapper? i'm using bootstrap theme.
Should i initially set a var in a theme_preprocess_page(), and then check its value in the template file?