Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

Fix for auto sorting items in select element #78

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

animator013
Copy link

There is a problem with select2 that if i had some optgroups and items in it and if i selected 1 item from the second and 1 item from the first, the second item was moved to first position.

I found in angular.js:

selectElement.bind('change', function() {
  scope.$apply(function() {
    var array = [];
    forEach(selectElement.find('option'), function(option) {
      if (option.selected) {
        array.push(option.value);
      }
    });
    ctrl.$setViewValue(array);
  });
});

that model is being filled by iterating the selected options in select element. So if we do this in ui-select2:

controller.$render = function () {
  if (isSelect) {
    elm.select2('val', controller.$viewValue);
  }
...
}

We actually update the element with model values and "resort" all items by the model. This is not a good approach I think.

I made a fix for it so the render will hit select element once (items get sorted at the beginning by model) and then after selecting options only model gets updated and items will be added. But they wont sync and user wont be confused by item getting away by sorting.

Also link to already reported issue #12

EDIT: i see it doesnt make through tests. If there is someone who can fix it properly it would be very helpful. I have no time for this at the moment.

@rwrrll
Copy link

rwrrll commented Mar 25, 2014

+1 this preserves the selection order.

@lkptrzk
Copy link
Contributor

lkptrzk commented Mar 25, 2014

@animator013 #180 fixes tests, many of the pull reqs are failing.

@dmunch
Copy link

dmunch commented Mar 25, 2014

So with the tests passing, are there any chances that this is going to be merged soon? Just stumbled upon the bug an hour ago and would need to fixed sort behaviour.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants