invisi/angular.js
1
0
Fork 0
Code Issues Pull Requests Actions Packages Releases Wiki Activity

fix(ngOptions): don't add comment nodes as empty options

Browse Source 
When the "empty option" element contains a transclusion directive, the result of the compilation always includes a comment node. Since we are adding / removing the "selected" attribute on the empty option, we need to make sure it's an actual element.

To solve this, we take advantage of the fact the each option element has an option directive that tries to register the option with the selectController. With ngOptions, this registerOption function is normally noop'd since it's not possible to add dynamic options. Now if the result of the empty option compilation is a comment, we re-define the function so that it catches empty options when they are actually linked / rendered.

Closes #15454
Closes #15459
This commit is contained in:
Martin Staffa
2016-12-02 18:48:49 +01:00
committed by GitHub
parent f5d2bf3d6e
commit 245b27101a
3 changed files with 131 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ng/directive/ngOptions.js
+30 -3
View File
@@ -420,6 +420,7 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
// option when the viewValue does not match any of the option values.
for (var i = 0, children = selectElement.children(), ii = children.length; i < ii; i++) {
if (children[i].value === '') {
selectCtrl.hasEmptyOption = true;
selectCtrl.emptyOption = children.eq(i);
break;
}
@@ -556,9 +557,35 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
// compile the element since there might be bindings in it
$compile(selectCtrl.emptyOption)(scope);
// remove the class, which is added automatically because we recompile the element and it
// becomes the compilation root
selectCtrl.emptyOption.removeClass('ng-scope');
if (selectCtrl.emptyOption[0].nodeType === NODE_TYPE_COMMENT) {
// This means the empty option has currently no actual DOM node, probably because
// it has been modified by a transclusion directive.
selectCtrl.hasEmptyOption = false;
// Redefine the registerOption function, which will catch
// options that are added by ngIf etc. (rendering of the node is async because of
// lazy transclusion)
selectCtrl.registerOption = function(optionScope, optionEl) {
if (optionEl.val() === '') {
selectCtrl.hasEmptyOption = true;
selectCtrl.emptyOption = optionEl;
selectCtrl.emptyOption.removeClass('ng-scope');
// This ensures the new empty option is selected if previously no option was selected
ngModelCtrl.$render();
optionEl.on('$destroy', function() {
selectCtrl.hasEmptyOption = false;
selectCtrl.emptyOption = undefined;
});
}
};
} else {
// remove the class, which is added automatically because we recompile the element and it
// becomes the compilation root
selectCtrl.emptyOption.removeClass('ng-scope');
}
}
selectElement.empty();
src/ng/directive/select.js
+11 -8
View File
@@ -32,6 +32,14 @@ var SelectController =
// to create it in <select> and IE barfs otherwise.
self.unknownOption = jqLite(window.document.createElement('option'));
// The empty option is an option with the value '' that te application developer can
// provide inside the select. When the model changes to a value that doesn't match an option,
// it is selected - so if an empty option is provided, no unknown option is generated.
// However, the empty option is not removed when the model matches an option. It is always selectable
// and indicates that a "null" selection has been made.
self.hasEmptyOption = false;
self.emptyOption = undefined;
self.renderUnknownOption = function(val) {
var unknownVal = self.generateUnknownOptionValue(val);
self.unknownOption.val(unknownVal);
@@ -55,13 +63,6 @@ var SelectController =
if (self.unknownOption.parent()) self.unknownOption.remove();
};
// The empty option is an option with the value '' that te application developer can
// provide inside the select. When the model changes to a value that doesn't match an option,
// it is selected - so if an empty option is provided, no unknown option is generated.
// However, the empty option is not removed when the model matches an option. It is always selectable
// and indicates that a "null" selection has been made.
self.emptyOption = undefined;
self.selectEmptyOption = function() {
if (self.emptyOption) {
$element.val('');
@@ -70,7 +71,7 @@ var SelectController =
};
self.unselectEmptyOption = function() {
if (self.emptyOption) {
if (self.hasEmptyOption) {
self.emptyOption.removeAttr('selected');
}
};
@@ -132,6 +133,7 @@ var SelectController =
assertNotHasOwnProperty(value, '"option value"');
if (value === '') {
self.hasEmptyOption = true;
self.emptyOption = element;
}
var count = optionsMap.get(value) || 0;
@@ -148,6 +150,7 @@ var SelectController =
if (count === 1) {
optionsMap.remove(value);
if (value === '') {
self.hasEmptyOption = false;
self.emptyOption = undefined;
}
} else {
test/ng/directive/ngOptionsSpec.js
+90
View File
@@ -2524,6 +2524,96 @@ describe('ngOptions', function() {
}
);
it('should select the correct option after linking when the ngIf expression is initially falsy', function() {
scope.values = [
{name:'black'},
{name:'white'},
{name:'red'}
];
scope.selected = scope.values[2];
expect(function() {
createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
scope.$apply();
}).not.toThrow();
expect(element.find('option')[2]).toBeMarkedAsSelected();
expect(linkLog).toEqual(['linkNgOptions']);
});
it('should add / remove the "selected" attribute on empty option which has an initially falsy ngIf expression', function() {
scope.values = [
{name:'black'},
{name:'white'},
{name:'red'}
];
scope.selected = scope.values[2];
createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
scope.$apply();
expect(element.find('option')[2]).toBeMarkedAsSelected();
scope.$apply('isBlank = true');
expect(element.find('option')[0].value).toBe('');
expect(element.find('option')[0]).not.toBeMarkedAsSelected();
scope.$apply('selected = null');
expect(element.find('option')[0].value).toBe('');
expect(element.find('option')[0]).toBeMarkedAsSelected();
scope.selected = scope.values[1];
scope.$apply();
expect(element.find('option')[0].value).toBe('');
expect(element.find('option')[0]).not.toBeMarkedAsSelected();
expect(element.find('option')[2]).toBeMarkedAsSelected();
});
it('should add / remove the "selected" attribute on empty option which has an initially truthy ngIf expression when no option is selected', function() {
scope.values = [
{name:'black'},
{name:'white'},
{name:'red'}
];
scope.isBlank = true;
createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
scope.$apply();
expect(element.find('option')[0].value).toBe('');
expect(element.find('option')[0]).toBeMarkedAsSelected();
scope.selected = scope.values[2];
scope.$apply();
expect(element.find('option')[0]).not.toBeMarkedAsSelected();
expect(element.find('option')[3]).toBeMarkedAsSelected();
});
it('should add the "selected" attribute on empty option which has an initially falsy ngIf expression when no option is selected', function() {
scope.values = [
{name:'black'},
{name:'white'},
{name:'red'}
];
createSingleSelect('<option ng-if="isBlank" value="">blank</option>');
scope.$apply();
expect(element.find('option')[0]).not.toBeMarkedAsSelected();
scope.isBlank = true;
scope.$apply();
expect(element.find('option')[0].value).toBe('');
expect(element.find('option')[0]).toBeMarkedAsSelected();
expect(element.find('option')[1]).not.toBeMarkedAsSelected();
});
it('should not throw when a directive compiles the blank option before ngOptions is linked', function() {
expect(function() {
createSelect({