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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/TransitionGroup.js
Expand Up @@ -191,24 +191,33 @@ class TransitionGroup extends React.Component {
let child = this.state.children[key];
if (child) {
let isCallbackRef = typeof child.ref !== 'string';
let factoryChild = this.props.childFactory(child);
let ref = (r) => {
this.childRefs[key] = r;
};

warning(isCallbackRef,
'string refs are not supported on children of TransitionGroup and will be ignored. ' +
'Please use a callback ref instead: https://facebook.github.io/react/docs/refs-and-the-dom.html#the-ref-callback-attribute');

// Always chaining the refs leads to problems when the childFactory
// wraps the child. The child ref callback gets called twice with the
// wrapper and the child. So we only need to chain the ref if the
// factoryChild is not different from child.
if (factoryChild === child && isCallbackRef) {
ref = chain(child.ref, ref);
}

// You may need to apply reactive updates to a child as it is leaving.
// The normal React way to do it won't work since the child will have
// already been removed. In case you need this behavior you can provide
// a childFactory function to wrap every child, even the ones that are
// leaving.
childrenToRender.push(React.cloneElement(
this.props.childFactory(child),
factoryChild,
{
key,
ref: chain(
isCallbackRef ? child.ref : null,
(r) => {
this.childRefs[key] = r;
}),
ref,
},
));
}
Expand Down
76 changes: 76 additions & 0 deletions test/TransitionGroup-test.js
@@ -1,6 +1,7 @@
import tsp from 'teaspoon';

let React;
let PropTypes;
let ReactDOM;
let TransitionGroup;

Expand All @@ -11,6 +12,7 @@ describe('TransitionGroup', () => {

beforeEach(() => {
React = require('react');
PropTypes = require('prop-types');
ReactDOM = require('react-dom');
TransitionGroup = require('../src/TransitionGroup');

Expand Down Expand Up @@ -72,6 +74,80 @@ describe('TransitionGroup', () => {
expect(ref).toHaveBeenCalled();
});

it('properly calls ref if childFactory doesn\'t create a wrapper', () => {
let transition;

const transitionRef = (r) => { transition = r; };
const childRef = jest.fn();

class Child extends React.Component {
render() {
return (<span />);
}
}

const rendered = tsp(
<TransitionGroup ref={transitionRef}>
<Child key="child" ref={childRef} />
</TransitionGroup>,
)
.render();

expect(transition.childRefs['.$child']).toEqual(jasmine.any(Child));

rendered.unmount();

for (let i in childRef.mock.calls) {
let call = childRef.mock.calls[i];
let valid = (call[0] === null || call[0] instanceof Child) && call.length === 1;

expect(valid).toBeTruthy();
}
});

it('properly calls ref if childFactory does create a wrapper', () => {
let transition;

const transitionRef = (r) => { transition = r; };
const childRef = jest.fn();

class Wrapper extends React.Component {
static propTypes = {
children: PropTypes.element,
}

render() {
return this.props.children;
}
}

class Child extends React.Component {
render() {
return (<span />);
}
}

const childFactory = x => React.createElement(Wrapper, null, x);

const rendered = tsp(
<TransitionGroup ref={transitionRef} childFactory={childFactory}>
<Child key="child" ref={childRef} />
</TransitionGroup>,
)
.render();

expect(transition.childRefs['.$child']).toEqual(jasmine.any(Wrapper));

rendered.unmount();

for (let i in childRef.mock.calls) {
let call = childRef.mock.calls[i];
let valid = (call[0] === null || call[0] instanceof Child) && call.length === 1;

expect(valid).toBeTruthy();
}
});

it('should handle willEnter correctly', () => {
let log = [];

Expand Down