Contrib.social

SDC ComponentElement: Transform slots TranslatableMarkup values to #markup instead of throwing an exception

Open on Drupal.org →
Created on 5 February 2025, 3 months ago

Problem/Motivation

This can be seen as a follow-up for #3391702 🐛 SDC ComponentElement: Transform slots scalar values to #plain_text instead of throwing an exception Fixed

On that issue, it was proposed that #slots were made more resilient and forgiving by accepting non-scalar values.

Another common Drupalism we might be facing on slots are translated strings, which

  1. Can include simple inline HTML tags, like <br> or <em>
  2. Will take the shape of a TranslatableMarkup, which will fail both the is_scalar and Element::isRenderArray checks and, as such, will be rejected witj an exception

Steps to reproduce

Try to feed a translated string (with or without HTML) to a slot:

      return [
        '#type' => 'component',
        '#component' => 'theme:icon_message',
        '#props' => [
          'icon' => $this->options['icon'],
        ],
        '#slots' => [
          'message' => $this->t('No @type were found.<br> Please try again later.', ['@type' => $this->options['content_type']]),
        ]
      ];

A 'Unable to render component "%s". A render array or a scalar is expected for the slot "%s" when using the render element with the "#slots" property', will be thrown.

Proposed resolution

As we did with scalars, wrap the translatable markup in order to make it pass the checks and render.

Remaining tasks

Propose a MR

User interface changes

None

Introduced terminology

None

API changes

None

Data model changes

None

Release notes snippet

✨ Feature request
Status

Active

Version

11.0 🔥

Component

single-directory components

Created by

🇪🇸Spain idiaz.roncero Madrid

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

Merge Requests

Comments & Activities

  • Issue created by @idiaz.roncero
  • Comment 3 months ago →
    🇪🇸Spain idiaz.roncero Madrid
  • Comment 3 months ago →
    🇪🇸Spain idiaz.roncero Madrid
  • Merge request !111143504477 - Make TranslatableMarkup slot-ready by wrapping it → (Open) created by idiaz.roncero
  • Comment 3 months ago →
    🇪🇸Spain idiaz.roncero Madrid

    Submitted MR that I have tested locally and works.

  • Pipeline finished with Failed
    3 months ago
    Total: 399s
    #415549
  • Comment 3 months ago →
    🇺🇸United States smustgrave

    Thank you for opening.

    Lets add some test coverage for the proposed change.

  • Pipeline finished with Failed
    3 months ago
    Total: 436s
    #416587
  • Pipeline finished with Failed
    3 months ago
    Total: 98s
    #416616
  • Pipeline finished with Success
    3 months ago
    #416640
  • Comment 3 months ago →
    🇪🇸Spain idiaz.roncero Madrid

    Added test, executed pipeline and it passed.

  • Comment 2 months ago →
    🇺🇸United States smustgrave

    Removing tests tag as the test-only run shows the coverage

    1) Drupal\KernelTests\Components\ComponentRenderTest::testRender
Drupal\Core\Render\Component\Exception\InvalidComponentDataException: Unable to render component "sdc_test:my-banner". A render array or a scalar is expected for the slot "banner_body" when using the render element with the "#slots" property

    Summary seems clear with clear proposal that lines up with the code change.

    Code change seems pretty straightforward and see no objection.

    LGTM

  • Comment about 2 months ago →
    🇨🇭Switzerland berdir Switzerland

    Can confirm that this fixes an issue I see with navigation:title the entity returns a TranslatableMarkup object.

    Left one comment on the MR whether this should be expanded to MarkupInterface or even Stringable.

  • Comment 29 days ago →
    🇬🇧United Kingdom catch

    I think we should use MarkupInterface here.

  • Comment 26 days ago →
    🇨🇭Switzerland berdir Switzerland

    Updated, also shortened the test method description, it was already over 80 characters and that made it even longer.

  • Pipeline finished with Success
    26 days ago
    Total: 457s
    #459229
  • Comment 22 days ago →
    🇺🇸United States smustgrave 
    1) Drupal\KernelTests\Components\ComponentRenderTest::testRender
Drupal\Core\Render\Component\Exception\InvalidComponentDataException: Unable to render component "sdc_test:my-banner". A render array or a scalar is expected for the slot "banner_body" when using the render element with the "#slots" property
/builds/issue/drupal-3504477/core/lib/Drupal/Core/Render/Element/ComponentElement.php:118
/builds/issue/drupal-3504477/core/lib/Drupal/Core/Render/Element/ComponentElement.php:65
/builds/issue/drupal-3504477/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php:107
/builds/issue/drupal-3504477/core/lib/Drupal/Core/Render/Renderer.php:830
/builds/issue/drupal-3504477/core/lib/Drupal/Core/Render/Renderer.php:387
/builds/issue/drupal-3504477/core/lib/Drupal/Core/Render/Renderer.php:459
/builds/issue/drupal-3504477/core/lib/Drupal/Core/Render/Renderer.php:203
/builds/issue/drupal-3504477/core/tests/Drupal/KernelTests/Components/ComponentKernelTestBase.php:95
/builds/issue/drupal-3504477/core/lib/Drupal/Core/Render/Renderer.php:593
/builds/issue/drupal-3504477/core/tests/Drupal/KernelTests/Components/ComponentKernelTestBase.php:95
/builds/issue/drupal-3504477/core/tests/Drupal/KernelTests/Components/ComponentRenderTest.php:313
/builds/issue/drupal-3504477/core/tests/Drupal/KernelTests/Components/ComponentRenderTest.php:45
ERRORS!

    Shows the test coverage

    Believe this would solve a problem I see on my patterns theme were I have to pass #markup => content, in the twig file.

    Change makes sense.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024