-
Notifications
You must be signed in to change notification settings - Fork 55
Conversation
Codecov Report
@@ Coverage Diff @@
## master #356 +/- ##
=========================================
- Coverage 89.75% 89.4% -0.35%
=========================================
Files 64 64
Lines 1259 1265 +6
Branches 182 163 -19
=========================================
+ Hits 1130 1131 +1
- Misses 126 131 +5
Partials 3 3
Continue to review full report at Codecov.
|
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.
looks good
@@ -34,6 +34,6 @@ export default class Ref extends Component<IRefProps> { | |||
} | |||
|
|||
render() { | |||
return Children.only(this.props.children) | |||
return this.props.children && Children.only(this.props.children) |
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.
why is this necessary?
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.
otherwise, in case if children
is not provided for the component, this will throw the error and fails rendering process for the whole tree - while, in contrast, if we will handle it this way, then for innerRef
callback null
will be validly provided as captured DOM element (as there are no elements rendered)
TODO
API changes
Target
Accepts DOM element that will be used as popup's target (instead of
trigger
element that is used by default). This prop will also allow to support 'bridge' scenarios, where it is necessary for client to display a popup for some DOM element that is not managed by React.This is an optional prop - if omitted (by default)
Popup
will still usetrigger
element as a target.