SDC components: Fix JSON schema usage

Really appreciate this @pdureau! We will try to get these in shortly.

I will have a look :)

Incorrect JSON schema type

accordion-item component's open prop: "mixed" is not a valid JSON schema type.

{"type":"mixed"}

Proposal:

{"type":"boolean"}

it was done for accordion component, but still need to be done for accordion-item

Array of undefined values

{"type":"array"}

• section's regions prop. Proposal: is it a slot? ( Refactor Implementation of Section and Layouts that implement sections 📌 Refactor Implementation of Section and Layouts that implement sections. Active )
• grid's items prop. Proposal: is it a slot?
• glide's items prop. Proposal: this is clearly a slot
• glide's classes prop. Proposal: add items/type = string to the schema
• tabs's items pop. Proposal: it may be better to have a slot, but let's add the expected data structure (id, body, heading...) if we prefer to keep it as a prop
• accordion's items pop. Proposal: it may be better to have a slot, but let's add the expected data structure (body, heading...) if we prefer to keep it as a prop
• text-and-media's ctas prop. Proposal: the same "links" data structure as the one proposed for billboard's ctas prop.
• carousel's items prop. Proposal: it may be better to have a slot, because the template is doing some risky work with an expected render array item.heading[0]['#text']
• card's ctas prop

Empty undefined object

{"type":"object"}

Found in:

• section's container_attributes prop. Proposal: reorganise the template to not need such a mapping of region => attributes ( Refactor Implementation of Section and Layouts that implement sections 📌 Refactor Implementation of Section and Layouts that implement sections. Active )
• layout--onecol's & layout--twocol's settings prop. Proposal: each setting must be a prop: mobile_order, bg_color, content_width ( Refactor Implementation of Section and Layouts that implement sections 📌 Refactor Implementation of Section and Layouts that implement sections. Active )
• layout--onecol's & layout--twocol's container_attributes prop. Proposal: reorganise the template to not need such a mapping of region => attributes ( Refactor Implementation of Section and Layouts that implement sections 📌 Refactor Implementation of Section and Layouts that implement sections. Active )
• card's media prop

List of undefined objects

billboard's component ctas prop:

{
  "items": {
    "type": "object"
  },
  "type": "array"
}

Proposal: in my humble opinion, it would be better to replace this prop a slot than hardcoding the call to kinetic:link--button in the template. There are no slots in this component, which is lacking flexibility.

If we prefer to keep it as a prop, you need to have a proper data structure:

{
  "type": "array",
  "items": {
    "type": "object",
    "properties": {
      "url": {
        "type": "string"
      },
      "title": {
        "type": "string"
      }
    }
  }
}

With "title" instead of "text" because it is the common link data structure, found in LinkItem and menu.html.twig for example. Let's be careful when we map this property to link--button in the template.

I made some more changes. I think we should have a different issue for considering things to be slots rather than props for things mentioned like accodion-item, ctas and the like. I wouldn't mind being pointed in the direction of some examples and articles to see what best practice is for those items. The current mindset is that we want to be super defined about what is accepted in certain places like ctas where we don't necessarily want just anything passed there. That being said @pdureau we are open to learning and changing our mindset.

The current mindset is that we want to be super defined about what is accepted in certain places like ctas where we don't necessarily want just anything passed there.

If you want to be superdefined about what is accepted in certain places, props are better than slots.

However, I would suggest to learn to "let go" while designing a design system. Components are meant to be shared and used with different business constraint and context. To achieve this, we need to both:

• Make the components flexible
• Make the components forward compatible

And slots are perfect for that. One day, instead of the super defined CTAs, your card may have something else. Those CTAs is not what make this component a card.

Also, in a Drupal context, slots have the advantage of being able to accept any renderable (rendered entities, views displays, forms...), so they are easy to use while site building from admin pages.

Anyway, if you keep them as props, {"type":"array"} and/or {"type":"array"} are not enough.

I wouldn't mind being pointed in the direction of some examples and articles to see what best practice is for those items.

Because this subject is still new in Drupal community, those articles are easier to find in other communities (like Vue.js, for example).

Also, witnessing the struggle of ReactJS with "prop drilling" teach us about the advantages of a proper slots system (which React lacks):

• https://www.freecodecamp.org/news/avoid-prop-drilling-in-react/
• https://dev.to/codeofrelevancy/what-is-prop-drilling-in-react-3kol

@pdureau taking into consideration your comments. Thank you for that. I feel that it could end up being a new ticket that revolves around making our base components more flexible and use slots but this would be more of a change than just schema. So TBD.

Going to mark this fixed in terms of the schema is fixed but not using slots yet to be more flexible. That is tracked in the related issue.

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

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