Skip to content

Commit a08cbc0

Browse files
vojtajinavojtajina
committed
feat($compile): do not interpolate boolean attributes, rather evaluate them
So that we can have non string values, e.g. ng-value="true" for radio inputs Breaks boolean attrs are evaluated rather than interpolated To migrate your code, change: <input ng-disabled="{{someBooleanVariable}}"> to: <input ng-disabled="someBooleanVariabla"> Affected directives: * ng-multiple * ng-selected * ng-checked * ng-disabled * ng-readonly * ng-required
1 parent 5502713 commit a08cbc0

File tree

7 files changed

+105
-106
lines changed

7 files changed

+105
-106
lines changed

‎docs/content/cookbook/advancedform.ngdoc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ detection, and preventing invalid form submission.
8585
<input type="text" ng-model="contact.value" required/>
8686
[ <a href="" ng-click="removeContact(contact)">X</a> ]
8787
</div>
88-
<button ng-click="cancel()" ng-disabled="{{isCancelDisabled()}}">Cancel</button>
89-
<button ng-click="save()" ng-disabled="{{isSaveDisabled()}}">Save</button>
88+
<button ng-click="cancel()" ng-disabled="isCancelDisabled()">Cancel</button>
89+
<button ng-click="save()" ng-disabled="isSaveDisabled()">Save</button>
9090
</form>
9191

9292
<hr/>

‎src/directive/booleanAttrDirs.js

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@
130130
<doc:example>
131131
<doc:source>
132132
Click me to toggle: <input type="checkbox" ng-model="checked"><br/>
133-
<button ng-model="button" ng-disabled="{{checked}}">Button</button>
133+
<button ng-model="button" ng-disabled="checked">Button</button>
134134
</doc:source>
135135
<doc:scenario>
136136
it('should toggle button', function() {
@@ -142,7 +142,7 @@
142142
</doc:example>
143143
*
144144
* @element INPUT
145-
* @param {template} ng-disabled any string which can contain '{{}}' markup.
145+
* @param {string} expression Angular expression that will be evaluated.
146146
*/
147147

148148

@@ -160,7 +160,7 @@
160160
<doc:example>
161161
<doc:source>
162162
Check me to check both: <input type="checkbox" ng-model="master"><br/>
163-
<input id="checkSlave" type="checkbox" ng-checked="{{master}}">
163+
<input id="checkSlave" type="checkbox" ng-checked="master">
164164
</doc:source>
165165
<doc:scenario>
166166
it('should check both checkBoxes', function() {
@@ -172,7 +172,7 @@
172172
</doc:example>
173173
*
174174
* @element INPUT
175-
* @param {template} ng-checked any string which can contain '{{}}' markup.
175+
* @param {string} expression Angular expression that will be evaluated.
176176
*/
177177

178178

@@ -191,7 +191,7 @@
191191
<doc:example>
192192
<doc:source>
193193
Check me check multiple: <input type="checkbox" ng-model="checked"><br/>
194-
<select id="select" ng-multiple="{{checked}}">
194+
<select id="select" ng-multiple="checked">
195195
<option>Misko</option>
196196
<option>Igor</option>
197197
<option>Vojta</option>
@@ -208,7 +208,7 @@
208208
</doc:example>
209209
*
210210
* @element SELECT
211-
* @param {template} ng-multiple any string which can contain '{{}}' markup.
211+
* @param {string} expression Angular expression that will be evaluated.
212212
*/
213213

214214

@@ -226,7 +226,7 @@
226226
<doc:example>
227227
<doc:source>
228228
Check me to make text readonly: <input type="checkbox" ng-model="checked"><br/>
229-
<input type="text" ng-readonly="{{checked}}" value="I'm Angular"/>
229+
<input type="text" ng-readonly="checked" value="I'm Angular"/>
230230
</doc:source>
231231
<doc:scenario>
232232
it('should toggle readonly attr', function() {
@@ -238,7 +238,7 @@
238238
</doc:example>
239239
*
240240
* @element INPUT
241-
* @param {template} ng-readonly any string which can contain '{{}}' markup.
241+
* @param {string} expression Angular expression that will be evaluated.
242242
*/
243243

244244

@@ -255,35 +255,60 @@
255255
* @example
256256
<doc:example>
257257
<doc:source>
258-
Check me to select: <input type="checkbox" ng-model="checked"><br/>
258+
Check me to select: <input type="checkbox" ng-model="selected"><br/>
259259
<select>
260260
<option>Hello!</option>
261-
<option id="greet" ng-selected="{{checked}}">Greetings!</option>
261+
<option id="greet" ng-selected="selected">Greetings!</option>
262262
</select>
263263
</doc:source>
264264
<doc:scenario>
265265
it('should select Greetings!', function() {
266266
expect(element('.doc-example-live #greet').prop('selected')).toBeFalsy();
267-
input('checked').check();
267+
input('selected').check();
268268
expect(element('.doc-example-live #greet').prop('selected')).toBeTruthy();
269269
});
270270
</doc:scenario>
271271
</doc:example>
272+
*
272273
* @element OPTION
273-
* @param {template} ng-selected any string which can contain '{{}}' markup.
274+
* @param {string} expression Angular expression that will be evaluated.
274275
*/
275-
276276

277-
function ngAttributeAliasDirective(propName, attrName) {
278-
ngAttributeAliasDirectives[directiveNormalize('ng-' + attrName)] = valueFn(
279-
function(scope, element, attr) {
280-
attr.$observe(directiveNormalize('ng-' + attrName), function(value) {
281-
attr.$set(attrName, value);
282-
});
283-
}
284-
);
285-
}
286277

287278
var ngAttributeAliasDirectives = {};
288-
forEach(BOOLEAN_ATTR, ngAttributeAliasDirective);
289-
ngAttributeAliasDirective(null, 'src');
279+
280+
281+
// boolean attrs are evaluated
282+
forEach(BOOLEAN_ATTR, function(propName, attrName) {
283+
var normalized = directiveNormalize('ng-' + attrName);
284+
ngAttributeAliasDirectives[normalized] = function() {
285+
return {
286+
compile: function(tpl, attr) {
287+
attr.$observers[attrName] = [];
288+
return function(scope, element, attr) {
289+
scope.$watch(attr[normalized], function(value) {
290+
attr.$set(attrName, value);
291+
});
292+
};
293+
}
294+
};
295+
};
296+
});
297+
298+
299+
// ng-src, ng-href are interpolated
300+
forEach(['src', 'href'], function(attrName) {
301+
var normalized = directiveNormalize('ng-' + attrName);
302+
ngAttributeAliasDirectives[normalized] = function() {
303+
return {
304+
compile: function(tpl, attr) {
305+
attr.$observers[attrName] = [];
306+
return function(scope, element, attr) {
307+
attr.$observe(normalized, function(value) {
308+
attr.$set(attrName, value);
309+
});
310+
};
311+
}
312+
};
313+
};
314+
});

‎src/service/compiler.js

Lines changed: 21 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,7 @@ function $CompileProvider($provide) {
128128
COMMENT_DIRECTIVE_REGEXP = /^\s*directive\:\s*([\d\w\-_]+)\s+(.*)$/,
129129
CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/,
130130
CONTENT_REGEXP = /\<\<content\>\>/i,
131-
HAS_ROOT_ELEMENT = /^\<[\s\S]*\>$/,
132-
SIDE_EFFECT_ATTRS = {};
133-
134-
forEach('src,href,multiple,selected,checked,disabled,readonly,required'.split(','), function(name) {
135-
SIDE_EFFECT_ATTRS[name] = name;
136-
SIDE_EFFECT_ATTRS[directiveNormalize('ng_' + name)] = name;
137-
});
131+
HAS_ROOT_ELEMENT = /^\<[\s\S]*\>$/;
138132

139133

140134
this.directive = function registerDirective(name, directiveFactory) {
@@ -861,44 +855,29 @@ function $CompileProvider($provide) {
861855

862856

863857
function addAttrInterpolateDirective(node, directives, value, name) {
864-
var interpolateFn = $interpolate(value, true),
865-
realName = SIDE_EFFECT_ATTRS[name],
866-
specialAttrDir = (realName && (realName !== name));
867-
868-
realName = realName || name;
858+
var interpolateFn = $interpolate(value, true);
869859

870-
if (specialAttrDir && isBooleanAttr(node, name)) {
871-
value = true;
872-
}
873860

874-
// no interpolation found and we are not a side-effect attr -> ignore
875-
if (!interpolateFn && !specialAttrDir) {
876-
return;
877-
}
861+
// no interpolation found -> ignore
862+
if (!interpolateFn) return;
878863

879864
directives.push({
880865
priority: 100,
881-
compile: function(element, attr) {
882-
if (interpolateFn) {
883-
return function(scope, element, attr) {
884-
if (name === 'class') {
885-
// we need to interpolate classes again, in the case the element was replaced
886-
// and therefore the two class attrs got merged - we want to interpolate the result
887-
interpolateFn = $interpolate(attr[name], true);
888-
}
889-
890-
// we define observers array only for interpolated attrs
891-
// and ignore observers for non interpolated attrs to save some memory
892-
attr.$observers[realName] = [];
893-
attr[realName] = undefined;
894-
scope.$watch(interpolateFn, function(value) {
895-
attr.$set(realName, value);
896-
});
897-
};
898-
} else {
899-
attr.$set(realName, value);
866+
compile: valueFn(function(scope, element, attr) {
867+
if (name === 'class') {
868+
// we need to interpolate classes again, in the case the element was replaced
869+
// and therefore the two class attrs got merged - we want to interpolate the result
870+
interpolateFn = $interpolate(attr[name], true);
900871
}
901-
}
872+
873+
// we define observers array only for interpolated attrs
874+
// and ignore observers for non interpolated attrs to save some memory
875+
attr.$observers[name] = [];
876+
attr[name] = undefined;
877+
scope.$watch(interpolateFn, function(value) {
878+
attr.$set(name, value);
879+
});
880+
})
902881
});
903882
}
904883

@@ -945,15 +924,12 @@ function $CompileProvider($provide) {
945924
var booleanKey = isBooleanAttr(this.$element[0], key.toLowerCase());
946925

947926
if (booleanKey) {
948-
value = toBoolean(value);
949927
this.$element.prop(key, value);
950-
this[key] = value;
951-
attrName = key = booleanKey;
952-
value = value ? booleanKey : undefined;
953-
} else {
954-
this[key] = value;
928+
attrName = booleanKey;
955929
}
956930

931+
this[key] = value;
932+
957933
// translate normalized key to actual key
958934
if (attrName) {
959935
this.$attr[key] = attrName;

‎test/directive/booleanAttrDirSpecs.js

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ describe('boolean attr directives', function() {
1717

1818

1919
it('should bind disabled', inject(function($rootScope, $compile) {
20-
element = $compile('<button ng-disabled="{{isDisabled}}">Button</button>')($rootScope)
20+
element = $compile('<button ng-disabled="isDisabled">Button</button>')($rootScope)
2121
$rootScope.isDisabled = false;
2222
$rootScope.$digest();
2323
expect(element.attr('disabled')).toBeFalsy();
@@ -28,7 +28,7 @@ describe('boolean attr directives', function() {
2828

2929

3030
it('should bind checked', inject(function($rootScope, $compile) {
31-
element = $compile('<input type="checkbox" ng-checked="{{isChecked}}" />')($rootScope)
31+
element = $compile('<input type="checkbox" ng-checked="isChecked" />')($rootScope)
3232
$rootScope.isChecked = false;
3333
$rootScope.$digest();
3434
expect(element.attr('checked')).toBeFalsy();
@@ -39,7 +39,7 @@ describe('boolean attr directives', function() {
3939

4040

4141
it('should bind selected', inject(function($rootScope, $compile) {
42-
element = $compile('<select><option value=""></option><option ng-selected="{{isSelected}}">Greetings!</option></select>')($rootScope)
42+
element = $compile('<select><option value=""></option><option ng-selected="isSelected">Greetings!</option></select>')($rootScope)
4343
jqLite(document.body).append(element)
4444
$rootScope.isSelected=false;
4545
$rootScope.$digest();
@@ -51,7 +51,7 @@ describe('boolean attr directives', function() {
5151

5252

5353
it('should bind readonly', inject(function($rootScope, $compile) {
54-
element = $compile('<input type="text" ng-readonly="{{isReadonly}}" />')($rootScope)
54+
element = $compile('<input type="text" ng-readonly="isReadonly" />')($rootScope)
5555
$rootScope.isReadonly=false;
5656
$rootScope.$digest();
5757
expect(element.attr('readOnly')).toBeFalsy();
@@ -62,7 +62,7 @@ describe('boolean attr directives', function() {
6262

6363

6464
it('should bind multiple', inject(function($rootScope, $compile) {
65-
element = $compile('<select ng-multiple="{{isMultiple}}"></select>')($rootScope)
65+
element = $compile('<select ng-multiple="isMultiple"></select>')($rootScope)
6666
$rootScope.isMultiple=false;
6767
$rootScope.$digest();
6868
expect(element.attr('multiple')).toBeFalsy();
@@ -88,24 +88,38 @@ describe('boolean attr directives', function() {
8888
expect(element.attr('href')).toEqual('http://server');
8989
expect(element.attr('rel')).toEqual('REL');
9090
}));
91+
});
9192

9293

93-
it('should bind Text with no Bindings', inject(function($compile, $rootScope) {
94-
forEach(['checked', 'disabled', 'multiple', 'readonly', 'selected'], function(name) {
95-
element = $compile('<div ng-' + name + '="some"></div>')($rootScope)
96-
$rootScope.$digest();
97-
expect(element.attr(name)).toBe(name);
98-
dealoc(element);
99-
});
94+
describe('ng-src', function() {
10095

101-
element = $compile('<div ng-src="some"></div>')($rootScope)
96+
it('should interpolate the expression and bind to src', inject(function($compile, $rootScope) {
97+
var element = $compile('<div ng-src="some/{{id}}"></div>')($rootScope)
10298
$rootScope.$digest();
103-
expect(element.attr('src')).toEqual('some');
99+
expect(element.attr('src')).toEqual('some/');
100+
101+
$rootScope.$apply(function() {
102+
$rootScope.id = 1;
103+
});
104+
expect(element.attr('src')).toEqual('some/1');
105+
104106
dealoc(element);
107+
}));
108+
});
109+
110+
111+
describe('ng-href', function() {
105112

106-
element = $compile('<div ng-href="some"></div>')($rootScope)
113+
it('should interpolate the expression and bind to href', inject(function($compile, $rootScope) {
114+
var element = $compile('<div ng-href="some/{{id}}"></div>')($rootScope)
107115
$rootScope.$digest();
108-
expect(element.attr('href')).toEqual('some');
116+
expect(element.attr('href')).toEqual('some/');
117+
118+
$rootScope.$apply(function() {
119+
$rootScope.id = 1;
120+
});
121+
expect(element.attr('href')).toEqual('some/1');
122+
109123
dealoc(element);
110124
}));
111125
});

‎test/directive/inputSpec.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -941,8 +941,8 @@ describe('input', function() {
941941

942942
describe('required', function() {
943943

944-
it('should allow bindings on required', function() {
945-
compileInput('<input type="text" ng-model="value" required="{{required}}" />');
944+
it('should allow bindings on ng-required', function() {
945+
compileInput('<input type="text" ng-model="value" ng-required="required" />');
946946

947947
scope.$apply(function() {
948948
scope.required = false;

‎test/directive/selectSpec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -780,7 +780,7 @@ describe('select', function() {
780780
createSelect({
781781
'ng-model': 'value',
782782
'ng-options': 'item.name for item in values',
783-
'ng-required': '{{required}}'
783+
'ng-required': 'required'
784784
}, true);
785785

786786

‎test/service/compilerSpec.js

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,22 +1411,6 @@ describe('$compile', function() {
14111411
});
14121412

14131413

1414-
it('should set boolean attributes', function() {
1415-
attr.$set('disabled', 'true');
1416-
attr.$set('readOnly', 'true');
1417-
expect(element.attr('disabled')).toEqual('disabled');
1418-
expect(element.attr('readonly')).toEqual('readonly');
1419-
1420-
attr.$set('disabled', 'false');
1421-
expect(element.attr('disabled')).toEqual(undefined);
1422-
1423-
attr.$set('disabled', false);
1424-
attr.$set('readOnly', false);
1425-
expect(element.attr('disabled')).toEqual(undefined);
1426-
expect(element.attr('readonly')).toEqual(undefined);
1427-
});
1428-
1429-
14301414
it('should remove attribute', function() {
14311415
attr.$set('ngMyAttr', 'value');
14321416
expect(element.attr('ng-my-attr')).toEqual('value');

0 commit comments

Comments
 (0)