lordfriend / nya-bootstrap-select

An AngularJS select replacement which build select like dropdown component with collection and ng-model support
http://nya.io/nya-bootstrap-select/
MIT License
179 stars 82 forks source link

Poor performance for live-search for lists with over 200 items (especially in IE) #71

Open gitcriel opened 9 years ago

gitcriel commented 9 years ago

Version: 2.0.10

I'm experiencing poor performance while using the live-search feature when drop-downs have too many items in them. I have a few drop-downs with over 200 items in them (one of them has 800 items so it's especially noticeable for that one.) The problem is noticeable in Chrome and is especially noticeable in IE11 and lower. When typing a search criteria, the UI hangs after each character is typed for about a second while it redraws the list.

I was able to trace the problem to this code:

           searchBox.children().on('input', function(){

            var searchKeyword = searchBox.children().val(),
              found = 0,
              options = dropdownMenu.children(),
              length = options.length,
              index,
              option,
              nyaBsOptionNode;

            if(searchKeyword) {
              for(index = 0; index < length; index++) {
                option = options.eq(index);
                if(option.hasClass('nya-bs-option')) {
                  if(!hasKeyword(option.find('a'), searchKeyword)) {
                    option.addClass('not-match');
                  } else {
                    option.removeClass('not-match');
                    found++;
                  }
                }
              }

              if(found === 0) {
                noSearchResult.addClass('show');
              } else {
                noSearchResult.removeClass('show');
              }
            } else {
              for(index = 0; index < length; index++) {
                option = options.eq(index);
                if(option.hasClass('nya-bs-option')) {
                  option.removeClass('not-match');
                }
              }
              noSearchResult.removeClass('show');
            }

            nyaBsOptionNode = findFocus(true);

            if(nyaBsOptionNode) {
              options.removeClass('active');
              jqLite(nyaBsOptionNode).addClass('active');
            }

          });

The entire list of items is traversed every time and a call to addClass or removeClass is made. This makes the browser rebuild the DOM and re-render. A better way to implement this is to clear the dropdownMenu and append the html of only the matching elements.

I have a potential solution (which is far from perfect, but it does greatly improve the performance.) This code replaces the code shown above:

        searchBox.children().on('input', function(){

            // Some caching here...this should probably be moved somewhere else...wherever the list first gets loaded
            if($scope.fastOptions === undefined) {
                $scope.fastOptions = [];
                $scope.allOptions = "";
                var options = dropdownMenu.children();
                angular.forEach(dropdownMenu.children(), function(option) {
                    var element = angular.element(option);
                    if(element.hasClass('nya-bs-option')) {
                        $scope.fastOptions.push({'value': element.find('a').text(), 'option': element});
                        $scope.allOptions = $scope.allOptions + element.prop('outerHTML');
                    }
                });
            }

            var searchKeyword = searchBox.children().val(),
              nyaBsOptionNode;

            // The timeout is to make this code asynchronous so that typing isn't delayed while the list loads
            $timeout(function() {
                dropdownMenu.empty();

                if(searchKeyword === undefined || searchKeyword.trim().length==0)
                    dropdownMenu.append($scope.allOptions);
                else {
                    var list = "";
                    angular.forEach($scope.fastOptions, function(item) {
                        if(item.value.toUpperCase().indexOf(searchKeyword.toUpperCase())>=0) {
                            list = list + item.option.prop('outerHTML');
                        }
                    });
                    if(list=="") 
                        list = noSearchResult.prop('outerHTML');

                    dropdownMenu.append(list);
                }

                if($scope.reducedList != undefined && $scope.reducedList.length>0) {
                    nyaBsOptionNode = findFocus(true);

                    if(nyaBsOptionNode) {
                      options.removeClass('active');
                      jqLite(nyaBsOptionNode).addClass('active');
                    }
                }
            });

          });
lordfriend commented 9 years ago

Thank you for reporting this bug and giving a solution.

You're right, the cost of transversing the DOM elements of the list is expensive. But manipulating another list is also too tricky and can make the nyaBsSelect directive too much more complex and wired. I hope using the nyaBsOption directive to manipulate the list DOM elements.

At the beginning stage of designing 2.0 version of this directive, I tried to use filter in nyaBsOption directive to make a straight forward solution for filter through the option list. But I quickly encounter the problem which is caused the scope of nyaBsSelect directive is not isolated or inherited from parent. So I couldn't add any property into current scope as the filter expression.

Maybe the currently the short solution is add timeout to make the transversing asynchronous.

gitcriel commented 9 years ago

Traversing the DOM elements isn't the expensive part. It's the updating of the DOM - by calling addClass/removeClass - in every iteration, that causes the performance problems. Also, adding timeout does not solve the problem, it only prevents blocking of the UI while typing but without improving the performance the timeout doesn't do anything useful.

The thing that (almost) completely solves the performance problem is the code that recreates the HTML for the result list in one go and appends that to the DOM once, and only once. This way the DOM is only recreated once.

So this part:

                 dropdownMenu.empty();  // Clear the list

                if(searchKeyword === undefined || searchKeyword.trim().length==0)
                    dropdownMenu.append($scope.allOptions);
                else {
                    var list = "";
                    angular.forEach($scope.fastOptions, function(item) {
                        if(item.value.toUpperCase().indexOf(searchKeyword.toUpperCase())>=0) {
                            list = list + item.option.prop('outerHTML');
                        }
                    });
                    if(list=="") 
                        list = noSearchResult.prop('outerHTML');

                    dropdownMenu.append(list); // Append the results only (the only DOM manipulation after the empty())
                }
gitcriel commented 9 years ago

Looks like my "fix" doesn't actually work... I ended up using a the typeahead control from UI-Bootstrap instead. I'll switch back to using this control if the performance problem ever gets fixed.

jibap commented 8 years ago

@gitcriel : Hi, i've got the same issue, but i need 4000 to 10000 items. ui-bootstrap is really fastest ? Thanks

@lordfriend : otherwise nya-bootstrap-select is the best ! ;-)

lordfriend commented 8 years ago

I haven't found a way which doesn't change the way to use live-search. Currently I recommend to use https://github.com/mbenford/ngTagsInput for super large list