Search code examples
javascriptangularjsangularjs-scopeangularjs-validationangularjs-digest

angular: checkbox group's validation is out of sync with the model


The group of checkboxes requires at least one to be checked in order enable the submit button.
Deselecting all checkboxes disables the button.
All checkboxes use the same scope property (showRequired), to mark them as required.

What happens is, the first click does not enable the button like it should. Selecting and deselecting other checkboxes throws the whole thing out of whack; the button may or many not be enabled on any given click, regardless of what state the checkboxes are in.

Troubling shooting it, what I see is that the class ng-valid is not always applied to all checkboxes. Sometimes it's on one checkbox, other times it's on all of them, or some combination of. Using the $timeout fixes everything, but I don't why, and using timers produces a bad code smell. There must be a better way.

I can manually call the validation function to bring the view inline with the model:

 angular.element(document.querySelectorAll('.cb')).scope().checkBoxes()

...leading me to think I'm steping on the $digest loop.

.
.
The following only works when using the $timeout:

angular.module('app', []).
    controller('testCtrl', function ($scope, $timeout) {
        $scope.guests = [
                            {name:'stephen', lname:'wille'},
                            {name:'mary', lname:'torccaso'}
                        ];
        $scope.form = {};
        $scope.form.cb = {one:false, two:false, three:false, four:false}
        $scope.showRequired = true;

        $scope.checkBoxes = function ()
        {
            console.log('checkbox');
            $scope.showRequired = !($scope.form.cb.one || $scope.form.cb.two ||
                                    $scope.form.cb.three || $scope.form.cb.four);
            $scope.$apply();

        };
        $scope.login = function () {
            console.log('login');
        };

        angular.element(document.querySelectorAll('.cb'))
                .on('click', function () {
                    console.log('click');
                    $scope.checkBoxes();

                /* 
                 * using the $timeout solves the problem, but why?
                 *  
                    $timeout(function () {
                        $scope.checkBoxes();
                    }, 0, false);
                */
                });
    });

The $timeout triggers another $digest, but so does $scoope.$apply().
So what gives?

Plunker example


Solution

  • You should just use ng-click instead of manually binding the event on DOM element. Also it is not a good practice accessing DOM in the controller. If you leav it to angular to handle the events you wont run into these issues and you can safely remove manual $digest invocation as well($scope.$apply()). Also use ng-change event instead of ng-click to properly get the model value since model value for check box gets updates at the change event.

    So basically your script would be just:-

    $scope.handleChange = function(){
        $scope.checkBoxes();
    };
    

    and html:-

        <input class="cb" type="checkbox" value="1" ng-model="form.cb.one"
                ng-required="showRequired" ng-change="handleClick()">one</input>
        <input class="cb" type="checkbox" value="2" ng-model="form.cb.two"
               ng-required="showRequired" ng-change="handleClick()">two</input>
        <input class="cb" type="checkbox" value="3" ng-model="form.cb.three"
               ng-required="showRequired" ng-change="handleClick()">three</input>
        <input class="cb" type="checkbox" value="4" ng-model="form.cb.four"
               ng-required="showRequired" ng-change="handleClick()">four</input>
    

    Demo

    You can actually improve this even more by using ng-repeat.

    if you rearrange your checkboxes in an array:-

    $scope.form.cb = [{text:'one', value:false}, {text:'two', value:false}, {text:'three', value:false}, {text:'four', value:false}];
    

    You could do away with just one:-

    <label ng-repeat="chk in form.cb">
     <input class="cb" type="checkbox" value="{{$index}}" 
          ng-model="chk.value" ng-required="showRequired" 
          ng-change="handleChange()"/> {{chk.text}}</label>
    

    And script will be just:-

    $scope.handleChange = function(){
      $scope.showRequired = $scope.form.cb.every(function(itm) {
         return !itm.value;
      });
    };
    

    Now this is flexible enough it is completely model driven and flexible enough to add any more checkboxes without modifying your html or logic.

    Demo2

    Array.prototype.every shim support for older browsers