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
Allow TransitionChild to passprops. Useful for component wrapper that… #4
Conversation
… clone children and modify props, new props should be passed down to child.
@@ -157,7 +157,9 @@ class CSSTransitionGroupChild extends React.Component { | |||
} | |||
|
|||
render() { | |||
return React.Children.only(this.props.children); | |||
const props = { ...this.props }; | |||
Object.keys(propTypes).forEach(key => delete props[key]); |
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.
this pattern feels a bit odd to see here – i'm afraid of people using this with the babel optimization to strip prop types and hitting breakage
hi there, thanks for the PR. I'm not sure I understand why this is needed. The user can already add any props to the children when they create them the first time. For a wrapper that controls/adjusts children why not wrap the TransitionGroup itself instead of the Component? let me know if I'm misunderstanding |
Here's the example that was unexpected:
|
Hi, is this repo still maintained? |
is it maintained? yes, yes it is. |
Is there anything blocking to have the PR merged? Thanks! |
I'm not sure this is the right approach... expectung the animation container to behave correctly when passing in arbitrary components (like List) seems fraught. who is to say that component will play nice. what if it doesn't render the children at all? in general it seems better for List to render a Transition group (ideally). @taion thoughts here? |
(sorry for the delay btw) |
I think the original intention of the
The same functionality would need to be built into other wrappers, like a I feel that its better practice for components that are most reusable to play nice rather than have dependencies above playing nice downwards. TransitionGroup assumed to be the wrapper hence it enforce either a div or a custom component, so it make sense that the tradeoffs of the assumption is taken care of. Alternatively, i propose |
@jquense Taking React-Bootstrap as an example, how would we make this work with something like |
yeah I guess it doesn't work in the case which isn't ideal. My concern here is that it may be misleading that it would work with any Component, which isn't true if the rendered root decides to do anything odd with the children. I guess its fine to let the buyer beware tho on those things. |
I think the goal is to reduce the cases where adding transitions via TransitionGroup fail to work well. I don't think any component is meant to work with everything, imagine |
fair enough |
… clone children and modify props, new props should be passed down to child.
Reference: facebook/react#8522