Overview

Based on this discussion with Pierre and Wim

Indeed, I would not recommend to add this to your component definitions:

    attributes:
      type: Drupal\Core\Template\Attribute
      name: Attributes
      title: Attributes

Because this prop is injected automatically by SDC anyway.

But most importantly, because using a PHP namespace as a prop type is not JSON Schema compliant. It is an SDC quirk, a Drupalism, we need to get rid off.

Related issue, kind of: #3457874: HTML attributes as Twig mappings instead of PHP objects

— @pdureau at #3468944-11: Update XB's `image` SDC to comply with best practices, and document those best practices

We can and should remove the attributes section of components, e.g.

https://git.drupalcode.org/project/experience_builder/-/blob/0.x/compone...

    # @see \Drupal\Core\Template\ComponentsTwigExtension::mergeAdditionalRenderContext()
    attributes:
      type: Drupal\Core\Template\Attribute
      name: Attributes
      title: Attributes

Proposed resolution

Remove them.

User interface changes

Issue fork experience_builder-3470575

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kristen pol created an issue. See original summary.

kristen pol credited pdureau.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

Adding credit.

omkar-pd made their first commit to this issue’s fork.

omkar-pd opened merge request !216

omkar-pd’s picture

omkar-pd commented
Status: Active » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Remove attributes in XB components » Remove `attributes` prop from all SDCs in the Experience Builder module
Component: Page builder » Code
Priority: Normal » Major
Issue summary: View changes
Issue tags: +DX (Developer Experience)
Related issues: +#3468944: Update XB's `image` SDC to comply with best practices, and document those best practices, +#3457874: HTML attributes as Twig mappings instead of PHP objects , +#3462705: [META] Missing features in SDC needed for XB

+1 for theory

First: I'd love to see this simplified: it's very unfortunate that SDC's metadata is all JSON schema, except for this. Both the SDC subsystem and the XB module have to do a fair bit of special casing to make this work. For example, in XB:

…
      // TRICKY: `attributes` is a special case — it is kind of a reserved
      // prop.
      // @see \Drupal\sdc\Twig\TwigExtension::mergeAdditionalRenderContext()
      // @see https://www.drupal.org/project/drupal/issues/3352063#comment-15277820
      if ($prop_name === 'attributes') {
        assert($prop_schema['type'][0] === Attribute::class);
        continue;
      }
…

… but in practice: not sure?

I'm not quite convinced yet. 😅

If this is such a bad practice, then why does every SDC in Drupal core have this?!

See:

  • core/profiles/demo_umami/themes/umami/components/branding/branding.component.yml
  • core/profiles/demo_umami/themes/umami/components/banner/banner.component.yml (literally the only prop there!)
  • core/themes/olivero/components/teaser/teaser.component.yml (literally the only prop there!)

So: can you point to a place where this removal was discussed?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Postponed (maintainer needs more info)

Project: Experience Builder » Drupal Canvas
Version: » 1.x-dev

Experience Builder has been renamed to Drupal Canvas in preparation for its beta release. You can now track issues on the new project page.