fix(uiSelectMultiple): Don't call onSelectCallback if limit already r…

…eached

Previously if a limit was defined for a multiple selection ui-select 
and that limit was reached, the onSelectCallback would still be fired 
even though the item wasn't actually selected. 

This commit moves the firing of the callback to after the select has 
actually taken place to ensure it is only fired on the correct 
occassions.

Fixes angular-ui#1794 
Closes angular-ui#1836

src/uiSelectController.js

@@ -21,7 +21,6 @@ uis.controller('uiSelectCtrl',
   ctrl.refreshing = false;
   ctrl.spinnerEnabled = uiSelectConfig.spinnerEnabled;
   ctrl.spinnerClass = uiSelectConfig.spinnerClass;
-
   ctrl.removeSelected = uiSelectConfig.removeSelected; //If selected item(s) should be removed from dropdown list
   ctrl.closeOnSelect = true; //Initialized inside uiSelect directive link function
   ctrl.skipFocusser = false; //Set to true to avoid returning focus to ctrl when item is selected
@@ -433,17 +432,7 @@ uis.controller('uiSelectCtrl',
         }
         _resetSearchInput();
         $scope.$broadcast('uis:select', item);
-
-        var locals = {};
-        locals[ctrl.parserResult.itemName] = item;
-
-        $timeout(function(){
-          ctrl.onSelectCallback($scope, {
-            $item: item,
-            $model: ctrl.parserResult.modelMapper($scope, locals)
-          });
-        });
-
+
         if (ctrl.closeOnSelect) {
           ctrl.close(skipFocusser);
         }

src/uiSelectMultipleDirective.js

@@ -178,6 +178,15 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec
           return;
         }
         $select.selected.push(item);
+        var locals = {};        
+        locals[$select.parserResult.itemName] = item;
+
+        $timeout(function(){
+          $select.onSelectCallback(scope, {
+            $item: item,
+            $model: $select.parserResult.modelMapper(scope, locals)
+          });
+        });
         $selectMultiple.updateModel();
       });
 

src/uiSelectSingleDirective.js

@@ -51,6 +51,15 @@ uis.directive('uiSelectSingle', ['$timeout','$compile', function($timeout, $comp
 
       scope.$on('uis:select', function (event, item) {
         $select.selected = item;
+        var locals = {};        
+        locals[$select.parserResult.itemName] = item;
+
+        $timeout(function(){
+          $select.onSelectCallback(scope, {
+            $item: item,
+            $model: $select.parserResult.modelMapper(scope, locals)
+          });
+        });
       });
 
       scope.$on('uis:close', function (event, skipFocusser) {

test/select.spec.js

@@ -1840,6 +1840,8 @@ describe('ui-select tests', function() {
             if (attrs.lockChoice !== undefined) { matchesAttrsHtml += ' ui-lock-choice="' + attrs.lockChoice + '"'; }
             if (attrs.removeSelected !== undefined) { attrsHtml += ' remove-selected="' + attrs.removeSelected + '"'; }
             if (attrs.resetSearchInput !== undefined) { attrsHtml += ' reset-search-input="' + attrs.resetSearchInput + '"'; }
+            if (attrs.limit !== undefined) { attrsHtml += ' limit="' + attrs.limit + '"'; }
+            if (attrs.onSelect !== undefined) { attrsHtml += ' on-select="' + attrs.onSelect + '"'; }
         }
 
         return compileTemplate(
@@ -2785,6 +2787,51 @@ describe('ui-select tests', function() {
          expect(el.scope().$select.selected.length).toBe(2);
      });
 
+     it('should set only 1 item in the selected items when limit = 1', function () {
+         var el = createUiSelectMultiple({limit: 1});
+         clickItem(el, 'Wladimir');
+         clickItem(el, 'Natasha');
+         expect(el.scope().$select.selected.length).toEqual(1);
+    });
+
+    it('should only have 1 item selected and onSelect function should only be handled once.',function(){
+        scope.onSelectFn = function ($item, $model) {
+          scope.$item = $item;
+          scope.$model = $model;
+        };
+        var el = createUiSelectMultiple({limit:1,onSelect:'onSelectFn($item, $model)'});
+
+        expect(scope.$item).toBeFalsy();
+        expect(scope.$model).toBeFalsy();
+
+        clickItem(el, 'Samantha');
+        $timeout.flush();
+        clickItem(el, 'Natasha');
+        $timeout.flush();
+        expect(scope.selection.selectedMultiple[0].name).toBe('Samantha');
+        expect(scope.$model.name).toEqual('Samantha');
+        expect(el.scope().$select.selected.length).toEqual(1);
+    });
+
+    it('should only have 2 items selected and onSelect function should be handeld.',function(){
+        scope.onSelectFn = function ($item, $model) {
+          scope.$item = $item;
+          scope.$model = $model;
+        };
+        var el = createUiSelectMultiple({onSelect:'onSelectFn($item, $model)'});
+
+        expect(scope.$item).toBeFalsy();
+        expect(scope.$model).toBeFalsy();
+
+        clickItem(el, 'Samantha');
+        $timeout.flush();
+        expect(scope.$model.name).toEqual('Samantha');
+        clickItem(el, 'Natasha');
+        $timeout.flush();
+        expect(scope.$model.name).toEqual('Natasha');
+        expect(el.scope().$select.selected.length).toEqual(2);
+    });
+
   describe('resetSearchInput option multiple', function () {
       it('should be true by default', function () {
         expect(createUiSelectMultiple().scope().$select.resetSearchInput).toBe(true);