Merge pull request facebook#4561 from djrodgerspryor/css_transition_group_robust_cleanup

Robust animation-end handling in ReactCSSTransitionGroup

Author: sophiebits

docs/docs/10.1-animation.md

@@ -44,7 +44,7 @@ var TodoList = React.createClass({
     return (
       <div>
         <button onClick={this.handleAdd}>Add Item</button>
-        <ReactCSSTransitionGroup transitionName="example">
+        <ReactCSSTransitionGroup transitionName="example" transitionEnterTimeout={500} transitionLeaveTimeout={300} >
           {items}
         </ReactCSSTransitionGroup>
       </div>
@@ -67,31 +67,29 @@ You can use these classes to trigger a CSS animation or transition. For example,
 
 .example-enter.example-enter-active {
   opacity: 1;
-  transition: opacity .5s ease-in;
+  transition: opacity 500ms ease-in;
 }
-```
-
-You'll notice that when you try to remove an item `ReactCSSTransitionGroup` keeps it in the DOM. If you're using an unminified build of React with add-ons you'll see a warning that React was expecting an animation or transition to occur. That's because `ReactCSSTransitionGroup` keeps your DOM elements on the page until the animation completes. Try adding this CSS:
 
-```css
 .example-leave {
   opacity: 1;
 }
 
 .example-leave.example-leave-active {
   opacity: 0.01;
-  transition: opacity .5s ease-in;
+  transition: opacity 300ms ease-in;
 }
 ```
 
+You'll notice that animation durations need to be specified in both the CSS and the render method; this tells React when to remove the animation classes from the element and -- if it's leaving -- when to remove the element from the DOM.
+
 ### Animate Initial Mounting
 
 `ReactCSSTransitionGroup` provides the optional prop `transitionAppear`, to add an extra transition phase at the initial mount of the component. There is generally no transition phase at the initial mount as the default value of `transitionAppear` is `false`. The following is an example which passes the prop `transitionAppear` with the value `true`.
 
 ```javascript{3-5}
   render: function() {
     return (
-      <ReactCSSTransitionGroup transitionName="example" transitionAppear={true}>
+      <ReactCSSTransitionGroup transitionName="example" transitionAppear={true} transitionAppearTimeout={500}>
         <h1>Fading at Initial Mount</h1>
       </ReactCSSTransitionGroup>
     );
@@ -184,7 +182,7 @@ var ImageCarousel = React.createClass({
   render: function() {
     return (
       <div>
-        <ReactCSSTransitionGroup transitionName="carousel">
+        <ReactCSSTransitionGroup transitionName="carousel" transitionEnterTimeout={300} transitionLeaveTimeout={300}>
           <img src={this.props.imageSrc} key={this.props.imageSrc} />
         </ReactCSSTransitionGroup>
       </div>

src/addons/transitions/ReactCSSTransitionGroup.js

@@ -23,6 +23,31 @@ var ReactCSSTransitionGroupChild = React.createFactory(
   require('ReactCSSTransitionGroupChild')
 );
 
+function createTransitionTimeoutPropValidator(transitionType) {
+  var timeoutPropName = 'transition' + transitionType + 'Timeout';
+  var enabledPropName = 'transition' + transitionType;
+
+  return function(props) {
+    // If the transition is enabled
+    if (props[enabledPropName]) {
+      // If no timeout duration is provided
+      if (!props[timeoutPropName]) {
+        return new Error(
+          timeoutPropName + ' wasn\'t supplied to ReactCSSTransitionGroup: ' +
+          'this can cause unreliable animations and won\'t be supported in ' +
+          'a future version of React. See ' +
+          'https://fb.me/react-animation-transition-group-timeout for more ' +
+          'information.'
+        );
+
+      // If the duration isn't a number
+      } else if (typeof props[timeoutPropName] !== 'number') {
+        return new Error(timeoutPropName + ' must be a number (in milliseconds)');
+      }
+    }
+  };
+}
+
 var ReactCSSTransitionGroup = React.createClass({
   displayName: 'ReactCSSTransitionGroup',
 
@@ -43,9 +68,13 @@ var ReactCSSTransitionGroup = React.createClass({
         appearActive: React.PropTypes.string,
       }),
     ]).isRequired,
+
     transitionAppear: React.PropTypes.bool,
     transitionEnter: React.PropTypes.bool,
     transitionLeave: React.PropTypes.bool,
+    transitionAppearTimeout: createTransitionTimeoutPropValidator('Appear'),
+    transitionEnterTimeout: createTransitionTimeoutPropValidator('Enter'),
+    transitionLeaveTimeout: createTransitionTimeoutPropValidator('Leave'),
   },
 
   getDefaultProps: function() {
@@ -66,6 +95,9 @@ var ReactCSSTransitionGroup = React.createClass({
         appear: this.props.transitionAppear,
         enter: this.props.transitionEnter,
         leave: this.props.transitionLeave,
+        appearTimeout: this.props.transitionAppearTimeout,
+        enterTimeout: this.props.transitionEnterTimeout,
+        leaveTimeout: this.props.transitionLeaveTimeout,
       },
       child
     );

src/addons/transitions/ReactCSSTransitionGroupChild.js

@@ -19,35 +19,31 @@ var CSSCore = require('CSSCore');
 var ReactTransitionEvents = require('ReactTransitionEvents');
 
 var onlyChild = require('onlyChild');
-var warning = require('warning');
 
 // We don't remove the element from the DOM until we receive an animationend or
 // transitionend event. If the user screws up and forgets to add an animation
 // their node will be stuck in the DOM forever, so we detect if an animation
 // does not start and if it doesn't, we just call the end listener immediately.
 var TICK = 17;
-var NO_EVENT_TIMEOUT = 5000;
-
-var noEventListener = null;
-
-
-if (__DEV__) {
-  noEventListener = function() {
-    warning(
-      false,
-      'transition(): tried to perform an animation without ' +
-      'an animationend or transitionend event after timeout (' +
-      '%sms). You should either disable this ' +
-      'transition in JS or add a CSS animation/transition.',
-      NO_EVENT_TIMEOUT
-    );
-  };
-}
 
 var ReactCSSTransitionGroupChild = React.createClass({
   displayName: 'ReactCSSTransitionGroupChild',
 
-  transition: function(animationType, finishCallback) {
+  propTypes: {
+    name: React.PropTypes.string.isRequired,
+
+    // Once we require timeouts to be specified, we can remove the
+    // boolean flags (appear etc.) and just accept a number
+    // or a bool for the timeout flags (appearTimeout etc.)
+    appear: React.PropTypes.bool,
+    enter: React.PropTypes.bool,
+    leave: React.PropTypes.bool,
+    appearTimeout: React.PropTypes.number,
+    enterTimeout: React.PropTypes.number,
+    leaveTimeout: React.PropTypes.number,
+  },
+
+  transition: function(animationType, finishCallback, userSpecifiedDelay) {
     var node = ReactDOM.findDOMNode(this);
 
     if (!node) {
@@ -59,16 +55,14 @@ var ReactCSSTransitionGroupChild = React.createClass({
 
     var className = this.props.name[animationType] || this.props.name + '-' + animationType;
     var activeClassName = this.props.name[animationType + 'Active'] || className + '-active';
-
-    var noEventTimeout = null;
+    var timeout = null;
 
     var endListener = function(e) {
       if (e && e.target !== node) {
         return;
       }
-      if (__DEV__) {
-        clearTimeout(noEventTimeout);
-      }
+
+      clearTimeout(timeout);
 
       CSSCore.removeClass(node, className);
       CSSCore.removeClass(node, activeClassName);
@@ -82,15 +76,18 @@ var ReactCSSTransitionGroupChild = React.createClass({
       }
     };
 
-    ReactTransitionEvents.addEndEventListener(node, endListener);
-
     CSSCore.addClass(node, className);
 
     // Need to do this to actually trigger a transition.
     this.queueClass(activeClassName);
 
-    if (__DEV__) {
-      noEventTimeout = setTimeout(noEventListener, NO_EVENT_TIMEOUT);
+    // If the user specified a timeout delay.
+    if (userSpecifiedDelay) {
+      // Clean-up the animation after the specified delay
+      timeout = setTimeout(endListener, userSpecifiedDelay);
+    } else {
+      // DEPRECATED: this listener will be removed in a future version of react
+      ReactTransitionEvents.addEndEventListener(node, endListener);
     }
   },
 
@@ -124,23 +121,23 @@ var ReactCSSTransitionGroupChild = React.createClass({
 
   componentWillAppear: function(done) {
     if (this.props.appear) {
-      this.transition('appear', done);
+      this.transition('appear', done, this.props.appearTimeout);
     } else {
       done();
     }
   },
 
   componentWillEnter: function(done) {
     if (this.props.enter) {
-      this.transition('enter', done);
+      this.transition('enter', done, this.props.enterTimeout);
     } else {
       done();
     }
   },
 
   componentWillLeave: function(done) {
     if (this.props.leave) {
-      this.transition('leave', done);
+      this.transition('leave', done, this.props.leaveTimeout);
     } else {
       done();
     }

src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js

@@ -29,9 +29,29 @@ describe('ReactCSSTransitionGroup', function() {
     spyOn(console, 'error');
   });
 
-  it('should warn after time with no transitionend', function() {
+  it('should warn if timeouts aren\'t specified', function() {
+    ReactDOM.render(
+      <ReactCSSTransitionGroup
+        transitionName="yolo"
+        transitionEnter={false}
+        transitionLeave={true}
+      >
+        <span key="one" id="one" />
+      </ReactCSSTransitionGroup>,
+      container
+    );
+
+    // Warning about the missing transitionLeaveTimeout prop
+    expect(console.error.argsForCall.length).toBe(1);
+  });
+
+  it('should clean-up silently after the timeout elapses', function() {
     var a = ReactDOM.render(
-      <ReactCSSTransitionGroup transitionName="yolo">
+      <ReactCSSTransitionGroup
+        transitionName="yolo"
+        transitionEnter={false}
+        transitionLeaveTimeout={200}
+      >
         <span key="one" id="one" />
       </ReactCSSTransitionGroup>,
       container
@@ -41,7 +61,11 @@ describe('ReactCSSTransitionGroup', function() {
     setTimeout.mock.calls.length = 0;
 
     ReactDOM.render(
-      <ReactCSSTransitionGroup transitionName="yolo">
+      <ReactCSSTransitionGroup
+        transitionName="yolo"
+        transitionEnter={false}
+        transitionLeaveTimeout={200}
+      >
         <span key="two" id="two" />
       </ReactCSSTransitionGroup>,
       container
@@ -53,14 +77,18 @@ describe('ReactCSSTransitionGroup', function() {
     // For some reason jst is adding extra setTimeout()s and grunt test isn't,
     // so we need to do this disgusting hack.
     for (var i = 0; i < setTimeout.mock.calls.length; i++) {
-      if (setTimeout.mock.calls[i][1] === 5000) {
+      if (setTimeout.mock.calls[i][1] === 200) {
         setTimeout.mock.calls[i][0]();
         break;
       }
     }
 
-    expect(ReactDOM.findDOMNode(a).childNodes.length).toBe(2);
-    expect(console.error.argsForCall.length).toBe(1);
+    // No warnings
+    expect(console.error.argsForCall.length).toBe(0);
+
+    // The leaving child has been removed
+    expect(ReactDOM.findDOMNode(a).childNodes.length).toBe(1);
+    expect(ReactDOM.findDOMNode(a).childNodes[0].id).toBe('two');
   });
 
   it('should keep both sets of DOM nodes around', function() {
@@ -170,5 +198,4 @@ describe('ReactCSSTransitionGroup', function() {
     expect(ReactDOM.findDOMNode(a).childNodes.length).toBe(1);
     expect(ReactDOM.findDOMNode(a).childNodes[0].id).toBe('one');
   });
-
 });