Feature/collapse card prop inverted #839
Conversation
|
Il y avait aussi un ajustement à faire sur les stories mais je ne suis plus sur. Est ce qu'il ne manque pas la story du CardBase ? |
J'ai fusionné le CollapseCard avec le CollapseCardBase donc on n'a plus qu'une seule story |
vharny
force-pushed
the
feature/CollapseCardPropInverted
branch
2 times, most recently
from
c499da6 to
d12c6b8
Compare
|
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Il faudrait prévoir le guide de migration du breaking change.
(https://github.com/AxaGuilDEv/react-toolkit/blob/master/MIGRATION.md)
| ? collapses[index] | ||
| : child.props.collapse, | ||
| isOpen: | ||
| collapses[index] !== undefined ? collapses[index] : child.props.isOpen, |
There was a problem hiding this comment.
Peut-on trouver un meilleur nom pour collapses ? Il reste ambigu.
There was a problem hiding this comment.
Je trouve "openComponents" assez ambigu, car c'est bien un composant Collapse qui se ferme et s'ouvre.
De plus, le collapse en question n'est pas forcément true il peut être false.
|
Cette revue est offerte par l'équipe transverse |
| ? collapses[index] | ||
| : child.props.collapse, | ||
| isOpen: | ||
| collapses[index] !== undefined ? collapses[index] : child.props.isOpen, |
J'ai ajouté le guide dans la PR. |
vharny
dismissed stale reviews from ArthurVT59, SamuelHsn, and arnaudforaison
via
b28d0d4
vharny
force-pushed
the
feature/CollapseCardPropInverted
branch
from
d12c6b8 to
b28d0d4
Compare
arnaudforaison
dismissed stale reviews from jforatier, johnmeunier, sisko59, and themself
via
5ab07ad
Related issue
Reference to the issue
Fixes #787
Description of the issue
The collapse property can be confusing so it will be replaced by isOpen.
I did a refacto that merges the CollapseCard and CollapseCardBase components because the only difference was the onToggle prop who could be add on CollapseCardBase.