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

Commit

Permalink
feat($compile): do not interpolate boolean attributes, rather evaluat…
Browse files Browse the repository at this point in the history
…e 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
  • Loading branch information
vojtajina committed Mar 27, 2012
1 parent 5502713 commit a08cbc0
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 106 deletions.
4 changes: 2 additions & 2 deletions docs/content/cookbook/advancedform.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ detection, and preventing invalid form submission.
<input type="text" ng-model="contact.value" required/>
[ <a href="" ng-click="removeContact(contact)">X</a> ]
</div>
<button ng-click="cancel()" ng-disabled="{{isCancelDisabled()}}">Cancel</button>
<button ng-click="save()" ng-disabled="{{isSaveDisabled()}}">Save</button>
<button ng-click="cancel()" ng-disabled="isCancelDisabled()">Cancel</button>
<button ng-click="save()" ng-disabled="isSaveDisabled()">Save</button>
</form>

<hr/>
Expand Down
73 changes: 49 additions & 24 deletions src/directive/booleanAttrDirs.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
<doc:example>
<doc:source>
Click me to toggle: <input type="checkbox" ng-model="checked"><br/>
<button ng-model="button" ng-disabled="{{checked}}">Button</button>
<button ng-model="button" ng-disabled="checked">Button</button>
</doc:source>
<doc:scenario>
it('should toggle button', function() {
Expand All @@ -142,7 +142,7 @@
</doc:example>
*
* @element INPUT
* @param {template} ng-disabled any string which can contain '{{}}' markup.
* @param {string} expression Angular expression that will be evaluated.
*/


Expand All @@ -160,7 +160,7 @@
<doc:example>
<doc:source>
Check me to check both: <input type="checkbox" ng-model="master"><br/>
<input id="checkSlave" type="checkbox" ng-checked="{{master}}">
<input id="checkSlave" type="checkbox" ng-checked="master">
</doc:source>
<doc:scenario>
it('should check both checkBoxes', function() {
Expand All @@ -172,7 +172,7 @@
</doc:example>
*
* @element INPUT
* @param {template} ng-checked any string which can contain '{{}}' markup.
* @param {string} expression Angular expression that will be evaluated.
*/


Expand All @@ -191,7 +191,7 @@
<doc:example>
<doc:source>
Check me check multiple: <input type="checkbox" ng-model="checked"><br/>
<select id="select" ng-multiple="{{checked}}">
<select id="select" ng-multiple="checked">
<option>Misko</option>
<option>Igor</option>
<option>Vojta</option>
Expand All @@ -208,7 +208,7 @@
</doc:example>
*
* @element SELECT
* @param {template} ng-multiple any string which can contain '{{}}' markup.
* @param {string} expression Angular expression that will be evaluated.
*/


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


Expand All @@ -255,35 +255,60 @@
* @example
<doc:example>
<doc:source>
Check me to select: <input type="checkbox" ng-model="checked"><br/>
Check me to select: <input type="checkbox" ng-model="selected"><br/>
<select>
<option>Hello!</option>
<option id="greet" ng-selected="{{checked}}">Greetings!</option>
<option id="greet" ng-selected="selected">Greetings!</option>
</select>
</doc:source>
<doc:scenario>
it('should select Greetings!', function() {
expect(element('.doc-example-live #greet').prop('selected')).toBeFalsy();
input('checked').check();
input('selected').check();
expect(element('.doc-example-live #greet').prop('selected')).toBeTruthy();
});
</doc:scenario>
</doc:example>
*
* @element OPTION
* @param {template} ng-selected any string which can contain '{{}}' markup.
* @param {string} expression Angular expression that will be evaluated.
*/


function ngAttributeAliasDirective(propName, attrName) {
ngAttributeAliasDirectives[directiveNormalize('ng-' + attrName)] = valueFn(
function(scope, element, attr) {
attr.$observe(directiveNormalize('ng-' + attrName), function(value) {
attr.$set(attrName, value);
});
}
);
}

var ngAttributeAliasDirectives = {};
forEach(BOOLEAN_ATTR, ngAttributeAliasDirective);
ngAttributeAliasDirective(null, 'src');


// boolean attrs are evaluated
forEach(BOOLEAN_ATTR, function(propName, attrName) {
var normalized = directiveNormalize('ng-' + attrName);
ngAttributeAliasDirectives[normalized] = function() {
return {
compile: function(tpl, attr) {
attr.$observers[attrName] = [];
return function(scope, element, attr) {
scope.$watch(attr[normalized], function(value) {
attr.$set(attrName, value);
});
};
}
};
};
});


// ng-src, ng-href are interpolated
forEach(['src', 'href'], function(attrName) {
var normalized = directiveNormalize('ng-' + attrName);
ngAttributeAliasDirectives[normalized] = function() {
return {
compile: function(tpl, attr) {
attr.$observers[attrName] = [];
return function(scope, element, attr) {
attr.$observe(normalized, function(value) {
attr.$set(attrName, value);
});
};
}
};
};
});
66 changes: 21 additions & 45 deletions src/service/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,7 @@ function $CompileProvider($provide) {
COMMENT_DIRECTIVE_REGEXP = /^\s*directive\:\s*([\d\w\-_]+)\s+(.*)$/,
CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/,
CONTENT_REGEXP = /\<\<content\>\>/i,
HAS_ROOT_ELEMENT = /^\<[\s\S]*\>$/,
SIDE_EFFECT_ATTRS = {};

forEach('src,href,multiple,selected,checked,disabled,readonly,required'.split(','), function(name) {
SIDE_EFFECT_ATTRS[name] = name;
SIDE_EFFECT_ATTRS[directiveNormalize('ng_' + name)] = name;
});
HAS_ROOT_ELEMENT = /^\<[\s\S]*\>$/;


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


function addAttrInterpolateDirective(node, directives, value, name) {
var interpolateFn = $interpolate(value, true),
realName = SIDE_EFFECT_ATTRS[name],
specialAttrDir = (realName && (realName !== name));

realName = realName || name;
var interpolateFn = $interpolate(value, true);

if (specialAttrDir && isBooleanAttr(node, name)) {
value = true;
}

// no interpolation found and we are not a side-effect attr -> ignore
if (!interpolateFn && !specialAttrDir) {
return;
}
// no interpolation found -> ignore
if (!interpolateFn) return;

directives.push({
priority: 100,
compile: function(element, attr) {
if (interpolateFn) {
return function(scope, element, attr) {
if (name === 'class') {
// we need to interpolate classes again, in the case the element was replaced
// and therefore the two class attrs got merged - we want to interpolate the result
interpolateFn = $interpolate(attr[name], true);
}

// we define observers array only for interpolated attrs
// and ignore observers for non interpolated attrs to save some memory
attr.$observers[realName] = [];
attr[realName] = undefined;
scope.$watch(interpolateFn, function(value) {
attr.$set(realName, value);
});
};
} else {
attr.$set(realName, value);
compile: valueFn(function(scope, element, attr) {
if (name === 'class') {
// we need to interpolate classes again, in the case the element was replaced
// and therefore the two class attrs got merged - we want to interpolate the result
interpolateFn = $interpolate(attr[name], true);
}
}

// we define observers array only for interpolated attrs
// and ignore observers for non interpolated attrs to save some memory
attr.$observers[name] = [];
attr[name] = undefined;
scope.$watch(interpolateFn, function(value) {
attr.$set(name, value);
});
})
});
}

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

if (booleanKey) {
value = toBoolean(value);
this.$element.prop(key, value);
this[key] = value;
attrName = key = booleanKey;
value = value ? booleanKey : undefined;
} else {
this[key] = value;
attrName = booleanKey;
}

this[key] = value;

// translate normalized key to actual key
if (attrName) {
this.$attr[key] = attrName;
Expand Down
46 changes: 30 additions & 16 deletions test/directive/booleanAttrDirSpecs.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('boolean attr directives', function() {


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


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


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


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


it('should bind multiple', inject(function($rootScope, $compile) {
element = $compile('<select ng-multiple="{{isMultiple}}"></select>')($rootScope)
element = $compile('<select ng-multiple="isMultiple"></select>')($rootScope)
$rootScope.isMultiple=false;
$rootScope.$digest();
expect(element.attr('multiple')).toBeFalsy();
Expand All @@ -88,24 +88,38 @@ describe('boolean attr directives', function() {
expect(element.attr('href')).toEqual('http://server');
expect(element.attr('rel')).toEqual('REL');
}));
});


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

element = $compile('<div ng-src="some"></div>')($rootScope)
it('should interpolate the expression and bind to src', inject(function($compile, $rootScope) {
var element = $compile('<div ng-src="some/{{id}}"></div>')($rootScope)
$rootScope.$digest();
expect(element.attr('src')).toEqual('some');
expect(element.attr('src')).toEqual('some/');

$rootScope.$apply(function() {
$rootScope.id = 1;
});
expect(element.attr('src')).toEqual('some/1');

dealoc(element);
}));
});


describe('ng-href', function() {

element = $compile('<div ng-href="some"></div>')($rootScope)
it('should interpolate the expression and bind to href', inject(function($compile, $rootScope) {
var element = $compile('<div ng-href="some/{{id}}"></div>')($rootScope)
$rootScope.$digest();
expect(element.attr('href')).toEqual('some');
expect(element.attr('href')).toEqual('some/');

$rootScope.$apply(function() {
$rootScope.id = 1;
});
expect(element.attr('href')).toEqual('some/1');

dealoc(element);
}));
});
4 changes: 2 additions & 2 deletions test/directive/inputSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -941,8 +941,8 @@ describe('input', function() {

describe('required', function() {

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

scope.$apply(function() {
scope.required = false;
Expand Down
2 changes: 1 addition & 1 deletion test/directive/selectSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ describe('select', function() {
createSelect({
'ng-model': 'value',
'ng-options': 'item.name for item in values',
'ng-required': '{{required}}'
'ng-required': 'required'
}, true);


Expand Down
16 changes: 0 additions & 16 deletions test/service/compilerSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1411,22 +1411,6 @@ describe('$compile', function() {
});


it('should set boolean attributes', function() {
attr.$set('disabled', 'true');
attr.$set('readOnly', 'true');
expect(element.attr('disabled')).toEqual('disabled');
expect(element.attr('readonly')).toEqual('readonly');

attr.$set('disabled', 'false');
expect(element.attr('disabled')).toEqual(undefined);

attr.$set('disabled', false);
attr.$set('readOnly', false);
expect(element.attr('disabled')).toEqual(undefined);
expect(element.attr('readonly')).toEqual(undefined);
});


it('should remove attribute', function() {
attr.$set('ngMyAttr', 'value');
expect(element.attr('ng-my-attr')).toEqual('value');
Expand Down

0 comments on commit a08cbc0

Please sign in to comment.