From b7475e79d141772bf0b7c24ae68e693ebe4c7a02 Mon Sep 17 00:00:00 2001 From: Dondi Dionisio Date: Wed, 23 Mar 2016 14:16:29 -0700 Subject: [PATCH 1/6] fix(select-choices): Possible fix for #1355 ...at the cost of potential unnecessary watches when dropdown is closed. But the premise is that, until the resulting errors are fixed, the preference is to avoid the errors for now. Closes #1355 --- src/uiSelectChoicesDirective.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/uiSelectChoicesDirective.js b/src/uiSelectChoicesDirective.js index 57d4ae0d9..1b4ea9d04 100644 --- a/src/uiSelectChoicesDirective.js +++ b/src/uiSelectChoicesDirective.js @@ -44,8 +44,7 @@ uis.directive('uiSelectChoices', throw uiSelectMinErr('rows', "Expected 1 .ui-select-choices-row but got '{0}'.", choices.length); } - choices.attr('ng-repeat', $select.parserResult.repeatExpression(groupByExp)) - .attr('ng-if', '$select.open'); //Prevent unnecessary watches when dropdown is closed + choices.attr('ng-repeat', $select.parserResult.repeatExpression(groupByExp)); if ($window.document.addEventListener) { //crude way to exclude IE8, specifically, which also cannot capture events choices.attr('ng-mouseenter', '$select.setActiveItem('+$select.parserResult.itemName +')') .attr('ng-click', '$select.select(' + $select.parserResult.itemName + ',$select.skipFocusser,$event)'); From 6a1c922ff413e3ae227ef3080ace40af813930c2 Mon Sep 17 00:00:00 2001 From: Dondi Dionisio Date: Wed, 23 Mar 2016 22:31:23 -0700 Subject: [PATCH 2/6] feat(isDisabled): Make removeSelected configurable Expose the removeSelected property, and adjust isDisabled behavior so that items that would have been removed are disabled instead. Work in progress: This broke some tests --- src/common.js | 1 + src/uiSelectController.js | 13 +++++++++---- src/uiSelectDirective.js | 5 +++++ src/uiSelectMultipleDirective.js | 1 - 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/common.js b/src/common.js index 8d9a56f9b..cbad55af2 100644 --- a/src/common.js +++ b/src/common.js @@ -101,6 +101,7 @@ var uis = angular.module('ui.select', []) closeOnSelect: true, skipFocusser: false, dropdownPosition: 'auto', + removeSelected: true, generateId: function() { return latestId++; }, diff --git a/src/uiSelectController.js b/src/uiSelectController.js index ba3e6d56d..c1645cf7c 100644 --- a/src/uiSelectController.js +++ b/src/uiSelectController.js @@ -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; @@ -298,6 +298,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; @@ -306,9 +311,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)) { 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 } @@ -356,7 +361,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; } diff --git a/src/uiSelectDirective.js b/src/uiSelectDirective.js index 9fdeb3e66..dca8e751c 100644 --- a/src/uiSelectDirective.js +++ b/src/uiSelectDirective.js @@ -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; diff --git a/src/uiSelectMultipleDirective.js b/src/uiSelectMultipleDirective.js index 75744a77e..3ad617112 100644 --- a/src/uiSelectMultipleDirective.js +++ b/src/uiSelectMultipleDirective.js @@ -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; From 295e10a7986adc6546a0fbaca1ecf944b9e5f5f7 Mon Sep 17 00:00:00 2001 From: Dondi Dionisio Date: Mon, 18 Apr 2016 14:59:20 -0700 Subject: [PATCH 3/6] Restore mis-deleted linebreak. --- src/uiSelectChoicesDirective.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uiSelectChoicesDirective.js b/src/uiSelectChoicesDirective.js index d361e37cd..a1c3967c7 100644 --- a/src/uiSelectChoicesDirective.js +++ b/src/uiSelectChoicesDirective.js @@ -60,6 +60,7 @@ uis.directive('uiSelectChoices', $select.onHighlightCallback = attrs.onHighlight; $select.dropdownPosition = attrs.position ? attrs.position.toLowerCase() : uiSelectConfig.dropdownPosition; + scope.$on('$destroy', function() { choices.remove(); }); From 3059f25b81f604f8147049bcb2d4c3f034f9233e Mon Sep 17 00:00:00 2001 From: Dondi Dionisio Date: Mon, 18 Apr 2016 18:02:43 -0700 Subject: [PATCH 4/6] Fix failing unit tests. --- src/uiSelectController.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uiSelectController.js b/src/uiSelectController.js index 1e2a249a2..cd99524ab 100644 --- a/src/uiSelectController.js +++ b/src/uiSelectController.js @@ -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); } From c94bddc6c99d23b35a4ba90802cb4887b51ff6e9 Mon Sep 17 00:00:00 2001 From: Dondi Dionisio Date: Tue, 24 May 2016 11:28:51 -0700 Subject: [PATCH 5/6] docs(removeSelected): add demo for `remove-selected` property Working to round out https://github.com/angular-ui/ui-select/pull/1567 --- docs/assets/demo.js | 1 + docs/examples/demo-multiple-selection.html | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/docs/assets/demo.js b/docs/assets/demo.js index 7aaf3364e..f9999d4a7 100644 --- a/docs/assets/demo.js +++ b/docs/assets/demo.js @@ -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 = ['samantha@email.com','wladimir@email.com']; + vm.multipleDemo.removeSelectIsFalse = [vm.people[2], vm.people[0]]; vm.appendToBodyDemo = { remainingToggleTime: 0, diff --git a/docs/examples/demo-multiple-selection.html b/docs/examples/demo-multiple-selection.html index 54d23e5be..799c8430d 100644 --- a/docs/examples/demo-multiple-selection.html +++ b/docs/examples/demo-multiple-selection.html @@ -72,4 +72,18 @@

Array of objects (with groupBy)

Selected: {{multipleDemo.selectedPeopleWithGroupBy}}

+
+

Disabling instead of removing selected items

+ + {{$item.name}} <{{$item.email}}> + +
+ + email: {{person.email}} + age: + + + +

Selected: {{multipleDemo.removeSelectIsFalse}}

+
From c180e618878abea63d1680232645ab2751aa60fd Mon Sep 17 00:00:00 2001 From: Dondi Dionisio Date: Tue, 24 May 2016 14:11:33 -0700 Subject: [PATCH 6/6] test(remove-select): add unit tests for remove-select Toward merging of https://github.com/angular-ui/ui-select/pull/1567 --- test/select.spec.js | 50 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/select.spec.js b/test/select.spec.js index ac3d3658e..5b814c348 100644 --- a/test/select.spec.js +++ b/test/select.spec.js @@ -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( + ' \ + {{$select.selected.name}} \ + \ +
\ +
\ + \ + ' + ); + + 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( + ' \ + {{$select.selected.name}} \ + \ +
\ +
\ + \ + ' + ); + + 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 tag', function () { var el = compileTemplate(