From 97b3e003bc60f979fddecd16bc50304b5bef7e53 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Sat, 9 Apr 2016 11:06:02 +0200 Subject: [PATCH] perf(ngOptions): use documentFragment to populate select This changes the way option elements are generated when the ngOption collection changes. Previously, we would re-use option elements when possible (updating their text and label). Now, we first remove all currently displayed options and the create new options for the collection. The new options are first appended to a documentFragment, which is in the end appended to the selectElement. Using documentFragment improves render performance in IE with large option collections (> 100 elements) considerably. Creating new options from scratch fixes issues in IE where the select would become unresponsive to user input. Fixes #13607 Fixes #13239 Fixes #12076 --- src/ng/directive/ngOptions.js | 134 +++++++++-------------------- test/ng/directive/ngOptionsSpec.js | 2 + 2 files changed, 41 insertions(+), 95 deletions(-) diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js index bcd67591c331..a4444f70df12 100644 --- a/src/ng/directive/ngOptions.js +++ b/src/ng/directive/ngOptions.js @@ -245,7 +245,7 @@ var NG_OPTIONS_REGEXP = /^\s*([\s\S]+?)(?:\s+as\s+([\s\S]+?))?(?:\s+group\s+by\s // jshint maxlen: 100 -var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { +var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile, $document, $parse) { function parseOptionsExpression(optionsExp, selectElement, scope) { @@ -432,7 +432,10 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { var options; var ngOptions = parseOptionsExpression(attr.ngOptions, selectElement, scope); - + // This stores the newly created options before they are appended to the select. + // Since the contents are removed from the fragment when it is appended, + // we only need to create it once. + var listFragment = $document[0].createDocumentFragment(); var renderEmptyOption = function() { if (!providedEmptyOption) { @@ -581,6 +584,8 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { emptyOption = jqLite(optionTemplate.cloneNode(false)); } + selectElement.empty(); + // We need to do this here to ensure that the options object is defined // when we first hit it in writeNgOptionsValue updateOptions(); @@ -590,6 +595,12 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { // ------------------------------------------------------------------ // + function addOptionElement(option, parent) { + var optionElement = optionTemplate.cloneNode(false); + parent.appendChild(optionElement); + updateOptionElement(option, optionElement); + } + function updateOptionElement(option, element) { option.element = element; @@ -606,133 +617,66 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { if (option.value !== element.value) element.value = option.selectValue; } - function addOrReuseElement(parent, current, type, templateElement) { - var element; - // Check whether we can reuse the next element - if (current && lowercase(current.nodeName) === type) { - // The next element is the right type so reuse it - element = current; - } else { - // The next element is not the right type so create a new one - element = templateElement.cloneNode(false); - if (!current) { - // There are no more elements so just append it to the select - parent.appendChild(element); - } else { - // The next element is not a group so insert the new one - parent.insertBefore(element, current); - } - } - return element; - } - - - function removeExcessElements(current) { - var next; - while (current) { - next = current.nextSibling; - jqLiteRemove(current); - current = next; - } - } - + function updateOptions() { + var previousValue = options && selectCtrl.readValue(); - function skipEmptyAndUnknownOptions(current) { - var emptyOption_ = emptyOption && emptyOption[0]; - var unknownOption_ = unknownOption && unknownOption[0]; - - // We cannot rely on the extracted empty option being the same as the compiled empty option, - // because the compiled empty option might have been replaced by a comment because - // it had an "element" transclusion directive on it (such as ngIf) - if (emptyOption_ || unknownOption_) { - while (current && - (current === emptyOption_ || - current === unknownOption_ || - current.nodeType === NODE_TYPE_COMMENT || - (nodeName_(current) === 'option' && current.value === ''))) { - current = current.nextSibling; + // We must remove all current options, but cannot simply set innerHTML = null + // since the providedEmptyOption might have an ngIf on it that inserts comments which we + // must preserve. + // Instead, iterate over the current option elements and remove them or their optgroup + // parents + if (options) { + + for (var i = options.items.length - 1; i >= 0; i--) { + var option = options.items[i]; + if (option.group) { + jqLiteRemove(option.element.parentNode); + } else { + jqLiteRemove(option.element); + } } } - return current; - } - - - function updateOptions() { - - var previousValue = options && selectCtrl.readValue(); options = ngOptions.getOptions(); - var groupMap = {}; - var currentElement = selectElement[0].firstChild; + var groupElementMap = {}; // Ensure that the empty option is always there if it was explicitly provided if (providedEmptyOption) { selectElement.prepend(emptyOption); } - currentElement = skipEmptyAndUnknownOptions(currentElement); - - options.items.forEach(function updateOption(option) { - var group; + options.items.forEach(function addOption(option) { var groupElement; - var optionElement; if (isDefined(option.group)) { // This option is to live in a group // See if we have already created this group - group = groupMap[option.group]; + groupElement = groupElementMap[option.group]; - if (!group) { + if (!groupElement) { - // We have not already created this group - groupElement = addOrReuseElement(selectElement[0], - currentElement, - 'optgroup', - optGroupTemplate); - // Move to the next element - currentElement = groupElement.nextSibling; + groupElement = optGroupTemplate.cloneNode(false); + listFragment.appendChild(groupElement); // Update the label on the group element groupElement.label = option.group; // Store it for use later - group = groupMap[option.group] = { - groupElement: groupElement, - currentOptionElement: groupElement.firstChild - }; - + groupElementMap[option.group] = groupElement; } - // So now we have a group for this option we add the option to the group - optionElement = addOrReuseElement(group.groupElement, - group.currentOptionElement, - 'option', - optionTemplate); - updateOptionElement(option, optionElement); - // Move to the next element - group.currentOptionElement = optionElement.nextSibling; + addOptionElement(option, groupElement); } else { // This option is not in a group - optionElement = addOrReuseElement(selectElement[0], - currentElement, - 'option', - optionTemplate); - updateOptionElement(option, optionElement); - // Move to the next element - currentElement = optionElement.nextSibling; + addOptionElement(option, listFragment); } }); - - // Now remove all excess options and group - Object.keys(groupMap).forEach(function(key) { - removeExcessElements(groupMap[key].currentOptionElement); - }); - removeExcessElements(currentElement); + selectElement[0].appendChild(listFragment); ngModelCtrl.$render(); diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js index 232f9193cbfc..74e4fadacba9 100644 --- a/test/ng/directive/ngOptionsSpec.js +++ b/test/ng/directive/ngOptionsSpec.js @@ -1946,6 +1946,8 @@ describe('ngOptions', function() { scope.options[1].unavailable = false; }); + options = element.find('option'); + expect(scope.options[1].unavailable).toEqual(false); expect(options.eq(1).prop('disabled')).toEqual(false); });