ngAnimate 1.4 Refactor #11336

matsko · 2015-03-16T19:12:12Z

All of ngAnimate has been rewritten to make the internals of the
animation code more flexible, reuseable and performant.

BREAKING CHANGE: JavaSript and CSS animations can no longer be run in
parallel. With earlier versions of ngAnimate, both CSS and JS animations
would be run together when multiple animations were detected. This
feature has now been removed, however, the same effect, with even more
possibilities, can be achieved by injecting $animateCss into a
JavaScript-defined animation and creating custom CSS-based animations
from there. Read the ngAnimate docs for more info.

BREAKING CHANGE: The function params for $animate.enabled() when an
element is used are now flipped. This fix allows the function to act as
a getter when a single element param is provided.

// < 1.4
$animate.enabled(false, element);

// 1.4+
$animate.enabled(element, false);

BREAKING CHANGE: In addition to disabling the children of the element,
$animate.enabled(element, false) will now also disable animations on
the element itself.

BREAKING CHANGE: Animation-related callbacks are now fired on
$animate.on instead of directly being on the element.

// < 1.4
element.on('$animate:before', function(e, data) {
  if (data.event === 'enter') { ... }
});
element.off('$animate:before', fn);

// 1.4+
$animate.on(element, 'enter', function(data) {
  //...
});
$animate.off(element, 'enter', fn);

BREAKING CHANGE: There is no need to call $scope.$apply or
$scope.$digest inside of a animation promise callback anymore
since the promise is resolved within a digest automatically (but a
digest is not run unless the promise is chained).

// < 1.4
$animate.enter(element).then(function() {
  $scope.$apply(function() {
    $scope.explode = true;
  });
});

// 1.4+
$animate.enter(element).then(function() {
  $scope.explode = true;
});

BREAKING CHANGE: When an enter, leave or move animation is triggered then it
will always end any pending or active parent class based animations
(animations triggered via ngClass) in order to ensure that any CSS
styles are resolved in time.

petebacondarwin · 2015-03-16T20:22:27Z

It would be helpful it was possible to see how the tests have changed from before.

petebacondarwin · 2015-03-18T11:34:27Z

src/ngAnimate/animateCss.js

+
+function parseMaxTime(str) {
+  var maxValue = 0;
+  var values = isString(str) ?


str is always a string so this test is not needed.

petebacondarwin · 2015-03-24T11:44:17Z

src/ngAnimate/animateCss.js

+function blockTransitions(node, applyBlock) {
+  var value = applyBlock ? 'none' : '';
+  var key = TRANSITION_PROP + PROPERTY_KEY;
+  applyInlineStyle(node, key, value);


Change this to...

applyInlineStyle(node, [key, value]);

gkalpak · 2015-04-01T07:29:02Z

src/ngAnimate/animateQueue.js

+    }
+
+    function areAnimationsAllowed(element, parent, event) {
+      var bodyElement = jqLite($document[0].body);


Couldn't this be moved outside of the function (for performance reasons) ?
What are the implications ? Can the body really change ?

gkalpak · 2015-04-01T07:40:48Z

No time to look into the tests, but the (non-test) code and the whole re-organization looks absolutely fantastic.
👍 👍 👍 @matsko

_{(Left a couple of inline comments that changed ngAnimate for ever :P)}

One...actualy two...more tiny things I would fix regarding the comments:

Shorten too long comment lines (e.g. in $animateCss). They are hard to review.
Add a few commas here and there.

Narretz · 2015-04-01T09:45:35Z

@matsko plnkr with modal works now! Awesome work!

Would it help you if you one of us prepared a commit with all typo fixes that you can cherry-pick into your branch?

petebacondarwin · 2015-04-01T14:40:43Z

@Narretz I think that is an excellent idea!

matsko · 2015-04-03T03:55:00Z

@Narretz @petebacondarwin @gkalpak any typo fixes are good to go now. The PR has been updated with all the fixes that @gkalpak provided as well as the fixes that @IgorMinar suggested during our review.

$$animateOptions is gone now in favor of some simple private option-related helper methods.

We're almost there. Once the safari and IE-related unit tests are fixed and some extra docs are added then we're good merge.

gkalpak · 2015-04-03T06:02:23Z

BREAKING CHANGE: The function params for $animate.enabled() when an
element is used are now flipped. This fix allows the function to act as
a getter when a single element param is provided.

Not that it matters much (since there are several breaking changes already), but couldn't this one be avoided ? Does the order of the arguments really matter ?
_{(I obviously think it doesn't or else I wouldn't ask this question :))}

gkalpak · 2015-04-03T06:10:12Z

src/ngAnimate/animateQueue.js

+            animateChildren = value;
+          }
+        }
+


At this point, if parentAnimationDetected === true and animateChildren === false, we could immediately return false. There is no need to keep looping.

Added. Thx.

matsko · 2015-04-07T07:03:43Z

OK @petebacondarwin @IgorMinar @Narretz @gkalpak we are finally ready to merge. All green. Tested properly. Let's go!

gkalpak · 2015-04-07T07:53:28Z

10, 9 . . . (6 numbers later) . . . 2, 1 -----> 🚀

✨ 👏 ✨ 👏 ✨

petebacondarwin · 2015-04-07T15:23:10Z

modalContent.html

+  <div class="loader" ng-show="!media.loaded">
+    Loading
+  </div>
+</div>


What's going on here? It doesn't seem right for this file to be in the root of the project.

All of ngAnimate has been rewritten to make the internals of the animation code more flexible, reuseable and performant. BREAKING CHANGE: JavaSript and CSS animations can no longer be run in parallel. With earlier versions of ngAnimate, both CSS and JS animations would be run together when multiple animations were detected. This feature has now been removed, however, the same effect, with even more possibilities, can be achieved by injecting `$animateCss` into a JavaScript-defined animation and creating custom CSS-based animations from there. Read the ngAnimate docs for more info. BREAKING CHANGE: The function params for `$animate.enabled()` when an element is used are now flipped. This fix allows the function to act as a getter when a single element param is provided. ```js // < 1.4 $animate.enabled(false, element); // 1.4+ $animate.enabled(element, false); ``` BREAKING CHANGE: In addition to disabling the children of the element, `$animate.enabled(element, false)` will now also disable animations on the element itself. BREAKING CHANGE: Animation-related callbacks are now fired on `$animate.on` instead of directly being on the element. ```js // < 1.4 element.on('$animate:before', function(e, data) { if (data.event === 'enter') { ... } }); element.off('$animate:before', fn); // 1.4+ $animate.on(element, 'enter', function(data) { //... }); $animate.off(element, 'enter', fn); ``` BREAKING CHANGE: There is no need to call `$scope.$apply` or `$scope.$digest` inside of a animation promise callback anymore since the promise is resolved within a digest automatically (but a digest is not run unless the promise is chained). ```js // < 1.4 $animate.enter(element).then(function() { $scope.$apply(function() { $scope.explode = true; }); }); // 1.4+ $animate.enter(element).then(function() { $scope.explode = true; }); ``` BREAKING CHANGE: When an enter, leave or move animation is triggered then it will always end any pending or active parent class based animations (animations triggered via ngClass) in order to ensure that any CSS styles are resolved in time.

matsko · 2015-04-09T21:47:40Z

Closed via c8700f0

googlebot added the cla: yes label Mar 16, 2015

matsko force-pushed the ng_animate_14 branch 2 times, most recently from bf413c0 to d8e9f08 Compare March 16, 2015 19:16

matsko force-pushed the ng_animate_14 branch 2 times, most recently from e7dd45d to 0e1c40d Compare March 17, 2015 00:25

petebacondarwin reviewed Mar 18, 2015
View reviewed changes

petebacondarwin added this to the 1.4.0-beta.7 / 1.3.16 milestone Mar 18, 2015

petebacondarwin added component: ngAnimate type: refactor branch: 1.4.x labels Mar 18, 2015

matsko force-pushed the ng_animate_14 branch from cc47295 to ae4d082 Compare March 24, 2015 07:59

petebacondarwin reviewed Mar 24, 2015
View reviewed changes

gkalpak reviewed Apr 1, 2015
View reviewed changes

matsko force-pushed the ng_animate_14 branch from 05ae7f1 to 02d79fb Compare April 3, 2015 03:52

matsko force-pushed the ng_animate_14 branch 2 times, most recently from 621dc47 to 06ccc5d Compare April 3, 2015 04:10

gkalpak reviewed Apr 3, 2015
View reviewed changes

matsko force-pushed the ng_animate_14 branch 7 times, most recently from 889cd25 to db2c8b5 Compare April 7, 2015 06:42

petebacondarwin reviewed Apr 7, 2015
View reviewed changes

matsko force-pushed the ng_animate_14 branch 3 times, most recently from e8f55ad to a2385bc Compare April 9, 2015 21:06

matsko force-pushed the ng_animate_14 branch from a2385bc to e7743d3 Compare April 9, 2015 21:12

matsko closed this Apr 9, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ngAnimate 1.4 Refactor #11336

ngAnimate 1.4 Refactor #11336

matsko commented Mar 16, 2015

petebacondarwin commented Mar 16, 2015

petebacondarwin Mar 18, 2015

petebacondarwin Mar 24, 2015

gkalpak Apr 1, 2015

matsko Apr 2, 2015

gkalpak commented Apr 1, 2015

Narretz commented Apr 1, 2015

petebacondarwin commented Apr 1, 2015

matsko commented Apr 3, 2015

gkalpak commented Apr 3, 2015

gkalpak Apr 3, 2015

matsko Apr 7, 2015

matsko commented Apr 7, 2015

gkalpak commented Apr 7, 2015

petebacondarwin Apr 7, 2015

matsko commented Apr 9, 2015

ngAnimate 1.4 Refactor #11336

ngAnimate 1.4 Refactor #11336

Conversation

matsko commented Mar 16, 2015

petebacondarwin commented Mar 16, 2015

petebacondarwin Mar 18, 2015

Choose a reason for hiding this comment

petebacondarwin Mar 24, 2015

Choose a reason for hiding this comment

gkalpak Apr 1, 2015

Choose a reason for hiding this comment

matsko Apr 2, 2015

Choose a reason for hiding this comment

gkalpak commented Apr 1, 2015

Narretz commented Apr 1, 2015

petebacondarwin commented Apr 1, 2015

matsko commented Apr 3, 2015

gkalpak commented Apr 3, 2015

gkalpak Apr 3, 2015

Choose a reason for hiding this comment

matsko Apr 7, 2015

Choose a reason for hiding this comment

matsko commented Apr 7, 2015

gkalpak commented Apr 7, 2015

petebacondarwin Apr 7, 2015

Choose a reason for hiding this comment

matsko commented Apr 9, 2015