-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix($animate): only block keyframes if a stagger is set to occur #6459
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1247,7 +1247,11 @@ angular.module('ngAnimate', ['ng']) | |
if(transitionDuration > 0) { | ||
blockTransitions(element, className, isCurrentlyAnimating); | ||
} | ||
if(animationDuration > 0) { | ||
|
||
//keyframes don't need to be blocked unless staggered and if a stagger | ||
//is present then only the items that are of current index > 0 are blocked (since | ||
//the element at index==0 will never have a stagger delay at all). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you rewrite this to make it easier to follow, right now you jump between what it "doesn't need to do" and what it "should do" and what it "actually does" - kind of confusing :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||
if(animationDuration > 0 && stagger.animationDelay > 0 && stagger.animationDuration === 0) { | ||
blockKeyframeAnimations(element); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -841,6 +841,49 @@ describe("ngAnimate", function() { | |
})); | ||
|
||
|
||
it("should block and unblock keyframe animations when a stagger animation kicks in while skipping the first element", | ||
inject(function($animate, $rootScope, $compile, $sniffer, $timeout, $document, $rootElement) { | ||
|
||
if(!$sniffer.animations) return; | ||
|
||
$animate.enabled(true); | ||
|
||
ss.addRule('.blocked-animation.ng-enter', | ||
'-webkit-animation:my_animation 1s;' + | ||
'animation:my_animation 1s;'); | ||
|
||
ss.addRule('.blocked-animation.ng-enter-stagger', | ||
'-webkit-animation-delay:0.2s;' + | ||
'animation-delay:0.2s;'); | ||
|
||
var container = $compile(html('<div></div>'))($rootScope); | ||
|
||
var elements = []; | ||
for(var i = 0; i < 4; i++) { | ||
var newScope = $rootScope.$new(); | ||
var element = $compile('<div class="blocked-animation"></div>')(newScope); | ||
$animate.enter(element, container); | ||
elements.push(element); | ||
}; | ||
|
||
$rootScope.$digest(); | ||
|
||
expect(elements[0].attr('style')).toBeFalsy(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't use toBeFalsy/toBeTruthy, use toBeUndefined instead if possible There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||
expect(elements[1].attr('style')).toMatch(/animation:.*?none/); | ||
expect(elements[2].attr('style')).toMatch(/animation:.*?none/); | ||
expect(elements[3].attr('style')).toMatch(/animation:.*?none/); | ||
|
||
$animate.triggerReflow(); | ||
|
||
expect(elements[0].attr('style')).toBeFalsy(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||
expect(elements[1].attr('style')).not.toMatch(/animation:.*?none/); | ||
expect(elements[1].attr('style')).toMatch(/animation-delay: 0.2\d*s/); | ||
expect(elements[2].attr('style')).not.toMatch(/animation:.*?none/); | ||
expect(elements[2].attr('style')).toMatch(/animation-delay: 0.4\d*s/); | ||
expect(elements[3].attr('style')).not.toMatch(/animation:.*?none/); | ||
expect(elements[3].attr('style')).toMatch(/animation-delay: 0.6\d*s/); | ||
})); | ||
|
||
it("should stagger items when multiple animation durations/delays are defined", | ||
inject(function($animate, $rootScope, $compile, $sniffer, $timeout, $document, $rootElement) { | ||
|
||
|
@@ -3013,7 +3056,7 @@ describe("ngAnimate", function() { | |
})); | ||
|
||
|
||
it('should block and unblock keyframe animations around the reflow operation', | ||
it('should not block keyframe animations around the reflow operation', | ||
inject(function($rootScope, $compile, $rootElement, $document, $animate, $sniffer, $timeout) { | ||
|
||
if (!$sniffer.animations) return; | ||
|
@@ -3032,15 +3075,19 @@ describe("ngAnimate", function() { | |
|
||
$animate.addClass(element, 'trigger-class'); | ||
|
||
expect(node.style[animationKey]).toContain('none'); | ||
expect(node.style[animationKey]).not.toContain('none'); | ||
|
||
$animate.triggerReflow(); | ||
|
||
expect(node.style[animationKey]).not.toContain('none'); | ||
|
||
browserTrigger(element, 'animationend', { timeStamp: Date.now() + 1000, elapsedTime: 1 }); | ||
|
||
expect(node.style[animationKey]).not.toContain('none'); | ||
})); | ||
|
||
|
||
it('should block and unblock keyframe animations before the followup JS animation occurs', function() { | ||
it('should not block keyframe animations at anytime before a followup JS animation occurs', function() { | ||
module(function($animateProvider) { | ||
$animateProvider.register('.special', function($sniffer, $window) { | ||
var prop = $sniffer.vendorPrefix == 'Webkit' ? 'WebkitAnimation' : 'animation'; | ||
|
@@ -3076,8 +3123,8 @@ describe("ngAnimate", function() { | |
|
||
var prop = $sniffer.vendorPrefix == 'Webkit' ? 'WebkitAnimation' : 'animation'; | ||
|
||
expect(element[0].style[prop]).toContain('none'); | ||
expect($window.getComputedStyle(element[0])[prop + 'Duration']).toBe('0s'); | ||
expect(element[0].style[prop]).not.toContain('none'); | ||
expect($window.getComputedStyle(element[0])[prop + 'Duration']).toBe('1s'); | ||
|
||
$animate.triggerReflow(); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"don't need to" or "should not" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the text was fully changed.