85

Clicking on a checkbox and calling ng-click: the model is not updated before ng-click kicks in so the checkbox value is wrongly presented in the UI:

This works in AngularJS 1.0.7 and seems broken in Angualar 1.2-RCx.

<div ng-app="myApp" ng-controller="Ctrl">
<li  ng-repeat="todo in todos">
  <input type='checkbox' ng-click='onCompleteTodo(todo)' ng-model="todo.done">
    {{todo.text}}
</li> 
<hr>
task: {{todoText}}
<hr><h2>Wrong value</h2>
     done: {{doneAfterClick}}

and controller:

angular.module('myApp', [])
  .controller('Ctrl', ['$scope', function($scope) {
    $scope.todos=[
        {'text': "get milk",
         'done': true
         },
        {'text': "get milk2",
         'done': false
         }
        ];


   $scope.onCompleteTodo = function(todo) {
    console.log("onCompleteTodo -done: " + todo.done + " : " + todo.text);
    $scope.doneAfterClick=todo.done;
    $scope.todoText = todo.text;

   };
}]);

Broken Fiddle w/ Angular 1.2 RCx - http://jsfiddle.net/supercobra/ekD3r/

Working fidddle w/ Angular 1.0.0 - http://jsfiddle.net/supercobra/8FQNw/

supercobra
  • 15,810
  • 9
  • 45
  • 51

10 Answers10

167

How about changing

<input type='checkbox' ng-click='onCompleteTodo(todo)' ng-model="todo.done">

to

<input type='checkbox' ng-change='onCompleteTodo(todo)' ng-model="todo.done">

From docs:

Evaluate given expression when user changes the input. The expression is not evaluated when the value change is coming from the model.

Note, this directive requires ngModel to be present.

Community
  • 1
  • 1
kakoni
  • 4,980
  • 3
  • 23
  • 14
12

As reported in https://github.com/angular/angular.js/issues/4765, switching from ng-click to ng-change seems to fix this (I am using Angular 1.2.14)

Peter Hollingsworth
  • 1,870
  • 1
  • 18
  • 18
9

The order in which ng-click and ng-model will be executed is ambiguous (since neither explicitly set their priority). The most stable solution to this would be to avoid using them on the same element.

Also, you probably do not want the behavior that the examples show; you want the checkbox to respond to clicks on the complete label text, not only the checkbox. Hence, the cleanest solution would be to wrap the input (with ng-model) inside a label (with ng-click):

<label ng-click="onCompleteTodo(todo)">
  <input type='checkbox' ng-model="todo.done">
  {{todo.text}}
</label>

Working example: http://jsfiddle.net/b3NLH/1/

musically_ut
  • 34,028
  • 8
  • 94
  • 106
8

Why dont you use

$watch('todo',function(.....

Or another solution would be to set the todo.done inside the ng-click callback and only use ng-click

<div ng-app="myApp" ng-controller="Ctrl">
<li  ng-repeat="todo in todos">
<input type='checkbox' ng-click='onCompleteTodo(todo)'>
    {{todo.text}} {{todo.done}}

and

$scope.onCompleteTodo = function(todo) {
        todo.done = !todo.done; //toggle value
        console.log("onCompleteTodo -done: " + todo.done + " : " + todo.text);
        $scope.current = todo;
}
supercobra
  • 15,810
  • 9
  • 45
  • 51
GuillaumeA
  • 3,493
  • 4
  • 35
  • 69
  • 2
    See @kakoni answer, I used ng-change instead of ng-click and the timing works great. This lets you keep the two way binding and is much cleaner approach. – Michael Moser Apr 19 '14 at 19:14
7

Replacing ng-model with ng-checked works for me.

zzjove
  • 195
  • 3
  • 3
2

It is kind of a hack but wrapping it in a timeout seems to accomplish what you are looking for:

angular.module('myApp', [])
    .controller('Ctrl', ['$scope', '$timeout', function ($scope, $timeout) {
    $scope.todos = [{
        'text': "get milk",
        'done': true
    }, {
        'text': "get milk2",
            'done': false
    }];

    $scope.onCompleteTodo = function (todo) {
        $timeout(function(){
            console.log("onCompleteTodo -done: " + todo.done + " : " + todo.text);
            $scope.doneAfterClick = todo.done;
            $scope.todoText = todo.text;
        });
    };
}]);
Brian Lewis
  • 5,739
  • 1
  • 21
  • 28
1

The ordering between ng-model and ng-click seems to be different and it's something you probably shouldn't rely on. Instead you could do something like this:

<div ng-app="myApp" ng-controller="Ctrl">
<li  ng-repeat="todo in todos">
<input type='checkbox' ng-model="todo.done" ng-click='onCompleteTodo(todo)'>
    {{todo.text}} {{todo.done}}
</li> 
    <hr>
        task: {{current.text}}
        <hr>
            <h2>Wrong value</h2>
         done: {{current.done}}
</div>

And your script:

angular.module('myApp', [])
    .controller('Ctrl', ['$scope', function($scope) {

        $scope.todos=[
            {'text': "get milk",
             'done': true
             },
            {'text': "get milk2",
             'done': false
             }
            ];

        $scope.current = $scope.todos[0];


       $scope.onCompleteTodo = function(todo) {
            console.log("onCompleteTodo -done: " + todo.done + " : " + todo.text);
    //$scope.doneAfterClick=todo.done;
    //$scope.todoText = todo.text;
       $scope.current = todo;

   };
}]);

What's different here is whenever you click a box, it sets that box as what's "current" and then display those values in the view. http://jsfiddle.net/QeR7y/

Manny D
  • 20,310
  • 2
  • 29
  • 31
0

Usually this is due to another directive in-between your ng-controller and your input that is creating a new scope. When the select writes out it value, it will write it up to the most recent scope, so it would write it to this scope rather than the parent that is further away.

The best practice is to never bind directly to a variable on the scope in an ng-model, this is also known as always including a "dot" in your ngmodel. For a better explanation of this, check out this video from John:

http://www.youtube.com/watch?v=DTx23w4z6Kc

Solution from: https://groups.google.com/forum/#!topic/angular/7Nd_me5YrHU

fergusrg
  • 59
  • 6
  • It'd be great if you provide a jump marker `#t=5m08s` in your YouTube link so it's not necessary to watch the full video. See http://www.mattcutts.com/blog/link-to-youtube-minute-second/ – Volker E. Jun 11 '14 at 14:21
0

I just replaced ng-model with ng-checked and it worked for me.

This issue was when I updated my angular version from 1.2.28 to 1.4.9

Also check if your ng-change is causing any issue here. I had to remove my ng-change as-well to make it working.

thatzprem
  • 4,697
  • 1
  • 33
  • 41
-1
.task{ng:{repeat:'task in model.tasks'}}
  %input{type:'checkbox',ng:{model:'$parent.model.tasks[$index].enabled'}}
Andrew WC Brown
  • 2,240
  • 5
  • 22
  • 24