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

CommentFileSizeAuthor
#4 2048637-4-type-attributes.patch4.85 KBjoelpittet
#4 interdiff.txt2.56 KBjoelpittet
#2 2048637-type-attributes.patch2.23 KBjoelpittet
#2 2048637-1-type-attributes.patch2.43 KBjoelpittet

Comments

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
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

joelpittet
he/him
Primary language English
Location Vancouver
commented
Issue summary: View changes

added before/after

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs review
StatusFileSize
new 2048637-1-type-attributes.patch2.43 KB
new 2048637-type-attributes.patch2.23 KB

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

jenlampton
she / her
commented

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

jenlampton
she / her
commented
Issue summary: View changes

Updated issue summary.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
StatusFileSize
new interdiff.txt2.56 KB
new 2048637-4-type-attributes.patch4.85 KB

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

joelpittet
he/him
Primary language English
Location Vancouver
commented

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

joelpittet
he/him
Primary language English
Location Vancouver
commented

"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

joelpittet
he/him
Primary language English
Location Vancouver
commented
Issue summary: View changes

Updated issue summary.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue summary: View changes
Issue tags: -DX +DX (Developer Experience)
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
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.