Skip to content

Commit b89584d

Browse files
matskomatsko
committed
fix($animate): avoid hanging animations if the active CSS transition class is missing
Closes angular#4732 Closes angular#4490
1 parent 41a2d5b commit b89584d

3 files changed

Lines changed: 103 additions & 17 deletions

File tree

‎css/angular.css‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,14 @@
99
ng\:form {
1010
display: block;
1111
}
12+
13+
/* The styles below ensure that the CSS transition will ALWAYS
14+
* animate and close. A nasty bug occurs with CSS transitions where
15+
* when the active class isn't set, or if the active class doesn't
16+
* contain any styles to transition to, then, if ngAnimate is used,
17+
* it will appear as if the webpage is broken due to the forever hanging
18+
* animations. The clip (!ie) and zoom (ie) CSS properties are used
19+
* since they trigger a transition without making the browser
20+
* animate anything and they're both highly underused CSS properties */
21+
.ng-animate { clip:rect(1px, auto, auto, 0); -ms-zoom:1.0001; }
22+
.ng-animate-active { clip:rect(0, auto, auto, 0); -ms-zoom:1; }

‎src/ngAnimate/animate.js‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -790,16 +790,22 @@ angular.module('ngAnimate', ['ng'])
790790
if(!data) {
791791
var transitionDuration = 0, transitionDelay = 0,
792792
animationDuration = 0, animationDelay = 0,
793-
transitionDelayStyle, animationDelayStyle;
793+
transitionDelayStyle, animationDelayStyle,
794+
transitionDurationStyle,
795+
transitionPropertyStyle;
794796

795797
//we want all the styles defined before and after
796798
forEach(element, function(element) {
797799
if (element.nodeType == ELEMENT_NODE) {
798800
var elementStyles = $window.getComputedStyle(element) || {};
799801

800-
transitionDuration = Math.max(parseMaxTime(elementStyles[transitionProp + durationKey]), transitionDuration);
802+
transitionDurationStyle = elementStyles[transitionProp + durationKey];
803+
804+
transitionDuration = Math.max(parseMaxTime(transitionDurationStyle), transitionDuration);
801805

802806
if(!onlyCheckTransition) {
807+
transitionPropertyStyle = elementStyles[transitionProp + propertyKey];
808+
803809
transitionDelayStyle = elementStyles[transitionProp + delayKey];
804810

805811
transitionDelay = Math.max(parseMaxTime(transitionDelayStyle), transitionDelay);
@@ -820,12 +826,14 @@ angular.module('ngAnimate', ['ng'])
820826
});
821827
data = {
822828
total : 0,
829+
transitionPropertyStyle: transitionPropertyStyle,
830+
transitionDurationStyle: transitionDurationStyle,
823831
transitionDelayStyle: transitionDelayStyle,
824-
transitionDelay : transitionDelay,
825-
transitionDuration : transitionDuration,
832+
transitionDelay: transitionDelay,
833+
transitionDuration: transitionDuration,
826834
animationDelayStyle: animationDelayStyle,
827-
animationDelay : animationDelay,
828-
animationDuration : animationDuration
835+
animationDelay: animationDelay,
836+
animationDuration: animationDuration
829837
};
830838
if(cacheKey) {
831839
lookupCache[cacheKey] = data;
@@ -896,7 +904,7 @@ angular.module('ngAnimate', ['ng'])
896904
node.style[transitionProp + propertyKey] = 'none';
897905
}
898906

899-
var activeClassName = '';
907+
var activeClassName = 'ng-animate-active ';
900908
forEach(className.split(' '), function(klass, i) {
901909
activeClassName += (i > 0 ? ' ' : '') + klass + '-active';
902910
});
@@ -910,25 +918,38 @@ angular.module('ngAnimate', ['ng'])
910918
return;
911919
}
912920

921+
var applyFallbackStyle, style = '';
913922
if(timings.transitionDuration > 0) {
914923
node.style[transitionProp + propertyKey] = '';
924+
925+
var propertyStyle = timings.transitionPropertyStyle;
926+
if(propertyStyle.indexOf('all') == -1) {
927+
applyFallbackStyle = true;
928+
var fallbackProperty = $sniffer.msie ? '-ms-zoom' : 'clip';
929+
style += prefix + 'transition-property: ' + propertyStyle + ', ' + fallbackProperty + '; ';
930+
style += prefix + 'transition-duration: ' + timings.transitionDurationStyle + ', ' + timings.transitionDuration + 's; ';
931+
}
915932
}
916933

917934
if(ii > 0) {
918-
var staggerStyle = '';
919935
if(stagger.transitionDelay > 0 && stagger.transitionDuration === 0) {
920-
staggerStyle += prefix + 'transition-delay: ' +
921-
prepareStaggerDelay(timings.transitionDelayStyle, stagger.transitionDelay, ii) + '; ';
936+
var delayStyle = timings.transitionDelayStyle;
937+
if(applyFallbackStyle) {
938+
delayStyle += ', ' + timings.transitionDelay + 's';
939+
}
940+
941+
style += prefix + 'transition-delay: ' +
942+
prepareStaggerDelay(delayStyle, stagger.transitionDelay, ii) + '; ';
922943
}
923944

924945
if(stagger.animationDelay > 0 && stagger.animationDuration === 0) {
925-
staggerStyle += prefix + 'animation-delay: ' +
926-
prepareStaggerDelay(timings.animationDelayStyle, stagger.animationDelay, ii) + '; ';
946+
style += prefix + 'animation-delay: ' +
947+
prepareStaggerDelay(timings.animationDelayStyle, stagger.animationDelay, ii) + '; ';
927948
}
949+
}
928950

929-
if(staggerStyle.length > 0) {
930-
formerStyle = applyStyle(node, staggerStyle);
931-
}
951+
if(style.length > 0) {
952+
formerStyle = applyStyle(node, style);
932953
}
933954

934955
element.addClass(activeClassName);

‎test/ngAnimate/animateSpec.js‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,59 @@ describe("ngAnimate", function() {
784784
});
785785

786786
describe("Transitions", function() {
787+
it("should only apply the fallback transition property unless all properties are being animated",
788+
inject(function($compile, $rootScope, $animate, $sniffer, $timeout) {
789+
790+
if (!$sniffer.animations) return;
791+
792+
ss.addRule('.all.ng-enter', '-webkit-transition:1s linear all;' +
793+
'transition:1s linear all');
794+
795+
ss.addRule('.one.ng-enter', '-webkit-transition:1s linear color;' +
796+
'transition:1s linear color');
797+
798+
var element = $compile('<div></div>')($rootScope);
799+
var child = $compile('<div class="all">...</div>')($rootScope);
800+
$rootElement.append(element);
801+
var body = jqLite($document[0].body);
802+
body.append($rootElement);
803+
804+
$animate.enter(child, element);
805+
$rootScope.$digest();
806+
$timeout.flush();
807+
808+
expect(child.attr('style') || '').not.toContain('transition-property');
809+
expect(child.hasClass('ng-animate')).toBe(true);
810+
expect(child.hasClass('ng-animate-active')).toBe(true);
811+
812+
browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 });
813+
$timeout.flush();
814+
815+
expect(child.hasClass('ng-animate')).toBe(false);
816+
expect(child.hasClass('ng-animate-active')).toBe(false);
817+
818+
child.remove();
819+
820+
var child2 = $compile('<div class="one">...</div>')($rootScope);
821+
822+
$animate.enter(child2, element);
823+
$rootScope.$digest();
824+
$timeout.flush();
825+
826+
//IE removes the -ms- prefix when placed on the style
827+
var fallbackProperty = $sniffer.msie ? 'zoom' : 'clip';
828+
var regExp = new RegExp("transition-property:\\s+color\\s*,\\s*" + fallbackProperty + "\\s*;");
829+
expect(child2.attr('style') || '').toMatch(regExp);
830+
expect(child2.hasClass('ng-animate')).toBe(true);
831+
expect(child2.hasClass('ng-animate-active')).toBe(true);
832+
833+
browserTrigger(child2,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 });
834+
$timeout.flush();
835+
836+
expect(child2.hasClass('ng-animate')).toBe(false);
837+
expect(child2.hasClass('ng-animate-active')).toBe(false);
838+
}));
839+
787840
it("should skip transitions if disabled and run when enabled",
788841
inject(function($animate, $rootScope, $compile, $sniffer, $timeout) {
789842

@@ -1013,7 +1066,7 @@ describe("ngAnimate", function() {
10131066
$rootScope.$digest();
10141067
$timeout.flush();
10151068

1016-
expect(elements[0].attr('style')).toBeFalsy();
1069+
expect(elements[0].attr('style')).not.toContain('transition-delay');
10171070
expect(elements[1].attr('style')).toMatch(/transition-delay: 2\.1\d*s,\s*4\.1\d*s/);
10181071
expect(elements[2].attr('style')).toMatch(/transition-delay: 2\.2\d*s,\s*4\.2\d*s/);
10191072
expect(elements[3].attr('style')).toMatch(/transition-delay: 2\.3\d*s,\s*4\.3\d*s/);
@@ -1030,7 +1083,7 @@ describe("ngAnimate", function() {
10301083
ss.addRule('.ani.ng-enter, .ani.ng-leave',
10311084
'-webkit-animation:my_animation 1s 1s, your_animation 1s 2s;' +
10321085
'animation:my_animation 1s 1s, your_animation 1s 2s;' +
1033-
'-webkit-transition:1s linear all 0s;' +
1086+
'-webkit-transition:1s linear all 1s;' +
10341087
'transition:1s linear all 1s;');
10351088

10361089
ss.addRule('.ani.ng-enter-stagger, .ani.ng-leave-stagger',
@@ -1731,6 +1784,7 @@ describe("ngAnimate", function() {
17311784
expect(child.css(propertyKey)).toBe('background-color');
17321785
child.remove();
17331786

1787+
child = $compile('<div class="ani">...</div>')($rootScope);
17341788
child.attr('class','trans');
17351789
$animate.enter(child, element);
17361790
$rootScope.$digest();

0 commit comments

Comments
 (0)