Fix to work enter animation with CSSTransitionGroup

[koba04]

This PR is to fix #5.

Currently, enter animation with CSSTransitionGroup seems not to work as mentioned in #5.

The following is a reproduce case.

• Using react-addons-css-transition-group (works fine)

  • http://www.webpackbin.com/NyJBZC7uf
  • 

• Using react-transition-group (enter animation doesn't work)

  • http://www.webpackbin.com/Ey8UzR7_M
  • 

According to the above Chrome timeline images, react-transition-group calls flushClassNameAndNodeQueue before recalculate and repaint.
I think this is the cause of the issue.

So the PR is using setTimeout with TICK = 17, which is same as the original implementation in react-addons-css-transition-group.

After applying the above patch, CSSTransitionGroupChild creates nested duplicate children.
It can fix not to pass children to React.cloneElement

[jquense]

thanks for the PR! we really want to use requestAnimationFrame here as that is what the original code is emulating (poorly). rather then go back to the old code, which causes other animation issues, I'd like to figure out whats not working and fix the current code thanks!

[koba04]

@jquense If you'd like to use rAF, there is a way to force a repaint by accessing a style property.

diff --git a/src/CSSTransitionGroupChild.js b/src/CSSTransitionGroupChild.js
index d3f5baa..bea150b 100644
--- a/src/CSSTransitionGroupChild.js
+++ b/src/CSSTransitionGroupChild.js
@@ -125,6 +125,11 @@ class CSSTransitionGroupChild extends React.Component {
   flushClassNameAndNodeQueue() {
     if (!this.unmounted) {
       this.classNameAndNodeQueue.forEach((obj) => {
+        // This is for to force a repaint,
+        // which is necessary in order to transition styles when adding a class name.
+        /* eslint-disable no-unused-expressions */
+        obj.node.scrollTop;
+        /* eslint-enable no-unused-expressions */
         addClass(obj.node, obj.className);
       });
     }
@@ -165,6 +170,7 @@ class CSSTransitionGroupChild extends React.Component {
     delete props.appearTimeout;
     delete props.enterTimeout;
     delete props.leaveTimeout;
+    delete props.children;
     return React.cloneElement(React.Children.only(this.props.children), props);
   }
 }

If you prefer this approach, I can update this PR.

[jquense]

hey thanks for looking into it. I'm not really sure why we'd need to trigger a repaint after a RAF honestly. RAF is very specifically for getting a callback on the next repaint, so you should need to trigger one, because teh callback is being called exactly when the browser is about to paint. My guess at what be happening here tho is:

first class is added somewhere between a paint frame and then the RAF callback is called at the end leading to both being added in the same repaint.

If so the best bet would be to trigger a reflow right after the first enter class is added instead of before the active class. I think ideally tho we should add the enter class in a RAF and then trigger the next RAF for the active class...

That may over complicate it all tho

[koba04]

@jquense Thanks!

If so the best bet would be to trigger a reflow right after the first enter class is added instead of before the active class.

I agree with this. The following patch also works fine.

diff --git a/src/CSSTransitionGroupChild.js b/src/CSSTransitionGroupChild.js
index d3f5baa..210c418 100644
--- a/src/CSSTransitionGroupChild.js
+++ b/src/CSSTransitionGroupChild.js
@@ -78,9 +78,10 @@ class CSSTransitionGroupChild extends React.Component {
     let removeListeners;
 
     addClass(node, className);
-
-    // Need to do this to actually trigger a transition.
-    this.queueClassAndNode(activeClassName, node);
+    raf(() => {
+      // Need to do this to actually trigger a transition.
+      this.queueClassAndNode(activeClassName, node);
+    });
 
     // Clean-up the animation after the specified delay
     let finish = (e) => {
@@ -165,6 +166,7 @@ class CSSTransitionGroupChild extends React.Component {
     delete props.appearTimeout;
     delete props.enterTimeout;
     delete props.leaveTimeout;
+    delete props.children;
     return React.cloneElement(React.Children.only(this.props.children), props);
   }
 }

I think rAF calls a callback before the next repaint. So we need to call nested rAFs.

• https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame

The window.requestAnimationFrame() method tells the browser that you wish to perform an animation and requests that the browser call a specified function to update an animation before the next repaint. The method takes as an argument a callback to be invoked before the repaint.
Note: Your callback routine must itself call requestAnimationFrame() if you want to animate another frame at the next repaint.

[jquense]

So i've tried this out a few times with a bunch of different approaches noted here. Ultimately I think that your thought: #13 (comment) is actually the simplest and most consistent fix, among the animations i've tried.

I was worried that reflow triggers in the RAF would cause a thrash, but looking at the timeline of it. it seems its actually the least noisy option

[koba04]

Thank you for your having time! I've updated this to using #13 (comment).

[koba04]

Thanks 🎉

Could you publish this on npm as a new version?
I'd like to use it in my project.