-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(slash): add ChildrenQuestion component #881
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je veux bien un screen du composant si possible pour voir ce que ça donne stp 😄
@GuillaumeKESTEMAN tu peux le voir dans le storybook ici https://67120d087b5afa65f0447b87-cdchcijzzp.chromatic.com/?path=/story/components-form-childrenquestion--playground |
26ac181
to
d3659ec
Compare
d3659ec
to
de6385d
Compare
|
Je suis curieux de voir quels sont les cas d'usage, ca ressemble à des sous questions (conditionnés ?) |
return ( | ||
<section className={componentClassName}> | ||
<div className={`${componentClassName}-arrow`} /> | ||
<section>{content}</section> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pourquoi ne pas utiliser le children
au lieu de créer une props
content ?
return ( | ||
<section className={componentClassName}> | ||
<div className={`${componentClassName}-arrow`} /> | ||
<section>{content}</section> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
je ne suis pas certain que 2 section imbriqué comme cela ca valide d'un point de vue a11y.
Je pense que ca devrait plutot s'appeler |
Peut etre rajouter de la doc sur le usecase avec un screenshot ou une plus grosse story pour mettre du contexte ? |
@@ -0,0 +1,12 @@ | |||
import "@axa-fr/design-system-slash-css/dist/Form/ChildrenQuestion/ChildrenQuestion.scss"; | |||
|
|||
export const ChildrenQuestion = ({ content }: { content: React.ReactNode }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: rajouter un className
optionnel pour pouvoir surcharger
Fix #879
Before merging this pull request, we will make a review with @adrien-dos to be sure the component implements the correct UX.