-
Notifications
You must be signed in to change notification settings - Fork 0
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(elements): add spotlighted number to icon button #1292
feat(elements): add spotlighted number to icon button #1292
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1292 +/- ##
=======================================
Coverage 57.49% 57.49%
=======================================
Files 55 55
Lines 807 807
Branches 274 267 -7
=======================================
Hits 464 464
- Misses 327 343 +16
+ Partials 16 0 -16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM, mais je l'aurais appelé badgeNumber
plutôt que spotlightedNumber
^^.
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.
Top, on va pouvoir l'utiliser sur Env aussi :-)
Pas besoin, de props pour changer la couleur de fond et celle du texte?
J'ai ajouté ces deux props |
@@ -76,23 +79,35 @@ export function IconButton({ | |||
case Accent.SECONDARY: | |||
return ( | |||
<Wrapper> | |||
{!!spotlightedNumber && <SpotlightedNumber size={size}>{spotlightedNumber}</SpotlightedNumber>} | |||
{!!badgeNumber && ( | |||
<BadgeNumber backgroundColor={badgeBackgroundColor} color={badgeColor} size={size}> |
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 vais faire ma relou mais il me semble qu'on s'était dit que les props css on les préfixaient avec $
, genre $backgroundColor
(désolée j'ai pas fait attention avant)
🎉 This PR is included in version 18.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Related Pull Requests & Issues
Preview URL
https://637e01cf5934a2ae881ccc9d-pfbyygqvhf.chromatic.com/