From b95bf9ffc7e5bab23fadfd871e451421764906cb Mon Sep 17 00:00:00 2001 From: user378230 Date: Fri, 8 Jul 2016 18:27:55 +0100 Subject: [PATCH 1/3] fix(isDisabled): do not modify item Previously the item from the list was modified with the _uiSelectChoiceDisabled property. This allowed a leakage of information from ui-select to outside the directive. It also caused issues when the was used outside of the directive. This commit adds a reference array to store disabled items and so prevents the need to modify the item in place. Closes #1200 and #1661 Partially supersedes #1641 --- src/uiSelectController.js | 40 +++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/src/uiSelectController.js b/src/uiSelectController.js index 1a5afec85..6cea09d81 100644 --- a/src/uiSelectController.js +++ b/src/uiSelectController.js @@ -324,18 +324,42 @@ uis.controller('uiSelectCtrl', ctrl.selected.filter(function (selection) { return angular.equals(selection, item); }).length > 0); }; + var disabledItems = []; + + function _updateItemDisabled(item, isDisabled) { + var disabledItemIndex = disabledItems.indexOf(item); + if (isDisabled && disabledItemIndex === -1) { + disabledItems.push(item); + } + + if (!isDisabled && disabledItemIndex > -1) { + disabledItems.splice(disabledItemIndex, 0); + } + } + + function _isItemDisabled(item) { + return disabledItems.indexOf(item) > -1; + } + ctrl.isDisabled = function(itemScope) { if (!ctrl.open) return; - var itemIndex = ctrl.items.indexOf(itemScope[ctrl.itemProperty]); + var item = itemScope[ctrl.itemProperty]; + var itemIndex = ctrl.items.indexOf(item); var isDisabled = false; - var item; + + if (itemIndex >= 0 && (angular.isDefined(ctrl.disableChoiceExpression) || ctrl.multiple)) { - if (itemIndex >= 0 && (!angular.isUndefined(ctrl.disableChoiceExpression) || ctrl.multiple)) { - item = ctrl.items[itemIndex]; - isDisabled = !!(itemScope.$eval(ctrl.disableChoiceExpression)) || _isItemSelected(item); // force the boolean value - item._uiSelectChoiceDisabled = isDisabled; // store this for later reference + if (ctrl.multiple) { + isDisabled = _isItemSelected(item); + } + + if (!isDisabled && angular.isDefined(ctrl.disableChoiceExpression)) { + isDisabled = !!(itemScope.$eval(ctrl.disableChoiceExpression)); + } + + _updateItemDisabled(item, isDisabled); } return isDisabled; @@ -344,11 +368,11 @@ uis.controller('uiSelectCtrl', // When the user selects an item with ENTER or clicks the dropdown ctrl.select = function(item, skipFocusser, $event) { - if (item === undefined || !item._uiSelectChoiceDisabled) { + if (item === undefined || !_isItemDisabled(item)) { if ( ! ctrl.items && ! ctrl.search && ! ctrl.tagging.isActivated) return; - if (!item || !item._uiSelectChoiceDisabled) { + if (!item || !_isItemDisabled(item)) { if(ctrl.tagging.isActivated) { // if taggingLabel is disabled and item is undefined we pull from ctrl.search if ( ctrl.taggingLabel === false ) { From fcd9bc5cc2472960d32b26b2b339215f802de57a Mon Sep 17 00:00:00 2001 From: user378230 Date: Fri, 8 Jul 2016 18:31:43 +0100 Subject: [PATCH 2/3] fix(tagging): infite digest loops when name is similar Resolves issue where isDisabled was running for tags and causing a infite loop to occur if the new tag value included a previous value. For example: tag1: "a" tag2: "aa" Fixes #1693 --- src/uiSelectController.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uiSelectController.js b/src/uiSelectController.js index 6cea09d81..5fb968c0c 100644 --- a/src/uiSelectController.js +++ b/src/uiSelectController.js @@ -351,6 +351,8 @@ uis.controller('uiSelectCtrl', if (itemIndex >= 0 && (angular.isDefined(ctrl.disableChoiceExpression) || ctrl.multiple)) { + if (item.isTag) return false; + if (ctrl.multiple) { isDisabled = _isItemSelected(item); } From c01d3638dd71a68a00b59ad5c5d72edc0a947d2c Mon Sep 17 00:00:00 2001 From: user378230 Date: Fri, 8 Jul 2016 21:50:46 +0100 Subject: [PATCH 3/3] fix(isLocked): do not modify item Previously setting a selected item as locked modified that item, this could cause issues if the item was used outside of the directive. This commit changes the directive to store the information internally thus preventing it from interfering with external uses. Closes #1269 and #514 Supersedes (and closes) #1641 and #952 --- src/uiSelectController.js | 49 ++++++++++++++++++++++++++++---- src/uiSelectMultipleDirective.js | 22 ++++++++------ test/select.spec.js | 37 ++++++++++++++++++++++-- 3 files changed, 92 insertions(+), 16 deletions(-) diff --git a/src/uiSelectController.js b/src/uiSelectController.js index 5fb968c0c..b7dc2801c 100644 --- a/src/uiSelectController.js +++ b/src/uiSelectController.js @@ -472,16 +472,53 @@ uis.controller('uiSelectCtrl', } }; - ctrl.isLocked = function(itemScope, itemIndex) { - var isLocked, item = ctrl.selected[itemIndex]; + // Set default function for locked choices - avoids unnecessary + // logic if functionality is not being used + ctrl.isLocked = function () { + return false; + }; + + $scope.$watch(function () { + return angular.isDefined(ctrl.lockChoiceExpression) && ctrl.lockChoiceExpression !== ""; + }, _initaliseLockedChoices); + + function _initaliseLockedChoices(doInitalise) { + if(!doInitalise) return; + + var lockedItems = []; + + function _updateItemLocked(item, isLocked) { + var lockedItemIndex = lockedItems.indexOf(item); + if (isLocked && lockedItemIndex === -1) { + lockedItems.push(item); + } + + if (!isLocked && lockedItemIndex > -1) { + lockedItems.splice(lockedItemIndex, 0); + } + } - if (item && !angular.isUndefined(ctrl.lockChoiceExpression)) { - isLocked = !!(itemScope.$eval(ctrl.lockChoiceExpression)); // force the boolean value - item._uiSelectChoiceLocked = isLocked; // store this for later reference + function _isItemlocked(item) { + return lockedItems.indexOf(item) > -1; + } + + ctrl.isLocked = function (itemScope, itemIndex) { + var isLocked = false, + item = ctrl.selected[itemIndex]; + + if(item) { + if (itemScope) { + isLocked = !!(itemScope.$eval(ctrl.lockChoiceExpression)); + _updateItemLocked(item, isLocked); + } else { + isLocked = _isItemlocked(item); + } } return isLocked; - }; + }; + } + var sizeWatch = null; var updaterScheduled = false; diff --git a/src/uiSelectMultipleDirective.js b/src/uiSelectMultipleDirective.js index 072eada43..c9be64e7b 100644 --- a/src/uiSelectMultipleDirective.js +++ b/src/uiSelectMultipleDirective.js @@ -37,10 +37,10 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec // Remove item from multiple select ctrl.removeChoice = function(index){ - var removedChoice = $select.selected[index]; + // if the choice is locked, don't remove it + if($select.isLocked(null, index)) return false; - // if the choice is locked, can't remove it - if(removedChoice._uiSelectChoiceLocked) return; + var removedChoice = $select.selected[index]; var locals = {}; locals[$select.parserResult.itemName] = removedChoice; @@ -59,6 +59,7 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec ctrl.updateModel(); + return true; }; ctrl.getPlaceholder = function(){ @@ -246,11 +247,16 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec case KEY.BACKSPACE: // Remove selected item and select previous/first if(~$selectMultiple.activeMatchIndex){ - $selectMultiple.removeChoice(curr); - return prev; - } - // Select last item - else return last; + if($selectMultiple.removeChoice(curr)) { + return prev; + } else { + return curr; + } + + } else { + // If nothing yet selected, select last item + return last; + } break; case KEY.DELETE: // Remove selected item and select next item diff --git a/test/select.spec.js b/test/select.spec.js index 8959381e5..5b735689f 100644 --- a/test/select.spec.js +++ b/test/select.spec.js @@ -1745,7 +1745,8 @@ describe('ui-select tests', function() { function createUiSelectMultiple(attrs) { var attrsHtml = '', - choicesAttrsHtml = ''; + choicesAttrsHtml = '', + matchesAttrsHtml = ''; if (attrs !== undefined) { if (attrs.disabled !== undefined) { attrsHtml += ' ng-disabled="' + attrs.disabled + '"'; } if (attrs.required !== undefined) { attrsHtml += ' ng-required="' + attrs.required + '"'; } @@ -1756,11 +1757,12 @@ describe('ui-select tests', function() { if (attrs.taggingLabel !== undefined) { attrsHtml += ' tagging-label="' + attrs.taggingLabel + '"'; } if (attrs.inputId !== undefined) { attrsHtml += ' input-id="' + attrs.inputId + '"'; } if (attrs.groupBy !== undefined) { choicesAttrsHtml += ' group-by="' + attrs.groupBy + '"'; } + if (attrs.lockChoice !== undefined) { matchesAttrsHtml += ' ui-lock-choice="' + attrs.lockChoice + '"'; } } return compileTemplate( ' \ - {{$item.name}} <{{$item.email}}> \ + {{$item.name}} <{{$item.email}}> \ \
\
\ @@ -1922,6 +1924,37 @@ describe('ui-select tests', function() { }); + it('should NOT remove highlighted match when pressing BACKSPACE key on a locked choice', function() { + + scope.selection.selectedMultiple = [scope.people[4], scope.people[5], scope.people[6]]; //Wladimir, Samantha & Nicole + var el = createUiSelectMultiple({lockChoice: "$item.name == '" + scope.people[6].name + "'"}); + var searchInput = el.find('.ui-select-search'); + + expect(isDropdownOpened(el)).toEqual(false); + triggerKeydown(searchInput, Key.Left); + triggerKeydown(searchInput, Key.Backspace); + expect(el.scope().$select.selected).toEqual([scope.people[4], scope.people[5], scope.people[6]]); //Wladimir, Samantha & Nicole + + expect(el.scope().$selectMultiple.activeMatchIndex).toBe(scope.selection.selectedMultiple.length - 1); + + }); + + it('should NOT remove highlighted match when pressing DELETE key on a locked choice', function() { + + scope.selection.selectedMultiple = [scope.people[4], scope.people[5], scope.people[6]]; //Wladimir, Samantha & Nicole + var el = createUiSelectMultiple({lockChoice: "$item.name == '" + scope.people[6].name + "'"}); + var searchInput = el.find('.ui-select-search'); + + expect(isDropdownOpened(el)).toEqual(false); + triggerKeydown(searchInput, Key.Left); + triggerKeydown(searchInput, Key.Delete); + expect(el.scope().$select.selected).toEqual([scope.people[4], scope.people[5], scope.people[6]]); //Wladimir, Samantha & Nicole + + expect(el.scope().$selectMultiple.activeMatchIndex).toBe(scope.selection.selectedMultiple.length - 1); + + }); + + it('should move to last match when pressing LEFT key from search', function() { var el = createUiSelectMultiple();