From f1fda8f24b387db096892434e6e59bfcd9668645 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Wed, 18 Nov 2015 12:10:50 +0100 Subject: [PATCH 1/2] fix($animateCss): respect transition styles already on the element Previously, $animateCss wouldn't use transition styles that were on the element before the animation process started. Precisely, transition property, timing-function and delay were overwritten in the process. Closes #12656 Closes #13333 --- src/ngAnimate/.jshintrc | 2 + src/ngAnimate/animateCss.js | 59 +++++++++--------- src/ngAnimate/shared.js | 2 + test/ngAnimate/animateCssSpec.js | 100 +++++++++++++++++++++++++++---- 4 files changed, 119 insertions(+), 44 deletions(-) diff --git a/src/ngAnimate/.jshintrc b/src/ngAnimate/.jshintrc index 9ab8bbe1b890..7079fddc9eee 100644 --- a/src/ngAnimate/.jshintrc +++ b/src/ngAnimate/.jshintrc @@ -33,6 +33,8 @@ "TRANSITION_DURATION_PROP": false, "TRANSITION_DELAY_PROP": false, + "TRANSITION_TIMING_PROP": false, + "TRANSITION_PROPERTY_PROP": false, "TRANSITION_PROP": false, "PROPERTY_KEY": false, "DURATION_KEY": false, diff --git a/src/ngAnimate/animateCss.js b/src/ngAnimate/animateCss.js index ba5e34773cf6..c790905e916b 100644 --- a/src/ngAnimate/animateCss.js +++ b/src/ngAnimate/animateCss.js @@ -223,12 +223,13 @@ var ELAPSED_TIME_MAX_DECIMAL_PLACES = 3; var CLOSING_TIME_BUFFER = 1.5; var DETECT_CSS_PROPERTIES = { - transitionDuration: TRANSITION_DURATION_PROP, - transitionDelay: TRANSITION_DELAY_PROP, - transitionProperty: TRANSITION_PROP + PROPERTY_KEY, - animationDuration: ANIMATION_DURATION_PROP, - animationDelay: ANIMATION_DELAY_PROP, - animationIterationCount: ANIMATION_PROP + ANIMATION_ITERATION_COUNT_KEY + transitionDuration: TRANSITION_DURATION_PROP, + transitionDelay: TRANSITION_DELAY_PROP, + transitionProperty: TRANSITION_PROP + PROPERTY_KEY, + transitionTimingFunction: TRANSITION_PROP + TIMING_KEY, + animationDuration: ANIMATION_DURATION_PROP, + animationDelay: ANIMATION_DELAY_PROP, + animationIterationCount: ANIMATION_PROP + ANIMATION_ITERATION_COUNT_KEY }; var DETECT_STAGGER_CSS_PROPERTIES = { @@ -292,14 +293,14 @@ function truthyTimingValue(val) { return val === 0 || val != null; } -function getCssTransitionDurationStyle(duration, applyOnlyDuration) { +function getCssTransitionStyle(timings, duration) { var style = TRANSITION_PROP; var value = duration + 's'; - if (applyOnlyDuration) { - style += DURATION_KEY; - } else { - value += ' linear all'; - } + + value += ' ' + timings[TRANSITION_TIMING_PROP]; + value += ' ' + timings[TRANSITION_PROPERTY_PROP]; + value += timings[TRANSITION_DELAY_PROP] ? ' ' + timings[TRANSITION_DELAY_PROP] + 's' : ''; + return [style, value]; } @@ -557,15 +558,6 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { temporaryStyles.push(transitionStyle); } - if (options.duration >= 0) { - applyOnlyDuration = node.style[TRANSITION_PROP].length > 0; - var durationStyle = getCssTransitionDurationStyle(options.duration, applyOnlyDuration); - - // we set the duration so that it will be picked up by getComputedStyle later - applyInlineStyle(node, durationStyle); - temporaryStyles.push(durationStyle); - } - if (options.keyframeStyle) { var keyframeStyle = [ANIMATION_PROP, options.keyframeStyle]; applyInlineStyle(node, keyframeStyle); @@ -578,8 +570,18 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { : gcsLookup.count(cacheKey) : 0; - var isFirst = itemIndex === 0; + var timings = computeTimings(node, fullClassName, cacheKey); + + if (options.duration > 0) { + // Duration in options overwrites duration set in style + timings.transitionDuration = options.duration; + } + var relativeDelay = timings.maxDelay; + maxDelay = Math.max(relativeDelay, 0); + maxDuration = timings.maxDuration; + + var isFirst = itemIndex === 0; // this is a pre-emptive way of forcing the setup classes to be added and applied INSTANTLY // without causing any combination of transitions to kick in. By adding a negative delay value // it forces the setup class' transition to end immediately. We later then remove the negative @@ -590,17 +592,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { blockTransitions(node, SAFE_FAST_FORWARD_DURATION_VALUE); } - var timings = computeTimings(node, fullClassName, cacheKey); - var relativeDelay = timings.maxDelay; - maxDelay = Math.max(relativeDelay, 0); - maxDuration = timings.maxDuration; - var flags = {}; flags.hasTransitions = timings.transitionDuration > 0; flags.hasAnimations = timings.animationDuration > 0; - flags.hasTransitionAll = flags.hasTransitions && timings.transitionProperty == 'all'; - flags.applyTransitionDuration = hasToStyles && ( - (flags.hasTransitions && !flags.hasTransitionAll) + flags.applyTransitionDuration = options.duration > 0 || hasToStyles && (flags.hasTransitions || (flags.hasAnimations && !flags.hasTransitions)); flags.applyAnimationDuration = options.duration && flags.hasAnimations; flags.applyTransitionDelay = truthyTimingValue(options.delay) && (flags.applyTransitionDuration || flags.hasTransitions); @@ -613,8 +608,7 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { if (flags.applyTransitionDuration) { flags.hasTransitions = true; timings.transitionDuration = maxDuration; - applyOnlyDuration = node.style[TRANSITION_PROP + PROPERTY_KEY].length > 0; - temporaryStyles.push(getCssTransitionDurationStyle(maxDuration, applyOnlyDuration)); + temporaryStyles.push(getCssTransitionStyle(timings, maxDuration)); } if (flags.applyAnimationDuration) { @@ -622,6 +616,7 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { timings.animationDuration = maxDuration; temporaryStyles.push(getCssKeyframeDurationStyle(maxDuration)); } + } if (maxDuration === 0 && !flags.recalculateTimingStyles) { diff --git a/src/ngAnimate/shared.js b/src/ngAnimate/shared.js index 308258ec78c0..3649119bcb47 100644 --- a/src/ngAnimate/shared.js +++ b/src/ngAnimate/shared.js @@ -68,6 +68,8 @@ var ANIMATION_DELAY_PROP = ANIMATION_PROP + DELAY_KEY; var ANIMATION_DURATION_PROP = ANIMATION_PROP + DURATION_KEY; var TRANSITION_DELAY_PROP = TRANSITION_PROP + DELAY_KEY; var TRANSITION_DURATION_PROP = TRANSITION_PROP + DURATION_KEY; +var TRANSITION_TIMING_PROP = TRANSITION_PROP + TIMING_KEY; +var TRANSITION_PROPERTY_PROP = TRANSITION_PROP + PROPERTY_KEY; var isPromiseLike = function(p) { return p && p.then ? true : false; diff --git a/test/ngAnimate/animateCssSpec.js b/test/ngAnimate/animateCssSpec.js index 74fa12fc4b75..444851de4a7b 100644 --- a/test/ngAnimate/animateCssSpec.js +++ b/test/ngAnimate/animateCssSpec.js @@ -634,6 +634,86 @@ describe("ngAnimate $animateCss", function() { keyframeProgress(element, 1, 20); assertAnimationComplete(true); })); + + it("should apply all transition shorthand properties that are already on the element", + inject(function($animateCss, $rootElement) { + + ss.addRule('.action', 'transition: color 1s cubic-bezier(0.25, 0.1, 0.25, 1) 5s;'); + element.addClass('action'); + + var options = { + to: { background: 'blue' } + }; + + var animator = $animateCss(element, options); + animator.start(); + triggerAnimationStartFrame(); + + expect(element.css('transition-duration')).toMatch('1s'); + expect(element.css('transition-delay')).toMatch('5s'); + expect(element.css('transition-property')).toMatch('color'); + expect(element.css('transition-timing-function')).toBe('cubic-bezier(0.25, 0.1, 0.25, 1)'); + })); + + it("should apply all explicit transition properties that are already on the element", + inject(function($animateCss, $rootElement) { + + ss.addRule('.action', 'transition-duration: 1s;' + + 'transition-timing-function: cubic-bezier(0.25, 0.1, 0.25, 1);' + + 'transition-property: color;' + + 'transition-delay: 5s'); + element.addClass('action'); + + var options = { + to: { background: 'blue' } + }; + + var animator = $animateCss(element, options); + animator.start(); + triggerAnimationStartFrame(); + + expect(element.css('transition-duration')).toMatch('1s'); + expect(element.css('transition-delay')).toMatch('5s'); + expect(element.css('transition-property')).toMatch('color'); + expect(element.css('transition-timing-function')).toBe('cubic-bezier(0.25, 0.1, 0.25, 1)'); + })); + + it("should use the default transition-property (spec: all) if none is supplied in shorthand", + inject(function($animateCss, $rootElement) { + + ss.addRule('.action', 'transition: 1s ease'); + element.addClass('action'); + + var options = { + to: { background: 'blue' } + }; + + var animator = $animateCss(element, options); + animator.start(); + triggerAnimationStartFrame(); + + expect(element.css('transition-property')).toBe('all'); + })); + + it("should use the default easing (spec: ease) if none is supplied in shorthand", + inject(function($animateCss, $rootElement) { + + ss.addRule('.action', 'transition: color 1s'); + element.addClass('action'); + + var options = { + to: { background: 'blue' } + }; + + var animator = $animateCss(element, options); + animator.start(); + triggerAnimationStartFrame(); + + // IE reports ease in cubic-bezier form + expect(element.css('transition-timing-function')).toBeOneOf('ease', 'cubic-bezier(0.25, 0.1, 0.25, 1)'); + })); + + }); describe("staggering", function() { @@ -2052,7 +2132,7 @@ describe("ngAnimate $animateCss", function() { var style = element.attr('style'); expect(style).toContain('3000s'); - expect(style).toContain('linear'); + expect(element.css('transition-timing-function')).toBeOneOf('ease', 'cubic-bezier(0.25, 0.1, 0.25, 1)'); })); it("should be applied to a CSS keyframe animation directly if keyframes are detected within the CSS class", @@ -2158,7 +2238,7 @@ describe("ngAnimate $animateCss", function() { expect(style).toMatch(/animation(?:-duration)?:\s*4s/); expect(element.css('transition-duration')).toMatch('4s'); expect(element.css('transition-property')).toMatch('all'); - expect(style).toContain('linear'); + expect(element.css('transition-timing-function')).toBeOneOf('linear', 'cubic-bezier(0, 0, 1, 1)'); })); }); @@ -2322,7 +2402,7 @@ describe("ngAnimate $animateCss", function() { var animator = $animateCss(element, options); expect(element.attr('style') || '').not.toContain('animation-delay'); - expect(element.attr('style') || '').not.toContain('transition-delay'); + expect(element.css('transition-delay')).toEqual('-2s'); expect(classSpy).not.toHaveBeenCalled(); //redefine the classSpy to assert that the delay values have been @@ -2500,10 +2580,9 @@ describe("ngAnimate $animateCss", function() { animator.start(); triggerAnimationStartFrame(); - var style = element.attr('style'); expect(element.css('transition-duration')).toMatch('4s'); expect(element.css('transition-property')).toMatch('color'); - expect(style).toContain('ease-in'); + expect(element.css('transition-timing-function')).toBeOneOf('ease-in', 'cubic-bezier(0.42, 0, 1, 1)'); })); it("should give priority to the provided delay value, but only update the delay style itself", @@ -2754,11 +2833,9 @@ describe("ngAnimate $animateCss", function() { animator.start(); triggerAnimationStartFrame(); - - var style = element.attr('style'); expect(element.css('transition-duration')).toMatch('2.5s'); expect(element.css('transition-property')).toMatch('all'); - expect(style).toContain('linear'); + expect(element.css('transition-timing-function')).toBeOneOf('ease', 'cubic-bezier(0.25, 0.1, 0.25, 1)'); })); it("should remove all inline transition styling when an animation completes", @@ -2889,7 +2966,7 @@ describe("ngAnimate $animateCss", function() { it("should apply a transition duration if the existing transition duration's property value is not 'all'", inject(function($animateCss, $rootElement) { - ss.addRule('.ng-enter', 'transition: 1s linear color'); + ss.addRule('.ng-enter', 'transition: 1s cubic-bezier(0.25, 0.1, 0.25, 1) color'); var emptyObject = {}; var options = { @@ -2903,10 +2980,9 @@ describe("ngAnimate $animateCss", function() { triggerAnimationStartFrame(); - var style = element.attr('style'); expect(element.css('transition-duration')).toMatch('1s'); - expect(element.css('transition-property')).toMatch('all'); - expect(style).toContain('linear'); + expect(element.css('transition-property')).toMatch('color'); + expect(element.css('transition-timing-function')).toBe('cubic-bezier(0.25, 0.1, 0.25, 1)'); })); it("should apply a transition duration and an animation duration if duration + styles options are provided for a matching keyframe animation", From 505c11a4a072c001570f8cc9378c7171bdbb33da Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Thu, 10 Dec 2015 12:46:11 +0100 Subject: [PATCH 2/2] refactor($animateCss): various improvements - since transition / animation styles are now set on the element even if the transition property is all, the condition for setting the transitionDuration can be simplified - explain why we test for cubic bezier and ease - rename a parameter --- src/ngAnimate/animateCss.js | 15 +++++++-------- test/ngAnimate/animateCssSpec.js | 20 ++++++++++++-------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/ngAnimate/animateCss.js b/src/ngAnimate/animateCss.js index c790905e916b..9cc2f3f4bab5 100644 --- a/src/ngAnimate/animateCss.js +++ b/src/ngAnimate/animateCss.js @@ -225,8 +225,8 @@ var CLOSING_TIME_BUFFER = 1.5; var DETECT_CSS_PROPERTIES = { transitionDuration: TRANSITION_DURATION_PROP, transitionDelay: TRANSITION_DELAY_PROP, - transitionProperty: TRANSITION_PROP + PROPERTY_KEY, - transitionTimingFunction: TRANSITION_PROP + TIMING_KEY, + transitionProperty: TRANSITION_PROPERTY_PROP, + transitionTimingFunction: TRANSITION_TIMING_PROP, animationDuration: ANIMATION_DURATION_PROP, animationDelay: ANIMATION_DELAY_PROP, animationIterationCount: ANIMATION_PROP + ANIMATION_ITERATION_COUNT_KEY @@ -293,13 +293,13 @@ function truthyTimingValue(val) { return val === 0 || val != null; } -function getCssTransitionStyle(timings, duration) { +function getCssTransitionStyle(styles, duration) { var style = TRANSITION_PROP; var value = duration + 's'; - value += ' ' + timings[TRANSITION_TIMING_PROP]; - value += ' ' + timings[TRANSITION_PROPERTY_PROP]; - value += timings[TRANSITION_DELAY_PROP] ? ' ' + timings[TRANSITION_DELAY_PROP] + 's' : ''; + value += ' ' + styles[TRANSITION_TIMING_PROP]; + value += ' ' + styles[TRANSITION_PROPERTY_PROP]; + value += styles[TRANSITION_DELAY_PROP] ? ' ' + styles[TRANSITION_DELAY_PROP] + 's' : ''; return [style, value]; } @@ -595,8 +595,7 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { var flags = {}; flags.hasTransitions = timings.transitionDuration > 0; flags.hasAnimations = timings.animationDuration > 0; - flags.applyTransitionDuration = options.duration > 0 || hasToStyles && (flags.hasTransitions - || (flags.hasAnimations && !flags.hasTransitions)); + flags.applyTransitionDuration = options.duration > 0 || hasToStyles && flags.hasTransitions; flags.applyAnimationDuration = options.duration && flags.hasAnimations; flags.applyTransitionDelay = truthyTimingValue(options.delay) && (flags.applyTransitionDuration || flags.hasTransitions); flags.applyAnimationDelay = truthyTimingValue(options.delay) && flags.hasAnimations; diff --git a/test/ngAnimate/animateCssSpec.js b/test/ngAnimate/animateCssSpec.js index 444851de4a7b..faef92d4b0f2 100644 --- a/test/ngAnimate/animateCssSpec.js +++ b/test/ngAnimate/animateCssSpec.js @@ -2,6 +2,10 @@ describe("ngAnimate $animateCss", function() { + // Firefox transforms all transition timing function values to their cubic bezier equivalents + var CUBIC_BEZIER_LINEAR_EQUIVALENT = 'cubic-bezier(0, 0, 1, 1)'; + var CUBIC_BEZIER_EASE_EQUIVALENT = 'cubic-bezier(0.25, 0.1, 0.25, 1)'; + beforeEach(module('ngAnimate')); beforeEach(module('ngAnimateMock')); @@ -710,7 +714,7 @@ describe("ngAnimate $animateCss", function() { triggerAnimationStartFrame(); // IE reports ease in cubic-bezier form - expect(element.css('transition-timing-function')).toBeOneOf('ease', 'cubic-bezier(0.25, 0.1, 0.25, 1)'); + expect(element.css('transition-timing-function')).toBeOneOf('ease', CUBIC_BEZIER_EASE_EQUIVALENT); })); @@ -2132,7 +2136,7 @@ describe("ngAnimate $animateCss", function() { var style = element.attr('style'); expect(style).toContain('3000s'); - expect(element.css('transition-timing-function')).toBeOneOf('ease', 'cubic-bezier(0.25, 0.1, 0.25, 1)'); + expect(element.css('transition-timing-function')).toBeOneOf('ease', CUBIC_BEZIER_EASE_EQUIVALENT); })); it("should be applied to a CSS keyframe animation directly if keyframes are detected within the CSS class", @@ -2238,7 +2242,7 @@ describe("ngAnimate $animateCss", function() { expect(style).toMatch(/animation(?:-duration)?:\s*4s/); expect(element.css('transition-duration')).toMatch('4s'); expect(element.css('transition-property')).toMatch('all'); - expect(element.css('transition-timing-function')).toBeOneOf('linear', 'cubic-bezier(0, 0, 1, 1)'); + expect(element.css('transition-timing-function')).toBeOneOf('linear', CUBIC_BEZIER_LINEAR_EQUIVALENT); })); }); @@ -2569,7 +2573,7 @@ describe("ngAnimate $animateCss", function() { inject(function($animateCss, $rootElement) { var options = { - transitionStyle: '5.5s ease-in color', + transitionStyle: '5.5s ease color', duration: 4, event: 'enter', structural: true @@ -2582,7 +2586,7 @@ describe("ngAnimate $animateCss", function() { expect(element.css('transition-duration')).toMatch('4s'); expect(element.css('transition-property')).toMatch('color'); - expect(element.css('transition-timing-function')).toBeOneOf('ease-in', 'cubic-bezier(0.42, 0, 1, 1)'); + expect(element.css('transition-timing-function')).toBeOneOf('ease', CUBIC_BEZIER_EASE_EQUIVALENT); })); it("should give priority to the provided delay value, but only update the delay style itself", @@ -2835,7 +2839,7 @@ describe("ngAnimate $animateCss", function() { expect(element.css('transition-duration')).toMatch('2.5s'); expect(element.css('transition-property')).toMatch('all'); - expect(element.css('transition-timing-function')).toBeOneOf('ease', 'cubic-bezier(0.25, 0.1, 0.25, 1)'); + expect(element.css('transition-timing-function')).toBeOneOf('ease', CUBIC_BEZIER_EASE_EQUIVALENT); })); it("should remove all inline transition styling when an animation completes", @@ -2966,7 +2970,7 @@ describe("ngAnimate $animateCss", function() { it("should apply a transition duration if the existing transition duration's property value is not 'all'", inject(function($animateCss, $rootElement) { - ss.addRule('.ng-enter', 'transition: 1s cubic-bezier(0.25, 0.1, 0.25, 1) color'); + ss.addRule('.ng-enter', 'transition: 1s linear color'); var emptyObject = {}; var options = { @@ -2982,7 +2986,7 @@ describe("ngAnimate $animateCss", function() { expect(element.css('transition-duration')).toMatch('1s'); expect(element.css('transition-property')).toMatch('color'); - expect(element.css('transition-timing-function')).toBe('cubic-bezier(0.25, 0.1, 0.25, 1)'); + expect(element.css('transition-timing-function')).toBeOneOf('linear', CUBIC_BEZIER_LINEAR_EQUIVALENT); })); it("should apply a transition duration and an animation duration if duration + styles options are provided for a matching keyframe animation",