SDC components: Fix JSON schema usage

Hey Pierre, thanks for a thorough check on the components and sorry for a late reply, I've been quite busy but I'll start cleaning this up soon and keep you posted on that

Hi Pierre, so far I fixed/improved the following:

• Heading component
• Nav-item component
• Accordion component
• Table component
• Array of HTML classes

Regarding title_prefix & title_suffix, these variables are not slots in the sense used by Drupal's SDC schema, but they serve a similar purpose in terms of allowing customization of the page title, for Radix design philosophy, I treat twig blocks as slots and not any other variables

I'd appreciate it if you check these changes out while I work on Toasts, thanks

I will have a look

Heading component

https://git.drupalcode.org/project/radix/-/tree/6.0.x/components/heading

Unused attributes variable

heading_attributes is used instead of the default attributes variable.

It is defined in the YML (like badge component do for badge_attributes, or page-footer do for footer_attributes) but it would be better to use the default attributes variable.

And it will prevent this kind of verbose precautions:
{% set heading_attributes = heading_attributes ?: create_attribute() %}

Duality slots / props

So, now we have heading_content slot as a Twig block AND title_link & content props inside the exact same block:

    {% block heading_content %}
      {% if title_link %}
        <a href="{{ title_link }}">{{ content }}</a>
      {% else %}
        {{ content }}
      {% endif %}
    {% endblock %}

What will happen if we call the component with only the slot ?

"#type" => "component",
"#component"  =>"radix:heading",
"#slots" => [
  "content" => ["#plain_text" => "Lorem ipusm"],
]

Only the props ?

"#type" => "component",
"#component"  =>"radix:heading",
"#props" => [
  "title_link" => "Foo",
  "content" => "Bar",
]

Both slot and props ?

Let's be careful with the Twig block system. It is useful for template inheritance, but in component driven development we don't do template inheritance.

nav-item component

https://git.drupalcode.org/project/radix/-/tree/6.0.x/components/nav-item

Unused attributes variable

nav_item_attributes is used in template but not defined anywhere.
But as seen for heading component, instead of adding the prop in the definition, it is better to use the default attributes variable.

link slot

The template looks now OK:
{{ link|add_class('nav-link', is_active ? 'active' : '') }}

But not the definition yet. It went from undefined object to undefined array.

Accordion component

Not reviewed yet. I will update this comment later.

Table component

table prop is now split in 5 slots (table_caption, table_colgroup, table_header, table_body & table_footer) and 5 props:

• caption: String
• colgroups: Undefined array
• header: Undefined array
• rows: Undefined array
• footer: String

Like for heading component, what to do with duality slots/props ?

Array of HTML classes

Perfect :)

title_prefix & title_suffix

Indeed, they are not always slots. Fro example, {{ title_suffix.contextual_links }} in media component.
But they need to be defined anywhere. What is expected inside ?

Review done. I would be happy to continue this conversation if you still need me.

Pierre great work as always, by any chance would you like to be a co-maintainer? makes rather more sense that way and you can just start contributing if you have free time in between all your other projects

Hi Sohail,

I would be honoured to become maintainer of Radix, but I am already maintaining a Bootstrap5 theme: https://www.drupal.org/project/ui_suite_bootstrap →

Sharing feedbacks about SDC integration is easy for me, because it is very operational and, most of the time, simple binary decision (it is OK ? is it not?). However, becoming more involved and part of the team, I may feel uncomfortable dealing with such similar projects.

yeah I see your point, and I need to give your theme a go, thanks a lot anyways but feel free whenever you wanted to jump in, let me know.
Otherwise I appreciate your contribution in general

and I need to give your theme a go

Great :)

If you are interested about our SDC implementation (which will become default this summer I hope, replacing current UI Patterns 1.x → components), we have a specific branch with them: https://git.drupalcode.org/issue/ui_suite_bootstrap-3412076/-/tree/34120...

Except table component which is under rework, there are pretty good IMHO.

links, variants and stories properties of components definitions are UI Patterns specific.

Thanks a lot for the help Pierre and a lot of good tips and hints, most are fixed now, some are opinionated but all is good so far. Feel free to re-open with your great suggestions, marking this as fixed for now

Automatically closed - issue fixed for 2 weeks with no activity.