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

Merge Requests

More

Recent comments

πŸ‡«πŸ‡·France pdureau Paris

Let's talk about that tomorrow

πŸ‡«πŸ‡·France pdureau Paris

Fixed in πŸ› PHP warnings with Metatag module Active according to https://www.drupal.org/project/ui_patterns/issues/3498456#comment-15951405 πŸ› PHP warnings with Metatag module Active

πŸ‡«πŸ‡·France pdureau Paris

The change is great, thanks.

Before merging and closing the issue, we need to create the related issue in https://www.drupal.org/project/issues/ui_patterns_ds β†’

Do you want to do it?

πŸ‡«πŸ‡·France pdureau Paris

I was not expecting us to add description but only to convert existing, but that's interesting.

I have proposed my takes.

πŸ‡«πŸ‡·France pdureau Paris

Crediting @pdureau

Thanks, it is an honor 😊

Code Components are rendered (without Twig) to an element t

So, does that mean we are switching /ui/src/ to Astro (still using React component, but different framework) ?

πŸ‡«πŸ‡·France pdureau Paris

Very cool. OK with everything

πŸ‡«πŸ‡·France pdureau Paris

Indeed, for the moment, ui_patterns module is not providing itself a way to guess the current entity inside ds_entity_view layout options

UI Patterns target both Core & contrib modules, but we target by default only Core plugin types: Blocks, Layouts, Formatters...

Display Suite is using both:

  • core plugin types (Layout for example) and it is supposed to work directly with ui_patterns_layouts sub-module if the usage of Dsipaly Suite is a "normal" (no weird additions or alteration)
  • a custom plugin type ("DS Field"), for this one, ui_patterns_ds is needed

So, are we 100% sure it is not working because of DS?

So, anyway, it seems there is something to do for the RC2.

πŸ‡«πŸ‡·France pdureau Paris

The main issue I encountered is that the url is a string instead of an object.

It was done in purpose. Everything related to the application state must be resolved before being sent to component template. SDC templates must be dumb, UI-logic only.

To print it in the template took extra checks and manipulation to handle the 3 different link urls.

Are you sure you need those checks to implement an UI component like DaisyUI's footer? I guess not :)

If yes, we can talk about changing LinksPropType::normalize() logic.

πŸ‡«πŸ‡·France pdureau Paris

The Links prop type is an abstract data structure made to be able to "host" many Drupal data sttucture (menu, links, pager, breadcrumb...)

It is normal the structure is a bit different before after the normalization: https://git.drupalcode.org/project/ui_patterns/-/blob/2.0.x/src/Plugin/U...

We hope this work will be done by Drupal Core in the future: ✨ Add unified Links data type for menus, pagers, breadcrumbs... Active

πŸ‡«πŸ‡·France pdureau Paris

Hi,

we have overridden Core's |add_class() and set_attribute() filters in UI Patterns (1 & 2) since May 2023, and... it works.

https://git.drupalcode.org/project/ui_patterns/-/blob/2.0.x/src/Template...

what about an array of arrays of render arrays?

As long as the array is a list, as expected.

It is a simple and useful solution, used by many projects for many months. No need to introduce complicated concepts like "repeatable slots" or "slots restrictions"

πŸ‡«πŸ‡·France pdureau Paris

A component placed in block layout has contextual edit links for Configure and Remove, but not for editing the menu.

Ok, I understand now. That could be a nice feature to add later: in a ComponentBlock, if the component has a links prop type which is set with a MenuSource value, add a "Menu" contextual edit link to the menu config page.

What will happen if the component has many links prop type? We take the first one with a MenuSource value?

I hope the implemnattion of this will not be too complicated for a small feature.

πŸ‡«πŸ‡·France pdureau Paris

Hi @mortona2k,

Thanks for your work. However, you created 3 components: footer, footer_menu, and aside.

footer_menu and aside don't exist in DaisyUI according to the documentation: https://daisyui.com/components/

Only footer is documented: https://daisyui.com/components/footer/

Moreover, looking at the templating, it seems aside is not a component but a simple HTML element:

<aside{{ attributes }}>
  {{ content }}
</aside>

And footer_menu markup doesn't look like the one of a self contained component.

Also, we don't need custom CSS here. The CSS is handled by DaisyUI. Maybe you needed to add some because of the 3 components instead of one.

What you need to do in my opinion:

  1. create footer as described in the DaisyUI documentation. Nothing more, nothing less.
  2. add footer_nav & footer_aside regions to theme info.yml
  3. call the component from page.html.twig by mapping the regions to the slots, and setting the default footer if you want
πŸ‡«πŸ‡·France pdureau Paris

I think they are normally printed in the block template, and the component just handles the menu links.

That's right. Contextual edit links for the component block works anyway? Is it enough?

πŸ‡«πŸ‡·France pdureau Paris

(There are some limitations in comparison, would be great to add those, like the options to render/link the selected parent item)

We are open to proposals: https://www.drupal.org/project/issues/ui_patterns β†’ :)

I was experimenting with the UI Suite ecosystem and DaisyUI, this is my first time with both.

I hope you are enjoying this discovery

πŸ‡«πŸ‡·France pdureau Paris

Can we move...

The declaration in some YAML files?

in order to be authorable by front developers.

For example, naive proposal in ui_suite_bootstrap.ui_skins.themes:

# No target because scopable.
# No value because same as plugin ID.
dark:
  label: "Dark"
  label_context: "color"
  key: "data-bs-theme"
  bg_utility:
    - bg-dark
  fg_utility:
    - text-bg-dark

# No target because scopable.
# No value because same as plugin ID.
light:
  label: "Light"
  label_context: "color"
  key: "data-bs-theme"
  utility_equivalent:
    - bg-light
    - text-bg-light

The Bootstrap::getThemeMapping() logic in UI Skins?

We can keep the apply in the ui_suite_bootstrap PHP code for now, so those function will call the new UI Skins API:

  • PreprocessPage::preprocess()
  • PreprocessMediaMediaLibrary::preprocess()
  • FormMediaLibraryAddFormAlter
  • ElementPreRenderLayoutBuilder
πŸ‡«πŸ‡·France pdureau Paris

Here's another approach, using UI Styles for color, padding, and the footer class.

Thanks to the attributes prop, it will be possible to use UI styles with the footer component.

But first, we need the footer component :)

The Menu Block module lets you set the theme hook to use if you want to change footer to use something else, or use another menu.

It is already handled by the MenuSource plugin for links props. If we do this component the right way, UI Patterns 2 will plug it to many Drupal API. It is magical :)

Layout builder and DaisyUI have a ton of options for building footer variations.

Let's not thing about the display builder while authoring the component. Let's build a good component, focused on UI and implemenattion of the upstream design system documentation, and it will work on layout builder and others.

My little proposal si still relevant IMHO: https://docs.google.com/document/d/1DJCsANcHH7lnbxfUlHnX7m_2kiWdqAN8YWM9...

πŸ‡«πŸ‡·France pdureau Paris

Congratulations for Drupal CMS 1.0 πŸ₯³

πŸ‡«πŸ‡·France pdureau Paris

Thinking about moving with_context: false to the .component.yml file.
This approach could allow component makers to decide when to restrict it or not, which would be helpful for nesting.

I am not comfortable with this:

  • with_context is not something located in the component itself, but where it is called.
  • with_context is a standard Twig feature
  • So, what is happening if the with_context information from .component.yml is conflicting with the one from Twig include() function?

Having something like utility_classes as a property in all components

This is the purpose of the automatically injected attributes prop, which can do more:

  • SEO modules can insert tagging attributes through it
  • Accessibility modules can alter/insert some attributes values (aria-*, role, alt...) through it
  • Translation modules can add the lang attribute when needed
  • Hypermedia modules (HTMX, Hotwire/Turbo...) can insert triggers and behaviours attributes through it
  • Drupal Core's |add_class() and |set_attribute() Twig filters rely on it
  • display builders (like Experience Builder) can insert annotation through it
  • ...

What to do from UI Patterns for example if all has utility_classes? and will they pass all values from the parent component to the deep child components?

This is the purpose of Drupal Core's |add_class() filter, which is an explicit and standardized way of doing it.

πŸ‡«πŸ‡·France pdureau Paris

Not possible to move it to the Schema Manager's Canonicalizer, because Canonicalizer service is called (from ComponentPluginManager::annotateProp() to PropTypePluginManager::getDefinitionFromSchema() to CompatibilityChecker::isCompatible()) before ComponentMetadata::parseSchemaInfo() is executed (ComponentPluginManager::createInstance() to Component::__construct() to ComponentMetadata::__construct()).

I have tried to alter it by overriding ComponentPluginManager::createInstance() but everything is read-only.

So, I guess we need to fix this directly in SDC with a Core issue, by simply removing:

      // All props should also support "object" this allows deferring rendering
      // in Twig to the render pipeline.
      $schema_props = $metadata_info['props'];
      foreach ($schema_props['properties'] ?? [] as $name => $prop_schema) {
        $type = $prop_schema['type'] ?? '';
        $schema['properties'][$name]['type'] = array_unique([
          ...(array) $type,
          'object',
        ]);
      }
πŸ‡«πŸ‡·France pdureau Paris

hi Rajab, this documentation must be updated. See 🌱 Clarify SDC documentation by toning down Twig blocks promotion Active

πŸ‡«πŸ‡·France pdureau Paris
πŸ‡«πŸ‡·France pdureau Paris

Auto-merge

πŸ‡«πŸ‡·France pdureau Paris

Decided collectively during the weekly.

Instead of a list of buttons inside a fieldset, let's try with a select form element with:

  • - Select a source to add - as empty option of the form element
  • An AJAX mechanism (using the Druapl Form State API) to submit on change
πŸ‡«πŸ‡·France pdureau Paris

2 feedbacks.

Source label in form: Instead of the confusing "Field prop: entity", some proposals: "Field item: entity", "Referenced entity" (singular)... what else?

Also, after selection, we don't see the selected source, but it will be fixed in its own ticket.

Those 2 sources setup are now equivalent because "Reference entities" now behave like the new source:

        slots:
          title:
            sources:
              -
                source:
                  derivable_context: 'entity_reference:node:article:field_tags:taxonomy_term:tags'
                  'entity_reference:node:article:field_tags:taxonomy_term:tags':
                    value:
                      sources:
                        -
                          source:
                            derivable_context: 'field:taxonomy_term:tags:name'
                            'field:taxonomy_term:tags:name':
                              value:
                                sources:
                                  -
                                    source_id: 'field_property:taxonomy_term:name:value'
                                    _weight: '0'
                          source_id: entity_field
                          _weight: '0'
                source_id: 'entity:field_property:node:field_tags:target_id'
                _weight: '0'
          message:
            sources:
              -
                source:
                  derivable_context: 'entity_reference:node:article:field_tags:taxonomy_term:tags'
                  'entity_reference:node:article:field_tags:taxonomy_term:tags':
                    value:
                      sources:
                        -
                          source:
                            derivable_context: 'field:taxonomy_term:tags:name'
                            'field:taxonomy_term:tags:name':
                              value:
                                sources:
                                  -
                                    source_id: 'field_property:taxonomy_term:name:value'
                                    _weight: '0'
                          source_id: entity_field
                          _weight: '0'
                source_id: entity_reference
                _weight: '0'
πŸ‡«πŸ‡·France pdureau Paris

OK for me because:

  • The new UI is good enough for me
  • It is the same data structure

Christian, if you are OK too, we merge:

                component_id: null
                variant_id:
                  source_id: select
                  source:
                    value: ''
                slots:
                  title:
                    sources:
                      -
                        source:
                          component:
                            component_id: 'ui_suite_dsfr:button'
                            variant_id:
                              source_id: select
                              source:
                                value: ''
                            props:
                              attributes:
                                source_id: attributes
                                source:
                                  value: ''
                              title:
                                source_id: textfield
                                source:
                                  value: ''
                              url:
                                source_id: url
                                source:
                                  value: ''
                              target:
                                source_id: select
                                source:
                                  value: ''
                              external:
                                source_id: checkbox
                              disabled:
                                source_id: checkbox
                              icon_position:
                                source_id: select
                                source:
                                  value: ''
                              tooltip:
                                source_id: textfield
                                source:
                                  value: ''
                              id:
                                source_id: textfield
                                source:
                                  value: ''
                              social:
                                source_id: select
                                source:
                                  value: ''
                              icon:
                                source_id: icon
                                source:
                                  value: null
                            slots:
                              slots: {  }
                        source_id: component
                        _weight: '0'
                props:
                  attributes:
                    source_id: attributes
                    source:
                      value: ''
                  dismissible:
                    source_id: select
                    source:
                      value: ''
                  close_title:
                    source_id: textfield
                    source:
                      value: ''
                  title_tag:
                    source_id: select
                    source:
                      value: ''
πŸ‡«πŸ‡·France pdureau Paris

Mikael,

I am happy Dale finds the new source useful :)

I will do the proper review tomorrow, but I have a question.

Is the combination of "Data from a field + "Field prop: entity" the same as "Reference entity" ?

πŸ‡«πŸ‡·France pdureau Paris

What about if a developer actually wants to have access to a variable that's available in the parent component?

The variables still go the same way (from Drupal template to component template), they just need to be explicitly passed instead of "magically". Is it an issue?

I doubt most people will be using web components/shadow DOM

No, but we expect to have more in the future, so we are very interested about this subject. Especially when we will start the 3.x branch of https://www.drupal.org/project/ui_suite_material β†’

πŸ‡«πŸ‡·France pdureau Paris

Hello you,

You can get some help from https://www.drupal.org/project/sdc_devel β†’ which is providing a drush command and a report page.

Still in alpha phase, there are still false positive β†’ , unclear messages and some missing warnings, but already very useful to get and fix most of the SDC component authoring errors.

πŸ‡«πŸ‡·France pdureau Paris

Yes, I'm sending the items (menu structure) to the items slot in the nav component.

That's the issue. This is not UI Patterns 2 related, but SDC related. The menu structure is not renderable. It is a rigid typed structure you are manipulating instead of just printing:

      {% for item in items %}
        {% set nav_item_classes = [
          'nav-item',
          item.is_expanded and item.below ? 'dropdown' : '',
          item.in_active_trail ? 'active',
        ] %}

So, you have 2 choices:

  • keep items a slot and send an already built menu renderable in it
  • make items a prop following the menu structure, something like that:
    type: array
    items:
      type: object
      properties:
        title:
          type: string
        url:
          type: string
          format: iri-reference
        below:
          type: array
          items:
            type: object
    
πŸ‡«πŸ‡·France pdureau Paris

Done for most, only Protocol and Material missing, issues ongoing.

Protocol doesn't have icons. Material will get icons in the upcoming beta1.

πŸ‡«πŸ‡·France pdureau Paris

I guess we are talking about https://git.drupalcode.org/project/varbase_components/-/blob/2.0.x/compo... :)

You are typing the slot, which is not a thing:

slots:
  items:
    type: array
    name: Items
    description: Navigation items.

Better to remove it:

slots:
  items:
    name: Items
    description: Navigation items.

Then, what do you send in this slot? I don't see anything explicit here:
{% include 'varbase_components:nav' with {
utility_classes: [
'social-media--menu',
'w-fc',
'h-px-32',
'align-items-center',
'gap-20',
],
} %}

I am afraid you are sending a menu structure to the slot instead of a renderable.

It is better to use include function, with with_context = false and explicit mapping:
{{ include('varbase_components:nav', {
utility_classes: [...]
items: ...,
}, with_context = false) }}

πŸ‡«πŸ‡·France pdureau Paris

For DaisyUI, the footer menu is not its own component, let's follow that, let's stick with upstream.

I think I needed those templates to get the contextual edit links to work.

We lose the capacity of using Footer Menu Block (and the related contextual links), but that's OK because we can inject the footer menu in the component prop. Moreover it is better to avoid code (templates suggestion here) directly referencing config, by skipping menu--footer.html.twig, because "footer" here is a config entity ID.

There are some differences, like active menu trail classes.

It will be managed by the LinksPropType::normalize() if we use the links items attributes in the template.

I think loading the SDC in the page template makes sense. But I'd rather find a way to make it flexible than hardcode the footer menu.

The way I am proposing is less "hardcoded" in my opinion. This is a pure UI/design implementation, without coupling to the Drupal application state (blocks & menus). It is a different way of theming, maybe a bit disturbing at first, but it worth it.

πŸ‡«πŸ‡·France pdureau Paris

Hello,

In my opinjion, footer must be a SDC component because it is a component in upstream documentation: https://daisyui.com/components/footer/

Little specification proposal for the component: https://docs.google.com/document/d/1DJCsANcHH7lnbxfUlHnX7m_2kiWdqAN8YWM9...

And the 2 slots must be theme regions and the component must be called from page.html.twig

The injection of footer Drupal menu must be hardcoded in page.html.twig or configurable in the theme settings, because there is no good display builder for pages yet.

No need for menu--footer.html.twig and block--system-menu-block--footer.html.twig.

If there is default blocks in the slots, it must be config entities in config/optional/ folder

πŸ‡«πŸ‡·France pdureau Paris

I have been working on other focuses, and needed a break from Drupal.

I hope you have rested and feel better.

I had a few plans for some feature improvements that I'll focus on targeting with the Drupal 11.1 release.

Great :) Tell us if you have questions.

One big one was the ability to override icons in themes. So a module could define an edit icon for use with functionality and provide a default icon, but themes could change the icon display to fit the theme aesthetic.

I woudl love to use such a feature.

πŸ‡«πŸ‡·France pdureau Paris

The most important is to not introduce breaking changes now. Shipping with a loosely defined schema is better than a breaking change in RC.

πŸ‡«πŸ‡·France pdureau Paris

niharika patch is not OK because:

  • a patch, not a PR
  • no change in the definition file
  • change in template is weird
  • no linting
πŸ‡«πŸ‡·France pdureau Paris

And also: remove the style.

πŸ‡«πŸ‡·France pdureau Paris

Normally, new icons are automatically discovered.

For example, I am on DSFR 1.12.1 locally and I already see digital/in-progress. Can you check?

πŸ‡«πŸ‡·France pdureau Paris

Mikael & me have decided to cancel this task because the upstream documentation is not guarantee to be stable (see DSFR's slack), so we may introduce painful breaking changes for nothing.

We will consider this again when DSFR 2.0 will be released.

πŸ‡«πŸ‡·France pdureau Paris
πŸ‡«πŸ‡·France pdureau Paris

Indeed. Can you?

  1. Update your local drupal/coder
  2. Restore SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses . See https://www.drupal.org/project/coder/releases/8.3.27 β†’
  3. Apply the new rules to the codebase & commit
πŸ‡«πŸ‡·France pdureau Paris

Do we really need is not empty everywhere? If it works without it, let's keep it simple:

{% if item.below or item.url %}
πŸ‡«πŸ‡·France pdureau Paris

Can we first fix the default behaviour ? And maybe add a confirmation option later in a feature ticket..

πŸ‡«πŸ‡·France pdureau Paris

Revert title update because it also changes the path

πŸ‡«πŸ‡·France pdureau Paris

Make more abvious this is about UI Patterns 1.x only

πŸ‡«πŸ‡·France pdureau Paris

The attribute-based solution being argued for (with a lot of data to back it up in #18, impressively so, @pdureau! πŸ˜„)

Ah ah indeed πŸ˜‰

Seeing the complexity of the last days discussion and the current MR adding 500 new LOC to the already huge XB codebase, I am still believing something specific has been done wrong earlier in the project and we are now fighting against it.

I hope everything will land well 🀞

πŸ‡«πŸ‡·France pdureau Paris

Hi Rahaf,

Thanks for using UI Patterns Release Candidate and share your feedbacks.

Can you tell us more? What is the component definition? In which prop do you wnat to send this menu data? How? From a presenter template ? A render element ?

πŸ‡«πŸ‡·France pdureau Paris

This does not need to be perfect, or feature-complete, or even close to it. This is strictly meant for a one-off, single-page, read-only demo of Experience Builder, which is under heavy development, and we are not (for now) going to allow people to build real production pages with these components.

Thanks @phenaproxima for the context I was missing.

πŸ‡«πŸ‡·France pdureau Paris

It will also be the opportunity of:

  • add an attributes prop by region like what UI suite Bootstrap is doing? It may be interesting to add the role="complementary" style="order: XXX" (let's not care about aside element because not like that in DSFR)
  • changing the ratio from 4/4/4 to 3/6/3 (visually, so 6/3/3 in markup)
  • adding a theme settings to manage the ratio? Because the page layout display builders are not ready yet.
πŸ‡«πŸ‡·France pdureau Paris

Let's check with UI Patterns 2 leaders: Christian & Michael.

πŸ‡«πŸ‡·France pdureau Paris

Indeed, there is an example with a form in a dropdown in upstream doc: https://getbootstrap.com/docs/5.3/components/dropdowns/#forms

We can imagine adding two slots: one before {% for item in content %}...{% endfor %} and one after.

However, that would mean doing heavy and risky change in the HTML structure, without the guarantee to work properly at the end:

  {% if content %}
    {% if before %}
    <div{{ dropdown_menu_attributes }}>
      {{ before }}
      <ul>
    {% else %} 
      <ul{{ dropdown_menu_attributes }}>
    {% endif %}
    {% for item in content %}

So, I advise not implementing this in the theme, and let users oiverriding the component if they need.

πŸ‡«πŸ‡·France pdureau Paris

No slots in this component? Why the slots get so little love in the SDC community those days? πŸ™ƒ

A few feedbacks.

image prop must be a slot

The $ref of this prop:

    image:
      title: 'Featured image'
      $ref: json-schema-definitions://experience_builder.module/image
      type: object
      examples:
        - src: https://placehold.co/600x450.png
          alt: 'Boring placeholder'
          width: 600
          height: 400

has 2 issues which can be resolved by switching to a slot:

  • an unnecessary dependency to experience builder. It is better to not bend the component definition to please the display builder, whatever it is.
  • a rigid structure to represent an object.

I have already warned XB team about this problematic $ref : https://www.drupal.org/project/experience_builder/issues/3468944#comment... πŸ“Œ Update XB's `image` SDC to comply with best practices, and document those best practices Needs review

Today, the MR template looks like that:

  <img alt="Placeholder Image" class="my-section__img" width="600" height="450" src="data:image/jpeg;base64...">

But I guess it will looks like that later:

<img class="my-section__img" src="{{ image.src }}" alt="{{ image.alt }}" width="{{ image.width }}" height="{{ image.height }}"/>

This rigid structure doesn't fit with real life usage. What will happen if some props are missing or empty? What about other classes? What about other attributes? cross origin rules? lazy loading? responsive sources?

A simple image slot with {{ image|add_class( "my-section__img") }} will fix all those issues.

Experience Builder is an exciting but early product, with some youthful quirks. Let's not impact the drupal_cms_olivero integrity with them. Lets' build the best components first and wait the display builders to get better.

cta as a slot to avoid prop drilling

We have this in the definition:

    cta:
      type: string
      title: CTA text
    ctahref:
      type: string
      format: uri
      title: CTA link

And this in the template:

<div class="featured__cta-wrapper">
   <a class="button--primary" href="{{ ctahref }}">{{ cta }}</a>
</div>

button--primary is clearly its own component, distinct from feature, so we have those 2 issues:

  • the button component markup is duplicate din the feature component template, which will cause maitainance errors when the former will evolve
  • the feature component is too busy (2 extra props!) because of the "prop drilling" (defining props in a component to pass them in a child component)

We need a cta slot, where it will be possible to inject a button component (or anything else πŸ˜‰).

Missing attributes object>/h3>

We currently have this in the MR:

<div{{ create_attribute({'class': classes}) }}>

2 issues:

  • classes is missing from the component definition
  • it is nice to allow the user to add HTML classes, but not enough

So, why not leveraging the Attribute object called attributes, injected in all component template by SDC, instead?

attributes is great because it is a standardized powerful API endpoint for all components:

  • SEO modules can insert tagging attributes through it
  • Accessibility modules can alter/insert some attributes values (aria-*, role, alt...) through it
  • Translation modules can add the lang attribute when needed
  • Styles utilities modules (like UI Styles, Style Options...) can insert helpers classes through it
  • Hypermedia modules (HTMX, Hotwire/Turbo...) can insert triggers and behaviours attributes through it
  • Drupal Core's |add_class() and |set_attribute() Twig filters rely on it
  • And, of course, display builders (like Experience Builder) can insert annotation through it

Happy to discuss further if you find those feedbacks useful.

Production build 0.71.5 2024