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

smustgrave is not available this week, let's close

🇫🇷France pdureau Paris

so you prefer to opt-in the expecting characters rather than opt-out the forbidden characters?

🇫🇷France pdureau Paris

Behaviours

The script to embed: https://git.drupalcode.org/project/ui_suite_dsfr/-/blob/aeb85b6a07c39f1e...

Example of behaviour:

Drupal.behaviors.myBehavior = {
    once('myCustomBehavior', 'input.myCustomBehavior', context).forEach(
      function (element) {
           // Your code is here
      }
    );
  }
};

Asset library management

In the MR, it is declared at the theme level: https://git.drupalcode.org/project/ui_suite_dsfr/-/blob/aeb85b6a07c39f1e...
With files in /css and /js folders

It would be better to add them directly in the component, so they are loaded only when the component is used:

🇫🇷France pdureau Paris

Let's create another issue for beta2 if we can do that today.

So, we need to replace \s\w\- in $double_quoted_value = '"[\s\w\-]*"'; by something catching all characters except:

Source: https://html.spec.whatwg.org/#attributes-2

🇫🇷France pdureau Paris

Hello;

Congratulation Alberto for becoming a maintainer of bootstrap. The Drupal community will benefit of the 2 most popular Bootstrap themes (100K & 40K) to join force.

I am one of the maintainer of another Drupal theme : https://www.drupal.org/project/ui_suite_bootstrap (156 installs) and I believe we may be helpful for future design system related work.

UI components

https://getbootstrap.com/docs/5.3/components

bootstrap theme and barrio theme are currently migrating to SDC. It is a task not as easy as it may look.

ui_suite_bootstrap has SDC-like components since 2020, they benefited of years of improvements from feedbacks and real life usage, and we have a branch for SDC conversion: https://git.drupalcode.org/issue/ui_suite_bootstrap-3412076/-/tree/34120...

Our SDC implementation has some dependencies to the upcoming ui_patterns 2.0 but they are easy to skip and bootstrap can benefit of those well crafted and tested components (and validated by me, one of the 2 maintainers of the SDC sub-system in Drupal Core ;) ).

We also provide a tool to validate SDC components if you want to keep yours own.

Other UI artfecats

A design system is not only UI components. Bootstrap also expose:

Those artefacts can be managed by custom PHP code in each theme, but we, the UI Suite team, provide generic modules to manage them with simple YAML files and automatically plug them to Drupal API:

Don't hesitate to contact us if you want to know more. We can do a demo of our implementations and tools.

🇫🇷France pdureau Paris

Beta2 is coming soon and we are late, so let's do only this for beta2:

From error to warning, related to components embedding:

  • Use slots instead of hard embedding a component in the template with `embed` >
  • Forbidden Twig function: `block`. Use slots instead of hard embedding a component in the template.

From error to warning, related to Twig condition testing:

  • The exact same as just testing the variable, empty is not needed.
  • Not needed in Drupal because strict_variables=false.

Other stuff:

  • You should declare a story in the definition > To remove because we will remove the stories from the definition anyway

I will move everything else, including the wording, to beta3

🇫🇷France pdureau Paris

Because it is the source which is making a mess here.

What would be the prop type adapter you are thinking about?

🇫🇷France pdureau Paris

Maybe we can leverage SourceInterface::getPropValue()

Proposal:

  public function getPropValue(): mixed {
    $value = $this->getSetting('value');
    $enum = $this->propDefinition['enum'];
    // Select form element is alwyas storing values as strings, but we want
    // a value with the same typing as the related enum value.
    return match (TRUE) {
      in_array($value, $enum, TRUE) => $value,
      in_array((int) $value, $enum, TRUE)  => (int) $value,
      in_array((float) $value, $enum, TRUE) => (float) $value,
      default => $value,
    };
  }

🇫🇷France pdureau Paris

Let's be careful with this one.

  • A new proptype adapter? It may be overkill
  • In SelectWidget? the best place to do the change. It may be the select form element which is casting the integer value as a string
  • in EnumPropType::normalize()? Look like a good place too, but the method is static and doesn't have access to the prop definition

So, what can we do in SelectWidget ? a #process callback?

🇫🇷France pdureau Paris

Thanks Brahim.

$attr_name = "[a-zA-Z\-]+";: can yo uupdate

$double_quoted_value = '"[\s\w\-]*"';

🇫🇷France pdureau Paris

Hi Sorya,

Thanks for the improvement. Let's ship it with the upcoming 1.0.2 release.

However, I am still not conformable with this change:

-

{{ title }}

+

{{ title }}

Because P element is not accepting block elements as children, so it is a bit risky for a slot, especially when used with a display builder like layout_builder. Is it mandatory to do?

(Careful, i have rebased your remote branch, you may need to recreate your local branch)

🇫🇷France pdureau Paris

Mikael, Can you have a look ? because you are currently working on the form

🇫🇷France pdureau Paris

I'm not sure we need to do anything special to 'take' it for core given there's no Drupal 8+ code ther

👍

We could file a project ownership issue just in case maybe?

It would be safer. However, will it make us responsible for the maintenance of the existing codebase? What will happen to https://www.drupal.org/project/issues/icon ?

if we did that, would the existing contrib module continue to provide those bits?

That's the idea, https://www.drupal.org/project/ui_icons will still exist and leverage the API found in Core by providing:

  • other extractor plugins
  • integration with Drupal API & tools
  • some extra stuff like library pages, form improvements...

We hope many other contrib modules will do the same.

🇫🇷France pdureau Paris

pipeline has run and everything is OK :)

🇫🇷France pdureau Paris

Can you have a look once the pipelines has run?

🇫🇷France pdureau Paris

I have a problem with some settings where errors can not be fixed, I guess it's from the yaml => php array => object conversion, but I cannot find a solution yet,

We got similar issue with Dilla's discovery and UI Patterns 2.x, I will have a look.

[settings.decimal.multipleOf] Use of exclusiveMinimum requires presence of minimum [settings.decimal] Failed to match all schemas

Werid

🇫🇷France pdureau Paris

Added: license, links & version

🇫🇷France pdureau Paris

ok, i will add a commit with: license, links & version

🇫🇷France pdureau Paris

"variant" removed in an other ticket

🇫🇷France pdureau Paris

I am still not reproducing.

Here is my 10.2 install

$ vendor/bin/drush st
Drupal version   : 10.3.3                                                        
Site URI         : http://default                                                
DB driver        : sqlite                                                        
DB port          :                                                               
DB username      :                                                               
DB name          : db.sqlite                                                     
Database         : Connected                                                     
Drupal bootstrap : Successful                                                    
Default theme    : ui_suite_bootstrap                                            
Admin theme      : claro                                                         
PHP binary       : /usr/bin/php                                                  
PHP config       : /etc/php.ini                                                  
PHP OS           : Linux                                                         
PHP version      : 8.3.11                                                        
Drush script     : /home/pierre/Projects/Drupal/d10/vendor/bin/drush             
Drush version    : 12.5.3.0                                                      
Drush temp       : /tmp                                                          
Drush configs    : /home/pierre/Projects/Drupal/d10/vendor/drush/drush/drush.yml 
Install profile  : standard                                                      
Drupal root      : /home/pierre/Projects/Drupal/d10/web                          
Site path        : sites/default                                                 
Files, Public    : sites/default/files                                           
Files, Temp      : /tmp                      

With up-to-date UI Patterns 2

commit 701bb9b423c19ea8ceaa838b0376cfb70db3fa31 (HEAD -> 2.0.x, origin/2.0.x)
Author: christian.wiedemann <7688-Christian.wiedemann@users.noreply.drupalcode.org>
Date:   Fri Sep 6 19:53:45 2024 +0000

    Issue #3469814 by christian.wiedemann, just_like_good_vibes: [2.0.0-beta2] Kernel Source Tests
🇫🇷France pdureau Paris

It is hard and heavy to extract icons from a proper font file (woff, ttf...).

Yes, but codepoints files are a mess -sometimes CSV, sometimes JSON, sometimes XML... no standardized structure) and not always available.

Maybe it is possible to extract the icon list using a lib (or a subset of a lib to avoid a dependency) like https://github.com/dompdf/php-font-lib

If we focus on only WOFF 2 for now, because all DS seems to propose this format, it may be simpler:

  • Bootstrap 5: bootstrap-icons.woff & bootstrap-icons.woff2
  • Font awesome: fa-brands-400.ttf fa-regular-400.woff2 fa-v4compatibility.ttf fa-brands-400.woff2 fa-solid-900.ttf fa v4compatibility.woff2 fa-regular-400.ttf fa-solid-900.woff2
  • Feather: ???
  • Material Symbols: https://fonts.gstatic.com/s/materialsymbolsoutlined/v207/{hash}.woff2
🇫🇷France pdureau Paris

I am not reproducing with drupal 11.0

I will test with drupal 10.3

🇫🇷France pdureau Paris

Thanks Mathieu.

This related issue will be useful if we store the thumbnails file outside the component directory and/or we name it something different from "thumbnail.png".

We can still process this issue without waiting the related issue to be done.

🇫🇷France pdureau Paris

PHPCS is KO

   | ERROR | [x] Use statements should be sorted alphabetically. The first
   |       |     wrong one is
   |       |     Drupal\Component\Plugin\Definition\PluginDefinitionInterface.
   |       |     (SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses.IncorrectlyOrderedUses)

Careful ( I have rebased the branch on git.drupalcode.org, you may need to delete and repull your local)

🇫🇷France pdureau Paris

Maybe the current MR is already enough. I will have a look.

🇫🇷France pdureau Paris

Hi,

Looking at: https://git.drupalcode.org/project/drupal/-/merge_requests/8860/diffs

    items:
      title: Tags Items
      type: array
      item:
        type: object
        properties:
          content:
            title: Tags Item
            type: object
          attributes:
            title: Item Attributes
            type: Drupal\Core\Template\Attribute

1. No need to have "Tags Items" as title, we know we are in a Tag component. "Items" is enough.

2. This is not JSON schema valid. You need items instead of item:

      type: array
      items:
        type: object

Source: https://json-schema.org/understanding-json-schema/reference/array

3. In your JSON object, content property is an object itself. Why? An object of what? Which properties? It is printed in the template, but a mapping is not printable in Twig (except when it is a Drupal renderable, but those are expected in slots). Would a simple string fit instead?

4. If you make content property a string, it would be the opportunity to adopt an existing Drupal data structure, like the menu structure, so it will be easier to plug it to Drupal API, by replacing content by title.

🇫🇷France pdureau Paris

Brahim, can you move the trait use to PropTypePluginBase?

🇫🇷France pdureau Paris

Moved to beta3 because:

  • There is many active and breaking changes currently nExperience builder
  • We may need to push a fix in Core before anyway
🇫🇷France pdureau Paris

already done in 📌 [2.0.0-beta2] ContextHelper cleaning Fixed

But we will not forget you in the credits

🇫🇷France pdureau Paris

as reported by multiple users, let's also correct that error in this issue with blocks :
Warning: Undefined array key "bundle" in Drupal\ui_patterns_blocks\Plugin\Block\ComponentBlock->getComponentSourceContexts() (line 137 of modules/contrib/ui_patterns/modules/ui_patterns_blocks/src/Plugin/Block/ComponentBlock.php).
it is probably already corrected in the current code :)

See: 🐛 [2.0.0-beta2] Undefined array key "bundle" in ComponentBlock Needs work

🇫🇷France pdureau Paris

One month after Florent has created this ticket, I am doing the same tests.

I don't meet the TypeError: Argument #1 ($callback) must be of type callable, string given or any other fatal error.
But the styles are not applied.

For example, if I create a section with Bootstrap alert component as a layout and I apply the bg-danger style utility to the heading slot, I see the style inside the region (so a temporary place sued by the layout PAI) and not inside the component slot (what will be rendered using SDC):

array:9 [
  "#type" => "component"
  "#component" => "ui_suite_bootstrap:alert"
  "#layout" => Drupal\ui_patterns_layouts\Plugin\Layout\ComponentLayout {#6647
    #pluginId: "ui_patterns:ui_suite_bootstrap:alert"
    ...
  }
  "#slots" => array:1 [
    "heading" => array:1 [
      "7562fca6-6168-41e6-aaa1-ee490bdff420" => array:10 [
        "#theme" => "block"
        "#plugin_id" => "field_block:node:article:title"
        "content" => array:2 [...]
        "#attributes" => []
      ]
    ]
  ]
  "heading" => array:2 [
    "7562fca6-6168-41e6-aaa1-ee490bdff420" => array:10 [
      "#theme" => "block"
      "content" => array:2 [...]
      "#attributes" => []
    ]
    "#attributes" => array:1 [
      "class" => array:1 [
        "colors_background_color" => "bg-danger"
      ]
    ]
  ]
  "message" => array:1 []
]
  

But this fix (which is the same 📌 [2.0.0-beta2] Drag & drop in layout builder Postponed , with only one line more) seems to work:

--- a/modules/ui_patterns_layouts/src/Element/ComponentAlterer.php
+++ b/modules/ui_patterns_layouts/src/Element/ComponentAlterer.php
@@ -39,8 +39,14 @@ class ComponentAlterer implements TrustedCallbackInterface {
       foreach (Element::children($element[$region_id]) as $block_id) {
         $element["#slots"][$region_id][$block_id] = $element[$region_id][$block_id];
       }
-      if (isset($element[$region_id]['#attributes'])) {
+      if (isset($element[$region_id]['#attributes']) && isset($element["#slots"][$region_id])) {
         $element['#region_attributes'][$region_id] = new Attribute($element[$region_id]['#attributes']);
+        $element["#slots"][$region_id] = [
+          "#type" => "html_tag",
+          "#tag" => "div",
+          "#attributes" => $element[$region_id]['#attributes'],
+          "content" => $element["#slots"][$region_id],
+        ];
       }
       unset($element[$region_id]);
     }

Florent, I give the ticket back to you to check my analysis.

🇫🇷France pdureau Paris

Looks great, I will merge this and target 1.0.2

🇫🇷France pdureau Paris

Hi, thanks for the MR

First look: you need to use the attribute object and add a default filter for title_tag in the template

🇫🇷France pdureau Paris

Hi @pdureau, the request is to replace or keep both available?

The existing form (3 parameters) is the expected one. It needs to be kept. The proposal is an alternative form.

Personally, I am not 100% convinced about introducing this new form.

So wouldn't be better to write {{ icon(iconDefinition) }} for example ?

Indeed, but iconDefinition must be a Twig mapping, not an executable PHP objetc

🇫🇷France pdureau Paris

Hi Jean,

I have added the JSON schema file: https://git.drupalcode.org/project/ui_icons/-/blob/cd3e748f361f4af084a53...

Some questions:

  • Is it the good opportunity to add other metadata in the iconpack definition? Like license or links ?
  • I wanted to make template required, but I see some tests files are not using it. What is happening during rendering if we don't set a template?
  • In settings, the "^\\w+$" pattern is accepting value with a dash ("-"). But AFAIK Twig is not accepting variables with a -. Do we change the pattern or do we keep the schema simple?
🇫🇷France pdureau Paris

In a way, it is more or less what SDC is doing:

ComponentsTwigExtension is adding 2 twig functions:

  • add_component_context which is adding the attribute Attribute object if missing
  • validate_component_props which is executing the JSON schema validator

They are not supposed to be used by templates owner, there exists because ComponentNodeVisitor is printing those Twig functions on every template, which will be executed on rendering in this order:

  1. attach_library
  2. add_component_context
  3. validate_component_props

In order to make the use of "template to template" Twig functions or tags (include and embed) more similar to the use of the Render API, leveraging the render element. For example, attach_library is used here because #attached is not executed.

🇫🇷France pdureau Paris

DRAFT - WORK IN PROGRESS: WILL BE UPDATED WITH BETTER INFO

So, let's see every possible situation.

No attribute in template context

No attribute in JSON schema

  1. add_component_context is adding an Attribute object
  2. validate_component_props is skipping the validation, so it is OK ✅

Attribute as a namespaced PHP class in JSON schema

The same:

  1. add_component_context is adding an Attribute object
  2. validate_component_props is skipping the validation, so it is OK ✅

Attribute as a proper JSON schema definition (the UI Patterns 2.x way)

  1. add_component_context is adding an Attribute object
  2. validate_component_props is doing the validation, and it is supposed to be KO ❌

attribute as a PHP object in template context

No attribute in JSON schema

  1. add_component_context is altering the Attribute object
  2. validate_component_props is skipping the validation, so it is OK ✅

attribute as a namespaced PHP class in JSON schema

The same:

  1. add_component_context is altering the Attribute object
  2. validate_component_props is skipping the validation, so it is OK ✅

Attribute as a proper JSON schema definition (the UI Patterns 2.x way)

  1. add_component_context is doing nothing
  2. validate_component_props is doing the validation, because a ❌

attribute as a PHP array in template context

This will be the situation if we remove AttributePropType::normalize()

No attribute in JSON schema

  1. add_component_context is doing nothing
  2. validate_component_props is skipping the validation, so it is OK ✅

Attribute as a namespaced PHP class in JSON schema

The same:

  1. add_component_context is doing nothing
  2. validate_component_props is skipping the validation, so it is OK ✅

Attribute as a proper JSON schema definition (the UI Patterns 2.x way)

  1. add_component_context is doing nothing
  2. validate_component_props is doing the validation, and it i supposed to be KO because a ❌
🇫🇷France pdureau Paris

SDC implementation looks a bit messy IMHO:

ComponentsTwigExtension is adding 2 twig functions:

  • add_component_context which is adding the attribute Attribute object if missing
  • validate_component_props which is executing the JSON schema validator

Why are they Twig functions instead of living in the rendering process? Who will ever use them in the template? I don't know

Then, ComponentNodeVisitor is ever more problematic, because it prints those Twig functions on every template, which will be executed on rendering:

  • attach_library
  • add_component_context
  • validate_component_props

And this is making all the mess. What we are paying here is the cost of the use of "template to template" Twig functions or tags (include and embed) instead of a dedicated mechanism which is loading the Render API and leveraging the render element. For example, attach_library is used here because #attached is not executed.

All this part of SDC would be to be removed and redone, but it will take time.

What can we do now, in this ticket, to overcome this?

🇫🇷France pdureau Paris

Disclaimer: This issue is not about computed field properties (in Drupal Core, found in: DateRangeItem, DateTimeItem, EntityReferenceItem, FileUriItem) but computed fields.

Unfortunately, AFAIK, there are no entity type with computed fields in Drupal Core.

So, considering the change is very light (a single line) and looks safe, I will merge it.

🇫🇷France pdureau Paris

Because we plan to extends the plugin manager, let's wait to see what will happen with the service decorator in 🐛 [2.0.0-beta2] plugin.manager.sdc conflict with Experience Builder Needs work

🇫🇷France pdureau Paris

Hi Brahim,

-    if (!$this->context["bundle"] && isset($this->context["entity"])) {
+    if (!isset($this->context["bundle"]) && isset($this->context["entity"])) {
Determine if a variable is considered set, this means if a variable is declared and is different than null.

Source: https://www.php.net/manual/fr/function.isset.php

Are we losing something with this change? What is happening if $this->context["bundle"] return a value resolving to false, but not to null?

Those are genuine questions, not rhetorical, I am not a PHP expert.

🇫🇷France pdureau Paris

Time flies. We have other priorities for beta2

🇫🇷France pdureau Paris

if we can make it a little bit more simple, just to list all icons, I think it will be easier for non tech savvy users.

So, without the settings form? If you define no settings in your icon pack definition, the settings form will not show up.

🇫🇷France pdureau Paris

Checked with Mikael

It is SDC's ComponentValidator::validateProps() which is calling a dependency method which is executing json_encode() which is casting the attribute object into a string.

So, we may need 2 different methods:

  • keep PropTypeInterface::normalize() before the ComponentValidator, which is always returning data conform to JSON Schema, so which is keeping the associative array for attributes
  • add a new method (PropTypeInterface::preprocess() ?) in between the ComponentValidator and the injection to Twig template, which is instantiating the Attribute object

Unfortunately, ComponentValidator may be executed very late, from a Twig extension, so we may need to add our own Twig extension to be executed after.

🇫🇷France pdureau Paris

the variant prop type uses enums, so fixing the issue with the enum prop type should also resolve it for the variant.

I am not sure about this sentence.

I tested all existing prop types and found no serialization errors.

This is the most important, I will do the review

Production build 0.71.5 2024