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

Thanks Wim for creating this issue and give us the opportunity to discuss about those subjects.

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.

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

• it could be leveraged 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

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: one of the 2 layers, the image component or the image prop, is superfluous 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 UI components. 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;

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 but 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 (still WIP but already useful). 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 carefully curated set of SDCs, tagged/categorized as "elements"? Maybe experience_builder:image is not a good example, 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.

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

Thanks! Created followup for XB:

#3470575: Remove `attributes` prop from all SDCs in the Experience Builder module

and reopened ours for SDDS:

#3469525: Update SDDS SDCs attributes prop to align with XB

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 #3454519: [META] Support component types other than SDC, block, and code components should be allowing media(and other entities?) to be placed directly in XB using view modes.

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.

Sorry for the late reply here! We don't have this experience fully designed (yet) but this was discussed on a high level as part of the IA exercise. The plan is to allow exactly this; allow images to be defined as props of components and as elements that could be placed in slots.

Because of that, for media, like videos, files, and images, there are elements inside the "Elements" group:

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

The "Dynamic Components" group includes a section that would allow inserting content directly using the entity view modes:

Next step is to design what the UX of inserting these would look like. That will likely be in ~3-4 months from now. In the meanwhile, there's some work around the image attributes in #3494545: [later phase] SDC must be able to specify minimum dimensions for a prop receiving an image.

Aside of whatever we need because of #3352063: Allow schema references in Single Directory Component prop schemas:

• attributes is being used properly in the twig template of the image component.
• Usage/ best practices of the image component is documented in Canvas SDC image docs.

So yes, feeling like this can be closed now.

This has been completed from my perspective – the image component looks good. There might be a follow-up discussion to be had on best practices regarding the use of attributes object.