fix(ngAnimate): close follow-up class-based animations when the same class is added/removed when removed/added #11755
Conversation
…class is added/removed when removed/added This patch ensures that if the same CSS class is added/removed within a follow-up digest then the previous class-based animation is cancelled beforehand. Closes angular#11717
| expect(closed).toBe(true); | ||
| })); | ||
|
|
||
| it('should cancel the previously running addClass animation if a follow-up removeClass animation is using the same class value', |
There was a problem hiding this comment.
aren't descriptions of those 2 tests swapped?
There was a problem hiding this comment.
I think you are right @mzgol.
| var closed = false; | ||
| runner.done(function(status) { | ||
| closed = true; | ||
| }); |
There was a problem hiding this comment.
Is there a reason why you are not using spies for this kind of thing? I find it much more readable:
var doneHandler = jasmine.createSpy('addClass done');
runner.done(doneHandler);
$$rAF.flush();
expect(doneHandler).not.toHaveBeenCalled();
$animate.removeClass(element, 'active-class');
$rootScope.$digest();
expect(doneHandler).toHaveBeenCalled();
|
Other than comments about descriptions and spies, this LGTM. |
|
I rebased this to merge but it causes another spec to fail at this line: angular.js/test/ngAnimate/animateSpec.js Line 1053 in 213c2a7 My suggested fix is to clear the animation state if a newly merged animation is invalid: angular.js/src/ngAnimate/animateQueue.js Line 361 in 213c2a7 So this bit of code would look like: if (!isValidAnimation) {
close();
clearElementAnimationState(element);
return runner;
} |
|
Closing in favour of #11927 |
|
Landed as db246eb |
This patch ensures that if the same CSS class is added/removed within a
follow-up digest then the previous class-based animation is cancelled
beforehand.
Closes #11717