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

Commit c4489eb

Browse files
committed
revert: fix($animateCss): remove animation end event listeners on close
This reverts commit c98e08f. This commit was identified as incompatible with ng-material at Google and is causing broken builds there. Proper fix to be investigated once the immediate regression is addressed.
1 parent 7caf913 commit c4489eb

File tree

3 files changed

+37
-145
lines changed

3 files changed

+37
-145
lines changed

src/ngAnimate/animateCss.js

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,6 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
474474
var maxDelayTime;
475475
var maxDuration;
476476
var maxDurationTime;
477-
var startTime;
478-
var events = [];
479477

480478
if (options.duration === 0 || (!$sniffer.animations && !$sniffer.transitions)) {
481479
return closeAndReturnNoopAnimator();
@@ -749,11 +747,6 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
749747
options.onDone();
750748
}
751749

752-
// Remove the transitionend / animationend listener(s)
753-
if (events) {
754-
element.off(events.join(' '), onAnimationProgress);
755-
}
756-
757750
// if the preparation function fails then the promise is not setup
758751
if (runner) {
759752
runner.complete(!rejected);
@@ -789,37 +782,15 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
789782
};
790783
}
791784

792-
function onAnimationProgress(event) {
793-
event.stopPropagation();
794-
var ev = event.originalEvent || event;
795-
var timeStamp = ev.$manualTimeStamp || ev.timeStamp || Date.now();
796-
797-
/* Firefox (or possibly just Gecko) likes to not round values up
798-
* when a ms measurement is used for the animation */
799-
var elapsedTime = parseFloat(ev.elapsedTime.toFixed(ELAPSED_TIME_MAX_DECIMAL_PLACES));
800-
801-
/* $manualTimeStamp is a mocked timeStamp value which is set
802-
* within browserTrigger(). This is only here so that tests can
803-
* mock animations properly. Real events fallback to event.timeStamp,
804-
* or, if they don't, then a timeStamp is automatically created for them.
805-
* We're checking to see if the timeStamp surpasses the expected delay,
806-
* but we're using elapsedTime instead of the timeStamp on the 2nd
807-
* pre-condition since animationPauseds sometimes close off early */
808-
if (Math.max(timeStamp - startTime, 0) >= maxDelayTime && elapsedTime >= maxDuration) {
809-
// we set this flag to ensure that if the transition is paused then, when resumed,
810-
// the animation will automatically close itself since transitions cannot be paused.
811-
animationCompleted = true;
812-
close();
813-
}
814-
}
815-
816785
function start() {
817786
if (animationClosed) return;
818787
if (!node.parentNode) {
819788
close();
820789
return;
821790
}
822791

792+
var startTime, events = [];
793+
823794
// even though we only pause keyframe animations here the pause flag
824795
// will still happen when transitions are used. Only the transition will
825796
// not be paused since that is not possible. If the animation ends when
@@ -982,6 +953,30 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
982953
element.removeData(ANIMATE_TIMER_KEY);
983954
}
984955
}
956+
957+
function onAnimationProgress(event) {
958+
event.stopPropagation();
959+
var ev = event.originalEvent || event;
960+
var timeStamp = ev.$manualTimeStamp || ev.timeStamp || Date.now();
961+
962+
/* Firefox (or possibly just Gecko) likes to not round values up
963+
* when a ms measurement is used for the animation */
964+
var elapsedTime = parseFloat(ev.elapsedTime.toFixed(ELAPSED_TIME_MAX_DECIMAL_PLACES));
965+
966+
/* $manualTimeStamp is a mocked timeStamp value which is set
967+
* within browserTrigger(). This is only here so that tests can
968+
* mock animations properly. Real events fallback to event.timeStamp,
969+
* or, if they don't, then a timeStamp is automatically created for them.
970+
* We're checking to see if the timeStamp surpasses the expected delay,
971+
* but we're using elapsedTime instead of the timeStamp on the 2nd
972+
* pre-condition since animations sometimes close off early */
973+
if (Math.max(timeStamp - startTime, 0) >= maxDelayTime && elapsedTime >= maxDuration) {
974+
// we set this flag to ensure that if the transition is paused then, when resumed,
975+
// the animation will automatically close itself since transitions cannot be paused.
976+
animationCompleted = true;
977+
close();
978+
}
979+
}
985980
}
986981
};
987982
}];

test/ngAnimate/.jshintrc

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@
88
"applyAnimationStyles": false,
99
"applyAnimationFromStyles": false,
1010
"applyAnimationToStyles": false,
11-
"applyAnimationClassesFactory": false,
12-
"TRANSITIONEND_EVENT": false,
13-
"TRANSITION_PROP": false,
14-
"ANIMATION_PROP": false,
15-
"ANIMATIONEND_EVENT": false
11+
"applyAnimationClassesFactory": false
1612
}
1713
}

test/ngAnimate/animateCssSpec.js

Lines changed: 10 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,6 @@ describe("ngAnimate $animateCss", function() {
1212
: expect(className).not.toMatch(regex);
1313
}
1414

15-
function keyframeProgress(element, duration, delay) {
16-
browserTrigger(element, 'animationend',
17-
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
18-
}
19-
20-
function transitionProgress(element, duration, delay) {
21-
browserTrigger(element, 'transitionend',
22-
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
23-
}
24-
2515
var fakeStyle = {
2616
color: 'blue'
2717
};
@@ -365,6 +355,16 @@ describe("ngAnimate $animateCss", function() {
365355
assert.toHaveClass('ng-enter-active');
366356
}
367357

358+
function keyframeProgress(element, duration, delay) {
359+
browserTrigger(element, 'animationend',
360+
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
361+
}
362+
363+
function transitionProgress(element, duration, delay) {
364+
browserTrigger(element, 'transitionend',
365+
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
366+
}
367+
368368
beforeEach(inject(function($rootElement, $document) {
369369
element = jqLite('<div></div>');
370370
$rootElement.append(element);
@@ -1404,105 +1404,6 @@ describe("ngAnimate $animateCss", function() {
14041404
expect(count.stagger).toBe(2);
14051405
}));
14061406
});
1407-
1408-
describe('transitionend/animationend event listeners', function() {
1409-
var element, elementOnSpy, elementOffSpy, progress;
1410-
1411-
function setStyles(event) {
1412-
switch (event) {
1413-
case TRANSITIONEND_EVENT:
1414-
ss.addRule('.ng-enter', 'transition: 10s linear all;');
1415-
progress = transitionProgress;
1416-
break;
1417-
case ANIMATIONEND_EVENT:
1418-
ss.addRule('.ng-enter', '-webkit-animation: animation 10s;' +
1419-
'animation: animation 10s;');
1420-
progress = keyframeProgress;
1421-
break;
1422-
}
1423-
}
1424-
1425-
beforeEach(inject(function($rootElement, $document) {
1426-
element = jqLite('<div></div>');
1427-
$rootElement.append(element);
1428-
jqLite($document[0].body).append($rootElement);
1429-
1430-
elementOnSpy = spyOn(element, 'on').andCallThrough();
1431-
elementOffSpy = spyOn(element, 'off').andCallThrough();
1432-
}));
1433-
1434-
they('should remove the $prop event listeners on cancel',
1435-
[TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
1436-
inject(function($animateCss) {
1437-
1438-
setStyles(event);
1439-
1440-
var animator = $animateCss(element, {
1441-
event: 'enter',
1442-
structural: true
1443-
});
1444-
1445-
var runner = animator.start();
1446-
triggerAnimationStartFrame();
1447-
1448-
expect(elementOnSpy).toHaveBeenCalledOnce();
1449-
expect(elementOnSpy.mostRecentCall.args[0]).toBe(event);
1450-
1451-
runner.cancel();
1452-
1453-
expect(elementOffSpy).toHaveBeenCalledOnce();
1454-
expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
1455-
});
1456-
});
1457-
1458-
they("should remove the $prop event listener when the animation is closed",
1459-
[TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
1460-
inject(function($animateCss) {
1461-
1462-
setStyles(event);
1463-
1464-
var animator = $animateCss(element, {
1465-
event: 'enter',
1466-
structural: true
1467-
});
1468-
1469-
var runner = animator.start();
1470-
triggerAnimationStartFrame();
1471-
1472-
expect(elementOnSpy).toHaveBeenCalledOnce();
1473-
expect(elementOnSpy.mostRecentCall.args[0]).toBe(event);
1474-
1475-
progress(element, 10);
1476-
1477-
expect(elementOffSpy).toHaveBeenCalledOnce();
1478-
expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
1479-
});
1480-
});
1481-
1482-
they("should remove the $prop event listener when the closing timeout occurs",
1483-
[TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
1484-
inject(function($animateCss, $timeout) {
1485-
1486-
setStyles(event);
1487-
1488-
var animator = $animateCss(element, {
1489-
event: 'enter',
1490-
structural: true
1491-
});
1492-
1493-
animator.start();
1494-
triggerAnimationStartFrame();
1495-
1496-
expect(elementOnSpy).toHaveBeenCalledOnce();
1497-
expect(elementOnSpy.mostRecentCall.args[0]).toBe(event);
1498-
1499-
$timeout.flush(15000);
1500-
1501-
expect(elementOffSpy).toHaveBeenCalledOnce();
1502-
expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
1503-
});
1504-
});
1505-
});
15061407
});
15071408

15081409
it('should avoid applying the same cache to an element a follow-up animation is run on the same element',

0 commit comments

Comments
 (0)