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_rdf_metadata

Twig Templates Modified

rdf-metadata.html.twig

Files: 
CommentFileSizeAuthor
#17 move_rdf_classes_from-2329787-17.patch1.49 KBcilefen
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,073 pass(es). View
#17 interdiff-12-17.txt1005 bytescilefen
#14 move_rdf_classes_from-2329787-14.patch1.4 KBcilefen
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,102 pass(es). View
#14 interdiff-12-14.txt561 bytescilefen
#12 move_rdf_classes_from-2329787-12.patch1.34 KBcilefen
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,086 pass(es). View
#12 interdiff-10-12.txt906 bytescilefen
#10 move_rdf_classes_from-2329787-10.patch1.33 KBcilefen
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,866 pass(es). View
#10 interdiff-8-10.txt575 bytescilefen
#8 move_rdf_classes-2329787-8.patch1.26 KBtuutti
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,865 pass(es), 0 fail(s), and 1 exception(s). View
#8 interdiff-2329787-3-8.txt380 bytestuutti
#3 move_rdf_classes-2329787-3.patch1.27 KBtuutti
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,872 pass(es), 0 fail(s), and 1 exception(s). View

Comments

davidhernandez’s picture

Issue tags: +FUDK
scor’s picture

Issue tags: +RDF code sprint
tuutti’s picture

Status: Active » Needs review
FileSize
1.27 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,872 pass(es), 0 fail(s), and 1 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 3: move_rdf_classes-2329787-3.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: move_rdf_classes-2329787-3.patch, failed testing.

Cottser’s picture

Thanks for the patch @tuutti!

+++ b/core/modules/rdf/templates/rdf-metadata.html.twig
@@ -13,5 +13,11 @@
+  <span{{ attributes }}{{ attributes.addClass(classes) }}></span>

This would result in double attributes. Should be:

<span{{ attributes.addClass(classes) }}></span>

tuutti’s picture

Status: Needs work » Needs review
FileSize
380 bytes
1.26 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,865 pass(es), 0 fail(s), and 1 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 8: move_rdf_classes-2329787-8.patch, failed testing.

cilefen’s picture

Status: Needs work » Needs review
FileSize
575 bytes
1.33 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,866 pass(es). View

It looks as though $attributes are not set sometimes when $variables is passed to the prerprocess function. The non-patched file sets $attributes['class'] to an array every time so it was not noticed. This patch sets $attributes to an empty array if it was not set.

Cottser’s picture

Status: Needs review » Needs work

Thanks @cilefen!

  1. +++ b/core/modules/rdf/rdf.module
    @@ -567,10 +567,9 @@ function rdf_preprocess_image(&$variables) {
    +    if (!isset($attributes)) {
    +      $attributes = array();
    +    }
    

    I think we should probably add a short code comment to explain why this is being added.

  2. +++ b/core/modules/rdf/templates/rdf-metadata.html.twig
    @@ -13,5 +13,11 @@
    +  {%
    +    set classes = [
    +      'rdf-meta',
    +      'hidden',
    +    ]
    +  %}
    +  <span{{ attributes.addClass(classes) }}></span>
    

    Since this is just two simple classes, maybe we should just do it inline?

    {{ attributes.addClass('rdf-meta', 'hidden') }}
    
cilefen’s picture

Status: Needs work » Needs review
FileSize
906 bytes
1.34 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,086 pass(es). View
Cottser’s picture

Status: Needs review » Needs work

Thanks!

To me that code comment just explains the code but not the "why", maybe something along the lines of:

In some cases $attributes is not set, in those cases make it an array for when we construct the Attribute object below.

Edit: or, shorter:

If $attributes is not set make it an empty array for constructing the Attribute object below.

cilefen’s picture

Status: Needs work » Needs review
FileSize
561 bytes
1.4 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,102 pass(es). View
Cottser’s picture

Status: Needs review » Reviewed & tested by the community

Makes sense to me, thanks @cilefen!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/rdf/rdf.module
@@ -567,10 +567,11 @@ function rdf_preprocess_image(&$variables) {
+    // In some cases $attributes is not set. In those cases make it an array
+    // for when we construct the Attribute object below.
+    if (!isset($attributes)) {

It is not that it is not set. It is that is null. This could be a if else too just set $variables['metadata'][$key] = new Attribute(); and then the comment could be shorter. Also the comment below about the doctype looks like it should be move to the twig template - can someone open a followup.

cilefen’s picture

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

Those are good suggestions @alexpott. I think we can do without a comment altogether this way.

#2336965: Move the XHTML inline comment in template_preprocess_rdf_metadata to the template file

Cottser’s picture

Status: Needs review » Reviewed & tested by the community

That seems reasonable to me, is_array() might be better than !is_null potentially but that's about it. Not sure if there any performance differences between the two.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed b29ad66 and pushed to 8.0.x. Thanks!

  • alexpott committed b29ad66 on 8.0.x
    Issue #2329787 by cilefen, tuutti | davidhernandez: Move RDF classes...

Status: Fixed » Closed (fixed)

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