Added Badge component.#2045
Conversation
|
Sounds like a good component! |
src/badge.jsx
Outdated
There was a problem hiding this comment.
You should be able to remove prefix. Our auto prefixed tool should do it for us.
There was a problem hiding this comment.
I wasn't sure if the prefixer covers similar properties or just prefixed properties with the same name, so I've left in the prefixed similar properties for now.
|
@oliviertassinari thanks for the good feedback. |
src/badge.jsx
Outdated
There was a problem hiding this comment.
Can't we create a local theme variable instead of the 6 call of getTheme()?
There was a problem hiding this comment.
getTheme() will only be invoked once or twice, so it might be more efficient not to store an unnecessary variable, but if you want to do it that way for legibility it's fine with me.
There was a problem hiding this comment.
Actually, you're right. The getTheme method is not necessary. I was basing the code off of another component in order to help me get started, but that method not necessary here.
|
@rhythnic I have done my final feedbacks. Could you have a look at them? Once you are happy with it, can you squash down the number of commit (will keep the history clean) and I will probably merge 👍 . |
src/badge.jsx
Outdated
|
@rhythnic Looks good to me now 👍. |
The badge is for displaying a small number or piece of text to the top-right of it's child. It's mostly useful for displaying a counter.
See Material Design Light's Badge.