Skip to content

Commit

Permalink
fix(ngOptions): provide robust identification of empty options with ngIf
Browse files Browse the repository at this point in the history
When an empty option has ngIf, the compilation result is different from
the extracted emptyOption, so the default way of identifying the empty
option doesn't work anymore. In that case, we need to make sure that we
don't identify comment nodes and options whose value is '' as valid
option elements.

Related angular#12952
Closes angular#12190
  • Loading branch information
Narretz committed Oct 7, 2015
1 parent beea571 commit 64d994b
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 3 deletions.
18 changes: 15 additions & 3 deletions src/ng/directive/ngOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,9 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
// The emptyOption allows the application developer to provide their own custom "empty"
// option when the viewValue does not match any of the option values.
var emptyOption;
// The empty option might have a directive that dynamically adds / removes it
var dynamicEmptyOption;

for (var i = 0, children = selectElement.children(), ii = children.length; i < ii; i++) {
if (children[i].value === '') {
emptyOption = children.eq(i);
Expand Down Expand Up @@ -550,6 +553,9 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {

// compile the element since there might be bindings in it
$compile(emptyOption)(scope);
if (emptyOption[0].nodeType === NODE_TYPE_COMMENT) {
dynamicEmptyOption = true;
}

// remove the class, which is added automatically because we recompile the element and it
// becomes the compilation root
Expand Down Expand Up @@ -619,9 +625,15 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
var unknownOption_ = unknownOption && unknownOption[0];

if (emptyOption_ || unknownOption_) {
while (current &&
(current === emptyOption_ ||
current === unknownOption_)) {

while (
current &&
(current === emptyOption_ ||
current === unknownOption_ ||
// When the empty option is dynamically added / removed, we cannot be sure that the
// emptyOption is the same as it was when it was extracted in the first place
dynamicEmptyOption && (current.nodeType === NODE_TYPE_COMMENT || current.value === ''))
) {
current = current.nextSibling;
}
}
Expand Down
47 changes: 47 additions & 0 deletions test/ng/directive/ngOptionsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2144,6 +2144,53 @@ describe('ngOptions', function() {
});


it('should be possible to use ngIf in the blank option', function() {
var option;
createSingleSelect('<option ng-if="isBlank" value="">blank</option>');

scope.$apply(function() {
scope.values = [{name: 'A'}];
scope.isBlank = true;
});

expect(element.find('option').length).toBe(2);
option = element.find('option').eq(0);
expect(option.val()).toBe('');
expect(option.text()).toBe('blank');

scope.$apply(function() {
scope.isBlank = false;
});

expect(element.find('option').length).toBe(1);
option = element.find('option').eq(0);
expect(option.text()).toBe('A');
});


it('should be possible to use ngIf in the blank option when values are available upon linking',
function() {
var options;

scope.values = [{name: 'A'}];
createSingleSelect('<option ng-if="isBlank" value="">blank</option>');

scope.$apply('isBlank = true');

options = element.find('option');
expect(options.length).toBe(2);
expect(options.eq(0).val()).toBe('');
expect(options.eq(0).text()).toBe('blank');

scope.$apply('isBlank = false');

options = element.find('option');
expect(options.length).toBe(1);
expect(options.eq(0).text()).toBe('A');
}
);


it('should not throw when a directive compiles the blank option before ngOptions is linked', function() {
expect(function() {
createSelect({
Expand Down

0 comments on commit 64d994b

Please sign in to comment.