Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
feat(ngAnimate): complete rewrite of animations
Browse files Browse the repository at this point in the history 
- ngAnimate directive is gone and was replaced with class based animations/transitions
- support for triggering animations on css class additions and removals
- done callback was added to all animation apis
- $animation and $animator where merged into a single $animate service with api:
  - $animate.enter(element, parent, after, done);
  - $animate.leave(element, done);
  - $animate.move(element, parent, after, done);
  - $animate.addClass(element, className, done);
  - $animate.removeClass(element, className, done);

BREAKING CHANGE: too many things changed, we'll write up a separate doc with migration instructions
  • Loading branch information
@matsko @mhevery
matsko authored and mhevery committed Jul 27, 2013
1 parent 11521a4 commit 81923f1
Show file tree
Hide file tree
Showing 40 changed files with 2,961 additions and 2,191 deletions.
5 changes: 5 additions & 0 deletions Gruntfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ module.exports = function(grunt) {
dest: 'build/angular-resource.js',
src: util.wrap(['src/ngResource/resource.js'], 'module')
},
animate: {
dest: 'build/angular-animate.js',
src: util.wrap(['src/ngAnimate/animate.js'], 'module')
},
route: {
dest: 'build/angular-route.js',
src: util.wrap([
Expand All @@ -178,6 +182,7 @@ module.exports = function(grunt) {

min: {
angular: 'build/angular.js',
animate: 'build/angular-animate.js',
cookies: 'build/angular-cookies.js',
loader: 'build/angular-loader.js',
mobile: 'build/angular-mobile.js',
Expand Down
5 changes: 3 additions & 2 deletions angularFiles.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ angularFiles = {
'src/auto/injector.js',

'src/ng/anchorScroll.js',
'src/ng/animation.js',
'src/ng/animator.js',
'src/ng/animate.js',
'src/ng/browser.js',
'src/ng/cacheFactory.js',
'src/ng/compile.js',
Expand Down Expand Up @@ -66,6 +65,7 @@ angularFiles = {
],

'angularSrcModules': [
'src/ngAnimate/animate.js',
'src/ngCookies/cookies.js',
'src/ngResource/resource.js',
'src/ngRoute/routeUtils.js',
Expand Down Expand Up @@ -107,6 +107,7 @@ angularFiles = {
'test/*.js',
'test/auto/*.js',
'test/ng/**/*.js',
'test/ngAnimate/*.js',
'test/ngCookies/*.js',
'test/ngResource/*.js',
'test/ngRoute/**/*.js',
Expand Down
4 changes: 4 additions & 0 deletions css/angular.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@
ng\:form {
display: block;
}

.ng-hide {
display: none;
}
39 changes: 17 additions & 22 deletions docs/component-spec/annotationsSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,41 +67,33 @@ describe('Docs Annotations', function() {

var $scope, parent, element, url, window;
beforeEach(function() {
module(function($provide, $animationProvider) {
module(function($provide, $animateProvider) {
$provide.value('$window', window = angular.mock.createMockWindow());
$animationProvider.register('foldout-enter', function($window) {
$animateProvider.register('.foldout', function($window) {
return {
start : function(element, done) {
enter : function(element, done) {
$window.setTimeout(done, 1000);
}
}
});
$animationProvider.register('foldout-hide', function($window) {
return {
start : function(element, done) {
},
show : function(element, done) {
$window.setTimeout(done, 500);
}
}
});
$animationProvider.register('foldout-show', function($window) {
return {
start : function(element, done) {
},
hide : function(element, done) {
$window.setTimeout(done, 200);
}
}
});
});
inject(function($rootScope, $compile, $templateCache, $rootElement, $animator) {
$animator.enabled(true);
inject(function($rootScope, $compile, $templateCache, $rootElement, $animate) {
$animate.enabled(true);
url = '/page.html';
$scope = $rootScope.$new();
parent = angular.element('<div class="parent"></div>');
element = angular.element('<div data-url="' + url + '" foldout></div>');

//we're injecting the element to the $rootElement since the changes in
//$animator only detect and perform animations if the root element has
//$animate only detect and perform animations if the root element has
//animations enabled. If the element is not apart of the DOM
//then animations are skipped.
element = angular.element('<div data-url="' + url + '" class="foldout" foldout></div>');
parent.append(element);
$rootElement.append(parent);
body.append($rootElement);
Expand Down Expand Up @@ -142,16 +134,19 @@ describe('Docs Annotations', function() {
$httpBackend.flush();
window.setTimeout.expect(1).process();
window.setTimeout.expect(1000).process();
window.setTimeout.expect(0).process();

//hide
element.triggerHandler('click');
window.setTimeout.expect(1).process();
window.setTimeout.expect(500).process();
window.setTimeout.expect(200).process();
window.setTimeout.expect(0).process();

//show
element.triggerHandler('click');
window.setTimeout.expect(1).process();
window.setTimeout.expect(200).process();
window.setTimeout.expect(500).process();
window.setTimeout.expect(0).process();
}));

});
Expand All @@ -160,7 +155,7 @@ describe('Docs Annotations', function() {

var window, $scope, ctrl;
beforeEach(function() {
module(function($provide, $animationProvider) {
module(function($provide, $animateProvider) {
$provide.value('$window', window = angular.mock.createMockWindow());
});
inject(function($rootScope, $controller, $location, $cookies, sections) {
Expand Down
5 changes: 3 additions & 2 deletions docs/components/angular-bootstrap/bootstrap-prettify.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,8 @@ directive.ngEvalJavascript = ['getEmbeddedTemplate', function(getEmbeddedTemplat
}];


directive.ngEmbedApp = ['$templateCache', '$browser', '$rootScope', '$location', '$sniffer',
function($templateCache, $browser, docsRootScope, $location, $sniffer) {
directive.ngEmbedApp = ['$templateCache', '$browser', '$rootScope', '$location', '$sniffer', '$animate',
function($templateCache, $browser, docsRootScope, $location, $sniffer, $animate) {
return {
terminal: true,
link: function(scope, element, attrs) {
Expand All @@ -193,6 +193,7 @@ directive.ngEmbedApp = ['$templateCache', '$browser', '$rootScope', '$location',
deregisterEmbedRootScope;

modules.push(['$provide', function($provide) {
$provide.value('$animate', $animate);
$provide.value('$templateCache', $templateCache);
$provide.value('$anchorScroll', angular.noop);
$provide.value('$browser', $browser);
Expand Down
9 changes: 4 additions & 5 deletions docs/components/angular-bootstrap/bootstrap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,12 +335,11 @@ directive.tabPane = function() {
};
};

directive.foldout = ['$http', '$animator','$window', function($http, $animator, $window) {
directive.foldout = ['$http', '$animate','$window', function($http, $animate, $window) {
return {
restrict: 'A',
priority : 500,
link: function(scope, element, attrs) {
var animator = $animator(scope, { ngAnimate: "'foldout'" });
var container, loading, url = attrs.url;
if(/\/build\//.test($window.location.href)) {
url = '/build/docs' + url;
Expand All @@ -353,7 +352,7 @@ directive.foldout = ['$http', '$animator','$window', function($http, $animator,
loading = true;
var par = element.parent();
container = angular.element('<div class="foldout">loading...</div>');
animator.enter(container, null, par);
$animate.enter(container, null, par);

$http.get(url, { cache : true }).success(function(html) {
loading = false;
Expand All @@ -367,12 +366,12 @@ directive.foldout = ['$http', '$animator','$window', function($http, $animator,
//avoid showing the element if the user has already closed it
if(container.css('display') == 'block') {
container.css('display','none');
animator.show(container);
$animate.show(container);
}
});
}
else {
container.css('display') == 'none' ? animator.show(container) : animator.hide(container);
container.hasClass('ng-hide') ? $animate.show(container) : $animate.hide(container);
}
});
});
Expand Down
2 changes: 1 addition & 1 deletion docs/src/example.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ exports.Example.prototype.toHtmlTabs = function() {

exports.Example.prototype.toHtmlEmbed = function() {
var out = [];
out.push('<div class="well doc-example-live animator-container"');
out.push('<div class="well doc-example-live animate-container"');
if(this.animations) {
out.push(" ng-class=\"{'animations-off':animationsOff == true}\"");
}
Expand Down
67 changes: 13 additions & 54 deletions docs/src/ngdoc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,19 @@ Doc.prototype = {
html_usage_parameters: function(dom) {
var self = this;
var params = this.param ? this.param : [];
if(this.animations) {
dom.h('Animations', this.animations, function(animations){
dom.html('<ul>');
var animations = animations.split("\n");
animations.forEach(function(ani) {
dom.html('<li>');
dom.text(ani);
dom.html('</li>');
});
dom.html('</ul>');
});
dom.html('<a href="api/ngAnimate.$animate">Click here</a> to learn more about the steps involved in the animation.');
}
if(params.length > 0) {
dom.html('<h2 id="parameters">Parameters</h2>');
dom.html('<table class="variables-matrix table table-bordered table-striped">');
Expand Down Expand Up @@ -538,18 +551,6 @@ Doc.prototype = {
dom.html('</tbody>');
dom.html('</table>');
}
if(this.animations) {
dom.h('Animations', this.animations, function(animations){
dom.html('<ul>');
var animations = animations.split("\n");
animations.forEach(function(ani) {
dom.html('<li>');
dom.text(ani);
dom.html('</li>');
});
dom.html('</ul>');
});
}
},

html_usage_returns: function(dom) {
Expand Down Expand Up @@ -665,48 +666,6 @@ Doc.prototype = {
dom.text('</' + element + '>');
});
}
if(self.animations) {
var animations = [], matches = self.animations.split("\n");
matches.forEach(function(ani) {
var name = ani.match(/^\s*(.+?)\s*-/)[1];
animations.push(name);
});

dom.html('with <span id="animations">animations</span>');
var comment;
if(animations.length == 1) {
comment = 'The ' + animations[0] + ' animation is supported';
}
else {
var rhs = animations[animations.length-1];
var lhs = '';
for(var i=0;i<animations.length-1;i++) {
if(i>0) {
lhs += ', ';
}
lhs += animations[i];
}
comment = 'The ' + lhs + ' and ' + rhs + ' animations are supported';
}
var element = self.element || 'ANY';
dom.code(function() {
dom.text('//' + comment + "\n");
dom.text('<' + element + ' ');
dom.text(dashCase(self.shortName));
renderParams('\n ', '="', '"', true);
dom.text(' ng-animate="{');
animations.forEach(function(ani, index) {
if (index) {
dom.text(', ');
}
dom.text(ani + ': \'' + ani + '-animation\'');
});
dom.text('}">\n ...\n');
dom.text('</' + element + '>');
});

dom.html('<a href="api/ng.$animator#Methods">Click here</a> to learn more about the steps involved in the animation.');
}
}
self.html_usage_directiveInfo(dom);
self.html_usage_parameters(dom);
Expand Down
Loading

6 comments on commit 81923f1

@damrbaby
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, while I like this approach (in theory), I have noticed that as a result of this change, just including ngAnimate in my mobile app is causing performance on mobile devices (iPad) to degrade in an ng-repeat of around 50 items. Basically the list takes a slight delay to appear (with a bit of a flicker), whereas before it would have appeared immediately. I am attributing this delay and performance hit to the DOM adding and removing the animation classes to and from all my list items individually. It's happening everywhere I have an ng-repeat defined (I have no animations defined in my CSS yet).

My problem with this approach is that it's no longer optional... if you include ngAnimate you will have the adding and removal of class names all over the place. What if I just want to animate ng-view without ng-repeat? Why should the DOM be adding all of those classes to elements that didn't even need an animation in the first place? That's a lot of overhead for no gain. DOM manipulation is expensive and noticabely slower in mobile browsers, even an iPad 4gen running iOS 6.

@matsko
Copy link
Contributor Author

@matsko matsko commented on 81923f1 Aug 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe the be a solution could be to add a .ng-animate-skip className for cases like this.

@mhevery
Copy link
Contributor

@mhevery mhevery commented on 81923f1 Aug 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds concerning. Could you provide us with a plunker which we could use to verify and debug this?

@damrbaby
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have set up a very basic plunkr here: http://plnkr.co/edit/x7baZHgI93pRWYScQIHs

The flickering is even visible in chrome, when removing items from the list.

If you remove ngAnimate as a dependency you'll see how this performs much smoother.

@damrbaby
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I encourage you to test this on apps in the wild (especially mobile ones) that have lots of ngRepeat to test it out...

Another thing I've been wondering about this approach is that it used to be possible to pass a function to ngAnimate ... like ng-animate="forwardOrBack()", and then programmatically choose the animation to apply based on the CSS classes that get added and removed. Now that the animations are just static css classes that automatically get added and removed, it doesn't seem possible to "choose" let's say, the direction of the animation (e.g. slide right or slide left depending on routing logic), other than targeting this with outer CSS classes.

I think that the declarative nature of ngAnimate directive is preferable to me over this "catch-all" approach. But I'd love to hear more about why this change was made!

@matsko
Copy link
Contributor Author

@matsko matsko commented on 81923f1 Aug 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We did think about how to do this and since ngAnimate is class-based you would use ngClass or an expression within a class attribute to toggle the classes on the element. When this changes, then the next animation will pay attention to the new CSS classes on the element. You can also use a function within the ngClass expression to return an object or string of classes. This all works, but there is a slight bug right now where ngRepeat, ngInclude or ngView doesn't pickup the ngClass changes before animating if the element is transcluded, but this will be shortly fixed (I also think that this relates to the issue you posted earlier).

Please sign in to comment.