Paris
Account created on 13 April 2012, over 12 years ago
  • Engagement Manager at Smile 
  • Business Unit Manager at Linagora 
#

Merge Requests

More

Recent comments

🇫🇷France pdureau Paris

Today, we decided to merge this issue because the Using XB components in UIP2 fix is enough for RC1 (after adding some tests, right?).

About Using UIP2 components in WB:

  • It seems XB is not resvolving $ref: ui-patterns:// but it may not resolved "$ref":"json-schema-definitions: either, so let's wait they implement a working reference resolver
  • when both XB and UIP2 are installed, XB is raising errors, so our component plugin manager decorators may not be fully compatible
  • XB is not loading UIP2 components (!) but is loading the ui_patterns_blocks's blocks twice, because of our (UIP2) context management; we derivate blocks twice, one with entity context and one without

Let's create a 2.1.0 issue.

🇫🇷France pdureau Paris

I am now hesitating between option 4

A single StringPropType with type: string JSON Schema, if the JSON schema definition has contentMediaType: "text/html" , then we allow HTML to be placed into the string prop.

And option 5:

A single StringPropType with type: string JSON Schema, if the JSON schema definition has contentMediaType: "text/plain" , then we strip the HTML.

Anyway, with both options, we are going in the right direction.

What do you think?

🇫🇷France pdureau Paris

I tested the UIP2 (beta6) a bit with multilingual locally.
My local env. is Drupal 11.0.9, PHP8.3. I also installed but did not enable the UI Suite DaisyUI (4.0alpha) theme which includes many ready components for testing.
For translation, I used Core Multilingual modules with installed Ukrainian as a second language (around 95% of interface translation is available by default)

User Interface translation

Not all props titles have a translation possibility e.g. "Attributes" because they are not wrapped by the translate function (/ui_patterns/src/ComponentPluginManager.php:261).
Meanwhile, descriptions, sources, and help text are translatable there (I added a Cyrillic word beside the English origin one to show that in the screenshots).

Props properties titles and descriptions implemented as a component in UI Suite DaisyUI (e.g. /ui_suite_daisyui/components/button/button.component.yml:87) can not be translated via the User interface translation page. E.g. the "Block?" title and nested "It is possible to display button as a block?" description text.
At the same time category, label, and description names in DaisyUI classes (/ui_suite_daisyui/ui_suite_daisyui.ui_styles.yml) are translatable.

Translation in the Views and Blocks has the same behavior.

As said by Grimreaper, this is the scope of another UI Patterns issue 💬 Components metadata translation questions/ideas Active .

It is also covered by this Core issue Props have no (translatable) labels Active

Config translation

I haven't tested layout builder overrides with symmetric and asymmetric translation yet because related contribs modules don't support Drupal 11. I need to set up the Drupal 10 environment for the test.

Indeed:

We need to test that, testign in a Drupal 10.3 or 10.4 environment. I may do it if I have time, but the task is open to anybody willing to take it.

🇫🇷France pdureau Paris

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

🇫🇷France pdureau Paris

I just add a little wording and i merge

🇫🇷France pdureau Paris

So, we don't trigger hook_field_formatter_third_party_settings_form in our Field Formatter Source plugin for slots. We need to trigger it.

Careful, there is maybe another hook to check at render time (field_formatter_range_preprocess_field?).

Olivier's job is done here. Who is taking the subject?

🇫🇷France pdureau Paris

Visualization with Grafana is too complicated to add in the gitlab CI pipeline. No asset in the tests, so it is useless for gitlab CI pipeline but still useful for local dev environment.

So, let's do that:

  1. rebase from 2.0.x
  2. fix PHPCS issues
  3. add a very explicit comment (README?) to tell people this is only useful when grafana is installed and how to set up the test
  4. merge
🇫🇷France pdureau Paris

It is because of ckeditor_layouts which is loading the layout through the theme manager but SDC is not passing through the theme manager.

      $template = ['#theme' => $layout->getThemeHook()];

https://git.drupalcode.org/project/ckeditor_layouts/-/blob/2.x/src/Plugi...

So, we don't do anything.

🇫🇷France pdureau Paris

Using XB components in UIP2

The issue was on UIP2 side. Fixed by Christian in https://git.drupalcode.org/issue/ui_patterns-3490873/-/commit/51c4ea3e22...

But he needs to add some tests.

Using UIP2 components in WB

We are not able to test because XB is currently broken (not the same bug as yesterday...). Let's wait a bit and pull again.

🇫🇷France pdureau Paris

AFAIK everything that was supposed to go with an SDC is supposed to go in the theme in the same folder as the component, or at least done in the theme. But I don't think it's best practice to do any of that in a theme?

The component are stored in a theme (or a module), but can be used outside of this theme (or module). Most of the time, component usage is stored in config entities related to display (block.block, entity view displays, views...). Sometimes in content (fields using ckeditor5 data or layout builder data...). Sometimes in code (custom plugins...)

🇫🇷France pdureau Paris

Step by step, we are gathering information to update this doc. That's nice

🇫🇷France pdureau Paris

Mikael, it looks like something you are currently working on, doesn't it?

🇫🇷France pdureau Paris

We detected this issue in 1.x, but maybe it impacts 2.x as well.

There are no variant specific or preview specific templates in UI Patterns 2

🇫🇷France pdureau Paris

Do you use the font extractor in one of your icon pack definition? If yes, you need ui_icons_font module. If not, let's investigate.

🇫🇷France pdureau Paris

Currently it is only possible to set the icon pack settings (size, width, height, etc.), in field formatter (, CKE5, menus) but not per field value.

It was done in purpose. Field storage is for data only (icon pack + icon ID), field formatter is for how it looks (settings). For example, yo uwant to add an icon to a content but this icon will look differently in full page, card, banner, search results...

But for Fontawesome icons, the ratio may change per icon. So even with a relative size in em, icons can be cropped or deformed.

Maybe the setting is not propertly defined. Can you share your iconpack definition?

🇫🇷France pdureau Paris

Hello Steven,

A few feedbacks about the current state of 4.x branch

Form components

Some components implement form element. Most SDC themes don't do that because SDC is not compatible with the Form API ( 📌 Compatibility between SDC and the Form API Active ). In UI Suite USWDS, we have a few of them, and there is nothign wrogn about that.

However, some don't keep slots opaque and manipulate renderables in the template.

For example, in combo_box:

{% if option.type == 'optgroup' or option['#type'] == 'optgroup'%}
{% set label = option.label ? option.label : option['#label'] %}

Is this component used somewhere? How? Can we prevent this behaviour?

Layout Options is not necessary anymore

UI Patterns 2.x is also replacing Layout Options:

Proposed resolution

  1. Create 4 components: grid_row_1, grid_row_2, grid_row_3 & grid_row_4
  2. Remove layout_options from info.yml & composer.json
  3. Remove ui_suite_uswds.layout_options.yml and ui_suite_uswds.layouts.yml
  4. Remove template/layout--grid.html.twig

Example of similar change: 📌 [1.1.0] From Layout Options to UI Patterns 2.x Active

landing_page ui_examples

You have to convert it to UI Patterns 2, to replace:

  • pattern render elements by component render elements
  • pattern_preview render elements by component render elements, with a story render property

It is an emergency to do it IMO, because you have already removed the dependency to ui_patterns_legacy

This will also be the opportunity to:

  • Remove the # prefix in the render property: "#type": html_tag and type: html_tag
  • Use the newly introduced Grid components instead of markup renderable like "#markup": '<div class="grid-row grid-gap"> <div class="tablet:grid-col-4">...'

Tests with UI Patterns 2

Have you already tried using ui_suite_uswds with UI Patterns 2 is projects? That's the best test to do ;)

🇫🇷France pdureau Paris

This looks like a need driven by a display builder UX, not by UI component modelling. So such feature doesn't seem to belong to SDC.

if the {type: boolean} cta.enabled prop is true, also require the cta.text and cta.url props)

Maybe the component is ill defined. Maybe the cta.enabled is not necessary because the same as the cta.text and cta.url expression.

🇫🇷France pdureau Paris

I guess this feature is expected by Experience Builder in order to fill the newly added component with initial data without having to rely on the unreliable examples property.

default must not be used in the rendering process (sending default value if slot value is missing or empty) because:

  • sometimes we want a default value in the display builder while allowing the user to set empty or missing value instead
  • Twig has already mechanisms to handle default value logic:
    • |default() when the slot is not a block
    • the block children nodes when the slot is a block
🇫🇷France pdureau Paris

For your information, in UI Patterns 2 , we use "meta:enum" which is not an official standard but supported by some popular projects:

props:
  type: object
  properties:
    position:
      type: string
      enum:
        - top
        - bottom
      "meta:enum":
        top: Top
        bottom: Bottom

If an item is in enum but not in meta:enum, its label will be the item string
If an item is in meta:enum but not in enum, it is ignored.

It would be great to stay compatible.

🇫🇷France pdureau Paris

About Form API. We (UI Patterns people) are trying to integrate UI components (our own format until recently, the SDC format now) with Form API for years now, without success. It would be a dream to get rid of Form Elements and use a superset of SDC to build forms. Did you already have an idea about how to do it? We can discuss about this in a dedicated issue.

See also 📌 Compatibility between SDC and the Form API Active

🇫🇷France pdureau Paris

Mandatory fixes

Slots are not typed

So let's remove the type: object here:

  metadata:
    title: Metadata
    description: Additional metadata
    type: object

Consistent slot notation

The 2 slots have different notations, one with a block and a print node, one with only a block:

{% block avatar %}{{ avatar }}{% endblock %}
{% block metadata %}{% endblock %}

It is better to stay consistent. My personal preference would be no blocks (see: 🌱 Clarify SDC documentation by toning down Twig blocks promotion Active ), but if there is a block I would advise to add also the print node.

A summary (without the only and with_context keywords, for clarity):

Deprecated Twig filter: `spaceless`

The spaceless filter is deprecated as of Twig 3.12. While not a full replacement, you can check the whitespace control features.

Other feedbacks

As always, the attributes prop declaration is not necessary because automatically added to all components, but it doesn't hurt to add it so keep if you want:

    attributes:
      title: Attributes
      description: Meta attributes.
      type: Drupal\Core\Template\Attribute

In attributes and author_attributes, using a PHP Namespace as a prop type is not JSON schema valid and is skipping the SDC validator. However, describing an Attribute object with JSON Schema is complex without a reference resolver, so we can let it like that for now.

🇫🇷France pdureau Paris

pdureau changed the visibility of the branch 3365377-metadata to hidden.

🇫🇷France pdureau Paris

No, it doesn't work.

Using XB components in UIP2

  • I install both ui_patterns_library and experience_builder
  • I got to /admin/appearance/ui/components/experience_builder/shoe_button
  • icon prop has this unknown prop type: {"$ref":"json-schema-definitions:\/\/experience_builder.module\/shoe-icon","type":"object"}

According to https://git.drupalcode.org/project/experience_builder/-/blob/0.x/schema.... the expected resolved json schema must be

title: Icon
type: object
properties:
  name:
    type: string
    title: Name
    default: primary
    enum:
    - moon-stars-fill
    - moon-stars
    - star-fill
    - star
    - stars
    - rocket-fill
    - rocket-takeoff-fill
    - rocket-takeoff
    - rocket

Which will be loaded as Enum prop by UIP2.

So, what is happening? Why the JSON schema reference is not resolved?

  • It is broken on Experience Builder side?
  • Is it broken because of having XB & UIP2 together?

Using UIP2 components in WB

I was not able to test because XB is currently broken:

Symfony\Component\Routing\Exception\RouteNotFoundException: Route "experience_builder.experience_builder" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 211 of core/lib/Drupal/Core/Routing/RouteProvider.php).

Let's try later.

🇫🇷France pdureau Paris

Maybe we can go easy with unicode PCRE patterns.

Ou regex will be less "good" but it may still be good enough and become compatible with Drupal's validatePattern function ?

🇫🇷France pdureau Paris

OK for me to be merged.

However, some tests are still commented:

     // "Standardized structure, flat, with objects" =>
      // self::standardizedFlatObjects(),
     ...
      // "Menu, as generated by the Menu module" => self::menu(),

https://git.drupalcode.org/project/ui_patterns/-/blob/cb853ea951afe89b24...

It would be great to uncomment before merging

🇫🇷France pdureau Paris

Mikael got a look, so back to Christian

🇫🇷France pdureau Paris

They have only a 'value' setting key, and they may use it in their form. in those particular cases, we process the default value indicated in the prop json schema.

So we are good, aren't we? :)

So, to fix this issue, maybe we need to make Drupal\ui_patterns\Plugin\UiPatterns\Source\AttributesWidget extending SourcePluginPropValue

🇫🇷France pdureau Paris

No error? That s'great news. I will check again.

By the way, I know they are still very early stage, and I don't expect the builder to work properly, I just want the 2 JSON schema resolvers ($ref: ui-patterns:// and $ref: json-schema-definitions://) to work well together.

🇫🇷France pdureau Paris

option 4 looks great: no new prop types, no confusion with slots, just an adaptation related to the prop schema.

I keep the issue on my side to check if no contentMediaType is stricter than contentMediaType: "text/html" in JSON schema.

🇫🇷France pdureau Paris

some kind of render array magic, I guess through preprocess functions in your .theme? How else would you use this method?

Most of the time, as the return value of configurable plugins related to display building:

Do we add this information to the SDc documentation too?

🇫🇷France pdureau Paris

Can we also remove this from ComponentElementBuilder::buildProp() ?

    if (empty($data) && $prop_type->getPluginId() !== 'attributes') {
      // For JSON Schema validator, empty value is not the same as missing
      // value, and we want to prevent some of the prop types rules to be
      // applied on empty values: string pattern, string format, enum, number
      // min/max...
      // However, we don't remove empty attributes to avoid an error with
      // Drupal\Core\Template\TwigExtension::createAttribute() when themers
      // forget to use the default({}) filter.
      return $build;
    }

It looks like a security we may not need anymore

🇫🇷France pdureau Paris

Hi Mikael,

so right now, default value in the definition seems not used in sources.

Can you have a look on https://git.drupalcode.org/project/ui_patterns/-/blob/2.0.x/src/SourcePl... .

  public function getSetting(string $key): mixed {
    $value = parent::getSetting($key);
    if (("value" === $key) && (NULL === $value)) {
      return $this->getDefaultFromPropDefinition();
    }
    return $value;
  }

  protected function getDefaultFromPropDefinition(): mixed {
    if (is_array($this->propDefinition) &&
      array_key_exists("default", $this->propDefinition)) {
      return $this->propDefinition["default"];
    }
    return NULL;
  }

  protected function mergeDefaults() : void {
    $defaultSettings = $this->defaultSettings();
    // -> we prefer the prop definition default value.
    if (array_key_exists("value", $defaultSettings)) {
      $defaultValueProp = $this->getDefaultFromPropDefinition();
      if (NULL !== $defaultValueProp) {
        $defaultSettings["value"] = $defaultValueProp;
      }
    }
    $this->settings += $defaultSettings;
    $this->defaultSettingsMerged = TRUE;
  }

Contributed by you 6 month ago: https://git.drupalcode.org/project/ui_patterns/-/commit/af53b6af42d7f622...

Is it still OK? Do we need to restore/replace this mechanism?

🇫🇷France pdureau Paris

Florent, are some Sviatoslav's feedbacks overlapping with 💬 Components metadata translation questions/ideas Active ?

🇫🇷France pdureau Paris

Thanks Olivier.

when using a field formatter to render the field inside a component's slot, the "Remove field markup" option is not there but that don't seems like a bug to me.

So, when using a field formatter to render the field inside a component's slot, field_formatter_range works but no nomarkup? If yes, we can close the issue.

So, when using a field formatter to render the field inside a component's slot, field_formatter_range doesn't work? If yes, we have to investigate

🇫🇷France pdureau Paris

effulgentsia:

This might be fine if we think SDC creators already know to output $attributes for other reasons, but #9 indicates that this isn't currently being consistently done in the wild.

luke.leber:

We made the decision not to use $attributes in our lowest level* twig, as it'd tightly couple things to Drupal.

I don't know what are "our lower level twig" here, but Experience Builder will be used with components from contrib or custom themes, so let's have a look on what themers are doing:

🇫🇷France pdureau Paris

The remaining problem is from $build[$region_name] = $regions[$region_name]; we did for Layout Builder integration, but may not work well with Field Layout.

We don't need to test with the Entity Form Displays because SDC is not compatible with the Form API yet, but Field Layout can also be used with Entity View Displays instead of Layout Builder.

🇫🇷France pdureau Paris
1) Drupal\Tests\ui_patterns_layouts\Functional\FieldLayoutRenderTest::testRenderForm
Behat\Mink\Exception\ElementNotFoundException: Element matching css ".field--name-body" not found.
/builds/project/ui_patterns/vendor/behat/mink/src/WebAssert.php:465
/builds/project/ui_patterns/modules/ui_patterns_layouts/tests/src/Functional/FieldLayoutRenderTest.php:115

2) Drupal\Tests\ui_patterns_layouts\Functional\LayoutBuilderRenderTest::testRenderProps
Behat\Mink\Exception\ElementNotFoundException: Element matching css ".layout-builder-block" not found.
/builds/project/ui_patterns/vendor/behat/mink/src/WebAssert.php:465
/builds/project/ui_patterns/modules/ui_patterns_layouts/tests/src/Functional/LayoutBuilderRenderTest.php:78
🇫🇷France pdureau Paris

Anyway, we can use our custom ruleset to override if necessary.

No custom ruleset. Let's just follow default vincentlanglet/twig-cs-fixer without overthinking

🇫🇷France pdureau Paris

Those tests belongs to ui_patterns_library

🇫🇷France pdureau Paris

Mikael, can you have a look before Christian do it? (You may also take it if Christian is busy)

🇫🇷France pdureau Paris

Hi Dale,

The proposal with #variant is for the render element only.

Don't worry, the call from template will still be:

{% embed 'my_theme:cool-component' with {
  variant: 'my-variant-name',
  prop1: 'foo',
  prop2: 'bar',
} %}

or:

{{ include('my_theme:cool-component', {
  variant: 'my-variant-name',
  prop1: 'foo',
  prop2: 'bar',
}) }}
🇫🇷France pdureau Paris

Hi smos,

Christian will review your gitlab CI addition.

It seems you didn't run the linter on the existing UIP2 codebase, because I have this:

+++ b/tests/modules/ui_patterns_test/components/test-component/test-component.twig
@@ -1,5 +1,5 @@
 {% set variant = variant|default('') %}
-<div{{ attributes.addClass(['ui-patterns-test-component', 'ui-patterns-test-component-variant-' ~ variant ]) }}>
+<div{{ attributes.addClass(['ui-patterns-test-component', 'ui-patterns-test-component-variant-' ~ variant]) }}>
   <div class="ui-patterns-props-string">
     {{ string }}
   </div>
🇫🇷France pdureau Paris

It would be nice to add an automatic test

🇫🇷France pdureau Paris

Thanks for your clear answer Lauriii.

Is there a concrete problem that you're anticipating?

I am expecting HTML comments to not be reliable place to put processable data, according to some HTML parsing behaviours I have witnessed a few times, long time ago, but it may be an outdated opinion.

If using the attributes variable doesn't fit your needs, I understand you are looking for other ways.

🇫🇷France pdureau Paris

A discussion has started in the meta issue: https://www.drupal.org/project/experience_builder/issues/3462705#comment... 📌 [SPIKE] Comprehensive plan for integrating with SDC Active
where I shared this advice:

This must not be a new SDC feature because a dynamic number of "columns" is already doable in a simple, solid and powerful way with a single slot where we are looping on the slot content (slots can have a renderable or a list of renderables)

🇫🇷France pdureau Paris

Ok, i will have a look today or tomorrow

Production build 0.71.5 2024