feat: add icon to the left and right button in alert component

[ClaraLpresta]

❓ Types of changes

• 📖 Documentation (updates to the documentation or readme)
• 🐞 Bug fix (a non-breaking change that fixes an issue)
• 👌 Enhancement (improving an existing functionality)
• 📦 New feature (a non-breaking change that adds functionality)
• ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

I add two props to Alert component for put an icon on the left or right to the button

It's a request for Customer Platform

📝 Checklist

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes
  I didn't make a test because there are tests for the button component
• The component exists on old Prestashop UIKit and my pull request on migrating documentation is accepted.

[MikeLebeau]

Hm... Dans ce cas la est-ce qu'il ne faudrait pas directement faire un slot "button" ? 🤔
Ou autre idée (je donne ca comme ca, je suis pas du tout sur du truc) mais est-ce qu'on se ferait pas plutot une props "buttonConf" ou autre et genre on lui passe un objet avec toutes les props du composant puik-button ? 🤔

[kktos]

Hm... Dans ce cas la est-ce qu'il ne faudrait pas directement faire un slot "button" ?

J'ai pensé à la même chose et je me suis dit qu'un slot, c'était en fait la porte ouverte à tout et il serait facile de "casser" le design en mettant ce que l'on veut....

Ou autre idée (je donne ca comme ca, je suis pas du tout sur du truc) mais est-ce qu'on se ferait pas plutôt une props "buttonConf" ou autre et genre on lui passe un objet avec toutes les props du composant puik-button ? 🤔

Là encore, ne risque t-on pas d'ouvrir le champ des possibles au delà de ce que les designers ont conçus ? Soit maintenant, soit plus tard si le button évolue.

J'ai vu la PR hier soir dans le train et j'ai failli répondre avant de réaliser tout ceci. ;)

[ClaraLpresta]

Hello vous deux ! Pour pouvoir intégrer l'icon j'ai justement utilisé un slot mais j'ai pensé que c'était plus propre de passer par les props car le bouton est déjà intégré au composant Alert.

S'il vaut mieux limiter la configuration du bouton pour par qu'on s'écarte du DS, est-ce qu'on pourrait pas trouver un juste milieux et créer une props "buttonConf" avec les 3 propriétés du bouton (label, iconRight et iconLeft) ?

[aAmorim27]

Hm... Dans ce cas la est-ce qu'il ne faudrait pas directement faire un slot "button" ? 🤔
Ou autre idée (je donne ca comme ca, je suis pas du tout sur du truc) mais est-ce qu'on se ferait pas plutot une props "buttonConf" ou autre et genre on lui passe un objet avec toutes les props du composant puik-button ? 🤔

Je pense que les deux solutions se valent mais elles présentent toutes quelques points négatif:
Les slots seront plus flexible vu qu'on peux passer ce qu'on veut dedans mais ca va présenter des limitations dans le cas ou du agir directement avec les props du button du genre le size, le variant mais faut il encore que coté design ça soit possible d'avoir un variant ou un size different par exemple

Pour ce qui est de l'objet de config c'est une solution qui est plus stricte et cadrée le plus gros soucis ici c'est que si y a un nouveau besoin qui apparaît il faudra mettre a jour le composant pour le prendre en compte donc un peu plus de travail niveau maintenance. Mais si coté design les cas d'affichage au niveau du bouton sont bien identifiés et qu'on change pas d'avis toutes les semaines ça pourrait fonctionner
Par contre attention vu qu'on a déja un props dédié au label il faudrait le déprécier pour le passer dans l'objet de config histoire d'uniformiser tout ca

J'ai pensé à la même chose et je me suis dit qu'un slot, c'était en fait la porte ouverte à tout et il serait facile de "casser" le design en mettant ce que l'on veut....

Certes c'est une porte ouverte à faire des choses non prévues par le composant que ce soit coté dev ou design mais si ceux qui intègrent le composant font n'importe quoi avec c'est leur problème

[mattgoud]

je vais déjà demander au design jusqu'à quel point on peut personnaliser l'alert

[mattgoud]

je vais déjà demander au design jusqu'à quel point on peut personnaliser l'alert

d'après les maquettes figma, on devrait même avoir la possibilité d'avoir deux boutons, ce qui complique encore les choses. Je serais d'avis qu'on supprime le bouton déjà en place et qu'on le remplace par un slot que l'on pourrait nommer "Actions". Coté design il faudra préciser dans la doc ce que l'on attend (ex: possibilité d'avoir max deux boutons, soit de type text soit tertiary en respectant le variant de l'alert dans ce cas). Sinon je crains qu'il faille multiplier les props. Vous en pensez quoi @ClaraLpresta @aAmorim27 @MikeLebeau @FrancoisQtr ?

[cnavarro-prestashop]

@mattgoud / @ClaraLpresta / @FrancoisQtr : est-ce que vous avez statué sur le composant alert ? Peut-on rajouter un ou plusieurs boutons ? Passe-ton par des slots permettant que des boutons (ça me semble plus propre effectivement) ?

[mattgoud]

@mattgoud / @ClaraLpresta / @FrancoisQtr : est-ce que vous avez statué sur le composant alert ? Peut-on rajouter un ou plusieurs boutons ? Passe-ton par des slots permettant que des boutons (ça me semble plus propre effectivement) ?

@cnavarro-prestashop j'aurais tendance à préférer un slot pour ma part ...

[ClaraLpresta]

Les slots paraient être une très bonne idée mais cela reste une dégression pour les personnes qui utilisent déjà le composant. Il faudrait donc les prévenir que le bouton ne sera plus disponible via les props

[krimz]

Les slots paraient être une très bonne idée mais cela reste une dégression pour les personnes qui utilisent déjà le composant. Il faudrait donc les prévenir que le bouton ne sera plus disponible via les props

Hello,
je plussois, l'utilisation des slots est une bonne chose pour imbriquer des composants les uns avec les autres comme c'est le cas ici -> Ajouter le composant Button qui a déjà des icons left et right :)
Il faudra par contre bien penser à être précis dans la documentation sur l'utilisation de ces slots avec les dos et donts.

[matks]

Plop! I take the opportunity of this notification to say 2 things:

1. This repository is public and we want to expand the use of PUIK outside of just the company, to the whole PrestaShop ecosystem. In order to do that, we need to speak in English 😉 . Let's start slowly, OK?
2. A design system is NOT a toolbox or a framework. A design system is created to provide a clear look-and-feel to all applications of a small brand. Hence the customization capabilities should exist, but be restricted.

So when I see:

(ex: possibilité d'avoir max deux boutons, soit de type text soit tertiary en respectant le variant de l'alert dans ce cas)

I say 😄 we must choose ONE possibility, implement it, and say to everyone: use this possibility, nothing else. That's the idea of a UI Kit. Enforce one look-and-feel, the same, in every app. If every app customizes PUIK then we don't have a UI Kit, we have a framework, like tailwind 😉

[krimz]

Les slots paraient être une très bonne idée mais cela reste une dégression pour les personnes qui utilisent déjà le composant. Il faudrait donc les prévenir que le bouton ne sera plus disponible via les props

Hello, je plussois, l'utilisation des slots est une bonne chose pour imbriquer des composants les uns avec les autres comme c'est le cas ici -> Ajouter le composant Button qui a déjà des icons left et right :) Il faudra par contre bien penser à être précis dans la documentation sur l'utilisation de ces slots avec les dos et donts.

Hello folks,
I reply to my old me: in the end, I think using slots, in this case, could be a mess... because it would allow people to add whatever they wanted to the alert banner component. That's not what we want! The purpose of this specific location is only to get a CTA or close action. I recommend using the CTA component directly. The management of the left or right icons in the CTA will be managed in the CTA component.

Ok for you guys?

[mattgoud]

Hello @ClaraLpresta , I suggest you close this PR replaced by #381. Tell me if it suits your needs (i put you reviewer)

[mattgoud]

@ClaraLpresta PR closed because the feature (possibility of having icons) was implemented by the following PR:
#381