Permalink
Comparing changes
Open a pull request
- 2 commits
- 8 files changed
- 0 commit comments
- 2 contributors
Commits on Jan 23, 2016
…imations Prior to this fix the addition and removal of a CSS class via ngAnimate would cause flicker effects because $animate was unable to keep track of the CSS classes once they were applied to the element. This fix ensures that ngAnimate always keeps a reference to the classes in the currently running animation so that cancelling works accordingly. The commit also adds a test for a previously untested animation merge path. Closes #10156 Closes #13822
Unified
Split
Showing
with
102 additions
and 39 deletions.
- +10 −5 docs/content/guide/filter.ngdoc
- +1 −1 src/ngAnimate/.jshintrc
- +23 −24 src/ngAnimate/animateQueue.js
- +7 −1 src/ngAnimate/shared.js
- +1 −1 test/ngAnimate/.jshintrc
- +23 −1 test/ngAnimate/animateSpec.js
- +11 −6 test/ngAnimate/animationHelperFunctionsSpec.js
- +26 −0 test/ngAnimate/integrationSpec.js
| @@ -32,10 +32,13 @@ E.g. the markup `{{ 1234 | number:2 }}` formats the number 1234 with 2 decimal p | ||
|
|
||
| ## Using filters in controllers, services, and directives | ||
|
|
||
| You can also use filters in controllers, services, and directives. For this, inject a dependency | ||
| with the name `<filterName>Filter` to your controller/service/directive. E.g. using the dependency | ||
| `numberFilter` will inject the number filter. The injected argument is a function that takes the | ||
| value to format as first argument and filter parameters starting with the second argument. | ||
| You can also use filters in controllers, services, and directives. | ||
|
|
||
| <div class="alert alert-info"> | ||
| For this, inject a dependency with the name `<filterName>Filter` into your controller/service/directive. | ||
| E.g. a filter called `number` is injected by using the dependency `numberFilter`. The injected argument | ||
| is a function that takes the value to format as first argument, and filter parameters starting with the second argument. | ||
| </div> | ||
|
|
||
| The example below uses the filter called {@link ng.filter:filter `filter`}. | ||
| This filter reduces arrays into sub arrays based on | ||
| @@ -108,6 +111,7 @@ text upper-case. | ||
| No filter: {{greeting}}<br> | ||
| Reverse: {{greeting|reverse}}<br> | ||
| Reverse + uppercase: {{greeting|reverse:true}}<br> | ||
| Reverse, filtered in controller: {{filteredGreeting}}<br> | ||
| </div> | ||
| </file> | ||
|
|
||
| @@ -127,8 +131,9 @@ text upper-case. | ||
| return out; | ||
| }; | ||
| }) | ||
| .controller('MyController', ['$scope', function($scope) { | ||
| .controller('MyController', ['$scope', 'reverseFilter', function($scope, reverseFilter) { | ||
| $scope.greeting = 'hello'; | ||
| $scope.filteredGreeting = reverseFilter($scope.greeting); | ||
| }]); | ||
| </file> | ||
| </example> | ||
| @@ -48,7 +48,7 @@ | ||
| "assertArg": false, | ||
| "isPromiseLike": false, | ||
| "mergeClasses": false, | ||
| "mergeAnimationOptions": false, | ||
| "mergeAnimationDetails": false, | ||
| "prepareAnimationOptions": false, | ||
| "applyAnimationStyles": false, | ||
| "applyAnimationFromStyles": false, | ||
| @@ -42,22 +42,21 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
| }); | ||
| } | ||
|
|
||
| function hasAnimationClasses(options, and) { | ||
| options = options || {}; | ||
| var a = (options.addClass || '').length > 0; | ||
| var b = (options.removeClass || '').length > 0; | ||
| function hasAnimationClasses(animation, and) { | ||
| var a = (animation.addClass || '').length > 0; | ||
| var b = (animation.removeClass || '').length > 0; | ||
| return and ? a && b : a || b; | ||
| } | ||
|
|
||
| rules.join.push(function(element, newAnimation, currentAnimation) { | ||
| // if the new animation is class-based then we can just tack that on | ||
| return !newAnimation.structural && hasAnimationClasses(newAnimation.options); | ||
| return !newAnimation.structural && hasAnimationClasses(newAnimation); | ||
| }); | ||
|
|
||
| rules.skip.push(function(element, newAnimation, currentAnimation) { | ||
| // there is no need to animate anything if no classes are being added and | ||
| // there is no structural animation that will be triggered | ||
| return !newAnimation.structural && !hasAnimationClasses(newAnimation.options); | ||
| return !newAnimation.structural && !hasAnimationClasses(newAnimation); | ||
| }); | ||
|
|
||
| rules.skip.push(function(element, newAnimation, currentAnimation) { | ||
| @@ -83,19 +82,17 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
| }); | ||
|
|
||
| rules.cancel.push(function(element, newAnimation, currentAnimation) { | ||
|
|
||
|
|
||
| var nA = newAnimation.options.addClass; | ||
| var nR = newAnimation.options.removeClass; | ||
| var cA = currentAnimation.options.addClass; | ||
| var cR = currentAnimation.options.removeClass; | ||
| var nA = newAnimation.addClass; | ||
| var nR = newAnimation.removeClass; | ||
| var cA = currentAnimation.addClass; | ||
| var cR = currentAnimation.removeClass; | ||
|
|
||
| // early detection to save the global CPU shortage :) | ||
| if ((isUndefined(nA) && isUndefined(nR)) || (isUndefined(cA) && isUndefined(cR))) { | ||
| return false; | ||
| } | ||
|
|
||
| return (hasMatchingClasses(nA, cR)) || (hasMatchingClasses(nR, cA)); | ||
| return hasMatchingClasses(nA, cR) || hasMatchingClasses(nR, cA); | ||
| }); | ||
|
|
||
| this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$HashMap', | ||
| @@ -167,8 +164,8 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
|
|
||
| var applyAnimationClasses = applyAnimationClassesFactory($$jqLite); | ||
|
|
||
| function normalizeAnimationOptions(element, options) { | ||
| return mergeAnimationOptions(element, options, {}); | ||
| function normalizeAnimationDetails(element, animation) { | ||
| return mergeAnimationDetails(element, animation, {}); | ||
| } | ||
|
|
||
| // IE9-11 has no method "contains" in SVG element and in Node.prototype. Bug #10259. | ||
| @@ -362,6 +359,8 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
| structural: isStructural, | ||
| element: element, | ||
| event: event, | ||
| addClass: options.addClass, | ||
| removeClass: options.removeClass, | ||
| close: close, | ||
| options: options, | ||
| runner: runner | ||
| @@ -374,11 +373,10 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
| close(); | ||
| return runner; | ||
| } else { | ||
| mergeAnimationOptions(element, existingAnimation.options, options); | ||
| mergeAnimationDetails(element, existingAnimation, newAnimation); | ||
| return existingAnimation.runner; | ||
| } | ||
| } | ||
|
|
||
| var cancelAnimationFlag = isAllowed('cancel', element, newAnimation, existingAnimation); | ||
| if (cancelAnimationFlag) { | ||
| if (existingAnimation.state === RUNNING_STATE) { | ||
| @@ -393,7 +391,8 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
| existingAnimation.close(); | ||
| } else { | ||
| // this will merge the new animation options into existing animation options | ||
| mergeAnimationOptions(element, existingAnimation.options, newAnimation.options); | ||
| mergeAnimationDetails(element, existingAnimation, newAnimation); | ||
|
|
||
| return existingAnimation.runner; | ||
| } | ||
| } else { | ||
| @@ -403,12 +402,12 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
| var joinAnimationFlag = isAllowed('join', element, newAnimation, existingAnimation); | ||
| if (joinAnimationFlag) { | ||
| if (existingAnimation.state === RUNNING_STATE) { | ||
| normalizeAnimationOptions(element, options); | ||
| normalizeAnimationDetails(element, newAnimation); | ||
| } else { | ||
| applyGeneratedPreparationClasses(element, isStructural ? event : null, options); | ||
|
|
||
| event = newAnimation.event = existingAnimation.event; | ||
| options = mergeAnimationOptions(element, existingAnimation.options, newAnimation.options); | ||
| options = mergeAnimationDetails(element, existingAnimation, newAnimation); | ||
|
|
||
| //we return the same runner since only the option values of this animation will | ||
| //be fed into the `existingAnimation`. | ||
| @@ -419,7 +418,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
| } else { | ||
| // normalization in this case means that it removes redundant CSS classes that | ||
| // already exist (addClass) or do not exist (removeClass) on the element | ||
| normalizeAnimationOptions(element, options); | ||
| normalizeAnimationDetails(element, newAnimation); | ||
| } | ||
|
|
||
| // when the options are merged and cleaned up we may end up not having to do | ||
| @@ -429,7 +428,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
| if (!isValidAnimation) { | ||
| // animate (from/to) can be quickly checked first, otherwise we check if any classes are present | ||
| isValidAnimation = (newAnimation.event === 'animate' && Object.keys(newAnimation.options.to || {}).length > 0) | ||
| || hasAnimationClasses(newAnimation.options); | ||
| || hasAnimationClasses(newAnimation); | ||
| } | ||
|
|
||
| if (!isValidAnimation) { | ||
| @@ -459,7 +458,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
| var isValidAnimation = parentElement.length > 0 | ||
| && (animationDetails.event === 'animate' | ||
| || animationDetails.structural | ||
| || hasAnimationClasses(animationDetails.options)); | ||
| || hasAnimationClasses(animationDetails)); | ||
|
|
||
| // this means that the previous animation was cancelled | ||
| // even if the follow-up animation is the same event | ||
| @@ -491,7 +490,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { | ||
|
|
||
| // this combined multiple class to addClass / removeClass into a setClass event | ||
| // so long as a structural event did not take over the animation | ||
| event = !animationDetails.structural && hasAnimationClasses(animationDetails.options, true) | ||
| event = !animationDetails.structural && hasAnimationClasses(animationDetails, true) | ||
| ? 'setClass' | ||
| : animationDetails.event; | ||
|
|
||
| @@ -217,7 +217,10 @@ function applyAnimationToStyles(element, options) { | ||
| } | ||
| } | ||
|
|
||
| function mergeAnimationOptions(element, target, newOptions) { | ||
| function mergeAnimationDetails(element, oldAnimation, newAnimation) { | ||
| var target = oldAnimation.options || {}; | ||
| var newOptions = newAnimation.options || {}; | ||
|
|
||
| var toAdd = (target.addClass || '') + ' ' + (newOptions.addClass || ''); | ||
| var toRemove = (target.removeClass || '') + ' ' + (newOptions.removeClass || ''); | ||
| var classes = resolveElementClasses(element.attr('class'), toAdd, toRemove); | ||
| @@ -249,6 +252,9 @@ function mergeAnimationOptions(element, target, newOptions) { | ||
| target.removeClass = null; | ||
| } | ||
|
|
||
| oldAnimation.addClass = target.addClass; | ||
| oldAnimation.removeClass = target.removeClass; | ||
|
|
||
| return target; | ||
| } | ||
|
|
||
| @@ -3,7 +3,7 @@ | ||
| "browser": true, | ||
| "newcap": false, | ||
| "globals": { | ||
| "mergeAnimationOptions": false, | ||
| "mergeAnimationDetails": false, | ||
| "prepareAnimationOptions": false, | ||
| "applyAnimationStyles": false, | ||
| "applyAnimationFromStyles": false, | ||
| @@ -6,11 +6,18 @@ describe("animations", function() { | ||
| beforeEach(module('ngAnimateMock')); | ||
|
|
||
| var element, applyAnimationClasses; | ||
|
|
||
| beforeEach(module(function() { | ||
| return function($$jqLite) { | ||
| applyAnimationClasses = applyAnimationClassesFactory($$jqLite); | ||
| }; | ||
| })); | ||
|
|
||
| afterEach(inject(function($$jqLite) { | ||
| applyAnimationClasses = applyAnimationClassesFactory($$jqLite); | ||
| dealoc(element); | ||
| })); | ||
|
|
||
|
|
||
| it('should allow animations if the application is bootstrapped on the document node', function() { | ||
| var capturedAnimation; | ||
|
|
||
| @@ -1118,6 +1125,21 @@ describe("animations", function() { | ||
| expect(doneHandler).toHaveBeenCalled(); | ||
| })); | ||
|
|
||
| it('should merge a follow-up animation that does not add classes into the previous animation (pre-digest)', | ||
| inject(function($animate, $rootScope) { | ||
|
|
||
| $animate.enter(element, parent); | ||
| $animate.animate(element, {height: 0}, {height: '100px'}); | ||
|
|
||
| $rootScope.$digest(); | ||
| expect(capturedAnimation).toBeTruthy(); | ||
| expect(capturedAnimation[1]).toBe('enter'); // make sure the enter animation is present | ||
|
|
||
| // fake the style setting (because $$animation is mocked) | ||
| applyAnimationStyles(element, capturedAnimation[2]); | ||
| expect(element.css('height')).toContain('100px'); | ||
| })); | ||
|
|
||
| it('should immediately skip the class-based animation if there is an active structural animation', | ||
| inject(function($animate, $rootScope) { | ||
|
|
||
| @@ -110,7 +110,7 @@ describe("animation option helper functions", function() { | ||
| })); | ||
| }); | ||
|
|
||
| describe('mergeAnimationOptions', function() { | ||
| describe('mergeAnimationDetails', function() { | ||
| it('should merge in new options', inject(function() { | ||
| element.attr('class', 'blue'); | ||
| var options = prepareAnimationOptions({ | ||
| @@ -120,11 +120,16 @@ describe("animation option helper functions", function() { | ||
| removeClass: 'blue gold' | ||
| }); | ||
|
|
||
| mergeAnimationOptions(element, options, { | ||
| age: 29, | ||
| addClass: 'gold brown', | ||
| removeClass: 'orange' | ||
| }); | ||
| var animation1 = { options: options }; | ||
| var animation2 = { | ||
| options: { | ||
| age: 29, | ||
| addClass: 'gold brown', | ||
| removeClass: 'orange' | ||
| } | ||
| }; | ||
|
|
||
| mergeAnimationDetails(element, animation1, animation2); | ||
|
|
||
| expect(options.name).toBe('matias'); | ||
| expect(options.age).toBe(29); | ||
| @@ -25,6 +25,32 @@ describe('ngAnimate integration tests', function() { | ||
| ss.destroy(); | ||
| }); | ||
|
|
||
| it('should cancel a running and started removeClass animation when a follow-up addClass animation adds the same class', | ||
| inject(function($animate, $rootScope, $$rAF, $document, $rootElement) { | ||
|
|
||
| jqLite($document[0].body).append($rootElement); | ||
| element = jqLite('<div></div>'); | ||
| $rootElement.append(element); | ||
|
|
||
| element.addClass('active-class'); | ||
|
|
||
| var runner = $animate.removeClass(element, 'active-class'); | ||
| $rootScope.$digest(); | ||
|
|
||
| var doneHandler = jasmine.createSpy('addClass done'); | ||
| runner.done(doneHandler); | ||
|
|
||
| $$rAF.flush(); // Trigger the actual animation | ||
|
|
||
| expect(doneHandler).not.toHaveBeenCalled(); | ||
|
|
||
| $animate.addClass(element, 'active-class'); | ||
| $rootScope.$digest(); | ||
|
|
||
| // Cancelling the removeClass animation triggers the done callback | ||
| expect(doneHandler).toHaveBeenCalled(); | ||
| })); | ||
|
|
||
| describe('CSS animations', function() { | ||
| if (!browserSupportsCssAnimations()) return; | ||
|
|
||