Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

feat(removeSelected): Implement removeSelected property for multiple selects #1622

Merged
merged 1 commit into from
May 31, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/assets/demo.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ app.controller('DemoCtrl', function ($scope, $http, $timeout, $interval) {
vm.multipleDemo.selectedPeople2 = vm.multipleDemo.selectedPeople;
vm.multipleDemo.selectedPeopleWithGroupBy = [vm.people[8], vm.people[6]];
vm.multipleDemo.selectedPeopleSimple = ['[email protected]','[email protected]'];
vm.multipleDemo.removeSelectIsFalse = [vm.people[2], vm.people[0]];

vm.appendToBodyDemo = {
remainingToggleTime: 0,
Expand Down
14 changes: 14 additions & 0 deletions docs/examples/demo-multiple-selection.html
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,18 @@ <h3>Array of objects (with groupBy)</h3>
</ui-select>
<p>Selected: {{multipleDemo.selectedPeopleWithGroupBy}}</p>

<hr>
<h3>Disabling instead of removing selected items</h3>
<ui-select multiple ng-model="ctrl.multipleDemo.removeSelectIsFalse" theme="bootstrap" ng-disabled="ctrl.disabled" close-on-select="false" style="width: 800px;" title="Choose a person" remove-selected="false">
<ui-select-match placeholder="Select person...">{{$item.name}} &lt;{{$item.email}}&gt;</ui-select-match>
<ui-select-choices repeat="person in ctrl.people | propsFilter: {name: $select.search, age: $select.search}">
<div ng-bind-html="person.name | highlight: $select.search"></div>
<small>
email: {{person.email}}
age: <span ng-bind-html="''+person.age | highlight: $select.search"></span>
</small>
</ui-select-choices>
</ui-select>
<p>Selected: {{multipleDemo.removeSelectIsFalse}}</p>

<div style="height:500px"></div>
1 change: 1 addition & 0 deletions src/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ var uis = angular.module('ui.select', [])
closeOnSelect: true,
skipFocusser: false,
dropdownPosition: 'auto',
removeSelected: true,
generateId: function() {
return latestId++;
},
Expand Down
17 changes: 11 additions & 6 deletions src/uiSelectController.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ uis.controller('uiSelectCtrl',
ctrl.refreshDelay = uiSelectConfig.refreshDelay;
ctrl.paste = uiSelectConfig.paste;

ctrl.removeSelected = false; //If selected item(s) should be removed from dropdown list
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
ctrl.search = EMPTY_SEARCH;
Expand Down Expand Up @@ -240,9 +240,9 @@ uis.controller('uiSelectCtrl',
}else{
if ( data !== undefined ) {
var filteredItems = data.filter(function(i) {
return selectedItems.every(function(selectedItem) {
return angular.isArray(selectedItems) ? selectedItems.every(function(selectedItem) {
return !angular.equals(i, selectedItem);
});
}) : !angular.equals(i, selectedItems);
});
ctrl.setItemsFn(filteredItems);
}
Expand Down Expand Up @@ -317,6 +317,11 @@ uis.controller('uiSelectCtrl',
return isActive;
};

var _isItemSelected = function (item) {
return (ctrl.selected && angular.isArray(ctrl.selected) &&
ctrl.selected.filter(function (selection) { return angular.equals(selection, item); }).length > 0);
};

ctrl.isDisabled = function(itemScope) {

if (!ctrl.open) return;
Expand All @@ -325,9 +330,9 @@ uis.controller('uiSelectCtrl',
var isDisabled = false;
var item;

if (itemIndex >= 0 && !angular.isUndefined(ctrl.disableChoiceExpression)) {
if (itemIndex >= 0 && (!angular.isUndefined(ctrl.disableChoiceExpression) || ctrl.multiple)) {
Copy link
Contributor

@user378230 user378230 May 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dondi could you add an extra set of brackets here just to make the condition precedence clear?

Eg.

if (itemIndex >= 0 && ((!angular.isUndefined(ctrl.disableChoiceExpression) || ctrl.multiple)))

Or..?

if ((itemIndex >= 0 && (!angular.isUndefined(ctrl.disableChoiceExpression)) || ctrl.multiple))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the parentheses already disambiguate the precedence. The current code is shown below, with the matching middle parentheses spaced out for emphasis:

if (itemIndex >= 0 &&   (  !angular.isUndefined(ctrl.disableChoiceExpression) || ctrl.multiple  )   ) {

Those parentheses ensure that the || in the middle takes precedence. The suggestions made above would result in double-parentheses on the same expression (the disjunction in the first example; the entire condition in the second example).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I totally misread it, my bad! 😳

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, all good—thanks for reviewing!

item = ctrl.items[itemIndex];
isDisabled = !!(itemScope.$eval(ctrl.disableChoiceExpression)); // force the boolean value
isDisabled = !!(itemScope.$eval(ctrl.disableChoiceExpression)) || _isItemSelected(item); // force the boolean value
item._uiSelectChoiceDisabled = isDisabled; // store this for later reference
}

Expand Down Expand Up @@ -375,7 +380,7 @@ uis.controller('uiSelectCtrl',
}
}
// search ctrl.selected for dupes potentially caused by tagging and return early if found
if ( ctrl.selected && angular.isArray(ctrl.selected) && ctrl.selected.filter( function (selection) { return angular.equals(selection, item); }).length > 0 ) {
if (_isItemSelected(item)) {
ctrl.close(skipFocusser);
return;
}
Expand Down
5 changes: 5 additions & 0 deletions src/uiSelectDirective.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ uis.directive('uiSelect',
$select.sortable = sortable !== undefined ? sortable : uiSelectConfig.sortable;
});

scope.$watch('removeSelected', function() {
var removeSelected = scope.$eval(attrs.removeSelected);
$select.removeSelected = removeSelected !== undefined ? removeSelected : uiSelectConfig.removeSelected;
});

attrs.$observe('disabled', function() {
// No need to use $eval() (thanks to ng-disabled) since we already get a boolean instead of a string
$select.disabled = attrs.disabled !== undefined ? attrs.disabled : false;
Expand Down
1 change: 0 additions & 1 deletion src/uiSelectMultipleDirective.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec
//$select.selected = raw selected objects (ignoring any property binding)

$select.multiple = true;
$select.removeSelected = true;

//Input that will handle focus
$select.focusInput = $select.searchInput;
Expand Down
50 changes: 50 additions & 0 deletions test/select.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1427,6 +1427,56 @@ describe('ui-select tests', function() {
expect($(el).scope().$select.selected).toEqual(['idontexist']);
});

it('should remove a choice when remove-selected is not given (default is true)', function () {

var el = compileTemplate(
'<ui-select multiple ng-model="selection.selected"> \
<ui-select-match placeholder="Pick one...">{{$select.selected.name}}</ui-select-match> \
<ui-select-choices repeat="person in people | filter: $select.search"> \
<div class="person-name" ng-bind-html="person.name" | highlight: $select.search"></div> \
<div ng-bind-html="person.email | highlight: $select.search"></div> \
</ui-select-choices> \
</ui-select>'
);

clickItem(el, 'Samantha');
clickItem(el, 'Adrian');

openDropdown(el);

var choicesEls = $(el).find('.ui-select-choices-row');
expect(choicesEls.length).toEqual(6);

['Adam', 'Amalie', 'Estefanía', 'Wladimir', 'Nicole', 'Natasha'].forEach(function (name, index) {
expect($(choicesEls[index]).hasClass('disabled')).toBeFalsy();
expect($(choicesEls[index]).find('.person-name').text()).toEqual(name);
});
});

it('should disable a choice instead of removing it when remove-selected is false', function () {

var el = compileTemplate(
'<ui-select multiple remove-selected="false" ng-model="selection.selected"> \
<ui-select-match placeholder="Pick one...">{{$select.selected.name}}</ui-select-match> \
<ui-select-choices repeat="person in people | filter: $select.search"> \
<div ng-bind-html="person.name" | highlight: $select.search"></div> \
<div ng-bind-html="person.email | highlight: $select.search"></div> \
</ui-select-choices> \
</ui-select>'
);

clickItem(el, 'Samantha');
clickItem(el, 'Adrian');

openDropdown(el);

var choicesEls = $(el).find('.ui-select-choices-row');
expect(choicesEls.length).toEqual(8);
[false, false, false, true /* Adrian */, false, true /* Samantha */, false, false].forEach(function (bool, index) {
expect($(choicesEls[index]).hasClass('disabled')).toEqual(bool);
});
});

it('should append/transclude content (with correct scope) that users add at <match> tag', function () {

var el = compileTemplate(
Expand Down