Allow instantiating Attribute objects within Twig

The Attribute class already includes the functions: addClass, removeClass, hasClass, setAttribute, removeAttribute.

Using this: /themes/my-theme/templates/html.html.twig

{% set attr = 'data-test' %}
{% set val = 'foobar' %}
{%
  set classes = [
    'top',
  ]
%}
<!DOCTYPE html>
<html{{ html_attributes }}>
  <head>
...
  </head>
  <body{{ attributes.addClass(classes).setAttribute(attr, val) }}>
    <ul class="skip-to-content">
...

the body element is rendered something like:

<body class="top toolbar-tray-open toolbar-fixed toolbar-horizontal" data-test="foobar">

So is this a case for documentation?

I agree that this would be useful.

Use case:
You have a field with many items, say images, and you're making a bootstrap carousel from them. You may want to define a default attributes object for each item that you can easily addClass for the "active" one to.

@nicholasThompson any chance you'd like to take a stab at creating this as a proof of concept?

I'd love to if I had the faintest idea of where to start... ;-)

Start by adding a function or filter in the TwigExtension class.
/core/lib/Drupal/Core/Template/TwigExtension.php

My guess would be a function that would create a new instance of an Attribute object.

That would likely be all that is need I guess.

#2571561: Add Twig filter for date formatting, #2398331: Add the ability to attach asset libraries directly from a template file, and #2308187: Provide a twig extension for file_create_url do something similar.

I'd totally appreciate this in order to be able to include templates with a completely new Attribute() object set in the include-statement. This is for example needed when building templates within the atomic design process, to be able to add modifier classes while including a subcomponent template.

+1 on the idea here. I'm going to mark this as novice, because creating a small helper function in TwigExtension per comment #8 is super simple to do.

I'll try work on this at Drupalcon Dublin

1. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -600,4 +601,17 @@ public function safeJoin(\Twig_Environment $env, $value, $glue = '') {
   +   * @param array $attributes
   +   *   Array (key, value) for attributes.
   

   This should match the parameter arguments provided by Attribute.

2. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -600,4 +601,17 @@ public function safeJoin(\Twig_Environment $env, $value, $glue = '') {
   +  public function addAttributes($attributes) {
   

   This doesn't add multiple attributes. Maybe calling this addAttribute would be better.

   No type hinting or default values so if someone doesn't pass anything, this is going to fail miserably. Take a look at the default constructor arguments on Attribute and copy/paste them into here.

3. Also, after uploading a patch, the status is normally changed to 'needs review'.

Called an array in a function __construct with a class Attribute

Thanks for working on this issue! This would be very useful improvement.

1. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -134,6 +134,7 @@ public function getFunctions() {
   +      new \Twig_SimpleFunction('addAttributes', [$this, 'addAttributes']),
   

   I think we should use underscore writing compound since all the other Twig functions are following that pattern

2. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -594,4 +595,20 @@ public function safeJoin(\Twig_Environment $env, $value, $glue = '') {
   +   * Create an Attribute object from an array.
   

   s/Create/Creates

3. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -594,4 +595,20 @@ public function safeJoin(\Twig_Environment $env, $value, $glue = '') {
   +   * @return Attribute
   

   This should include the full namespace

4. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -594,4 +595,20 @@ public function safeJoin(\Twig_Environment $env, $value, $glue = '') {
   +    if (!empty($attributes)) {
   

   I think this should return new Attributes object even if $attributes is empty

5. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -594,4 +595,20 @@ public function safeJoin(\Twig_Environment $env, $value, $glue = '') {
   +        new Attribute($attribute);
   

   The attributes object should be returned in the end

This patch include all items comments into #15 and #17

I iterated that a little further

Here's interdiff

1. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -54,6 +54,13 @@ class TwigExtension extends \Twig_Extension {
   +   * An empty attributes object that can be cloned.
   

   nit: An empty Attribute object that can be cloned.

2. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -600,4 +608,25 @@ public function safeJoin(\Twig_Environment $env, $value, $glue = '') {
   +    if (empty($attributes)) {
   

   This outside if is not needed. It should always check attributes !isset($this->attributes) and if it's not there create the empty new object.

   After clone if it's ! empty, maybe something like this. (like the constructor but not in the constructor)

   foreach ($attributes as $name => $value) {
   $cloned_attributes[$name] = $value;
   }

If that's getting to messy we can look at that in a follow-up issue for a slight performance gain.

#23 makes sense. I went a little further and renamed the property from $this->attributes => $this->attribute. Interdiff attached.

Since this is a new feature, also moving to 8.3. Although (hint, hint) this would be awful nice in 8.2 as well.

I think this looks very well put together. I'll add this to the twig_extensions if it takes a while to get in.

Didn't mean to change the version.

@joelpittet, that seems to just be a wrapper around twig upstream. Maybe opening an issue in twig_tweaks or some other twig++ contrib module is a better fit. And given this is slotted for 8.3, it will be a few months before folks start seeing it generally available.

Hmm good point, don't want to scope creep my own project, lol. Yeah, I can do something like that too, hopefully in a non-disruptive to when the filter gets into core.

In this patch add multiple attributes #30

Here I applied correctly patch #24 and add attributes asked in comment #30

1. +++ b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php
   @@ -309,14 +309,15 @@ public function testCreateAttribute() {
   +  ¶
   ...
   +    ¶
   

   Whitespace.

2. +++ b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php
   @@ -309,14 +309,15 @@ public function testCreateAttribute() {
   +      ['class' => ['puppies']],
   +      ['class' => ['dollies']],
   

   Let's add some attributes that aren't class, maybe try a data-* attribute. And also, let's leave some empty ones too, just for completeness.

Fixed 2 items to comments #34
1.Whitespace
2. Add some attributes

1. +++ b/core/lib/Drupal.php
   @@ -81,7 +81,7 @@ class Drupal {
   -  const VERSION = '8.2.1-dev';
   

   fixed

2. +++ b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php
   @@ -299,6 +299,26 @@ public function testRenderVarWithGeneratedLink() {
   +  ¶
   

   space fixed

3. +++ b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php
   @@ -299,6 +299,26 @@ public function testRenderVarWithGeneratedLink() {
   +    $expected = '<div class="kittens" data-toggle="modal" data-lang="es"></div><div id="puppies" data-value="foo" data-lang="en"></div>';
   

   added empty div

#30 is addressed

+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -600,4 +608,26 @@ public function safeJoin(\Twig_Environment $env, $value, $glue = '') {
+    if (!isset($this->attribute)) {
+      $this->attribute = new Attribute();
+    }
+    $cloned_attributes = clone $this->attribute;

Why the cloning dance?

@alexpott it's what we do in core for attributes that get created often for performance instead of re-instantiating the same class.

References:

core/includes/theme.inc:1571
core/lib/Drupal/Core/Theme/ThemeManager.php:364

We can do without it too, it's just to continue the pattern.

There is a missing space here that could be fixed on commit:
'data-value' =>['foo']

Or I'll fix it in a re-roll if we are going to remove the performance clone stuff.

I don't think that that micro-optimisation is worth in a method with only a single test call in core. If it becomes an issue then we could change it. Also I doubt that this test result still holds for PHP7... https://stackoverflow.com/questions/18040137/which-is-more-efficient-whe...

Removed the micro-optimization, thanks @alexpott. Also fixed the array space, added a description to the test and removed the extra arrays in the attributes that aren't needed(only class needs to be an array)

@vaplas Nice script, what version of PHP were you using and which method was which approach(could you edit your comment)?, I tried it and couldn't get a big diff either way which was closer to what @alexpott mentioned.

Latest patch works and looks good!

We also need a CR to tell people about this new functionality.

Created CR

After fixing this issue the name will forever

That is not correct. We can change it later with BC layer in case we feel like changing it.

I'm not against the name we chose, the short hand ones are a no go at all, as we try to use full words and not abbreviations in core.

Also this has some sense, it's clear what it's doing. It creates an Attribute object, so follows the name and doesn't obfuscate that fact, even though that name singular has thrown me a few times, it makes sense.

+1 on create_attribute. It was what I naturally was planning to call this thing when I first looked at this issue.

This should work as a test, uses the method on the object without any args added to the create_attribute function.

Looks great