🇫🇷France @pdureau

2 feedbacks

Remove the merge logic from stories

You removed the call to ::mergeStories() without replacing it by StoryPluginManager::getComponentStories() :

-    // ::mergeStories() is a temporary compatibility layer.
-    // @todo Replace by StoryPluginManager::getComponentStories() before 2.0.0
-    // release.
-    $component = $this->storyPluginManager->mergeStories($component['id'], $component);

So, I am afraid it is not working well. Did you test by looking the component library?

What else?

Do you know any other similar compatibility layers to remove? I am not sure I was thinking about everything while authoring this ticket.

pdureau → created an issue.

Let's imagine we are leveraging contentMediaType and create a new PropType plugin to make the distinction between Markup and PlainText:

• we can create a Markup prop type with type: string; contentMediaType: "text/html" and keep type: string for plain text
• or we can create a PlainText prop type with type: string; contentMediaType: "text/plain" and keep type: string for markup

What would be the usage of this Markup prop type? It will be very similar to SlotPropType :

• accept the same source plugins as SlotPropType: WYSIWYG, Component, Block, Field Formatter...
• do the same data normalization as SlotPropType, with only one step more: the renderable is renderer

Would it make sense? I don't think so.

first, I guess the format of the default value is different depending on the source plugins or is it the source plugin responsibility to transform this default value to fit in its code?

Today situation: if NULL, the prop is unset. So, it may be good.

second, if point one is not doable and default value format depends on the source plugin, when defining a component, it is not possible to guess which source plugin will be selected by default so this could lead to empty config form by default.

The default value in a prop must be valid with the prop type schema. It the job of the source plugins to alter the default value to fit with the form element(s) if necessary.

For example, in theory, an attribute prop will have this default value:

class: ["foo", "bar]
role: "info"

• The attributes widget will transform the default value to class="foo bar" role="info"
• The classes widget will transform the default value to foo bar

Do we agree on this mechanism? Is it the current implementation?

Expects auto merged

I have mixed feelings (so an open mind) on this subject:

• in a pure JSON Schema point of view, strings with markup are still strings, so no stripping
• in a SDC (or any similar UI component technology) point of view, markup (already rendered or not) belongs to slots, and string props may be considered as plain text only

Meeting with Christian, Florent, Mikael & Pierre

We are happy with the current situation. Let's close the issue

pdureau → created an issue.

Doing some tests with mkdocs, so the current issue will also cover 📌 [2.0.0-beta3] Move specs to markdown in git repo Active

Will be done in 📌 [2.0.0-rc] Update documentation Active

Grimreaper, Christian, and all,

It looks already great, but let's discuss this MR together to find the perfect files layout.

I downloaded UI Patterns, but there are no SDCs in there as examples

We don't ship SDC component in UI Patterns 2 by design. UI Patterns 2 is only a "bridge" between SDC and other Drupal API.

However, we are currently adding an extensive documentation to UI Patterns 2, expected before Christmas, and I hope it will give you the information you need.

Also, do you have examples of how you're using SDCs?

UI Suite team (the team doing UI Patterns, which I am part of) are also maintaining SDC themes:

• https://git.drupalcode.org/project/ui_suite_bootstrap/-/tree/5.1.x/compo...
• https://git.drupalcode.org/project/ui_suite_daisyui/-/tree/4.0.x/components
• https://git.drupalcode.org/project/ui_suite_dsfr/-/tree/1.1.x/components
• ...

Beware, there are 2 additions from UI Patterns 2 in those components compared to regular SDC components:

• ✨ Introduce component variants to SDC Active has been already implemented
• we use the native JSON schema's $def/$ref mechanism to avoid repeating complex prop schemas

Beside that, you can get inspiration from them.

Also, the tool we are using to check our SDC good practices, https://www.drupal.org/project/sdc_devel → , is already usable, but the rules set needs some tweaking (wording, scope, levels...) which will happen around Christmas.

I don't get how I can do slots with include and again, the docs only say use embed with slots.

That's why the doc needs to be updated. The issue description has some examples of using include with slots.

Mikael, because you have added tests/src/Kernel/PropTypesNormalizationTest.php in beta6

Do we still need?

• tests/src/Unit/PropTypeNormalization/AttributesPropTypeNormalizationTest.php
• tests/src/Unit/PropTypeNormalization/LinksPropTypeNormalizationTest.php
• tests/src/Unit/PropTypeNormalization/UrlPropTypeNormalizationTest.php

Can we consolidate those in a single collection of kernel tests?

Mikael's answer:

we can consolidate those tests yes and refactor. better to keep as kernel, and continue what i have introduced if possible. the file EnumNormalizationTest has been deleted, or should have been.

Can we have a tests/src/Kernel/PropTypeNormalization/ folder, with one file per prop type? a bit like what I did in tests/src/Unit/PropTypeNormalization ?

Hi Grimreaper,

Let's not forget this issue. Do we have something to do for 2.0.0-rc1?

Indeed, that's the view-table template which is causing trouble. Maybe because views module is doing some unexpected early rendering

Clear. Moved

repeatable/undefined number of slots, e.g. a dynamic columns component where users could place 1–n columns

This must not be a new SDC feature. A dynamic number of "columns" is a single slotswhere we are looping on the slot content.

Example with items slot:

   {% for item in items %}
    <div class="grid__item">
      {{ item }}
    </div>
  {% endfor %}

Also, it can be safer to check if there is a single renderable and wrap it into a sequence before looping:

{% set items = items and items is not sequence ? [items] : items %}

smustgrave → credited pdureau → .

grimreaper → credited pdureau → .

Indeed, EnumPropType is a scale prop type.

I will merge today

Thanks Pamela for the notice. I totally understand the situation. Let's focus on delivering the first releases of both Drupal CMS and UI Patterns 2, and let's talk again about possible synergies when possible.

Can you take it Mikael?

Hi @dalemoore,

Would you also say that doing this:

attributes:
      type: Drupal\Core\Template\Attribute
      title: 'Attributes'
      description: 'HTML attributes to apply to the component.'

in the component.yml is incorrect?

It is not incorrect in a SDC POV but:

• useless because the attributes variable is automatically injected to the component anyway, so no need to add it to the component definition
• ugly because using PHP namespaces (like Drupal\Core\Template\Attribute) as types is not compliant with JSON Schema and SDC has implemented a quirk to mange them. So let's avoid this

If I try to use the 'only' keyword, like this then I cannot include slots/blocks that aren't explicitly included in the Twig template as blocks.

I ddin't know this about embed, that's one more reason to promote Twig include() function (function, not tag !) (with with_context = false !) instead of the messy embed tag.

If the attributes prop isn't right, then I would ask: how do we add a class, ID, or other attribute to our components from presenter templates? Having to manually add a class prop (or other attribute) onto every SDC seem to be the only way, but and is burdensome.

I would not recommend adding a class prop. attributes props is perfect for this and it is not what is causing issue here. attributes also allows to attach other attributes, and is expected by any mechanism adding semantic annotation, SEO tags, styles utilities from design systems, accessibility attributes... It is very valuable.

If including an attributes prop, I wonder if it would be better to name it component_attributes instead of just attributes to prevent the leaking of attributes from the presenter template. We already have title_attributes, content_attributes, etc. for other things.

Radix theme did this mistake: 🐛 Inconsistent `attributes` handling Active Now the have have both COMPONENT_attributes and attributes in some templates, which is confusing and may lead to other issues.

Using the standard attributes prop must be enough and work well.

With your change, do we lose this logic?

      // Twig `is sequence` and `is mapping `tests are not useful when a list
      // of renderables has mapping keys (non consecutive, strings) instead of
      // sequence (integer, consecutive) keys. For example a list of blocks
      // from page layout or layout builder: each block is keyed by its UUID.
      // So, transform this list of renderables to a proper Twig sequence.
      return array_values($value);

I don't see the array_values equivalent in:

      $value = array_map(static fn($item) => self::normalize($item), $value);
      return (count($value) === 1) ? reset($value) : $value;

pdureau → created an issue.

I take it

Hi Rodrigo,

I might be wrong but it seems like bootswatch themes are compiled sith SCSS variables and not CSS variables the also include some CSS for each theme.

That's a good things for us. Because with UI suite we are strictly differentiating:

• buildtime variables, AKA "design tokens", like SCSS or SASS variables >> we expect themes/modes to be built from them
• runtime variables, so the CSS variables, KA "custom properties" >> we use them one by one, for specific targeted overrides

In my UI Suite Bootstrap subtheme I was able to apply one of the bootswatch themes with

Indeed, that's the "classic" way of doing that. But we lose here the knowledge about this theme, we are just using the default theme but replacing one of its CSS files. UI Skins → modules allow to declare proper themes/modes plugins in YAML, in order to maniopualte them as first class objects, without overrides.

Examples:

• https://git.drupalcode.org/project/ui_suite_bootstrap/-/blob/5.0.x/ui_su...
• https://git.drupalcode.org/project/ui_suite_daisyui/-/blob/4.0.x/ui_suit...
• https://git.drupalcode.org/project/ui_suite_dsfr/-/blob/1.0.x/ui_suite_d...

pdureau → created an issue.

pdureau → created an issue.

pdureau → created an issue.

more generally, do we need a better normalizing for string props? in that case the title of a link maybe normalized the same way string props are/would be normalized ?

Unfortunately, we cant' apply StringPropType normalization to links item titles, because we discovered than links item titles can be renderables.

grimreaper → credited pdureau → .

pdureau → made their first commit to this issue’s fork.

grimreaper → credited pdureau → .

pdureau → created an issue.

pdureau → created an issue.

I would love to see an simple example EXTENSION_NAME.icons.yml in the change record
The extractor example file is quite overwhelming

The examples in the change records are EXTENSION_NAME.icons.yml.

Indeed, we have shown complex stuff to display the power of the API. We will see if we can show something simpler.

Move the logic to ::normalize()

Add test

Careful, I have rebased your MR, you may need to pull again

pdureau → made their first commit to this issue’s fork.

Personally, I prefer the whitelist

pdureau → created an issue. See original summary → .

We keep them. A follow-up issue in RC1 to fix this.