Skip to content

Commit 5a8ad8f

Browse files
mheverymhevery
committed
Closes angular#170. Corrected the behavior of select when options are ng:repeated
 - Delete $postEval method, as it was a hack
1 parent 41d5938 commit 5a8ad8f

19 files changed

Lines changed: 328 additions & 229 deletions

‎regression/issue-170.html‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<!DOCTYPE HTML>
2+
<html xmlns:ng="http://angularjs.org">
3+
<head>
4+
<script type="text/javascript" src="../lib/jquery/jquery-1.4.2.js"></script>
5+
<script type="text/javascript" src="../src/angular-bootstrap.js" ng:autobind></script>
6+
<head>
7+
<body>
8+
<select name='selection0' style="display:block;">
9+
<option ng:repeat='value in ["FOO","BAR"]'">{{value}}</option>
10+
</select>
11+
{{selection0}} &lt;-- FOO should be shown here
12+
13+
<hr/>
14+
15+
<select ng:init="selection1='ignore'" name='selection1' style="display:block;">
16+
<option ng:repeat='value in ["FOO","BAR"]' ng:bind-attr="{selected:'{{value==\'BAR\'}}'}">{{value}}</option>
17+
</select>
18+
{{selection1}} &lt;-- BAR should be shown here
19+
20+
<hr/>
21+
22+
<select ng:init="selection2=1" name="selection2" style="display:block;">
23+
<option value="{{$index}}" ng:repeat="opt in ['zero', 'one']">{{opt}}</option>
24+
</select>
25+
{{selection2}} &lt;-- 1 should be shown here
26+
27+
</body>
28+
</html>

‎src/Angular.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ function fromCharCode(code) { return String.fromCharCode(code); }
5353
var _undefined = undefined,
5454
_null = null,
5555
$$element = '$element',
56+
$$update = '$update',
57+
$$scope = '$scope',
58+
$$validate = '$validate',
5659
$angular = 'angular',
5760
$array = 'array',
5861
$boolean = 'boolean',
@@ -69,6 +72,8 @@ var _undefined = undefined,
6972
$number = 'number',
7073
$object = 'object',
7174
$string = 'string',
75+
$value = 'value',
76+
$selected = 'selected',
7277
$undefined = 'undefined',
7378
NG_EXCEPTION = 'ng-exception',
7479
NG_VALIDATION_ERROR = 'ng-validation-error',

‎src/Compiler.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Template.prototype = {
3030
if (this.newScope) {
3131
childScope = createScope(scope);
3232
scope.$onEval(childScope.$eval);
33+
element.data($$scope, childScope);
3334
}
3435
foreach(this.inits, function(fn) {
3536
queue.push(function() {
@@ -68,6 +69,17 @@ Template.prototype = {
6869
}
6970
};
7071

72+
/*
73+
* Function walks up the element chain looking for the scope associated with the give element.
74+
*/
75+
function retrieveScope(element) {
76+
var scope;
77+
while (element && !(scope = element.data($$scope))) {
78+
element = element.parent();
79+
}
80+
return scope;
81+
}
82+
7183
///////////////////////////////////
7284
//Compiler
7385
//////////////////////////////////
@@ -97,6 +109,7 @@ Compiler.prototype = {
97109
element = jqLite(element);
98110
var scope = parentScope && parentScope.$eval ?
99111
parentScope : createScope(parentScope);
112+
element.data($$scope, scope);
100113
return extend(scope, {
101114
$element:element,
102115
$init: function() {

‎src/Scope.js‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,6 @@ function createScope(parent, providers, instanceCache) {
243243
parent = Parent.prototype = (parent || {});
244244
var instance = new Parent();
245245
var evalLists = {sorted:[]};
246-
var postList = [], postHash = {}, postId = 0;
247246

248247
extend(instance, {
249248
'this': instance,
@@ -371,11 +370,6 @@ function createScope(parent, providers, instanceCache) {
371370
instance.$tryEval(queue[j].fn, queue[j].handler);
372371
}
373372
}
374-
while(postList.length) {
375-
fn = postList.shift();
376-
delete postHash[fn.$postEvalId];
377-
instance.$tryEval(fn);
378-
}
379373
} else if (type === $function) {
380374
return exp.call(instance);
381375
} else if (type === 'string') {
@@ -549,27 +543,6 @@ function createScope(parent, providers, instanceCache) {
549543
});
550544
},
551545

552-
/**
553-
* @workInProgress
554-
* @ngdoc function
555-
* @name angular.scope.$postEval
556-
* @function
557-
*/
558-
$postEval: function(expr) {
559-
if (expr) {
560-
var fn = expressionCompile(expr);
561-
var id = fn.$postEvalId;
562-
if (!id) {
563-
id = '$' + instance.$id + "_" + (postId++);
564-
fn.$postEvalId = id;
565-
}
566-
if (!postHash[id]) {
567-
postList.push(postHash[id] = fn);
568-
}
569-
}
570-
},
571-
572-
573546
/**
574547
* @workInProgress
575548
* @ngdoc function

‎src/directives.js‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,8 @@ angularDirective("ng:bind-template", function(expression, element){
304304
var REMOVE_ATTRIBUTES = {
305305
'disabled':'disabled',
306306
'readonly':'readOnly',
307-
'checked':'checked'
307+
'checked':'checked',
308+
'selected':'selected'
308309
};
309310
/**
310311
* @workInProgress
@@ -359,27 +360,31 @@ var REMOVE_ATTRIBUTES = {
359360
angularDirective("ng:bind-attr", function(expression){
360361
return function(element){
361362
var lastValue = {};
362-
var updateFn = element.parent().data('$update');
363+
var updateFn = element.data($$update) || noop;
363364
this.$onEval(function(){
364-
var values = this.$eval(expression);
365+
var values = this.$eval(expression),
366+
dirty = noop;
365367
for(var key in values) {
366368
var value = compileBindTemplate(values[key]).call(this, element),
367369
specialName = REMOVE_ATTRIBUTES[lowercase(key)];
368370
if (lastValue[key] !== value) {
369371
lastValue[key] = value;
370372
if (specialName) {
371-
if (element[specialName] = toBoolean(value)) {
372-
element.attr(specialName, value);
373+
if (toBoolean(value)) {
374+
element.attr(specialName, specialName);
375+
element.attr('ng-' + specialName, value);
373376
} else {
374-
element.removeAttr(key);
377+
element.removeAttr(specialName);
378+
element.removeAttr('ng-' + specialName);
375379
}
376-
(element.data('$validate')||noop)();
380+
(element.data($$validate)||noop)();
377381
} else {
378382
element.attr(key, value);
379383
}
380-
this.$postEval(updateFn);
384+
dirty = updateFn;
381385
}
382386
}
387+
dirty();
383388
}, element);
384389
};
385390
});

‎src/validators.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ extend(angularValidator, {
394394
element.removeClass('ng-input-indicator-wait');
395395
scope.$invalidWidgets.markValid(element);
396396
}
397-
element.data('$validate')();
397+
element.data($$validate)();
398398
scope.$root.$eval();
399399
});
400400
} else if (inputState.inFlight) {

‎src/widgets.js‎

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ function valueAccessor(scope, element) {
282282
required = requiredExpr === '';
283283
}
284284

285-
element.data('$validate', validate);
285+
element.data($$validate, validate);
286286
return {
287287
get: function(){
288288
if (lastError)
@@ -391,6 +391,7 @@ var textWidget = inputWidget('keyup change', modelAccessor, valueAccessor, initW
391391
// 'file': fileWidget???
392392
};
393393

394+
394395
function initWidgetValue(initValue) {
395396
return function (model, view) {
396397
var value = view.get();
@@ -461,18 +462,13 @@ function inputWidget(events, modelAccessor, viewAccessor, initFn) {
461462
this.$eval(element.attr('ng:init')||'');
462463
// Don't register a handler if we are a button (noopAccessor) and there is no action
463464
if (action || modelAccessor !== noopAccessor) {
464-
element.bind(events, function(event){
465+
element.bind(events, function (){
465466
model.set(view.get());
466467
lastValue = model.get();
467468
scope.$tryEval(action, element);
468469
scope.$root.$eval();
469470
});
470471
}
471-
function updateView(){
472-
view.set(lastValue = model.get());
473-
}
474-
updateView();
475-
element.data('$update', updateView);
476472
scope.$watch(model.get, function(value){
477473
if (lastValue !== value) {
478474
view.set(lastValue = value);
@@ -494,15 +490,50 @@ angularWidget('select', function(element){
494490
return inputWidgetSelector.call(this, element);
495491
});
496492

493+
494+
/*
495+
* Consider this:
496+
* <select name="selection">
497+
* <option ng:repeat="x in [1,2]">{{x}}</option>
498+
* </select>
499+
*
500+
* The issue is that the select gets evaluated before option is unrolled.
501+
* This means that the selection is undefined, but the browser
502+
* default behavior is to show the top selection in the list.
503+
* To fix that we register a $update function on the select element
504+
* and the option creation then calls the $update function when it is
505+
* unrolled. The $update function then calls this update function, which
506+
* then tries to determine if the model is unassigned, and if so it tries to
507+
* chose one of the options from the list.
508+
*/
497509
angularWidget('option', function(){
498510
this.descend(true);
499511
this.directives(true);
500512
return function(element) {
501-
this.$postEval(element.parent().data('$update'));
513+
var select = element.parent();
514+
var scope = retrieveScope(select);
515+
var model = modelFormattedAccessor(scope, select);
516+
var view = valueAccessor(scope, select);
517+
var option = element;
518+
var lastValue = option.attr($value);
519+
var lastSelected = option.attr('ng-' + $selected);
520+
element.data($$update, function(){
521+
var value = option.attr($value);
522+
var selected = option.attr('ng-' + $selected);
523+
var modelValue = model.get();
524+
if (lastSelected != selected || lastValue != value) {
525+
lastSelected = selected;
526+
lastValue = value;
527+
if (selected || modelValue == _null || modelValue == _undefined)
528+
model.set(value);
529+
if (value == modelValue) {
530+
view.set(lastValue);
531+
}
532+
}
533+
});
502534
};
503535
});
504536

505-
506537
/**
507538
* @workInProgress
508539
* @ngdoc widget

‎test/AngularSpec.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ describe('Angular', function(){
1010
scope.$init();
1111
scope.$eval();
1212
expect(onUpdateView).wasCalled();
13+
dealoc(scope);
1314
});
1415
});
1516

‎test/BinderTest.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ BinderTest.prototype.setUp = function(){
55

66
this.compile = function(html, initialScope, parent) {
77
var compiler = new Compiler(angularTextMarkup, angularAttrMarkup, angularDirective, angularWidget);
8+
if (self.element) dealoc(self.element);
89
var element = self.element = jqLite(html);
910
var scope = compiler.compile(element)(element);
1011

‎test/CompilerSpec.js‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
describe('compiler', function(){
2-
var compiler, markup, directives, widgets, compile, log;
2+
var compiler, markup, directives, widgets, compile, log, scope;
33

44
beforeEach(function(){
55
log = "";
@@ -32,6 +32,10 @@ describe('compiler', function(){
3232
return scope;
3333
};
3434
});
35+
36+
afterEach(function(){
37+
dealoc(scope);
38+
});
3539

3640
it('should recognize a directive', function(){
3741
var e = jqLite('<div directive="expr" ignore="me"></div>');
@@ -44,20 +48,21 @@ describe('compiler', function(){
4448
};
4549
};
4650
var template = compiler.compile(e);
47-
var init = template(e).$init;
51+
scope = template(e);
52+
var init = scope.$init;
4853
expect(log).toEqual("found");
4954
init();
5055
expect(e.hasClass('ng-directive')).toEqual(true);
5156
expect(log).toEqual("found:init");
5257
});
5358

5459
it('should recurse to children', function(){
55-
var scope = compile('<div><span hello="misko"/></div>');
60+
scope = compile('<div><span hello="misko"/></div>');
5661
expect(log).toEqual("hello misko");
5762
});
5863

5964
it('should watch scope', function(){
60-
var scope = compile('<span watch="name"/>');
65+
scope = compile('<span watch="name"/>');
6166
expect(log).toEqual("");
6267
scope.$eval();
6368
scope.$set('name', 'misko');
@@ -71,7 +76,7 @@ describe('compiler', function(){
7176

7277
it('should prevent descend', function(){
7378
directives.stop = function(){ this.descend(false); };
74-
var scope = compile('<span hello="misko" stop="true"><span hello="adam"/></span>');
79+
scope = compile('<span hello="misko" stop="true"><span hello="adam"/></span>');
7580
expect(log).toEqual("hello misko");
7681
});
7782

@@ -87,7 +92,7 @@ describe('compiler', function(){
8792
});
8893
};
8994
};
90-
var scope = compile('before<span duplicate="expr">x</span>after');
95+
scope = compile('before<span duplicate="expr">x</span>after');
9196
expect(sortedHtml(scope.$element)).toEqual('<div>before<#comment></#comment><span>x</span>after</div>');
9297
scope.$eval();
9398
expect(sortedHtml(scope.$element)).toEqual('<div>before<#comment></#comment><span>x</span><span>x</span>after</div>');
@@ -103,7 +108,7 @@ describe('compiler', function(){
103108
textNode[0].nodeValue = 'replaced';
104109
}
105110
});
106-
var scope = compile('before<span>middle</span>after');
111+
scope = compile('before<span>middle</span>after');
107112
expect(sortedHtml(scope.$element[0], true)).toEqual('<div>before<span class="ng-directive" hello="middle">replaced</span>after</div>');
108113
expect(log).toEqual("hello middle");
109114
});
@@ -116,7 +121,7 @@ describe('compiler', function(){
116121
log += 'init';
117122
};
118123
};
119-
var scope = compile('<ng:button>push me</ng:button>');
124+
scope = compile('<ng:button>push me</ng:button>');
120125
expect(lowercase(scope.$element[0].innerHTML)).toEqual('<div>button</div>');
121126
expect(log).toEqual('init');
122127
});
@@ -135,7 +140,7 @@ describe('compiler', function(){
135140
if (text == '{{1+2}}')
136141
parent.text('3');
137142
});
138-
var scope = compile('<div><h1>ignore me</h1></div>');
143+
scope = compile('<div><h1>ignore me</h1></div>');
139144
expect(scope.$element.text()).toEqual('3');
140145
});
141146

@@ -158,7 +163,7 @@ describe('compiler', function(){
158163
textNode.remove();
159164
}
160165
});
161-
var scope = compile('A---B---C===D');
166+
scope = compile('A---B---C===D');
162167
expect(sortedHtml(scope.$element)).toEqual('<div>A<hr></hr>B<hr></hr>C<p></p>D</div>');
163168
});
164169

0 commit comments

Comments
 (0)