Skip to content

Commit c01d363

Browse files
committed
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 angular-ui#1269 and angular-ui#514 Supersedes (and closes) angular-ui#1641 and angular-ui#952
1 parent fcd9bc5 commit c01d363

File tree

3 files changed

+92
-16
lines changed

3 files changed

+92
-16
lines changed

Diff for: src/uiSelectController.js

+43-6
Original file line numberDiff line numberDiff line change
@@ -472,16 +472,53 @@ uis.controller('uiSelectCtrl',
472472
}
473473
};
474474

475-
ctrl.isLocked = function(itemScope, itemIndex) {
476-
var isLocked, item = ctrl.selected[itemIndex];
475+
// Set default function for locked choices - avoids unnecessary
476+
// logic if functionality is not being used
477+
ctrl.isLocked = function () {
478+
return false;
479+
};
480+
481+
$scope.$watch(function () {
482+
return angular.isDefined(ctrl.lockChoiceExpression) && ctrl.lockChoiceExpression !== "";
483+
}, _initaliseLockedChoices);
484+
485+
function _initaliseLockedChoices(doInitalise) {
486+
if(!doInitalise) return;
487+
488+
var lockedItems = [];
489+
490+
function _updateItemLocked(item, isLocked) {
491+
var lockedItemIndex = lockedItems.indexOf(item);
492+
if (isLocked && lockedItemIndex === -1) {
493+
lockedItems.push(item);
494+
}
495+
496+
if (!isLocked && lockedItemIndex > -1) {
497+
lockedItems.splice(lockedItemIndex, 0);
498+
}
499+
}
477500

478-
if (item && !angular.isUndefined(ctrl.lockChoiceExpression)) {
479-
isLocked = !!(itemScope.$eval(ctrl.lockChoiceExpression)); // force the boolean value
480-
item._uiSelectChoiceLocked = isLocked; // store this for later reference
501+
function _isItemlocked(item) {
502+
return lockedItems.indexOf(item) > -1;
503+
}
504+
505+
ctrl.isLocked = function (itemScope, itemIndex) {
506+
var isLocked = false,
507+
item = ctrl.selected[itemIndex];
508+
509+
if(item) {
510+
if (itemScope) {
511+
isLocked = !!(itemScope.$eval(ctrl.lockChoiceExpression));
512+
_updateItemLocked(item, isLocked);
513+
} else {
514+
isLocked = _isItemlocked(item);
515+
}
481516
}
482517

483518
return isLocked;
484-
};
519+
};
520+
}
521+
485522

486523
var sizeWatch = null;
487524
var updaterScheduled = false;

Diff for: src/uiSelectMultipleDirective.js

+14-8
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec
3737
// Remove item from multiple select
3838
ctrl.removeChoice = function(index){
3939

40-
var removedChoice = $select.selected[index];
40+
// if the choice is locked, don't remove it
41+
if($select.isLocked(null, index)) return false;
4142

42-
// if the choice is locked, can't remove it
43-
if(removedChoice._uiSelectChoiceLocked) return;
43+
var removedChoice = $select.selected[index];
4444

4545
var locals = {};
4646
locals[$select.parserResult.itemName] = removedChoice;
@@ -59,6 +59,7 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec
5959

6060
ctrl.updateModel();
6161

62+
return true;
6263
};
6364

6465
ctrl.getPlaceholder = function(){
@@ -246,11 +247,16 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec
246247
case KEY.BACKSPACE:
247248
// Remove selected item and select previous/first
248249
if(~$selectMultiple.activeMatchIndex){
249-
$selectMultiple.removeChoice(curr);
250-
return prev;
251-
}
252-
// Select last item
253-
else return last;
250+
if($selectMultiple.removeChoice(curr)) {
251+
return prev;
252+
} else {
253+
return curr;
254+
}
255+
256+
} else {
257+
// If nothing yet selected, select last item
258+
return last;
259+
}
254260
break;
255261
case KEY.DELETE:
256262
// Remove selected item and select next item

Diff for: test/select.spec.js

+35-2
Original file line numberDiff line numberDiff line change
@@ -1745,7 +1745,8 @@ describe('ui-select tests', function() {
17451745

17461746
function createUiSelectMultiple(attrs) {
17471747
var attrsHtml = '',
1748-
choicesAttrsHtml = '';
1748+
choicesAttrsHtml = '',
1749+
matchesAttrsHtml = '';
17491750
if (attrs !== undefined) {
17501751
if (attrs.disabled !== undefined) { attrsHtml += ' ng-disabled="' + attrs.disabled + '"'; }
17511752
if (attrs.required !== undefined) { attrsHtml += ' ng-required="' + attrs.required + '"'; }
@@ -1756,11 +1757,12 @@ describe('ui-select tests', function() {
17561757
if (attrs.taggingLabel !== undefined) { attrsHtml += ' tagging-label="' + attrs.taggingLabel + '"'; }
17571758
if (attrs.inputId !== undefined) { attrsHtml += ' input-id="' + attrs.inputId + '"'; }
17581759
if (attrs.groupBy !== undefined) { choicesAttrsHtml += ' group-by="' + attrs.groupBy + '"'; }
1760+
if (attrs.lockChoice !== undefined) { matchesAttrsHtml += ' ui-lock-choice="' + attrs.lockChoice + '"'; }
17591761
}
17601762

17611763
return compileTemplate(
17621764
'<ui-select multiple ng-model="selection.selectedMultiple"' + attrsHtml + ' theme="bootstrap" style="width: 800px;"> \
1763-
<ui-select-match placeholder="Pick one...">{{$item.name}} &lt;{{$item.email}}&gt;</ui-select-match> \
1765+
<ui-select-match "' + matchesAttrsHtml + ' placeholder="Pick one...">{{$item.name}} &lt;{{$item.email}}&gt;</ui-select-match> \
17641766
<ui-select-choices repeat="person in people | filter: $select.search"' + choicesAttrsHtml + '> \
17651767
<div ng-bind-html="person.name | highlight: $select.search"></div> \
17661768
<div ng-bind-html="person.email | highlight: $select.search"></div> \
@@ -1922,6 +1924,37 @@ describe('ui-select tests', function() {
19221924

19231925
});
19241926

1927+
it('should NOT remove highlighted match when pressing BACKSPACE key on a locked choice', function() {
1928+
1929+
scope.selection.selectedMultiple = [scope.people[4], scope.people[5], scope.people[6]]; //Wladimir, Samantha & Nicole
1930+
var el = createUiSelectMultiple({lockChoice: "$item.name == '" + scope.people[6].name + "'"});
1931+
var searchInput = el.find('.ui-select-search');
1932+
1933+
expect(isDropdownOpened(el)).toEqual(false);
1934+
triggerKeydown(searchInput, Key.Left);
1935+
triggerKeydown(searchInput, Key.Backspace);
1936+
expect(el.scope().$select.selected).toEqual([scope.people[4], scope.people[5], scope.people[6]]); //Wladimir, Samantha & Nicole
1937+
1938+
expect(el.scope().$selectMultiple.activeMatchIndex).toBe(scope.selection.selectedMultiple.length - 1);
1939+
1940+
});
1941+
1942+
it('should NOT remove highlighted match when pressing DELETE key on a locked choice', function() {
1943+
1944+
scope.selection.selectedMultiple = [scope.people[4], scope.people[5], scope.people[6]]; //Wladimir, Samantha & Nicole
1945+
var el = createUiSelectMultiple({lockChoice: "$item.name == '" + scope.people[6].name + "'"});
1946+
var searchInput = el.find('.ui-select-search');
1947+
1948+
expect(isDropdownOpened(el)).toEqual(false);
1949+
triggerKeydown(searchInput, Key.Left);
1950+
triggerKeydown(searchInput, Key.Delete);
1951+
expect(el.scope().$select.selected).toEqual([scope.people[4], scope.people[5], scope.people[6]]); //Wladimir, Samantha & Nicole
1952+
1953+
expect(el.scope().$selectMultiple.activeMatchIndex).toBe(scope.selection.selectedMultiple.length - 1);
1954+
1955+
});
1956+
1957+
19251958
it('should move to last match when pressing LEFT key from search', function() {
19261959

19271960
var el = createUiSelectMultiple();

0 commit comments

Comments
 (0)