Problem/Motivation

We need a way to let type's define attributes.

Proposed resolution

  • Setup a #type #pre_render hook for attributes: drupal_pre_render_attributes to build attributes for a type.

Before

// With lazy loading keys ('attributes', 'content_attributes', 'title_attributes')
function template_preprocess_something(&$variables) {
  // Special variable keys.
  $variables['attributes'] = array(
    'class' => array('i', 'am', 'lazy', 'and', 'special'),
  );
  $variables['title_attributes'] = array(
    'class' => array('me', 'two'),
  );
  $variables['content_attributes'] = array(
    'title' => "Don't forget me too",
  );
  // The rest of the gang.
  $variables['nested']['attributes'] = new Attribute(array(
    'src' => '/not/special/either.gif')
  );
  $variables['whatever_attributes'] = new Attribute(array(
    'alt' => 'Un fair:-('
  );
}

After

// Still converted to Attribute object's but later in the process for the twig layer.
function template_preprocess_something(&$variables) {
  // Playing fair and consistant.
  $variables['attributes'] = array(
    '#type' => 'attributes',
    'class' => array('i', 'am', 'a', 'work', 'horse'))
  );
  $variables['title_attributes'] = array(
    '#type' => 'attributes',
    'class' => array('me', 'two'))
  );
  $variables['content_attributes'] = array(
    '#type' => 'attributes',
    'title' => "not as fast but fair is fair.")
  );
  // The rest of the gang.
  $variables['nested']['attributes'] = array(
    '#type' => 'attributes',
    'src' => '/img/same.gif')
  );
  $variables['whatever_attributes'] = array(
    '#type' => 'attributes',
    'alt' => 'just like funky and the bunch',
  );
}

Remaining tasks

TBD

API changes

TBD

#1757550: [Meta] Convert core theme functions to Twig templates
#2006282: Refactor Attribute classes - Cleanup, Security, and Readability and minor performance
#2108771: Remove special cased title_attributes and content_attributes for Attribute creation

Files: 
CommentFileSizeAuthor
#4 2048637-4-type-attributes.patch4.85 KBjoelpittet
FAILED: [[SimpleTest]]: [MySQL] 54,083 pass(es), 917 fail(s), and 1,588 exception(s). View
#4 interdiff.txt2.56 KBjoelpittet
#2 2048637-type-attributes.patch2.23 KBjoelpittet
FAILED: [[SimpleTest]]: [MySQL] 59,052 pass(es), 1 fail(s), and 4 exception(s). View
#2 2048637-1-type-attributes.patch2.43 KBjoelpittet
PASSED: [[SimpleTest]]: [MySQL] 58,939 pass(es). View

Comments

joelpittet’s picture

Title: Add drupal_pre_render_attributes » Add #type => 'attributes' and wrap in Attribute class with drupal_pre_render_attributes
Issue tags: +Performance, +Twig, +DX

Changing title and adding tags

joelpittet’s picture

Issue summary: View changes

added before/after

joelpittet’s picture

Status: Active » Needs review
FileSize
2.43 KB
PASSED: [[SimpleTest]]: [MySQL] 58,939 pass(es). View
2.23 KB
FAILED: [[SimpleTest]]: [MySQL] 59,052 pass(es), 1 fail(s), and 4 exception(s). View

Here's a patch to see if this will work. I'd much rather return an Attribute from drupal_render()... I think. My problem is that I want the variable passed to twig to be of type Attribute... so my test needs to change and probably have to add in a loop inside theme over all the variables... which could be not so efficient...

I attached my first attempt too which will fail just to show what I did.

jenlampton’s picture

I like this idea!

Also see my comment here.

We should not have anything named title_attributes or content_attributes. If we can fix that (by removing those and correctly adding only attributes where needed) this code starts to look much nicer too. :)

jenlampton’s picture

Issue summary: View changes

Updated issue summary.

joelpittet’s picture

FileSize
2.56 KB
4.85 KB
FAILED: [[SimpleTest]]: [MySQL] 54,083 pass(es), 917 fail(s), and 1,588 exception(s). View

I'm abusing those default keys for the sake of a quick test. I've typed 'attributes', 'content_attributes', 'title_attributes'.

Also, @Fabianx probably has a better way to deal with all the intricacies of late rendering and converting the attributes at time of printing, but I had a hard time dealing with {{ test.attributes.class }} {{ test.attributes }} when I tried my hand at doing the 'casting' in twig_render_var().

// In bartik.theme
function bartik_preprocess_page(&$variables) {
$variables['test']['attributes'] = array(
    '#type' => 'attributes',
    'id' => 'test-id',
    'class' => array('test-class-1' ,'test-class-2'),
    'boolean' => TRUE,
  );
...
        // In bartik's page.html.twig.
        just class attribute value: {{ test.attributes.class }} <br>
        remainder: {{ test.attributes }}<br>

If anybody want's a quick test you can use that to see if things are working.

I'll run a performance test on this patch to see how it fairs with that recursion in there.

joelpittet’s picture

profiling is slow, so yeah, need a hand getting this type'd array turned into Attribute either in twig parsing or variable rendering, which ever works with making attribute values drillable.

=== 8.x..8.x compared (5257bf3a694a2..5257bf7e7133b):

ct  : 66,990|66,990|0|0.0%
wt  : 307,495|309,127|1,632|0.5%
cpu : 271,913|272,138|225|0.1%
mu  : 15,476,624|15,476,624|0|0.0%
pmu : 15,629,960|15,629,960|0|0.0%

=== 8.x..2048637-type-attributes compared (5257bf3a694a2..5257bfb7ad158):

ct  : 66,990|72,253|5,263|7.9%
wt  : 307,495|319,780|12,285|4.0%
cpu : 271,913|283,301|11,388|4.2%
mu  : 15,476,624|15,517,824|41,200|0.3%
pmu : 15,629,960|15,795,568|165,608|1.1%

Status: Needs review » Needs work

The last submitted patch, 2048637-4-type-attributes.patch, failed testing.

joelpittet’s picture

"xxx" is an invalid render array key is my nemesis. Anyways I'm sure all those errors don't help performance either.

joelpittet’s picture

Issue summary: View changes

Updated issue summary.

tim.plunkett’s picture

Issue summary: View changes
Issue tags: -DX +DX (Developer Experience)
joelpittet’s picture

Status: Needs work » Closed (won't fix)

I no longer want to do this... so if someone wants to re-open this giver. And ask any questions of me on IRC if you are curious.