Render fixes #19
Render fixes #19
Conversation
|
Hey @souporserious, what do you think of this? It's currently blocking |
| @@ -30,7 +30,7 @@ const attachmentPositions = [ | |||
| class TetherComponent extends Component { | |||
| static propTypes = { | |||
| renderElementTag: PropTypes.string, | |||
| renderElementTo: PropTypes.string, | |||
| renderElementTo: PropTypes.any, | |||
There was a problem hiding this comment.
Should we use React.PropTypes.oneOfType here?
There was a problem hiding this comment.
Yes @rafeememon please implement oneOfType here.
There was a problem hiding this comment.
Should the oneOfType be [PropTypes.string, PropTypes.any]? This is equivalent to PropTypes.any =P
There was a problem hiding this comment.
I think it should either be a string or a DOM node. So i'm guessing we would need a custom proptype checking if it's a DOM element? Something like this http://stackoverflow.com/a/385427/1461204
|
@rafeememon so sorry I just saw this. It's been a crazy last month. This all looks great! Just one thing, can we use a getter for |
|
For those of us who came from this issue: Hacker0x01/react-datepicker#444 (comment) - |
|
Added an |
|
Thanks for the work on this! Merged :) |
This PR fixes a couple issues I ran into:
renderElementTonode after Problem withrenderElementTo#16 was closed. I think the root of the problem is that since the render node is computed as soon as theTetherComponentis mounted, it doesn't "give me a chance" to find the node I want to render into. If we instead lazily evaluate the node retrieval (i.e. only inupdate) we can specify nodes that might not exist yet.renderElementToto a string (i.e. a query selector) but sometimes it may not be possible to give a selector; for instance if the target is a React component, all that is available is the DOM element (adding IDs/classes for this seems very anti-React). This adds the ability to specify either a query selector or a DOM node.