Skip to content

Commit

Permalink
fix(isLocked): do not modify item
Browse files Browse the repository at this point in the history
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 angular-ui#1269 and angular-ui#514
Supersedes (and closes) angular-ui#1641 and angular-ui#952
  • Loading branch information
user378230 committed Jul 9, 2016
1 parent 3b5752e commit 6745437
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 16 deletions.
47 changes: 41 additions & 6 deletions src/uiSelectController.js
Original file line number Diff line number Diff line change
Expand Up @@ -473,16 +473,51 @@ 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() {
var lockedItems = [];

function _updateItemLocked(item, isLocked) {
var lockedItemIndex = lockedItems.indexOf(item);
if (isLocked && lockedItemIndex === -1) {
lockedItems.push(item);
}

if (item && !angular.isUndefined(ctrl.lockChoiceExpression)) {
isLocked = !!(itemScope.$eval(ctrl.lockChoiceExpression)); // force the boolean value
item._uiSelectChoiceLocked = isLocked; // store this for later reference
if (!isLocked && lockedItemIndex > -1) {
lockedItems.splice(lockedItemIndex, 0);
}
}

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;
Expand Down
22 changes: 14 additions & 8 deletions src/uiSelectMultipleDirective.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -59,6 +59,7 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec

ctrl.updateModel();

return true;
};

ctrl.getPlaceholder = function(){
Expand Down Expand Up @@ -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
Expand Down
37 changes: 35 additions & 2 deletions test/select.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 + '"'; }
Expand All @@ -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(
'<ui-select multiple ng-model="selection.selectedMultiple"' + attrsHtml + ' theme="bootstrap" style="width: 800px;"> \
<ui-select-match placeholder="Pick one...">{{$item.name}} &lt;{{$item.email}}&gt;</ui-select-match> \
<ui-select-match "' + matchesAttrsHtml + ' placeholder="Pick one...">{{$item.name}} &lt;{{$item.email}}&gt;</ui-select-match> \
<ui-select-choices repeat="person in people | filter: $select.search"' + choicesAttrsHtml + '> \
<div ng-bind-html="person.name | highlight: $select.search"></div> \
<div ng-bind-html="person.email | highlight: $select.search"></div> \
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 6745437

Please sign in to comment.