Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Leave Animation callback overridden by next animation #12271

Closed
wants to merge 1 commit into from
Closed

Leave Animation callback overridden by next animation #12271

wants to merge 1 commit into from

Conversation

sreeramu
Copy link
Contributor

@sreeramu sreeramu commented Jul 5, 2015

This patch will fix the overriding or loosing the leave callback object of the animation.

Example:

var app = angular.module("app", ['ngMessages', 'ngAnimate']);

app.directive('iholder', ['$animate','$timeout', function($animate,$timeout) {
    return {
        restrict: 'EA',
        scope: {},
        template: '<div class="level1 level2" ng-click="clickHandler()" style="width:100%;height:10em;background-color:red;"></div>',
        replace: true,
        link: function($scope, element, attrs) {
            $scope.clickHandler = function() {
                $animate.leave(element);
                $animate.removeClass(element,'level2');
            }
        }
    }
}]);

In above example removeClass is called as soon as the leave is called so in queueAnimation the the leave animation option (callback) is overridden with the removeClass option so the element is not removed.

This patch will fix the #12249 issue , in this issue leave is called by ngRepeatDirective for removing the element and on destroy ngForm called removeClass so the leave callback is overridden in mergeAnimationOptions so the element is not removed.

Review on Reviewable

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@sreeramu
Copy link
Contributor Author

sreeramu commented Jul 5, 2015

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Jul 5, 2015
@Narretz
Copy link
Contributor

Narretz commented Jul 6, 2015

Hi, thanks for the PR. Would it be possible for you to add a test for this?

@sreeramu
Copy link
Contributor Author

sreeramu commented Jul 8, 2015

@Narretz Test script are updated for this bug.

@petebacondarwin petebacondarwin modified the milestones: 1.4.3, 1.4.4 Jul 16, 2015
@Narretz
Copy link
Contributor

Narretz commented Jul 24, 2015

@matsko can you take a look at this?

@hparfr
Copy link

hparfr commented Aug 11, 2015

Thanks sreeramu, this PR fix an issue I had (similar to #12161)

matsko pushed a commit to matsko/angular.js that referenced this pull request Aug 12, 2015
@matsko matsko mentioned this pull request Aug 12, 2015
@matsko
Copy link
Contributor

matsko commented Aug 12, 2015

@sreeramu I've taken your PR and removed some extra code: #12567

I'm waiting for Travis to be green and then it's in.

Excellent work on this! Thank you for putting in the time.

@matsko matsko closed this in 92e41ac Aug 12, 2015
lee-tunnicliffe pushed a commit to lee-tunnicliffe/angular.js that referenced this pull request Aug 13, 2015
@sreeramu
Copy link
Contributor Author

Thanks @matsko

@sreeramu sreeramu deleted the patch-1 branch August 15, 2015 06:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants