Skip to content

Commit

Permalink
fix(input): listen on "change" for radio and checkbox
Browse files Browse the repository at this point in the history
input[radio] and inout[checkbox] now listen on the change event instead
of the click event. This fixes issue with 3rd party libraries that trigger
a change event on inputs, e.g. Bootstrap 3 custom checkbox / radio button
toggles.
It also makes it easier to prevent specific events that can cause a checkbox / radio
to change, e.g. click events. Previously, this was difficult because the custom click
handler had to be registered before the input directive's click handler.

It is possible that radio and checkbox listened to click because IE8 has
broken support for listening on change, see http://www.quirksmode.org/dom/events/change.html

Closes angular#4516
Closes angular#14667

BREAKING CHANGE:

input[radio] and input[checkbox] now need to be attached to the document to propagate events
correctly. This should only be of concern in unit-tests that compile input elements and
trigger click events on them. This is because we now listen to the change event which
gets automatically triggered by browsers when a checkbox or radio is clicked. However,
this may fail in some browsers when the elements are not attached to the document.

Before:

```js
    it('should update the model', inject(function($compile, $rootScope) {
      var inputElm = $compile('<input type="checkbox" ng-model="checkbox" />')($rootScope);

      browserTrigger(inputElm[0], 'click');
      expect($rootScope.checkbox).toBe(true);
    });
```

With this patch, `$rootScope.checkbox` might not be true, because the click event
hasn't triggered the change event. To make the test, work append the inputElm to the app's
$rootElement, and the $rootElement to the $document:

After:

```js
    it('should update the model', inject(function($compile, $rootScope, $rootElement, $document) {
      var inputElm = $compile('<input type="checkbox" ng-model="checkbox" />')($rootScope);

      $rootElement.append(inputElm);
      $document.append($rootElement);

      browserTrigger(inputElm[0], 'click');
      expect($rootScope.checkbox).toBe(true);
    });
```
  • Loading branch information
Narretz committed May 27, 2016
1 parent 4d5f5ff commit ee23d70
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/ng/directive/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -1472,7 +1472,7 @@ function radioInputType(scope, element, attr, ctrl) {
}
};

element.on('click', listener);
element.on('change', listener);

ctrl.$render = function() {
var value = attr.value;
Expand Down Expand Up @@ -1505,7 +1505,7 @@ function checkboxInputType(scope, element, attr, ctrl, $sniffer, $browser, $filt
ctrl.$setViewValue(element[0].checked, ev && ev.type);
};

element.on('click', listener);
element.on('change', listener);

ctrl.$render = function() {
element[0].checked = ctrl.$viewValue;
Expand Down
5 changes: 4 additions & 1 deletion test/helpers/testabilityPatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ function generateInputCompilerHelper(helper) {
};
});
});
inject(function($compile, $rootScope, $sniffer) {
inject(function($compile, $rootScope, $sniffer, $document, $rootElement) {

helper.compileInput = function(inputHtml, mockValidity, scope) {

Expand All @@ -381,6 +381,9 @@ function generateInputCompilerHelper(helper) {
// Compile the lot and return the input element
$compile(helper.formElm)(scope);

$rootElement.append(helper.formElm);
jqLite($document[0].body).append($rootElement);

spyOn(scope.form, '$addControl').and.callThrough();
spyOn(scope.form, '$$renameControl').and.callThrough();

Expand Down
22 changes: 20 additions & 2 deletions test/ng/directive/inputSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2932,7 +2932,7 @@ describe('input', function() {

describe('radio', function() {

it('should update the model', function() {
they('should update the model on $prop event', ['click', 'change'], function(event) {
var inputElm = helper.compileInput(
'<input type="radio" ng-model="color" value="white" />' +
'<input type="radio" ng-model="color" value="red" />' +
Expand All @@ -2948,7 +2948,8 @@ describe('input', function() {
expect(inputElm[1].checked).toBe(true);
expect(inputElm[2].checked).toBe(false);

browserTrigger(inputElm[2], 'click');
if (event === 'change') inputElm[2].checked = true;
browserTrigger(inputElm[2], event);
expect($rootScope.color).toBe('blue');
});

Expand Down Expand Up @@ -3005,6 +3006,23 @@ describe('input', function() {
});


they('should update the model on $prop event', ['click', 'change'], function(event) {
var inputElm = helper.compileInput('<input type="checkbox" ng-model="checkbox" />');

expect(inputElm[0].checked).toBe(false);

$rootScope.$apply("checkbox = true");
expect(inputElm[0].checked).toBe(true);

$rootScope.$apply("checkbox = false");
expect(inputElm[0].checked).toBe(false);

if (event === 'change') inputElm[0].checked = true;
browserTrigger(inputElm[0], event);
expect($rootScope.checkbox).toBe(true);
});


it('should format booleans', function() {
var inputElm = helper.compileInput('<input type="checkbox" ng-model="name" />');

Expand Down

0 comments on commit ee23d70

Please sign in to comment.