perf(ngOptions): use documentFragment to populate select#14400
perf(ngOptions): use documentFragment to populate select#14400Narretz merged 1 commit intoangular:masterfrom
Conversation
Narretz
force-pushed
the
fix-select-freeze
branch
from
1026b4d to
8e2100d
Compare
Narretz
force-pushed
the
fix-select-freeze
branch
3 times, most recently
from
340dd24 to
d73ab07
Compare
|
I checked this with the ngOptions benchmark the project and there is no discernable difference in speed on Chrome, Firefox or Safarin on OS/X. So I am happy to go with using this much simpler (and less buggy when it comes to IE) algorithm. |
|
@Narretz please merge into master and 1.5.x |
src/ng/directive/ngOptions.js
Outdated
|
|
||
| var groupMap = {}; | ||
| var currentElement = selectElement[0].firstChild; | ||
| var listFragment = $document[0].createDocumentFragment(); |
There was a problem hiding this comment.
Just noticed that it should be possible to create the documentFragment only once per ngOptions (instead of once per update), as the fragment contents are removed once it has been appended to a container
|
So, except for a couple of minor (ignorable) comments it LGTM. |
|
Wrt to the tests - maybe. But the test was only working before because it relied on the implementation (option elements are reused). And tests shouldn't do that. |
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 angular#13607 Fixes angular#13239 Fixes angular#12076
Narretz
force-pushed
the
fix-select-freeze
branch
from
091bbc8 to
97b3e00
Compare
4 participants
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
perf/fix
What is the current behavior? (You can also link to an open issue here)
Does this PR introduce a breaking change?
a tiny one possibly - now the select element is completely emptied before the options are appended
Please check if the PR fulfills these requirements
Other information:
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.
Always creating new options fixes issues in IE where the select would become unresponsive to user
input.
Fixes #13607
Fixes #12076