From a08cbc02e78e789a66e9af771c410e8ad1646e25 Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Fri, 23 Mar 2012 15:53:04 -0700 Subject: [PATCH] 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: to: Affected directives: * ng-multiple * ng-selected * ng-checked * ng-disabled * ng-readonly * ng-required --- docs/content/cookbook/advancedform.ngdoc | 4 +- src/directive/booleanAttrDirs.js | 73 ++++++++++++++++-------- src/service/compiler.js | 66 +++++++-------------- test/directive/booleanAttrDirSpecs.js | 46 +++++++++------ test/directive/inputSpec.js | 4 +- test/directive/selectSpec.js | 2 +- test/service/compilerSpec.js | 16 ------ 7 files changed, 105 insertions(+), 106 deletions(-) diff --git a/docs/content/cookbook/advancedform.ngdoc b/docs/content/cookbook/advancedform.ngdoc index 37c5da0d377f..1501f955609f 100644 --- a/docs/content/cookbook/advancedform.ngdoc +++ b/docs/content/cookbook/advancedform.ngdoc @@ -85,8 +85,8 @@ detection, and preventing invalid form submission. [ X ] - - + +
diff --git a/src/directive/booleanAttrDirs.js b/src/directive/booleanAttrDirs.js index 10c6eee892a8..7da52db0c005 100644 --- a/src/directive/booleanAttrDirs.js +++ b/src/directive/booleanAttrDirs.js @@ -130,7 +130,7 @@ Click me to toggle:
- + it('should toggle button', function() { @@ -142,7 +142,7 @@ * * @element INPUT - * @param {template} ng-disabled any string which can contain '{{}}' markup. + * @param {string} expression Angular expression that will be evaluated. */ @@ -160,7 +160,7 @@ Check me to check both:
- + it('should check both checkBoxes', function() { @@ -172,7 +172,7 @@ * * @element INPUT - * @param {template} ng-checked any string which can contain '{{}}' markup. + * @param {string} expression Angular expression that will be evaluated. */ @@ -191,7 +191,7 @@ Check me check multiple:
- @@ -208,7 +208,7 @@ * * @element SELECT - * @param {template} ng-multiple any string which can contain '{{}}' markup. + * @param {string} expression Angular expression that will be evaluated. */ @@ -226,7 +226,7 @@ Check me to make text readonly:
- + it('should toggle readonly attr', function() { @@ -238,7 +238,7 @@ * * @element INPUT - * @param {template} ng-readonly any string which can contain '{{}}' markup. + * @param {string} expression Angular expression that will be evaluated. */ @@ -255,35 +255,60 @@ * @example - Check me to select:
+ Check me to select:
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(); }); + * * @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); + }); + }; + } + }; + }; +}); diff --git a/src/service/compiler.js b/src/service/compiler.js index 8ddf77ae5614..a22c5d669d03 100644 --- a/src/service/compiler.js +++ b/src/service/compiler.js @@ -128,13 +128,7 @@ function $CompileProvider($provide) { COMMENT_DIRECTIVE_REGEXP = /^\s*directive\:\s*([\d\w\-_]+)\s+(.*)$/, CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/, CONTENT_REGEXP = /\<\\>/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) { @@ -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); + }); + }) }); } @@ -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; diff --git a/test/directive/booleanAttrDirSpecs.js b/test/directive/booleanAttrDirSpecs.js index 8d71c2d858ec..7a4244a8f74e 100644 --- a/test/directive/booleanAttrDirSpecs.js +++ b/test/directive/booleanAttrDirSpecs.js @@ -17,7 +17,7 @@ describe('boolean attr directives', function() { it('should bind disabled', inject(function($rootScope, $compile) { - element = $compile('')($rootScope) + element = $compile('')($rootScope) $rootScope.isDisabled = false; $rootScope.$digest(); expect(element.attr('disabled')).toBeFalsy(); @@ -28,7 +28,7 @@ describe('boolean attr directives', function() { it('should bind checked', inject(function($rootScope, $compile) { - element = $compile('')($rootScope) + element = $compile('')($rootScope) $rootScope.isChecked = false; $rootScope.$digest(); expect(element.attr('checked')).toBeFalsy(); @@ -39,7 +39,7 @@ describe('boolean attr directives', function() { it('should bind selected', inject(function($rootScope, $compile) { - element = $compile('')($rootScope) + element = $compile('')($rootScope) jqLite(document.body).append(element) $rootScope.isSelected=false; $rootScope.$digest(); @@ -51,7 +51,7 @@ describe('boolean attr directives', function() { it('should bind readonly', inject(function($rootScope, $compile) { - element = $compile('')($rootScope) + element = $compile('')($rootScope) $rootScope.isReadonly=false; $rootScope.$digest(); expect(element.attr('readOnly')).toBeFalsy(); @@ -62,7 +62,7 @@ describe('boolean attr directives', function() { it('should bind multiple', inject(function($rootScope, $compile) { - element = $compile('')($rootScope) + element = $compile('')($rootScope) $rootScope.isMultiple=false; $rootScope.$digest(); expect(element.attr('multiple')).toBeFalsy(); @@ -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('
')($rootScope) - $rootScope.$digest(); - expect(element.attr(name)).toBe(name); - dealoc(element); - }); +describe('ng-src', function() { - element = $compile('
')($rootScope) + it('should interpolate the expression and bind to src', inject(function($compile, $rootScope) { + var element = $compile('
')($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('
')($rootScope) + it('should interpolate the expression and bind to href', inject(function($compile, $rootScope) { + var element = $compile('
')($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); })); }); diff --git a/test/directive/inputSpec.js b/test/directive/inputSpec.js index 8d0e44b3eeb8..e5f083b37b81 100644 --- a/test/directive/inputSpec.js +++ b/test/directive/inputSpec.js @@ -941,8 +941,8 @@ describe('input', function() { describe('required', function() { - it('should allow bindings on required', function() { - compileInput(''); + it('should allow bindings on ng-required', function() { + compileInput(''); scope.$apply(function() { scope.required = false; diff --git a/test/directive/selectSpec.js b/test/directive/selectSpec.js index 3f8fb56e6045..2e3cfaafe744 100644 --- a/test/directive/selectSpec.js +++ b/test/directive/selectSpec.js @@ -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); diff --git a/test/service/compilerSpec.js b/test/service/compilerSpec.js index 698fc23e0ae4..dc2e20cf4feb 100644 --- a/test/service/compilerSpec.js +++ b/test/service/compilerSpec.js @@ -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');