Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit c3bfd7f

Browse files
committed
fix(ngOptions): don't duplicate groups with falsy values
Previously, ngOptions would fail to remove optgroups with falsy values when the options were changed / removed. Related #14781 PR (#14784)
1 parent 553cccd commit c3bfd7f

File tree

2 files changed

+84
-2
lines changed

2 files changed

+84
-2
lines changed

src/ng/directive/ngOptions.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
629629

630630
for (var i = options.items.length - 1; i >= 0; i--) {
631631
var option = options.items[i];
632-
if (option.group) {
632+
if (isDefined(option.group)) {
633633
jqLiteRemove(option.element.parentNode);
634634
} else {
635635
jqLiteRemove(option.element);
@@ -661,7 +661,8 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
661661
listFragment.appendChild(groupElement);
662662

663663
// Update the label on the group element
664-
groupElement.label = option.group;
664+
// "null" is special cased because of Safari
665+
groupElement.label = option.group === null ? 'null' : option.group;
665666

666667
// Store it for use later
667668
groupElementMap[option.group] = groupElement;

test/ng/directive/ngOptionsSpec.js

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1789,6 +1789,87 @@ describe('ngOptions', function() {
17891789
});
17901790

17911791

1792+
it('should group if the group has a falsy value (except undefined)', function() {
1793+
createSelect({
1794+
'ng-model': 'selected',
1795+
'ng-options': 'item.name group by item.group for item in values'
1796+
});
1797+
1798+
scope.$apply(function() {
1799+
scope.values = [{name: 'A'},
1800+
{name: 'B', group: ''},
1801+
{name: 'C', group: null},
1802+
{name: 'D', group: false},
1803+
{name: 'E', group: 0}];
1804+
scope.selected = scope.values[0];
1805+
});
1806+
1807+
var optgroups = element.find('optgroup');
1808+
var options = element.find('option');
1809+
1810+
expect(optgroups.length).toEqual(4);
1811+
expect(options.length).toEqual(5);
1812+
1813+
expect(optgroups[0].label).toBe('');
1814+
expect(optgroups[1].label).toBe('null');
1815+
expect(optgroups[2].label).toBe('false');
1816+
expect(optgroups[3].label).toBe('0');
1817+
1818+
expect(options[0].textContent).toBe('A');
1819+
expect(options[0].parentNode).toBe(element[0]);
1820+
1821+
expect(options[1].textContent).toBe('B');
1822+
expect(options[1].parentNode).toBe(optgroups[0]);
1823+
1824+
expect(options[2].textContent).toBe('C');
1825+
expect(options[2].parentNode).toBe(optgroups[1]);
1826+
1827+
expect(options[3].textContent).toBe('D');
1828+
expect(options[3].parentNode).toBe(optgroups[2]);
1829+
1830+
expect(options[4].textContent).toBe('E');
1831+
expect(options[4].parentNode).toBe(optgroups[3]);
1832+
});
1833+
1834+
1835+
it('should not duplicate a group with a falsy value when the options are updated', function() {
1836+
1837+
scope.$apply(function() {
1838+
scope.values = [{value: 'A', group: ''},
1839+
{value: 'B', group: 'First'}];
1840+
scope.selected = scope.values[0];
1841+
});
1842+
1843+
createSelect({
1844+
'ng-model': 'selected',
1845+
'ng-options': 'item.value group by item.group for item in values'
1846+
});
1847+
1848+
scope.$apply(function() {
1849+
scope.values.push({value: 'C', group: false});
1850+
});
1851+
1852+
var optgroups = element.find('optgroup');
1853+
var options = element.find('option');
1854+
1855+
expect(optgroups.length).toEqual(3);
1856+
expect(options.length).toEqual(3);
1857+
1858+
expect(optgroups[0].label).toBe('');
1859+
expect(optgroups[1].label).toBe('First');
1860+
expect(optgroups[2].label).toBe('false');
1861+
1862+
expect(options[0].textContent).toBe('A');
1863+
expect(options[0].parentNode).toBe(optgroups[0]);
1864+
1865+
expect(options[1].textContent).toBe('B');
1866+
expect(options[1].parentNode).toBe(optgroups[1]);
1867+
1868+
expect(options[2].textContent).toBe('C');
1869+
expect(options[2].parentNode).toBe(optgroups[2]);
1870+
});
1871+
1872+
17921873
it('should bind to scope value and track/identify objects', function() {
17931874
createSelect({
17941875
'ng-model': 'selected',

0 commit comments

Comments
 (0)