fix(directive): ng:options now support binding to expression

Closes angular#449
mhevery · Jul 20, 2011 · c32184d · c32184d
1 parent a4f1ed5
commit c32184d
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,8 @@
 <a name="0.9.18"><a/>
 # <angular/> 0.9.18 jiggling-armfat (in-progress) #
 
+### Bug Fixes
+- Issue #449: [ng:options] should support binding to a property of an item.
 
 
 

diff --git a/src/widgets.js b/src/widgets.js
@@ -596,12 +596,14 @@ angularWidget('button', inputWidgetSelector);
  *   * binding to a value not in list confuses most browsers.
  *
  * @element select
- * @param {comprehension_expression} comprehension _expresion_ `for` _item_ `in` _array_.
+ * @param {comprehension_expression} comprehension _select_ `as` _label_ `for` _item_ `in` _array_.
  *
  *   * _array_: an expression which evaluates to an array of objects to bind.
  *   * _item_: local variable which will refer to the item in the _array_ during the iteration
- *   * _expression_: The result of this expression will be `option` label. The
- *        `expression` most likely refers to the _item_ variable.
+ *   * _select_: The result of this expression will be assigned to the scope.
+ *      The _select_ can be ommited, in which case the _item_ itself will be assigned.
+ *   * _label_: The result of this expression will be the `option` label. The
+ *        `expression` most likely reffers to the _item_ variable. (optional)
  *
  * @example
     <doc:example>
@@ -657,7 +659,7 @@ angularWidget('button', inputWidgetSelector);
     </doc:example>
  */
 
-var NG_OPTIONS_REGEXP = /^(.*)\s+for\s+([\$\w][\$\w\d]*)\s+in\s+(.*)$/;
+var NG_OPTIONS_REGEXP = /^\s*((.*)\s+as\s+)?(.*)\s+for\s+([\$\w][\$\w\d]*)\s+in\s+(.*)$/;
 angularWidget('select', function(element){
   this.descend(true);
   this.directives(true);
@@ -669,12 +671,13 @@ angularWidget('select', function(element){
   }
   if (! (match = expression.match(NG_OPTIONS_REGEXP))) {
     throw Error(
-        "Expected ng:options in form of '_expresion_ for _item_ in _collection_' but got '" +
+        "Expected ng:options in form of '(_expression_ as)? _expresion_ for _item_ in _collection_' but got '" +
         expression + "'.");
   }
-  var displayFn = expressionCompile(match[1]).fnSelf;
-  var itemName = match[2];
-  var collectionFn = expressionCompile(match[3]).fnSelf;
+  var displayFn = expressionCompile(match[3]).fnSelf;
+  var itemName = match[4];
+  var itemFn = expressionCompile(match[2] || itemName).fnSelf;
+  var collectionFn = expressionCompile(match[5]).fnSelf;
   // we can't just jqLite('<option>') since jqLite is not smart enough
   // to create it in <select> and IE barfs otherwise.
   var option = jqLite(document.createElement('option'));
@@ -696,24 +699,33 @@ angularWidget('select', function(element){
       var collection = collectionFn(scope) || [];
       var value = select.val();
       var index, length;
-      if (isMultiselect) {
-        value = [];
-        for (index = 0, length = optionElements.length; index < length; index++) {
-          if (optionElements[index][0].selected) {
-            value.push(collection[index]);
+      var tempScope = scope.$new();
+      try {
+        if (isMultiselect) {
+          value = [];
+          for (index = 0, length = optionElements.length; index < length; index++) {
+            if (optionElements[index][0].selected) {
+              tempScope[itemName] = collection[index];
+              value.push(itemFn(tempScope));
+            }
           }
-        }
-      } else {
-        if (value == '?') {
-          value = undefined;
         } else {
-          value = (value == '' ? null : collection[value]);
+          if (value == '?') {
+            value = undefined;
+          } else if (value == ''){
+            value = null;
+          } else {
+            tempScope[itemName] = collection[value];
+            value = itemFn(tempScope);
+          }
         }
+        if (!isUndefined(value)) model.set(value);
+        scope.$tryEval(function(){
+          scope.$root.$eval();
+        });
+      } finally {
+        tempScope = null; // TODO(misko): needs to be $destroy
       }
-      if (!isUndefined(value)) model.set(value);
-      scope.$tryEval(function(){
-        scope.$root.$eval();
-      });
     });
 
     scope.$onEval(function(){
@@ -731,17 +743,19 @@ angularWidget('select', function(element){
       var selectValue = '';
       var isMulti = isMultiselect;
 
-      if (isMulti) {
-        selectValue = new HashMap();
-        if (modelValue && isNumber(length = modelValue.length)) {
-          for (index = 0; index < length; index++) {
-            selectValue.put(modelValue[index], true);
+      try {
+        if (isMulti) {
+          selectValue = new HashMap();
+          if (modelValue && isNumber(length = modelValue.length)) {
+            for (index = 0; index < length; index++) {
+              selectValue.put(modelValue[index], true);
+            }
           }
         }
-      }
-      try {
+
         for (index = 0, length = collection.length; index < length; index++) {
-          currentItem = optionScope[itemName] = collection[index];
+          optionScope[itemName] = collection[index];
+          currentItem = itemFn(optionScope);
           optionText = displayFn(optionScope);
           if (optionTexts.length > index) {
             // reuse
@@ -799,7 +813,7 @@ angularWidget('select', function(element){
         }
 
       } finally {
-        optionScope = null;
+        optionScope = null; // TODO(misko): needs to be $destroy()
       }
     });
   };

diff --git a/test/widgetsSpec.js b/test/widgetsSpec.js
@@ -576,22 +576,31 @@ describe("widget", function(){
   describe('ng:options', function(){
     var select, scope;
 
-    function createSelect(multiple, blank, unknown){
-      select = jqLite(
-          '<select name="selected" ' + (multiple ? ' multiple' : '') +
-                 ' ng:options="value.name for value in values">' +
-            (blank ? '<option value="">blank</option>' : '') +
-            (unknown ? '<option value="?">unknown</option>' : '') +
-          '</select>');
+    function createSelect(attrs, blank, unknown){
+      var html = '<select';
+      forEach(attrs, function(value, key){
+        if (typeof value == 'boolean') {
+          if (value) html += ' ' + key;
+        } else {
+          html+= ' ' + key + '="' + value + '"';
+        }
+      });
+      html += '>' +
+        (blank ? '<option value="">blank</option>' : '') +
+        (unknown ? '<option value="?">unknown</option>' : '') +
+      '</select>';
+      select = jqLite(html);
       scope = compile(select);
     };
 
     function createSingleSelect(blank, unknown){
-      createSelect(false, blank, unknown);
+      createSelect({name:'selected', 'ng:options':'value.name for value in values'},
+          blank, unknown);
     };
 
     function createMultiSelect(blank, unknown){
-      createSelect(true, blank, unknown);
+      createSelect({name:'selected', multiple:true, 'ng:options':'value.name for value in values'},
+          blank, unknown);
     };
 
     afterEach(function(){
@@ -602,7 +611,7 @@ describe("widget", function(){
     it('should throw when not formated "? for ? in ?"', function(){
       expect(function(){
         compile('<select name="selected" ng:options="i dont parse"></select>');
-      }).toThrow("Expected ng:options in form of '_expresion_ for _item_ in _collection_' but got 'i dont parse'.");
+      }).toThrow("Expected ng:options in form of '(_expression_ as)? _expresion_ for _item_ in _collection_' but got 'i dont parse'.");
 
       $logMock.error.logs.shift();
     });
@@ -712,6 +721,18 @@ describe("widget", function(){
         expect(select.val()).toEqual('1');
       });
 
+      it('should bind to scope value through experession', function(){
+        createSelect({name:'selected', 'ng:options':'item.id as item.name for item in values'});
+        scope.values = [{id:10, name:'A'}, {id:20, name:'B'}];
+        scope.selected = scope.values[0].id;
+        scope.$eval();
+        expect(select.val()).toEqual('0');
+
+        scope.selected = scope.values[1].id;
+        scope.$eval();
+        expect(select.val()).toEqual('1');
+      });
+
       it('should insert a blank option if bound to null', function(){
         createSingleSelect();
         scope.values = [{name:'A'}];
@@ -771,6 +792,18 @@ describe("widget", function(){
         expect(scope.selected).toEqual(scope.values[1]);
       });
 
+      it('should update model on change through expression', function(){
+        createSelect({name:'selected', 'ng:options':'item.id as item.name for item in values'});
+        scope.values = [{id:10, name:'A'}, {id:20, name:'B'}];
+        scope.selected = scope.values[0].id;
+        scope.$eval();
+        expect(select.val()).toEqual('0');
+
+        select.val('1');
+        browserTrigger(select, 'change');
+        expect(scope.selected).toEqual(scope.values[1].id);
+      });
+
       it('should update model to null on change', function(){
         createSingleSelect(true);
         scope.values = [{name:'A'}, {name:'B'}];