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

fix(ngAnimate): only buffer rAF requests within the animation runners #12619

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 3 additions & 41 deletions src/ng/raf.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function $$RAFProvider() { //rAF
$window.webkitCancelRequestAnimationFrame;

var rafSupported = !!requestAnimationFrame;
var rafFn = rafSupported
var raf = rafSupported
? function(fn) {
var id = requestAnimationFrame(fn);
return function() {
Expand All @@ -24,46 +24,8 @@ function $$RAFProvider() { //rAF
};
};

queueFn.supported = rafSupported;
raf.supported = rafSupported;

var cancelLastRAF;
var taskCount = 0;
var taskQueue = [];
return queueFn;

function flush() {
for (var i = 0; i < taskQueue.length; i++) {
var task = taskQueue[i];
if (task) {
taskQueue[i] = null;
task();
}
}
taskCount = taskQueue.length = 0;
}

function queueFn(asyncFn) {
var index = taskQueue.length;

taskCount++;
taskQueue.push(asyncFn);

if (index === 0) {
cancelLastRAF = rafFn(flush);
}

return function cancelQueueFn() {
if (index >= 0) {
taskQueue[index] = null;
index = null;

if (--taskCount === 0 && cancelLastRAF) {
cancelLastRAF();
cancelLastRAF = null;
taskQueue.length = 0;
}
}
};
}
return raf;
}];
}
4 changes: 2 additions & 2 deletions src/ngAnimate/animateJs.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
// by the time...

var $$AnimateJsProvider = ['$animateProvider', function($animateProvider) {
this.$get = ['$injector', '$$AnimateRunner', '$$rAFMutex', '$$jqLite',
function($injector, $$AnimateRunner, $$rAFMutex, $$jqLite) {
this.$get = ['$injector', '$$AnimateRunner', '$$jqLite',
function($injector, $$AnimateRunner, $$jqLite) {

var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
// $animateJs(element, 'enter');
Expand Down
27 changes: 21 additions & 6 deletions src/ngAnimate/animateRunner.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,33 @@
'use strict';

var $$rAFMutexFactory = ['$$rAF', function($$rAF) {
var $$AnimateAsyncRunFactory = ['$$rAF', function($$rAF) {
var waitQueue = [];

function waitForTick(fn) {
waitQueue.push(fn);
if (waitQueue.length > 1) return;
$$rAF(function() {
for (var i = 0; i < waitQueue.length; i++) {
waitQueue[i]();
}
waitQueue = [];
});
}

return function() {
var passed = false;
$$rAF(function() {
waitForTick(function() {
passed = true;
});
return function(fn) {
passed ? fn() : $$rAF(fn);
return function(callback) {
passed ? callback() : waitForTick(callback);
};
};
}];

var $$AnimateRunnerFactory = ['$q', '$$rAFMutex', function($q, $$rAFMutex) {
var $$AnimateRunnerFactory = ['$q', '$sniffer', '$$animateAsyncRun',
function($q, $sniffer, $$animateAsyncRun) {

var INITIAL_STATE = 0;
var DONE_PENDING_STATE = 1;
var DONE_COMPLETE_STATE = 2;
Expand Down Expand Up @@ -57,7 +72,7 @@ var $$AnimateRunnerFactory = ['$q', '$$rAFMutex', function($q, $$rAFMutex) {
this.setHost(host);

this._doneCallbacks = [];
this._runInAnimationFrame = $$rAFMutex();
this._runInAnimationFrame = $$animateAsyncRun();
this._state = 0;
}

Expand Down
5 changes: 2 additions & 3 deletions src/ngAnimate/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
/* global angularAnimateModule: true,

$$BodyProvider,
$$rAFMutexFactory,
$$AnimateAsyncRunFactory,
$$AnimateChildrenDirective,
$$AnimateRunnerFactory,
$$AnimateQueueProvider,
Expand Down Expand Up @@ -745,9 +745,8 @@ angular.module('ngAnimate', [])

.directive('ngAnimateChildren', $$AnimateChildrenDirective)

.factory('$$rAFMutex', $$rAFMutexFactory)

.factory('$$AnimateRunner', $$AnimateRunnerFactory)
.factory('$$animateAsyncRun', $$AnimateAsyncRunFactory)

.provider('$$animateQueue', $$AnimateQueueProvider)
.provider('$$animation', $$AnimationProvider)
Expand Down
63 changes: 44 additions & 19 deletions src/ngMock/angular-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -763,25 +763,50 @@ angular.mock.animate = angular.module('ngAnimateMock', ['ng'])
return reflowFn;
});

$provide.decorator('$animate', ['$delegate', '$timeout', '$browser', '$$rAF', '$$forceReflow',
function($delegate, $timeout, $browser, $$rAF, $$forceReflow) {
$provide.factory('$$animateAsyncRun', function() {
var queue = [];
var queueFn = function() {
return function(fn) {
queue.push(fn);
};
};
queueFn.flush = function() {
if (queue.length === 0) return false;

for (var i = 0; i < queue.length; i++) {
queue[i]();
}
queue = [];

return true;
};
return queueFn;
});

$provide.decorator('$animate', ['$delegate', '$timeout', '$browser', '$$rAF', '$$forceReflow', '$$animateAsyncRun',
function($delegate, $timeout, $browser, $$rAF, $$forceReflow, $$animateAsyncRun) {

var animate = {
queue: [],
cancel: $delegate.cancel,
on: $delegate.on,
off: $delegate.off,
pin: $delegate.pin,
get reflows() {
return $$forceReflow.totalReflows;
},
enabled: $delegate.enabled,
triggerCallbackEvents: function() {
$$rAF.flush();
},
triggerCallbackPromise: function() {
$timeout.flush(0);
},
triggerCallbacks: function() {
this.triggerCallbackEvents();
this.triggerCallbackPromise();
flush: function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't you want to document the flush method as public api?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's do this one in a follow up commit later this week. That way we can have a page in the guide that explains animation testing.

var rafsFlushed = false;
if ($$rAF.queue.length) {
$$rAF.flush();
rafsFlushed = true;
}

var animatorsFlushed = $$animateAsyncRun.flush();
if (!rafsFlushed && !animatorsFlushed) {
throw new Error('No pending animations ready to be closed or flushed');
}
}
};

Expand Down Expand Up @@ -1733,28 +1758,28 @@ angular.mock.$TimeoutDecorator = ['$delegate', '$browser', function($delegate, $
}];

angular.mock.$RAFDecorator = ['$delegate', function($delegate) {
var queue = [];
var rafFn = function(fn) {
var index = queue.length;
queue.push(fn);
var index = rafFn.queue.length;
rafFn.queue.push(fn);
return function() {
queue.splice(index, 1);
rafFn.queue.splice(index, 1);
};
};

rafFn.queue = [];
rafFn.supported = $delegate.supported;

rafFn.flush = function() {
if (queue.length === 0) {
if (rafFn.queue.length === 0) {
throw new Error('No rAF callbacks present');
}

var length = queue.length;
var length = rafFn.queue.length;
for (var i = 0; i < length; i++) {
queue[i]();
rafFn.queue[i]();
}

queue = queue.slice(i);
rafFn.queue = rafFn.queue.slice(i);
};

return rafFn;
Expand Down
4 changes: 2 additions & 2 deletions test/ng/directive/ngClassSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ describe('ngClass animations', function() {
};
});
});
inject(function($compile, $rootScope, $browser, $rootElement, $animate, $timeout, $$body, $$rAF) {
inject(function($compile, $rootScope, $browser, $rootElement, $animate, $timeout, $$body) {
$animate.enabled(true);

$rootScope.val = 'crazy';
Expand All @@ -488,7 +488,7 @@ describe('ngClass animations', function() {
expect(enterComplete).toBe(false);

$rootScope.$digest();
$$rAF.flush();
$animate.flush();
$rootScope.$digest();

expect(element.hasClass('crazy')).toBe(true);
Expand Down
18 changes: 10 additions & 8 deletions test/ng/directive/ngIncludeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,9 +428,11 @@ describe('ngInclude', function() {
});

expect(autoScrollSpy).not.toHaveBeenCalled();
expect($animate.queue.shift().event).toBe('enter');
$animate.triggerCallbacks();

$animate.flush();
$rootScope.$digest();

expect($animate.queue.shift().event).toBe('enter');
expect(autoScrollSpy).toHaveBeenCalledOnce();
}));

Expand All @@ -446,7 +448,6 @@ describe('ngInclude', function() {
});

expect($animate.queue.shift().event).toBe('enter');
$animate.triggerCallbacks();

$rootScope.$apply(function() {
$rootScope.tpl = 'another.html';
Expand All @@ -455,7 +456,6 @@ describe('ngInclude', function() {

expect($animate.queue.shift().event).toBe('leave');
expect($animate.queue.shift().event).toBe('enter');
$animate.triggerCallbacks();

$rootScope.$apply(function() {
$rootScope.tpl = 'template.html';
Expand All @@ -464,7 +464,9 @@ describe('ngInclude', function() {

expect($animate.queue.shift().event).toBe('leave');
expect($animate.queue.shift().event).toBe('enter');
$animate.triggerCallbacks();

$animate.flush();
$rootScope.$digest();

expect(autoScrollSpy).toHaveBeenCalled();
expect(autoScrollSpy.callCount).toBe(3);
Expand All @@ -480,7 +482,6 @@ describe('ngInclude', function() {
});

expect($animate.queue.shift().event).toBe('enter');
$animate.triggerCallbacks();
expect(autoScrollSpy).not.toHaveBeenCalled();
}));

Expand All @@ -496,7 +497,6 @@ describe('ngInclude', function() {
});

expect($animate.queue.shift().event).toBe('enter');
$animate.triggerCallbacks();

$rootScope.$apply(function() {
$rootScope.tpl = 'template.html';
Expand All @@ -518,7 +518,9 @@ describe('ngInclude', function() {

$rootScope.$apply("tpl = 'template.html'");
expect($animate.queue.shift().event).toBe('enter');
$animate.triggerCallbacks();

$animate.flush();
$rootScope.$digest();

expect(autoScrollSpy).toHaveBeenCalledOnce();
}
Expand Down
4 changes: 2 additions & 2 deletions test/ng/directive/ngRepeatSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1462,7 +1462,7 @@ describe('ngRepeat animations', function() {
}));

it('should not change the position of the block that is being animated away via a leave animation',
inject(function($compile, $rootScope, $animate, $document, $window, $sniffer, $timeout, $$rAF) {
inject(function($compile, $rootScope, $animate, $document, $window, $sniffer, $timeout) {
if (!$sniffer.transitions) return;

var item;
Expand All @@ -1487,7 +1487,7 @@ describe('ngRepeat animations', function() {
$rootScope.$digest();

expect(element.text()).toBe('123'); // the original order should be preserved
$$rAF.flush();
$animate.flush();
$timeout.flush(1500); // 1s * 1.5 closing buffer
expect(element.text()).toBe('13');
} finally {
Expand Down
40 changes: 0 additions & 40 deletions test/ng/rafSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,46 +31,6 @@ describe('$$rAF', function() {
expect(present).toBe(true);
}));

it('should only consume only one RAF if multiple async functions are registered before the first frame kicks in', inject(function($$rAF) {
if (!$$rAF.supported) return;

//we need to create our own injector to work around the ngMock overrides
var rafLog = [];
var injector = createInjector(['ng', function($provide) {
$provide.value('$window', {
location: window.location,
history: window.history,
webkitRequestAnimationFrame: function(fn) {
rafLog.push(fn);
}
});
}]);

$$rAF = injector.get('$$rAF');

var log = [];
function logFn() {
log.push(log.length);
}

$$rAF(logFn);
$$rAF(logFn);
$$rAF(logFn);

expect(log).toEqual([]);
expect(rafLog.length).toBe(1);

rafLog[0]();

expect(log).toEqual([0,1,2]);
expect(rafLog.length).toBe(1);

$$rAF(logFn);

expect(log).toEqual([0,1,2]);
expect(rafLog.length).toBe(2);
}));

describe('$timeout fallback', function() {
it("it should use a $timeout incase native rAF isn't suppored", function() {
var timeoutSpy = jasmine.createSpy('callback');
Expand Down
Loading