Contrib.social

Process #attributes render property

Open on Drupal.org →
Created on 26 March 2025, 7 months ago

Problem/Motivation

SDC automatically add a \Drupal\Core\Template\Attribute attributes prop in ComponentsTwigExtension::mergeAdditionalRenderContext.

This prop can be used as any "normal" prop:

$element['#props']['attributes'] = $value;

Some existing Drupal mechanisms, in Core & Contrib, expect and leverage this prop. For example, |add_class() Twig filter:

  public function addClass(array $element, ...$classes): array {
    $attributes = new Attribute($element['#attributes'] ?? []);
    $attributes->addClass(...$classes);
    $element['#attributes'] = $attributes->toArray();
    // Make sure element gets rendered again.
    unset($element['#printed']);
    return $element;
  }

However, calling $element['#attributes'] is not currently working with an SDC component, causing unexpected behaviours.

Proposed resolution

Simply move the value of #attributes property to $element["#props"]["attributes"] before the rendering.

You can take inspiration from ComponentElementAlter::processAttributesRenderProperty

  public function processAttributesRenderProperty(array $element): array {
    if (!isset($element["#attributes"])) {
      return $element;
    }
    if (is_a($element["#attributes"], '\Drupal\Core\Template\Attribute')) {
      $element["#attributes"] = $element["#attributes"]->toArray();
    }
    // Like \Drupal\Core\Template\Attribute::merge(), we use
    // NestedArray::mergeDeep().
    // This function is similar to PHP's array_merge_recursive() function, but
    // it handles non-array values differently. When merging values that are
    // not both arrays, the latter value replaces the former rather than
    // merging with it.
    $element["#props"]["attributes"] = NestedArray::mergeDeep(
      $element["#attributes"],
      $element["#props"]["attributes"] ?? []
    );
    return $element;
  }

And add a related test.

API changes

We don't break anything, we just improve compatibility with existing Drupal mechanisms.

✨ Feature request
Status

Active

Version

11.0 🔥

Component

single-directory components

Created by

🇫🇷France pdureau Paris

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @pdureau
  • Comment 7 months ago →
    🇫🇷France pdureau Paris
  • Comment 7 months ago →
    🇫🇷France pdureau Paris
  • Comment 7 months ago →
    🇮🇳India dipakmdhrm
  • Merge request !11649Merge component ['#attributes'] with the ['#props']['attributes']. → (Closed) created by dipakmdhrm
  • Pipeline finished with Failed
    7 months ago
    Total: 447s
    #458798
  • Pipeline finished with Success
    7 months ago
    Total: 422s
    #458825
  • Pipeline finished with Success
    7 months ago
    Total: 783s
    #458873
  • Comment 7 months ago →
    🇮🇳India dipakmdhrm
  • Pipeline finished with Success
    7 months ago
    Total: 335s
    #458899
  • Comment 7 months ago →
    🇫🇷France goz

    Looks great.
    What's about a test to confirm props attributes will keep its values in case keys exist in both.

        $build = [
      '#type' => 'component',
      '#component' => 'sdc_theme_test:my-card',
      '#props' => [
        'header' => 'Drupal.org',
        'attributes' => new Attribute([
          'foo' => 'bar',
        ]),
      ],
      '#attributes' => [
        'foo => 'third',
      ],
    ];

    I think in this case, foo should still be bar.

    And what's about attribute with array like classes for example ?

        $build = [
      '#type' => 'component',
      '#component' => 'sdc_theme_test:my-card',
      '#props' => [
        'header' => 'Drupal.org',
        'attributes' => new Attribute([
          'foo' => ['bar', 'ter'],
        ]),
      ],
      '#attributes' => [
        'foo' => ['quater'],
      ],
    ];
  • Comment 7 months ago →
    🇫🇷France pdureau Paris

    Thanks you @goz. Updating issue status.

  • Comment 6 months ago →
    🇫🇷France Grimreaper France 🇫🇷
  • Comment 6 months ago →
    🇫🇷France Grimreaper France 🇫🇷

    MR rebased and updated regarding comment 7.

  • Pipeline finished with Success
    6 months ago
    Total: 326s
    #497318
  • Comment 6 months ago →
    🇫🇷France pdureau Paris

    genklegni

  • Pipeline finished with Canceled
    6 months ago
    Total: 285s
    #497359
  • Comment 6 months ago →
    🇫🇷France pdureau Paris

    Oops, it was not something to merge yet. Merge train cancelled.

  • Assigned to pdureau
  • Status changed to Needs review 4 months ago
  • Comment 4 months ago →
    🇺🇸United States smustgrave

    @pdureau what's needed for this one?

  • Comment 4 months ago →
    🇫🇷France pdureau Paris

    Thanks Steven. I rebase and check the pipeline first. Review later.

  • Pipeline finished with Failed
    4 months ago
    Total: 159s
    #540850
  • Pipeline finished with Success
    4 months ago
    Total: 397s
    #540860
  • Pipeline finished with Success
    about 2 months ago
    Total: 779s
    #594081
  • Comment about 2 months ago →
    🇺🇸United States smustgrave

    Ran the test-only feature

    1) Drupal\KernelTests\Components\ComponentRenderTest::testRender
<html><body><div id="sdc-wrapper"><div foo="bar" data-component-id="sdc_theme_test:my-card">
  <h2 class="component--my-card__header">Drupal.org</h2>
  <div class="component--my-card__body">
          Default contents for a card
      </div>
</div>
</div>
</body></html>
Failed asserting that an object is not empty.
/builds/issue/drupal-3515506/core/tests/Drupal/KernelTests/Components/ComponentRenderTest.php:302
/builds/issue/drupal-3515506/core/tests/Drupal/KernelTests/Components/ComponentRenderTest.php:44
FAILURES!
Tests: 3, Assertions: 29, Failures: 1.
Exiting with EXIT_CODE=1

    Which shows the coverage. Don't see any open threads but want to leave assigned to pdureau :)

    Would it be worth a change record to announce things like |add_class now work with attributes?

  • Pipeline finished with Success
    about 1 month ago
    Total: 876s
    #600945
  • Pipeline finished with Success
    about 1 month ago
    Total: 746s
    #600959
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024