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

Commit 975a617

Browse files
committed
fix(aria/ngModel): do not overwrite the default $isEmpty() method for checkboxes
Previously, `ngAria` would overwrite the default `ngModelController.$isEmpty()` method for custom `checkbox`-shaped controls (e.g. `role="checkbox"` or `role="menuitemcheckbox"`), using the same implementation as `input[checkbox]` (i.e. `value === false`). While this makes sense for `input[checkbox]` which also defines a custom parser, it doesn't make sense for a custom `checkbox` out-of-the-box. For example, an unintialized value (`undefined`) would make the checkbox appear as "checked". If the user wants to provide a custom parser (e.g. setting falsy values to `false`), then they should also provide a custom `$isEmpty()` method. As a side effect, this commit solves issue #14621. (We could have solved it in different ways.) Fixes #14621 Closes #14625 BREAKING CHANGE: Custom `checkbox`-shaped controls (e.g. checkboxes, menuitemcheckboxes), no longer have a custom `$isEmpty()` method on their `NgModelController` that checks for `value === false`. Unless overwritten, the default `$isEmpty()` method will be used, which treats `undefined`, `null`, `NaN` and `''` as "empty". **Note:** The `$isEmpty()` method is used to determine if the checkbox is checked ("not empty" means "checked") and thus it can indirectly affect other things, such as the control's validity with respect to the `required` validator (e.g. "empty" + "required" --> "invalid"). Before: ```js var template = '<my-checkbox role="checkbox" ng-model="value"></my-checkbox>'; var customCheckbox = $compile(template)(scope); var ctrl = customCheckbox.controller('ngModel'); scope.$apply('value = false'); console.log(ctrl.$isEmpty()); //--> true scope.$apply('value = true'); console.log(ctrl.$isEmpty()); //--> false scope.$apply('value = undefined'/* or null or NaN or '' */); console.log(ctrl.$isEmpty()); //--> false ``` After: ```js var template = '<my-checkbox role="checkbox" ng-model="value"></my-checkbox>'; var customCheckbox = $compile(template)(scope); var ctrl = customCheckbox.controller('ngModel'); scope.$apply('value = false'); console.log(ctrl.$isEmpty()); //--> false scope.$apply('value = true'); console.log(ctrl.$isEmpty()); //--> false scope.$apply('value = undefined'/* or null or NaN or '' */); console.log(ctrl.$isEmpty()); //--> true ``` -- If you want to have a custom `$isEmpty()` method, you need to overwrite the default. For example: ```js .directive('myCheckbox', function myCheckboxDirective() { return { require: 'ngModel', link: function myCheckboxPostLink(scope, elem, attrs, ngModelCtrl) { ngModelCtrl.$isEmpty = function myCheckboxIsEmpty(value) { return !value; // Any falsy value means "empty" // Or to restore the previous behavior: // return value === false; }; } }; }) ```
1 parent e8d7496 commit 975a617

File tree

3 files changed

+149
-55
lines changed

3 files changed

+149
-55
lines changed

docs/content/guide/accessibility.ngdoc

+95-41
Original file line numberDiff line numberDiff line change
@@ -59,79 +59,133 @@ attributes (if they have not been explicitly specified by the developer):
5959
* aria-invalid
6060
* aria-required
6161
* aria-readonly
62+
* aria-disabled
6263

6364
### Example
6465

6566
<example module="ngAria_ngModelExample" deps="angular-aria.js" name="accessibility-ng-model">
6667
<file name="index.html">
67-
<form ng-controller="formsController">
68-
<some-checkbox role="checkbox" ng-model="checked" ng-class="{active: checked}"
69-
ng-disabled="isDisabled" ng-click="toggleCheckbox()"
70-
aria-label="Custom Checkbox" show-attrs>
71-
<span class="icon" aria-hidden="true"></span>
72-
Custom Checkbox
73-
</some-checkbox>
68+
<form>
69+
<custom-checkbox role="checkbox" ng-model="checked" required
70+
aria-label="Custom checkbox" show-attrs>
71+
Custom checkbox
72+
</custom-checkbox>
7473
</form>
74+
<hr />
75+
<b>Is checked:</b> {{ !!checked }}
7576
</file>
7677
<file name="script.js">
77-
angular.module('ngAria_ngModelExample', ['ngAria'])
78-
.controller('formsController', function($scope) {
79-
$scope.checked = false;
80-
$scope.toggleCheckbox = function() {
81-
$scope.checked = !$scope.checked;
82-
};
83-
})
84-
.directive('someCheckbox', function() {
78+
angular.
79+
module('ngAria_ngModelExample', ['ngAria']).
80+
directive('customCheckbox', customCheckboxDirective).
81+
directive('showAttrs', showAttrsDirective);
82+
83+
function customCheckboxDirective() {
8584
return {
8685
restrict: 'E',
87-
link: function($scope, $el, $attrs) {
88-
$el.on('keypress', function(event) {
86+
require: 'ngModel',
87+
transclude: true,
88+
template:
89+
'<span class="icon" aria-hidden="true"></span> ' +
90+
'<ng-transclude></ng-transclude>',
91+
link: function(scope, elem, attrs, ctrl) {
92+
// Overwrite necessary `NgModelController` methods
93+
ctrl.$isEmpty = isEmpty;
94+
ctrl.$render = render;
95+
96+
// Bind to events
97+
elem.on('click', function(event) {
98+
event.preventDefault();
99+
scope.$apply(toggleCheckbox);
100+
});
101+
elem.on('keypress', function(event) {
89102
event.preventDefault();
90103
if (event.keyCode === 32 || event.keyCode === 13) {
91-
$scope.toggleCheckbox();
92-
$scope.$apply();
104+
scope.$apply(toggleCheckbox);
93105
}
94106
});
107+
108+
// Helpers
109+
function isEmpty(value) {
110+
return !value;
111+
}
112+
113+
function render() {
114+
elem[ctrl.$viewValue ? 'addClass' : 'removeClass']('checked');
115+
}
116+
117+
function toggleCheckbox() {
118+
ctrl.$setViewValue(!ctrl.$viewValue);
119+
ctrl.$render();
120+
}
95121
}
96122
};
97-
})
98-
.directive('showAttrs', function() {
99-
return function($scope, $el, $attrs) {
123+
}
124+
125+
function showAttrsDirective($timeout) {
126+
return function(scope, elem, attrs) {
100127
var pre = document.createElement('pre');
101-
$el.after(pre);
102-
$scope.$watch(function() {
103-
var $attrs = {};
104-
Array.prototype.slice.call($el[0].attributes, 0).forEach(function(item) {
105-
if (item.name !== 'show-$attrs') {
106-
$attrs[item.name] = item.value;
107-
}
128+
elem.after(pre);
129+
130+
scope.$watchCollection(function() {
131+
return Array.prototype.slice.call(elem[0].attributes).reduce(function(aggr, attr) {
132+
if (attr.name !== attrs.$attr.showAttrs) aggr[attr.name] = attr.value;
133+
return aggr;
134+
}, {});
135+
}, function(newValues) {
136+
$timeout(function() {
137+
pre.textContent = angular.toJson(newValues, 2);
108138
});
109-
return $attrs;
110-
}, function(newAttrs, oldAttrs) {
111-
pre.textContent = JSON.stringify(newAttrs, null, 2);
112-
}, true);
139+
});
113140
};
114-
});
141+
}
115142
</file>
116143
<file name="style.css">
117-
[role=checkbox] {
144+
custom-checkbox {
118145
cursor: pointer;
119146
display: inline-block;
120147
}
121-
[role=checkbox] .icon:before {
148+
149+
custom-checkbox .icon:before {
122150
content: '\2610';
123151
display: inline-block;
124152
font-size: 2em;
125153
line-height: 1;
126-
vertical-align: middle;
127154
speak: none;
155+
vertical-align: middle;
128156
}
129-
[role=checkbox].active .icon:before {
157+
158+
custom-checkbox.checked .icon:before {
130159
content: '\2611';
131160
}
132-
pre {
133-
white-space: pre-wrap;
134-
}
161+
</file>
162+
<file name="protractor.js" type="protractor">
163+
var checkbox = element(by.css('custom-checkbox'));
164+
var checkedCheckbox = element(by.css('custom-checkbox.checked'));
165+
166+
it('should have the `checked` class only when checked', function() {
167+
expect(checkbox.isPresent()).toBe(true);
168+
expect(checkedCheckbox.isPresent()).toBe(false);
169+
170+
checkbox.click();
171+
expect(checkedCheckbox.isPresent()).toBe(true);
172+
173+
checkbox.click();
174+
expect(checkedCheckbox.isPresent()).toBe(false);
175+
});
176+
177+
it('should have the `aria-checked` attribute set to the appropriate value', function() {
178+
expect(checkedCheckbox.isPresent()).toBe(false);
179+
expect(checkbox.getAttribute('aria-checked')).toBe('false');
180+
181+
checkbox.click();
182+
expect(checkedCheckbox.isPresent()).toBe(true);
183+
expect(checkbox.getAttribute('aria-checked')).toBe('true');
184+
185+
checkbox.click();
186+
expect(checkedCheckbox.isPresent()).toBe(false);
187+
expect(checkbox.getAttribute('aria-checked')).toBe('false');
188+
});
135189
</file>
136190
</example>
137191

src/ngAria/aria.js

-8
Original file line numberDiff line numberDiff line change
@@ -252,14 +252,6 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
252252
var shape = getShape(attr, elem);
253253

254254
return {
255-
pre: function(scope, elem, attr, ngModel) {
256-
if (shape === 'checkbox') {
257-
//Use the input[checkbox] $isEmpty implementation for elements with checkbox roles
258-
ngModel.$isEmpty = function(value) {
259-
return value === false;
260-
};
261-
}
262-
},
263255
post: function(scope, elem, attr, ngModel) {
264256
var needsTabIndex = shouldAttachAttr('tabindex', 'tabindex', elem, false);
265257

test/ngAria/ariaSpec.js

+54-6
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ describe('$aria', function() {
8484
});
8585
});
8686

87-
8887
describe('aria-hidden when disabled', function() {
8988
beforeEach(configAriaProvider({
9089
ariaHidden: false
@@ -115,15 +114,41 @@ describe('$aria', function() {
115114
});
116115

117116
it('should attach itself to custom checkbox', function() {
118-
compileElement('<div role="checkbox" ng-model="val">');
117+
compileElement('<div role="checkbox" ng-model="val"></div>');
119118

120-
scope.$apply('val = true');
119+
scope.$apply('val = "checked"');
121120
expect(element.attr('aria-checked')).toBe('true');
122121

123-
scope.$apply('val = false');
122+
scope.$apply('val = null');
124123
expect(element.attr('aria-checked')).toBe('false');
125124
});
126125

126+
it('should use `$isEmpty()` to determine if the checkbox is checked',
127+
function() {
128+
compileElement('<div role="checkbox" ng-model="val"></div>');
129+
var ctrl = element.controller('ngModel');
130+
ctrl.$isEmpty = function(value) {
131+
return value === 'not-checked';
132+
};
133+
134+
scope.$apply('val = true');
135+
expect(ctrl.$modelValue).toBe(true);
136+
expect(element.attr('aria-checked')).toBe('true');
137+
138+
scope.$apply('val = false');
139+
expect(ctrl.$modelValue).toBe(false);
140+
expect(element.attr('aria-checked')).toBe('true');
141+
142+
scope.$apply('val = "not-checked"');
143+
expect(ctrl.$modelValue).toBe('not-checked');
144+
expect(element.attr('aria-checked')).toBe('false');
145+
146+
scope.$apply('val = "checked"');
147+
expect(ctrl.$modelValue).toBe('checked');
148+
expect(element.attr('aria-checked')).toBe('true');
149+
}
150+
);
151+
127152
it('should not handle native checkbox with ngChecked', function() {
128153
var element = $compile('<input type="checkbox" ng-checked="val">')(scope);
129154

@@ -210,11 +235,12 @@ describe('$aria', function() {
210235
});
211236

212237
it('should attach itself to role="menuitemcheckbox"', function() {
213-
scope.val = true;
214238
compileElement('<div role="menuitemcheckbox" ng-model="val"></div>');
239+
240+
scope.$apply('val = "checked"');
215241
expect(element.attr('aria-checked')).toBe('true');
216242

217-
scope.$apply('val = false');
243+
scope.$apply('val = null');
218244
expect(element.attr('aria-checked')).toBe('false');
219245
});
220246

@@ -832,6 +858,28 @@ describe('$aria', function() {
832858
expect(element.attr('tabindex')).toBeUndefined();
833859
});
834860
});
861+
862+
describe('ngModel', function() {
863+
it('should not break when manually compiling', function() {
864+
module(function($compileProvider) {
865+
$compileProvider.directive('foo', function() {
866+
return {
867+
priority: 10,
868+
terminal: true,
869+
link: function(scope, elem) {
870+
$compile(elem, null, 10)(scope);
871+
}
872+
};
873+
});
874+
});
875+
876+
injectScopeAndCompiler();
877+
compileElement('<div role="checkbox" ng-model="value" foo />');
878+
879+
// Just check an arbitrary feature to make sure it worked
880+
expect(element.attr('tabindex')).toBe('0');
881+
});
882+
});
835883
});
836884

837885
function expectAriaAttrOnEachElement(elem, ariaAttr, expected) {

0 commit comments

Comments
 (0)