🇫🇷France @pdureau

I also want to review before the merge

I am only care about the wording results, if you agree on a simpler way of doing that, let's go :)

It is not a UI Patterns 2 issue but a Core SDC issue.

#attributes property, which is an expected "classic" of render elements is not processed:

[
    "#type" => "component",
    "#component" => "ui_suite_uswds:logo",
    "#attributes" => [
      "foo" => "bar",
    ],
    "#slots" => [
      "content" => "Heloooo",
    ]
]

That's not good because many Drupal mechanisms are expecting this render property and inject data in it.

That also shows than, one year after the landing of SDC in Drupal Core, only a few people is using it with the Render API.

A dirty fix would be to add this to ComponentElement::preRenderComponent():

    if (isset($element["#attributes"])) {
      $props["attributes"] = new Attribute($element["#attributes"]);
    }

But we need to :

• consider the merging with existing values in $props["attributes"]
• check where the default attribute object is created (not in ComponentElement, which is surprising)

Not enough time left in Beta1.

Not enough time left in Beta1.

We already have a ticket for that: 🌱 [2.1.x] UI Patterns Library: add some interactions Postponed

I will close this issue which is becoming less and less relevant as long as other smaller issues are doing the expected work.

When i say:

Remove "Field" from fields select list

I mean, going from "Field Title" to "Title", from "Field Body" to "Body". We already know it is a field.

Don't implement "required" property

SDC is supporting "required" for both slots and props:

name: Tabs
props:
  type: object
  required:
    - foo
  properties:
    foo:
      title: foo
      type: string
slots:
  body:
    title: Body
    required: true

In my humble opinion, this is not a feature but an issue I was very vocal against. Required data is best fitting with business modelling, content management and data storage. UI artefacts, including components, must be the more loose possible. It is better to rely on default values.

Also, it is very hard to enforce required data. With the form builder, it is possible, by checking the filled data. But only for "widget" sources (textfield, URL, number...), not for "data from Drupal" sources (menu, field property...).

Moreover,

Or, maybe, only for props, sometimes

However, UI Patterns Settings 2.x is also proposing such a feature:

  settings:
    textfield:
      type: textfield
      label: Textfield
      preview: text preview
      required: true

And we want to cover the full scope of UI Patterns Settings (not all the setting type, but all the mechanisms).

So, let's implement required for props, and props only, for some widgets. While keeping a warning in ui_patterns_devel's Component validator telling using a default() Twig filter would be better.

Implementation proposal

On PropTypePluginBase::getSummary(), add a message telling the prop is mandatory (only for props). Careful, the information is outside the prop definition.

On source widgets (only for props), add the Form API #required property (default value: FALSE, of course):

• AttributesWidget?
• CheckboxesWidget ?
• CheckboxWidget
• ListTextareaWidget
• NumberWidget
• SelectWidget
• TextfieldWidget

Don't forget to add the migration of this property in a href="https://git.drupalcode.org/project/ui_patterns/-/blob/2.0.x/modules/ui_p...">ComponentConverter::getPropsFromSettings()

So...

OK with that?

Hi Steven, following 🐛 [2.0.0-beta1] Blocks implementing PluginFormInterface don't work with BlockSource Needs review , is it working better?

pdureau → created an issue.

pdureau → created an issue.

Understood now. I will share my thoughts.

Is it a beta1 issue?

This is done wrong in Block Source which we propably remove.

We have an issue about that: 🐛 [2.0.0-beta1] Blocks implementing PluginFormInterface don't work with BlockSource Needs review
We will not remove BlockSource but introduce a whitelist.

2. I am not sure about that. Actually I think it is good to have allways the same key in every consumer. We may provide a getConfigurationKey() function to overwrite that? Should we? @pdureau @Grimreaper
3. Should we provide an own trait for that? I would vote for that- @pdureau @Grimreaper

No opinion. I assign to @Grimreaper

Not remove UiPatternsOperations and maybe rename it to ComponentOperations

We have an issue about that: 📌 [2.0.0-beta2] Remove UiPatternsOperations form element Active

We amy replace it with ✨ Add new Splitbutton render element to eventually replace Dropbutton Needs work

i will have a look

pdureau → created an issue.

what about custom blocks which would have a blockSubmit but without breaking the config structure VS blockForm structure.

Custom blocks as content entity ? We are not supporting them because embedding such a block in a config entity will create a dependency from config to content, which is something to avoid in Drupal.

Maybe we can add a comment in the code to tell about this decision? And also why we are not supporting BlockSource?

Also, do you plan to do this:

   // @todo Add a way to alter this list properly (hook, event, etc).

i think layout_builder blocks are not shown here... so we can remove it safely from the whitelist.

I was not talking about layout_builder blocks. I was just telling this whitelist idea seems to look like what LAyout Builder is doing.

about "ui_patterns", "ui_patterns_blocks" blocks, maybe you are right, we could remove them.

the only reason i see now to desire a block VS a component, would be to have the "title" of the block or a specific template associated to it..

We don't care about specific block template. One of the long term goal of UI Suite is to get rid of node.html.twig, user.html.twig, field.html.twig, block.html.twig, views.html.twig...

So, let's remove them from the whitelist.

interesting. So, we do like Layout Builder is doing?

// @todo Add a way to alter this list properly (hook, event, etc).

indeed :)

    $whitelisted_blocks = [
      "id" => ["search_form_block", "system_menu_block"],
      "provider" => ["layout_builder", "ui_patterns", "ui_patterns_blocks", "views"],
    ];

do we whitelist ui_patterns & ui_patterns_blocks's blocks now we have ComponentSource?

I have this:

Line src/SourcePluginManager.php
------ ---------------------------------------------------------------------
150 PHPDoc tag @param for parameter $tag_filter with type array|null is not subtype of native type array.

To test.

ComponentFormatterBase::getComponentSourceContexts()

Test a formatter with a context-related source (field formatter)

ComponentStyle::addAjaxGroupTitleOption()

Was removed by 📌 [2.0.0-beta1] Do we take care of the big object in views config? Fixed

EntityFieldSourceDeriverBase::getEntityFieldsMetadata()

EntityFieldSourceDeriverBase::getDerivativeDefinitions()

Test a formatter with a "Data from field" source

SourcePluginManager::getNativeDefinitionsForPropType()

No dedicated test

i will have a llok

Leaving this as one prop is the right approach, IMO. What we're doing here is deriving a two-letter acronym for use when a menu item doesn't have an assigned icon. The requirement is to always use the first two letters. The API for the component is simpler to just ask for the one prop, rather than force the consumer to pass both and do the work before passing in. We can also enforce a two-letter acronym this way.

That's very interesting. Thanks.

Maybe it can be clearer in the Twig templates:

• by adding a comment
• and/or by creating intermediary variable(s) with explicit name(s)

Just an humble, non tested, proposal;

{% if text and text|length > 1 %}
  {% set icon_fallback = text|slice(0, 2)|join('') %}
  {% set attributes = attributes.setAttribute('data-index-text', text|first|lower).setAttribute('data-icon-text', icon_fallback) %}
{% endif %}

Will this upcoming Core form element be useful for our source selector in slots ✨ Add new Splitbutton render element to eventually replace Dropbutton Needs work ?

pdureau → created an issue.

i take it

Indeed, I am reproducing exactly the same issue

Hi @smustgrave

I am not reproducing, with which block did you try exactly?

This is the follow-up of 🐛 [2.0.0-beta1] [ui_patterns_views] AJAX _options break on the first plugin selction Fixed

Following 📌 [2.0.0-beta1] Sources must add dependencies to config entities Needs review

we also need to deal with:

ComponentElementBuilder::calculateComponentDependencies(): Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10.

OK, i check ui_patterns_legacy again

✨ [2.0.0-beta1] Add ComponentSource plugin Fixed was merged, don't forget to do this source too.

Since the last update, I have this error

An exception has been thrown during the rendering of a template ("Drupal\ui_patterns_legacy\RenderableConverter::getNamespacedId(): Argument #1 ($component_id) must be of type string, null given, called in modules/ui_patterns_legacy/src/RenderableConverter.php on line 81").

On those components, everytim it is because of the pattern() function:

• ui_suite_dsfr:consent_banner
• ui_suite_dsfr:consent_manager because of:
  

    {{ pattern('button_group', {
      attributes: {class: ['fr-consent-manager__buttons', 'fr-btns-group--inline-sm']},
      buttons: [pattern('button', {label: 'Confirm my choices'|t})]
    }, 'right') }}

• ui_suite_bootstrap:dropdown
• ui_suite_uswds:modal
• ui_suite_uswds:tooltip because of:
  

  {{ pattern('button', {
    'attributes': attributes,
    'label': label|default('Show')
  }) }}

But not every component template with pattern() function raise that. Mysterious...

pdureau → created an issue.

It seems to be an error from ui_patterns_legacy, for example on ui_suite_dsfr:translate the description of the props 'links' is empty, when transformed by ui_patterns_legacy it become: description: null, and so the Validator raise this schema error.

interesting, thanks. I will create a beta1 issue about this

I couldn't find why so for now I removed to proxy to componentValidator until I can do it properly.

that was the right thing to do

Another false positive found:

[props.properties.links.description] NULL value found, but a string is required

Every components have this error, even if no links prop.

Anyway, description property is not mandatory in props defrinitions.

pdureau → created an issue.

G4MBINI → credited pdureau → .

i do the review

let's wait 📌 [2.0.0-alpha3] Use Drupal entities & plugins for the Library pages Active before doing anything

Maybe a field formatter plugin will fix that

pdureau → created an issue.

pdureau → created an issue.

Be careful about context passing

pdureau → created an issue.

Jean, what is your opinion about this?

In 📌 Use webcomponents for dropbutton Needs review , @nod_ said "with webcomponents we don't need behaviors or once".

And @brianperry shared about "html web components", components that add behaviors to mostly existing markup and don't use the shadow dom.

So, i droppped this comment:

Such an interesting issue! Replacing all Drupal behaviours by WebComponents looks exciting: one less Drupalism & better performance.

Is it possible to do the same with "normal" markup instead of a custom element? Leveraging the is attribute and using the web component only as an (efficient) behaviour bag:

<div is="drupal-dropbutton" class="dropbutton-wrapper dropbutton-multiple">
<div class="dropbutton-widget">
<!-- ul, li, li, li, li -->
</div>
</div>

Advantages:

• faster to convert the renderable once the JS is updated, just replace data- attributes by the is attribute
• even faster rendering because server-side goodness
• no accessibility issues because we are back to good old markup
• Selenium may prefer this one :)

Unfortunately, Safari would need a polyfill for now.

pdureau → created an issue.

pdureau → created an issue.