Update XB's `image` SDC to comply with best practices, and document those best practices

Wim Leers → credited pdureau → .

👋 @pdureau — time to finally respond in detail to what you wrote on August 1!

• it has a single prop, also called "image", so the component looks like a meaningless wrapper around this single prop

Why is that meaningless?

When is it appropriate to use nested key-value pairs?

Not being an SDC expert, I think the current structure of components/image/image.component.yml makes sense: the image property fully encapsulates all concerns/semantics of what "an image" constitutes.

• it doesn't use the expected attributes object

If this is expected, then why isn't this imposed by SDC? 🤔

• no HTML classes (and the component has no dedicated CSS), so no specific UI purpose

Correct, this is meant to be one of the "elements" (see 1.1 Elements in the product requirements).

IOW: this is intended to be a building block in other components.

• it is nothing more than a rigid HTML element, like an image renderable but less powerful because with hardcoded attributes (so not compatible with lazy loading or any API working with images)

That's a good argument to add an attributes property in this particular case, but why is that generically true?

attributes object

If this is expected, then why isn't this imposed by SDC? 🤔
That's a good argument to add an attributes property in this particular case, but why is that generically true?

It is the other way around. Every component template has an attributes object automatically injected.

Its is good practice to use this already available attributes object in the template:

• because he can be leverage by some API or mechanism, which are injecting class utilities, attributes used by display builders, ARIA attributes for accessibility, semantic annotation (RDFa, schema.org...), SEO tagguing, JS events.... without duplications of attributes
• it is a more elegant way to allow the injection custom classes than a extra_classes array of strings prop, because it also checks duplicates

However, I believe this attributes can be even better by allowing #attributes key alongside #props["attributes"], and I may create a Core issue as soon as UI Patterns 2.0 is released.

Image component and image prop

Why is that meaningless?
When is it appropriate to use nested key-value pairs?

Because the full component and its single prop are the exact same thing :

name: Image
props:
  type: object
  properties:
    image: 
    title: image
      type: object
      required:
      - src
      properties:
        src:
          title: Image URL
          "$ref": json-schema-definitions://experience_builder.module/image-uri
        alt:
          title: Alternative text
          type: string
        width:
          title: Image width
          type: integer
        height:
          title: Image height
          type: integer

With:

<img src="{{ image.src }}" alt="{{ image.alt }}" width="{{ image.width }}" height="{{ image.height }}"></img>

It says something about the architecture. So we have a layer, the image component or the image prop, which is not expected here.

If we remove the prop, we get that:

name: Image
props:
  type: object
  required:
  - src
  properties:
    src:
      title: Image URL
      "$ref": json-schema-definitions://experience_builder.module/image-uri
    alt:
      title: Alternative text
      type: string
    width:
      title: Image width
      type: integer
    height:
      title: Image height
      type: integer

With:

<img src="{{ src }}" alt="{{ alt }}" width="{{ width }}" height="{{ height }}"></img>

But we can remove the component instead, as proposed in the next section.

HTML elements as SDC components

Correct, this is meant to be one of the "elements" (see 1.1 Elements in the product requirements).
IOW: this is intended to be a building block in other components.

Indeed, we need HTML elements renderables which are lighter than components:

• no templates, no predefined attachments
• no props and slots but the standardized #attributes and the element content

Oh wait, we have this already 😉

[
    '#type' => 'html_tag',
    '#tag' => 'p',
    '#value' => $this->t('Hello World'),
    '#attributes' => [
      'foo' => 'bar',
      'height' => 50,
    ],
];

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

No need to create a SDC component for each HTML elements, like:

<img src="{{ src }}" alt="{{ alt }}" width="{{ width }}" height="{{ height }}"></img>

No need to create a generic HTML element as a SDC component with:

<{{ html_tag }}{{attributes}}>{{ content }}</{{ html_tag }}>

Because HTML elements are not component. They don't have a design system meaning. We don't want to have them in the component libraries, we don't want them to show in component selectors, we don't want to load the full SDC API to render them.

Not every renderable must become a SDC component. I know, it is tempting, with such a great hammer, everything looks like a nail.

HTML elements, Icons (see https://www.drupal.org/project/ui_icons → ), Inline Templates, Plain Text... are renderables but not components. And we will discover more on our journey to design system implementations.

So, maybe Experience Builder needs to handle non-SDC renderables in slots. For example, a WYSIWYG for #markup;

It is the other way around. Every component template has an attributes object automatically injected.

TIL. But my question still stands, albeit differently: why doesn't SDC then not complain about this Twig template not printing the attributes variable at all?

Indeed, we need HTML elements renderables which are lighter than components:

• no templates, no predefined attachments
• no props and slots but the standardized #attributes and the element content

Oh wait, we have this already 😉

This is not the way @lauriii has been talking about "elements" (during https://wimleers.com/xb-week-5#chaos-origin). He's been referring to them as a carefully curated set of SDCs, tagged/categorized as "elements".

IOW: "elements" !== "HTML elements".

So, maybe Experience Builder needs to handle non-SDC renderables in slots. For example, a WYSIWYG for #markup;

We need a WYSIWYG to be able to provide HTML markup that goes into a prop. See 📌 [later phase] `StringSemanticsConstraint::MARKUP`: agree how SDC prop JSON schema can convey it should be markup, and allow using CKEditor 5 Needs review — I bet you'll want to chime in on that issue too :)

WRT definition of "elements" — see discussion at 📌 Clarify "components" vs "elements" vs "patterns" Active , there's no definitive answer yet.

TIL. But my question still stands, albeit differently: why doesn't SDC then not complain about this Twig template not printing the attributes variable at all?

SDC is currently not validating the component templates. It doesn't check used by undefined slots or props, it doesn't check defined but unused slots or props.

ui_patterns_devel module has a drush command to do such checks. I hope it will be useful and appreciated by the SDC community.

This is not the way @lauriii has been talking about "elements" (during https://wimleers.com/xb-week-5#chaos-origin). He's been referring to them as a carefully curated set of SDCs, tagged/categorized as "elements".

IOW: "elements" !== "HTML elements".

OK. Do you have an example of such ? Maybe https://git.drupalcode.org/issue/experience_builder-3446722/-/tree/exper... is not a good one, because this one is clearly an HTML element.

We need a WYSIWYG to be able to provide HTML markup that goes into a prop. See #3467959: `StringSemanticsConstraint::MARKUP`: agree how SDC prop JSON schema can convey it should be markup, and allow using CKEditor 5 — I bet you'll want to chime in on that issue too :)

Of course I will add a comment on this issue ;)

I enjoy our conversation, but I am also surprised we (Experience Builder team & UI Suite team) have different understandings and opinions on those SDC fundamentals: what is a component? how to use it? how to chose between slots and props? and other topics...

I know Experience Builder is trying to build both a display builder (a better Layout Builder) and an UI components loader/manager (like UI Patterns 2), and I understand it is not an easy task. But I am also afraid the components management may be bent in some ways we may regret later, in order to comply with the current state and needs of the display builder.

I hope it will be useful and appreciated by the SDC community.

I think that belongs in Drupal core! (Although I could see that the performance cost might be too high 😬)

Do you have an example […]

No, not yet. 😭 This is something I've been asking @lauriii to clarify and formally document in 📌 Clarify "components" vs "elements" vs "patterns" Active .

[…] but I am also surprised […] But I am also afraid the components management may be bent in some ways we may regret later, in order to comply with the current state and needs of the display builder.

Great concerns. I very strongly agree.

Two observations:

1. I don't know what those different perspectives are. 📌 Document supported component modeling approaches Active (another issue needing formal documentation…) is the closest we have to answering what "our perspective" is. You'll see that I'm not leading that conversation, because I know that I'm not an expert. It's @lauriii and @ctrlADel who've been the voices in that issue. I think it'd be great for you to participate in that issue's discussion 🙏
2. The current SDC-based implementation is just an early implementation, in service of 🌱 Milestone 0.1.0: Experience Builder Demo Active . After that's done (after DrupalCon Barcelona), attention will shift towards generalizing, i.e. supporting more "component types": 🌱 [META] Support component types other than SDC Needs work . IOW: the XB codebase will evolve.

(FYI: the ADR + docs that were added last week in 📌 Document the current JSON-based data model, and describe in an ADR Active finally formally document what the status quo is and describes the need minimum evolutions we'll need to meet all XB product requirements.)

This was very educational!

For example, I didn't realize that attributes were injected as I saw them explicitly in XB components, e.g.

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

Based on this, I'm assuming we can just remove them from our SDCs.

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: ✨ HTML attributes as Twig mappings instead of PHP objects Active

Thanks! Created followup for XB:

📌 Remove attributes in XB components Needs review

and reopened ours for SDDS:

📌 Update SDDS SDCs attributes prop to align with XB Fixed

#11 + #12: wow, that means this is yet another thing that SDC in core should be doing differently/better. That pattern is literally all over Drupal core! 🤪

Responding in more detail over at 📌 Remove attributes in XB components Needs review — let's continue that there.

@pdureau detailed response at #3470575-7: Remove `attributes` prop from all SDCs in the Experience Builder module → , looking forward to your feedback! 🤓🙏

Elements are ultimately built-in building blocks provided by the Experience Builder. These will be eventually used by the component to build components that would be exposed within the component library.

It seems that the confusion is likely arising from the fact that currently both elements, and components are defined using SDC and from that perspective they look alike. As argued by #5, there could be ways to define them outside of SDC but I'm not sure I understand why that would be required, so long as we can separate elements from components?

Even if they are both defined as SDC, they are different from UX perspective, and elements would not be stored as part of a component library. Currently the UX designs are separating the elements from the component library for this reason.

Many of the elements could be expressed as:
<{{ html_tag }}{{attributes}}>{{ content }}</{{ html_tag }}>

Even if we do that, we need to define the html tags / attributes available somewhere so that we can optimize the UX for the specific element. For example, it may not make sense to show the background color or font family options for an image whereas it makes sense for a container. Also, images need to have a field for alt text whereas other elements don't.

Images fall into the weird category of being both an element and a component. They should be individually placeable within slots or on a page but are also a fundamental building block of other components.

it has a single prop, also called "image", so the component looks like a meaningless wrapper around this single prop

I've found this to be a good way to future proof component definitions, it appears like a meaningless wrapper until there's a need to add another prop that is not directly an attribute of the image.

it doesn't use the expected attributes object

Attributes has always felt like an SDC antipattern that was included only to ease the integration with Drupal. This image use case is a great example of how a Drupal render array ends up having significant undefined impacts on the rendering of the component. An image component schema that fully defines/handles all the various ways to render an image as an html element would be way better then relying on whatever is in the attributes object but would be challenging with how much of an image's format/style is currently done in Drupal's render layer.

This issue is also making me wonder if part of the scope of 🌱 [META] Support component types other than SDC Needs work should be allowing media(and other entities?) to be placed directly in XB using view modes.

This issue is also making me wonder if part of the scope of 🌱 [META] Support component types other than SDC Needs work should be allowing media(and other entities?) to be placed directly in XB using view modes.

Interesting! 🤔 I'll think about that some more… (this reminds me of the block_content block plugin, which I've been thinking about in the context of 📌 Add support for Blocks as Components Active .)

Based on #15 and #16, I think this needs input from @pdureau.

Attributes has always felt like an SDC antipattern that was included only to ease the integration with Drupal.
[…]
An image component schema that fully defines/handles all the various ways to render an image as an html element would be way better then relying on whatever is in the attributes object

💯 — it's where the SDC abstraction feels broken/wrong/in violation with itself.