Skip to content
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

Proposed fix for #29 #39

Merged
merged 3 commits into from
May 2, 2017
Merged

Proposed fix for #29 #39

merged 3 commits into from
May 2, 2017

Conversation

aligon
Copy link
Contributor

@aligon aligon commented Apr 19, 2017

This was hijacking the child ref onto the CSSTransitionGroupChild, this fix for #29 leaves the ref on child and lets the TransitionGroup add a ref to the wrapped child produced from childFactory(). (assumption: childFactory() always wraps the child) An improvement would to detect if the child was wrapped or not and chain appropriately.

This was hijacking the child ref onto the `CSSTransitionGroupChild`, this fix for reactjs#29 leaves the ref on child and lets the `TransitionGroup` add a ref to the wrapped child produced from `childFactory()`.  (assumption: `childFactory()` always wraps the child) An improvement would to detect if the child was wrapped or not and chain appropriately.
Copy link
Collaborator

@jquense jquense left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi thanks for the PR. Can you write a test asserting the refs are both correct in this case?

@aligon
Copy link
Contributor Author

aligon commented Apr 26, 2017

@jquense I added some tests, and went ahead and made the change to intelligently chain the refs or not if the childFactory creates a wrapper.

@jquense jquense merged commit e91cb25 into reactjs:master May 2, 2017
@jsg2021
Copy link

jsg2021 commented May 2, 2017

🎉

marnusw added a commit to marnusw/react-css-transition-replace that referenced this pull request Oct 11, 2019
It was called first with the child and a second time with the
CSSTransitionGroupChild. The same issue was fixed in
reactjs/react-transition-group#39.
Since this library always creates a wrapper the solution is
simpler: just don't chain the ref at all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants