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

Bug ngAnimate 1.2.0-rc2 with side effects on watches on form$invalid when combined with ng-if in some inputs #4226

Closed
bfcamara opened this issue Oct 2, 2013 · 6 comments
Closed

Bug ngAnimate 1.2.0-rc2 with side effects on watches on form$invalid when combined with ng-if in some inputs #4226

bfcamara opened this issue Oct 2, 2013 · 6 comments
Assignees
@IgorMinar
Labels
component: ngAnimate type: bug
Milestone
1.2.0 - timely-de...

Comments

@bfcamara
Copy link

bfcamara commented Oct 2, 2013

I am wathcing a strange behavior when including ngAnimate as dependency.

I have a button with ng-disabled="$form.invalid" which is not being updated on some user events made on the form.

Here is an example ro reroduce the bug

    <!DOCTYPE html>
    <html ng-app="app">
    <head></head>
    <body ng-controller="MainCtrl">

            <form name="myForm" novalidate>
                <p>
                <label>Required Input </label>
                <input type="text" name="mytext" ng-model="vm.mytext" required/>
                </p>
                <p>
                    Option 1 <input type="checkbox" name="option1" ng-model="vm.option1"/>
                    Option 2<input type="checkbox" name="option2" ng-model="vm.option2"/>
                </p>
                <div>
                    <p>Fill Fields</p>
                    <div ng-if="vm.option1">Fields for Option 1
                        Required
                        <textarea name="myField1" required ng-model="vm.myField1"></textarea>
                    </div>

                    <div ng-if="vm.option2">Fields for Option 2
                        Required
                        <textarea name="myField2" required ng-model="vm.myField2"></textarea>
                    </div>
                </div>
                <hr/>
                <button ng-disabled="myForm.$invalid" ng-click="submit()">Submit</button>
            </form> 

        <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.0-rc.2/angular.min.js"></script>
        <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.0-rc.2/angular-route.min.js"></script>
        <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.0-rc.2/angular-animate.min.js"></script>
        <script>
    (function () {
        "use strict";

        var app = angular.module('app', ['ngAnimate']);     //WITH ngAnimate as dependency, the button ng-disabled does not respond to checkbox changes
        //var app = angular.module('app', []);              //Comment the previous line and uncomment this to have this sample working as expected

        app.controller('MainCtrl', ['$scope', 
            function ($scope) {
                $scope.vm = {}

                $scope.submit = function() {
                    console.log('submitted');

                };
            }
        ]);
    })();
        </script>
    </body>
    </html>

Here the steps to reproduce the bug:

1 - fill some text in text box 'Required Input' (the button becomes enabled)
2 - check Option 1 (the button becomes disabled)
3 - fill textarea of Option 1 (the button becomes enabled)
4 - check Option 2 (the button becomes disabled)
5 - uncheck Option 2 the button remains disabled, which is not correct

Now, if we do not include ngAnimate as dependency
1 - fill some text in text box 'Required Input' (the button becomes enabled)
2 - check Option 1 (the button becomes disabled)
3 - fill textarea of Option 1 (the button becomes enabled)
4 - check Option 2 (the button becomes disabled)
5 - uncheck Option 2 the button becomes enabled which is correct

Here is a jsfiddle http://jsfiddle.net/bfcamara/8s6r2/
@adityavm
Copy link

adityavm commented Oct 3, 2013

Just to point out, if you watch myForm.$invalid, you'll notice that it doesn't change (or its change isn't caught by the digest) when you un-tick a checkbox. If you re-tick the checkbox, the watch function is fired twice.

Updated fiddle: http://jsfiddle.net/8s6r2/2/

@matsko
Copy link
Contributor

matsko commented Oct 24, 2013

@bfcamara Looks like it's working in @adityavm's code. Can you please verify it?

@adityavm
Copy link

@matsko Actually, it isn't. Un-ticking the first box (after ticking it) should re-enable the button, but it doesn't.

In my comment, I was just pointing out how the change seems to get queued (if you look at it in the console).

@matsko
Copy link
Contributor

matsko commented Oct 24, 2013

Alright I see it now. Looking into it.

@ghost ghost assigned matsko Oct 24, 2013
@sod
Copy link

sod commented Oct 31, 2013

The issue here is the different behavior of the function leave() in vanilla angular vs. angular-animate.

angular leave():

      leave : function(element, done) {
        element.remove();
        done && $timeout(done, 0, false);
      },

angular-animate leave():

        leave : function(element, done) {
          cancelChildAnimations(element);
          this.enabled(false, element);
          $rootScope.$$postDigest(function() {
            performAnimation('leave', 'ng-leave', element, null, null, function() {
              $delegate.leave(element, done);
            });
          });
        },

On unchecking the option, the digest loops watcher on <input type="checkbox" /> calls ngIfWatchAction() => $animate.leave() => element.remove() in angular (that changes the formular requirements by removing the <textarea required /> from the DOM) and afterwards in the same digest loop calls the ng-disabled watcher on the submit button (that changes its state because of the previous require change).

But in angular-animate the element.remove() is postponed so $form.invalid is stil on true as the digest loop checks the submit button.

Have no solution, though. Just research :(

@ghost ghost assigned IgorMinar Oct 31, 2013
@matsko
Copy link
Contributor

matsko commented Nov 4, 2013

@bfcamara @sod

Yes, the issue with this is leave. And @IgorMinar has a fix in mind which he is putting together.

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
@IgorMinar @jamesdaily
fix(ngModel): deregister from the form on scope not DOM destruction
af46386 
Due to animations, DOM might get destroyed much later than scope and so the element $destroy event
might get fired outside of $digest, which causes changes to the validation model go unobserved
until the next digest. By deregistering on scope  event, the deregistration always happens
in $digest and the form validation model changes will be observed.

Closes angular#4226
Closes angular#4779
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
@IgorMinar @jamesdaily
fix(ngModel): deregister from the form on scope not DOM destruction
861955c 
Due to animations, DOM might get destroyed much later than scope and so the element $destroy event
might get fired outside of $digest, which causes changes to the validation model go unobserved
until the next digest. By deregistering on scope  event, the deregistration always happens
in $digest and the form validation model changes will be observed.

Closes angular#4226
Closes angular#4779
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@IgorMinar IgorMinar

Labels
component: ngAnimate type: bug
Projects
None yet
Milestone
1.2.0 - timely-delivery
Development

No branches or pull requests
5 participants
@adityavm @matsko @IgorMinar @bfcamara @sod