Closes #170. Corrected the behavior of select when options are ng:rep…

…eated - Delete $postEval method, as it was a hack
angular · Dec 3, 2010 · 5a8ad8f · 5a8ad8f
1 parent 41d5938
commit 5a8ad8f
Show file tree

Hide file tree

Showing 19 changed files with 328 additions and 229 deletions.
diff --git a/regression/issue-170.html b/regression/issue-170.html
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML>
+<html xmlns:ng="http://angularjs.org">
+<head>
+  <script type="text/javascript" src="../lib/jquery/jquery-1.4.2.js"></script>
+  <script type="text/javascript" src="../src/angular-bootstrap.js" ng:autobind></script>
+<head>
+<body>
+  <select name='selection0' style="display:block;">
+      <option ng:repeat='value in ["FOO","BAR"]'">{{value}}</option>
+  </select>
+  {{selection0}} &lt;-- FOO should be shown here
+
+  <hr/>
+
+  <select ng:init="selection1='ignore'" name='selection1' style="display:block;">
+      <option ng:repeat='value in ["FOO","BAR"]' ng:bind-attr="{selected:'{{value==\'BAR\'}}'}">{{value}}</option>
+  </select>
+  {{selection1}} &lt;-- BAR should be shown here
+
+  <hr/>
+
+  <select ng:init="selection2=1" name="selection2" style="display:block;">
+    <option value="{{$index}}" ng:repeat="opt in ['zero', 'one']">{{opt}}</option>
+  </select>
+  {{selection2}} &lt;-- 1 should be shown here
+
+</body>
+</html>
diff --git a/src/Angular.js b/src/Angular.js
@@ -53,6 +53,9 @@ function fromCharCode(code) { return String.fromCharCode(code); }
 var _undefined        = undefined,
     _null             = null,
     $$element         = '$element',
+    $$update          = '$update',
+    $$scope           = '$scope',
+    $$validate        = '$validate',
     $angular          = 'angular',
     $array            = 'array',
     $boolean          = 'boolean',
@@ -69,6 +72,8 @@ var _undefined        = undefined,
     $number           = 'number',
     $object           = 'object',
     $string           = 'string',
+    $value            = 'value',
+    $selected         = 'selected',
     $undefined        = 'undefined',
     NG_EXCEPTION      = 'ng-exception',
     NG_VALIDATION_ERROR = 'ng-validation-error',

diff --git a/src/Compiler.js b/src/Compiler.js
@@ -30,6 +30,7 @@ Template.prototype = {
     if (this.newScope) {
       childScope = createScope(scope);
       scope.$onEval(childScope.$eval);
+      element.data($$scope, childScope);
     }
     foreach(this.inits, function(fn) {
       queue.push(function() {
@@ -68,6 +69,17 @@ Template.prototype = {
   }
 };
 
+/*
+ * Function walks up the element chain looking for the scope associated with the give element.
+ */
+function retrieveScope(element) {
+  var scope;
+  while (element && !(scope = element.data($$scope))) {
+    element = element.parent();
+  }
+  return scope;
+}
+
 ///////////////////////////////////
 //Compiler
 //////////////////////////////////
@@ -97,6 +109,7 @@ Compiler.prototype = {
       element = jqLite(element);
       var scope = parentScope && parentScope.$eval ?
           parentScope : createScope(parentScope);
+      element.data($$scope, scope);
       return extend(scope, {
         $element:element,
         $init: function() {

diff --git a/src/Scope.js b/src/Scope.js
@@ -243,7 +243,6 @@ function createScope(parent, providers, instanceCache) {
   parent = Parent.prototype = (parent || {});
   var instance = new Parent();
   var evalLists = {sorted:[]};
-  var postList = [], postHash = {}, postId = 0;
 
   extend(instance, {
     'this': instance,
@@ -371,11 +370,6 @@ function createScope(parent, providers, instanceCache) {
             instance.$tryEval(queue[j].fn, queue[j].handler);
           }
         }
-        while(postList.length) {
-          fn = postList.shift();
-          delete postHash[fn.$postEvalId];
-          instance.$tryEval(fn);
-        }
       } else if (type === $function) {
         return exp.call(instance);
       } else  if (type === 'string') {
@@ -549,27 +543,6 @@ function createScope(parent, providers, instanceCache) {
       });
     },
 
-    /**
-     * @workInProgress
-     * @ngdoc function
-     * @name angular.scope.$postEval
-     * @function
-     */
-    $postEval: function(expr) {
-      if (expr) {
-        var fn = expressionCompile(expr);
-        var id = fn.$postEvalId;
-        if (!id) {
-          id = '$' + instance.$id + "_" + (postId++);
-          fn.$postEvalId = id;
-        }
-        if (!postHash[id]) {
-          postList.push(postHash[id] = fn);
-        }
-      }
-    },
-
-
     /**
      * @workInProgress
      * @ngdoc function

diff --git a/src/directives.js b/src/directives.js
@@ -304,7 +304,8 @@ angularDirective("ng:bind-template", function(expression, element){
 var REMOVE_ATTRIBUTES = {
   'disabled':'disabled',
   'readonly':'readOnly',
-  'checked':'checked'
+  'checked':'checked',
+  'selected':'selected'
 };
 /**
  * @workInProgress
@@ -359,27 +360,31 @@ var REMOVE_ATTRIBUTES = {
 angularDirective("ng:bind-attr", function(expression){
   return function(element){
     var lastValue = {};
-    var updateFn = element.parent().data('$update');
+    var updateFn = element.data($$update) || noop;
     this.$onEval(function(){
-      var values = this.$eval(expression);
+      var values = this.$eval(expression),
+          dirty = noop;
       for(var key in values) {
         var value = compileBindTemplate(values[key]).call(this, element),
             specialName = REMOVE_ATTRIBUTES[lowercase(key)];
         if (lastValue[key] !== value) {
           lastValue[key] = value;
           if (specialName) {
-            if (element[specialName] = toBoolean(value)) {
-              element.attr(specialName, value);
+            if (toBoolean(value)) {
+              element.attr(specialName, specialName);
+              element.attr('ng-' + specialName, value);
             } else {
-              element.removeAttr(key);
+              element.removeAttr(specialName);
+              element.removeAttr('ng-' + specialName);
             }
-            (element.data('$validate')||noop)();
+            (element.data($$validate)||noop)();
           } else {
             element.attr(key, value);
           }
-          this.$postEval(updateFn);
+          dirty = updateFn;
         }
       }
+      dirty();
     }, element);
   };
 });

diff --git a/src/validators.js b/src/validators.js
@@ -394,7 +394,7 @@ extend(angularValidator, {
           element.removeClass('ng-input-indicator-wait');
           scope.$invalidWidgets.markValid(element);
         }
-        element.data('$validate')();
+        element.data($$validate)();
         scope.$root.$eval();
       });
     } else if (inputState.inFlight) {

diff --git a/src/widgets.js b/src/widgets.js
@@ -282,7 +282,7 @@ function valueAccessor(scope, element) {
     required = requiredExpr === '';
   }
 
-  element.data('$validate', validate);
+  element.data($$validate, validate);
   return {
     get: function(){
       if (lastError)
@@ -391,6 +391,7 @@ var textWidget = inputWidget('keyup change', modelAccessor, valueAccessor, initW
 //      'file':            fileWidget???
     };
 
+
 function initWidgetValue(initValue) {
   return function (model, view) {
     var value = view.get();
@@ -461,18 +462,13 @@ function inputWidget(events, modelAccessor, viewAccessor, initFn) {
     this.$eval(element.attr('ng:init')||'');
     // Don't register a handler if we are a button (noopAccessor) and there is no action
     if (action || modelAccessor !== noopAccessor) {
-      element.bind(events, function(event){
+      element.bind(events, function (){
         model.set(view.get());
         lastValue = model.get();
         scope.$tryEval(action, element);
         scope.$root.$eval();
       });
     }
-    function updateView(){
-      view.set(lastValue = model.get());
-    }
-    updateView();
-    element.data('$update', updateView);
     scope.$watch(model.get, function(value){
       if (lastValue !== value) {
         view.set(lastValue = value);
@@ -494,15 +490,50 @@ angularWidget('select', function(element){
   return inputWidgetSelector.call(this, element);
 });
 
+
+/*
+ * Consider this:
+ * <select name="selection">
+ *   <option ng:repeat="x in [1,2]">{{x}}</option>
+ * </select>
+ * 
+ * The issue is that the select gets evaluated before option is unrolled.
+ * This means that the selection is undefined, but the browser
+ * default behavior is to show the top selection in the list. 
+ * To fix that we register a $update function on the select element
+ * and the option creation then calls the $update function when it is 
+ * unrolled. The $update function then calls this update function, which 
+ * then tries to determine if the model is unassigned, and if so it tries to
+ * chose one of the options from the list.
+ */
 angularWidget('option', function(){
   this.descend(true);
   this.directives(true);
   return function(element) {
-    this.$postEval(element.parent().data('$update'));
+    var select = element.parent();
+    var scope = retrieveScope(select);
+    var model = modelFormattedAccessor(scope, select);
+    var view = valueAccessor(scope, select);
+    var option = element;
+    var lastValue = option.attr($value);
+    var lastSelected = option.attr('ng-' + $selected);
+    element.data($$update, function(){
+      var value = option.attr($value);
+      var selected = option.attr('ng-' + $selected);
+      var modelValue = model.get();
+      if (lastSelected != selected || lastValue != value) {
+        lastSelected = selected;
+        lastValue = value;
+        if (selected || modelValue == _null || modelValue == _undefined) 
+          model.set(value);
+        if (value == modelValue) {
+          view.set(lastValue);
+        }
+      }
+    });
   };
 });
 
-
 /**
  * @workInProgress
  * @ngdoc widget

diff --git a/test/AngularSpec.js b/test/AngularSpec.js
@@ -10,6 +10,7 @@ describe('Angular', function(){
     scope.$init();
     scope.$eval();
     expect(onUpdateView).wasCalled();
+    dealoc(scope);
   });
 });
 

diff --git a/test/BinderTest.js b/test/BinderTest.js
@@ -5,6 +5,7 @@ BinderTest.prototype.setUp = function(){
 
   this.compile = function(html, initialScope, parent) {
     var compiler = new Compiler(angularTextMarkup, angularAttrMarkup, angularDirective, angularWidget);
+    if (self.element) dealoc(self.element);
     var element = self.element = jqLite(html);
     var scope = compiler.compile(element)(element);
 

diff --git a/test/CompilerSpec.js b/test/CompilerSpec.js
@@ -1,5 +1,5 @@
 describe('compiler', function(){
-  var compiler, markup, directives, widgets, compile, log;
+  var compiler, markup, directives, widgets, compile, log, scope;
 
   beforeEach(function(){
     log = "";
@@ -32,6 +32,10 @@ describe('compiler', function(){
       return scope;
     };
   });
+
+  afterEach(function(){
+    dealoc(scope);
+  });
 
   it('should recognize a directive', function(){
     var e = jqLite('<div directive="expr" ignore="me"></div>');
@@ -44,20 +48,21 @@ describe('compiler', function(){
       };
     };
     var template = compiler.compile(e);
-    var init = template(e).$init;
+    scope = template(e);
+    var init = scope.$init;
     expect(log).toEqual("found");
     init();
     expect(e.hasClass('ng-directive')).toEqual(true);
     expect(log).toEqual("found:init");
   });
 
   it('should recurse to children', function(){
-    var scope = compile('<div><span hello="misko"/></div>');
+    scope = compile('<div><span hello="misko"/></div>');
     expect(log).toEqual("hello misko");
   });
 
   it('should watch scope', function(){
-    var scope = compile('<span watch="name"/>');
+    scope = compile('<span watch="name"/>');
     expect(log).toEqual("");
     scope.$eval();
     scope.$set('name', 'misko');
@@ -71,7 +76,7 @@ describe('compiler', function(){
 
   it('should prevent descend', function(){
     directives.stop = function(){ this.descend(false); };
-    var scope = compile('<span hello="misko" stop="true"><span hello="adam"/></span>');
+    scope = compile('<span hello="misko" stop="true"><span hello="adam"/></span>');
     expect(log).toEqual("hello misko");
   });
 
@@ -87,7 +92,7 @@ describe('compiler', function(){
         });
       };
     };
-    var scope = compile('before<span duplicate="expr">x</span>after');
+    scope = compile('before<span duplicate="expr">x</span>after');
     expect(sortedHtml(scope.$element)).toEqual('<div>before<#comment></#comment><span>x</span>after</div>');
     scope.$eval();
     expect(sortedHtml(scope.$element)).toEqual('<div>before<#comment></#comment><span>x</span><span>x</span>after</div>');
@@ -103,7 +108,7 @@ describe('compiler', function(){
         textNode[0].nodeValue = 'replaced';
       }
     });
-    var scope = compile('before<span>middle</span>after');
+    scope = compile('before<span>middle</span>after');
     expect(sortedHtml(scope.$element[0], true)).toEqual('<div>before<span class="ng-directive" hello="middle">replaced</span>after</div>');
     expect(log).toEqual("hello middle");
   });
@@ -116,7 +121,7 @@ describe('compiler', function(){
         log += 'init';
       };
     };
-    var scope = compile('<ng:button>push me</ng:button>');
+    scope = compile('<ng:button>push me</ng:button>');
     expect(lowercase(scope.$element[0].innerHTML)).toEqual('<div>button</div>');
     expect(log).toEqual('init');
   });
@@ -135,7 +140,7 @@ describe('compiler', function(){
       if (text == '{{1+2}}')
         parent.text('3');
     });
-    var scope = compile('<div><h1>ignore me</h1></div>');
+    scope = compile('<div><h1>ignore me</h1></div>');
     expect(scope.$element.text()).toEqual('3');
   });
 
@@ -158,7 +163,7 @@ describe('compiler', function(){
         textNode.remove();
       }
     });
-    var scope = compile('A---B---C===D');
+    scope = compile('A---B---C===D');
     expect(sortedHtml(scope.$element)).toEqual('<div>A<hr></hr>B<hr></hr>C<p></p>D</div>');
   });