Doesn't render the badge if there's no children #78
Conversation
Doesn't render the badge if there's no children
|
Sorry, it's not what I'm talking about. It's about The point is that we're wrapping some element in |
|
Here's a selector |
|
Haha ok ;) Yep, actually I figured out it make no sense if the children was null as well anyway ;) |
|
Let's assume that we have some icon for notifications and we want to show a badge only if there's a notification (mostly like on github, but with numbers), with the current implementation we will wrap component in a badge if there's notifications or don't wrap if there's none. Not so good. Instead I guess it's assumed by upstream that if we don't want to show badge we just don't set I can make a PR to show the difference. |
|
Ok so you suggest that if |
|
Not exactly. I'll make a PR to explain better :-) |
|
It throws now when you return |
|
What if instead of checking for a single element and so on and so forth we just wrap element in span in any case? Try to wrap |
|
No need to add an extra container for that. |
| text: PropTypes.string.isRequired | ||
| } | ||
|
|
||
| render() { | ||
| var { children } = this.props; | ||
|
|
||
| // No badge if no children | ||
| if(!React.Children.count(children)) return null; |
There was a problem hiding this comment.
I'm talking about this line.
It seems like stateless components do not support returning null from render.
Current test suite skips it somehow.
There was a problem hiding this comment.
Hmm weird. Do you know if there's already a bug about that on react?
Seems weird the behavior from a stateless component is different in React and in a Shallow renderer.
There was a problem hiding this comment.
I guess may be we just don't get stateless components right?)
I didn't search for a bug.
fixes #76