Fix problem with animation callbacks incorrectly getting called, refa…

…ctor willRenderAnimations in LayoutCalculator
junction · Mar 16, 2011 · 763c490 · 763c490
1 parent 082c9ec
commit 763c490
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 22 deletions.
diff --git a/Buildfile b/Buildfile
@@ -96,8 +96,8 @@ config :core_tools, :required => [:desktop, :datastore, :animation, :forms]
 %w(tests test_controls docs welcome).each do |app_target|
   config app_target, 
     :required => [:desktop, :datastore, :core_tools],
-    :theme => :ace,
-    :html5_history => true
+    :theme => :ace
+    # :html5_history => true
 
   # mode :debug do
   #   config app_target, :combine_javascript => false

diff --git a/frameworks/core_foundation/tests/views/view/animation.js b/frameworks/core_foundation/tests/views/view/animation.js
@@ -171,6 +171,35 @@ if (SC.platform.supportsCSSTransitions) {
     SC.RunLoop.end();
   });
 
+  test("should not cancel callback when value hasn't changed", function() {
+    var callbacks = 0, wasCancelled = NO, check = 0;
+    stop(2000);
+
+    SC.RunLoop.begin();
+    view.invokeLater(function() {
+      // this triggers the initial layoutStyle code
+      view.animate('left', 79, 0.500, function(data) {
+        callbacks++;
+        wasCancelled = data.isCancelled;
+      });
+      // this triggers a re-render, re-running the layoutStyle code
+      view.displayDidChange();
+    }, 1);
+    SC.RunLoop.end();
+
+    setTimeout(function() {
+      // capture the callbacks value
+      check = callbacks;
+    }, 250);
+
+    setTimeout(function() {
+      start();
+      equals(check, 0, "the callback should not have been cancelled initially");
+      equals(callbacks, 1, "the callback should have been fired");
+      equals(wasCancelled, NO, "the callback should not have been cancelled");
+    }, 1000);
+  });
+
   test("should handle transform attributes", function(){
     SC.RunLoop.begin();
     view.animate('rotateX', 45, { duration: 1 });

diff --git a/frameworks/core_foundation/views/view/layout_style.js b/frameworks/core_foundation/views/view/layout_style.js
@@ -457,37 +457,51 @@ SC.View.LayoutStyleCalculator = SC.Object.extend({
           layer = view.get('layer'),
           currentStyle = layer ? layer.style : null,
           newStyle = view.get('layoutStyle'),
+          activeAnimations = this._activeAnimations, activeAnimation,
+          pendingAnimations = this._pendingAnimations, pendingAnimation,
+          animatedTransforms = this._animatedTransforms,
+          transformsLength = animatedTransforms ? animatedTransforms.length : 0,
           transitionStyle = newStyle[SC.platform.domCSSPrefix+"Transition"],
           layout = view.get('layout'),
-          key, callback, idx;
-
-      // Handle existing animations
-      if (this._activeAnimations) {
-        for(key in this._activeAnimations){
-          // TODO: Check for more than duration
-          if (
-            newStyle[key] !== (currentStyle ? currentStyle[key] : null) ||
-            !this._pendingAnimations || !this._pendingAnimations[key] ||
-            this._activeAnimations[key].duration !== this._pendingAnimations[key].duration
-          ) {
-            callback = this._activeAnimations[key].callback;
-            if (callback) {
-              if (this._animatedTransforms && this._animatedTransforms.length > 0) {
-                for (idx=0; idx < this._animatedTransforms.length; idx++) {
-                  this.runAnimationCallback(callback, null, this._animatedTransforms[idx], YES);
+          key, callback, idx, shouldCancel;
+
+      if (pendingAnimations) {
+        if (!activeAnimations) activeAnimations = {};
+
+        for (key in pendingAnimations) {
+          if (!pendingAnimations.hasOwnProperty(key)) continue;
+
+          pendingAnimation = pendingAnimations[key];
+          activeAnimation = activeAnimations[key];
+          shouldCancel = NO;
+
+          if (newStyle[key] !== (currentStyle ? currentStyle[key] : null)) shouldCancel = YES;
+
+          // if we have a new animation (an animation property has changed), cancel current
+          if (activeAnimation && (activeAnimation.duration !== pendingAnimation.duration || activeAnimation.timing !== pendingAnimation.timing)) {
+            shouldCancel = YES;
+          }
+
+          if (shouldCancel && activeAnimation) {
+            if (callback = activeAnimation.callback) {
+              if (transformsLength > 0) {
+                for (idx=0; idx < transformsLength; idx++) {
+                  this.runAnimationCallback(callback, null, animatedTransforms[idx], YES);
                 }
                 this._animatedTransforms = null;
               } else {
                 this.runAnimationCallback(callback, null, key, YES);
               }
             }
-
+            
             this.removeAnimationFromLayout(key, YES);
           }
+
+          activeAnimations[key] = pendingAnimation;
         }
       }
-
-      this._activeAnimations = this._pendingAnimations;
+      
+      this._activeAnimations = activeAnimations;
       this._pendingAnimations = null;
     }
   },
@@ -521,7 +535,7 @@ SC.View.LayoutStyleCalculator = SC.Object.extend({
 
     animation = this._activeAnimations ? this._activeAnimations[propertyName] : null;
 
-    if(animation) {
+    if (animation) {
       if (animation.callback) {
         // Charles says this is a good idea
         SC.RunLoop.begin();