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

fix(uiSelectMatch): set model value to null when cleared #1959

Merged
merged 1 commit into from
Apr 12, 2017
Merged

fix(uiSelectMatch): set model value to null when cleared #1959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions src/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ var KEY = {
}
};

function isNil(value) {
return angular.isUndefined(value) || value === null;
}

/**
* Add querySelectorAll() to jqLite.
*
Expand Down
6 changes: 3 additions & 3 deletions src/uiSelectController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ uis.controller('uiSelectCtrl',
}

ctrl.isEmpty = function() {
return angular.isUndefined(ctrl.selected) || ctrl.selected === null || ctrl.selected === '' || (ctrl.multiple && ctrl.selected.length === 0);
return isNil(ctrl.selected) || ctrl.selected === '' || (ctrl.multiple && ctrl.selected.length === 0);
};

function _findIndex(collection, predicate, thisArg){
Expand Down Expand Up @@ -379,7 +379,7 @@ uis.controller('uiSelectCtrl',

// When the user selects an item with ENTER or clicks the dropdown
ctrl.select = function(item, skipFocusser, $event) {
if (item === undefined || !_isItemDisabled(item)) {
if (isNil(item) || !_isItemDisabled(item)) {

if ( ! ctrl.items && ! ctrl.search && ! ctrl.tagging.isActivated) return;

Expand Down Expand Up @@ -454,7 +454,7 @@ uis.controller('uiSelectCtrl',
};

ctrl.clear = function($event) {
ctrl.select(undefined);
ctrl.select(null);
$event.stopPropagation();
$timeout(function() {
ctrl.focusser[0].focus();
Expand Down
10 changes: 5 additions & 5 deletions src/uiSelectMultipleDirective.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec
// Make sure that model value is array
if(!angular.isArray(ngModel.$viewValue)){
// Have tolerance for null or undefined values
if(angular.isUndefined(ngModel.$viewValue) || ngModel.$viewValue === null){
if (isNil(ngModel.$viewValue)){
ngModel.$viewValue = [];
} else {
throw uiSelectMinErr('multiarr', "Expected model value to be array but got '{0}'", ngModel.$viewValue);
Expand All @@ -178,7 +178,7 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec
return;
}
$select.selected.push(item);
var locals = {};
var locals = {};
locals[$select.parserResult.itemName] = item;

$timeout(function(){
Expand Down Expand Up @@ -261,11 +261,11 @@ uis.directive('uiSelectMultiple', ['uiSelectMinErr','$timeout', function(uiSelec
} else {
return curr;
}

} else {
// If nothing yet selected, select last item
return last;
}
return last;
}
break;
case KEY.DELETE:
// Remove selected item and select next item
Expand Down
16 changes: 13 additions & 3 deletions src/uiSelectSingleDirective.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ uis.directive('uiSelectSingle', ['$timeout','$compile', function($timeout, $comp

//From view --> model
ngModel.$parsers.unshift(function (inputValue) {
// Keep original value for undefined and null
if (isNil(inputValue)) {
return inputValue;
}

var locals = {},
result;
locals[$select.parserResult.itemName] = inputValue;
Expand All @@ -18,6 +23,11 @@ uis.directive('uiSelectSingle', ['$timeout','$compile', function($timeout, $comp

//From model --> view
ngModel.$formatters.unshift(function (inputValue) {
// Keep original value for undefined and null
if (isNil(inputValue)) {
return inputValue;
}

var data = $select.parserResult && $select.parserResult.source (scope, { $select : {search:''}}), //Overwrite $search
locals = {},
result;
Expand Down Expand Up @@ -51,13 +61,13 @@ uis.directive('uiSelectSingle', ['$timeout','$compile', function($timeout, $comp

scope.$on('uis:select', function (event, item) {
$select.selected = item;
var locals = {};
var locals = {};
locals[$select.parserResult.itemName] = item;

$timeout(function(){
$timeout(function() {
$select.onSelectCallback(scope, {
$item: item,
$model: $select.parserResult.modelMapper(scope, locals)
$model: isNil(item) ? item : $select.parserResult.modelMapper(scope, locals)
});
});
});
Expand Down
38 changes: 33 additions & 5 deletions test/select.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ describe('ui-select tests', function () {
Escape: 27
};

function isNil(value) {
return angular.isUndefined(value) || value === null;
}

//create a directive that wraps ui-select
angular.module('wrapperDirective', ['ui.select']);
angular.module('wrapperDirective').directive('wrapperUiSelect', function () {
Expand Down Expand Up @@ -49,8 +53,8 @@ describe('ui-select tests', function () {
restrict: 'A',
require: 'ngModel',
link: function (scope, element, attrs, ngModel) {
ngModel.$validators.testValidator = function (modelValue, viewValue) {
if (angular.isUndefined(modelValue) || modelValue === null) {
ngModel.$validators.testValidator = function(modelValue, viewValue) {
if (isNil(modelValue)) {
return true;
} else if (angular.isArray(modelValue)) {
var allValid = true, idx = modelValue.length;
Expand Down Expand Up @@ -611,11 +615,10 @@ describe('ui-select tests', function () {

// Trigger clear.
el.find('.select2-search-choice-close').click();
expect(scope.selection.selected).toEqual(undefined);
expect(scope.selection.selected).toEqual(null);

// If there is no selection it the X icon should be gone.
// If there is no selection the X icon should be gone.
expect(el.find('.select2-search-choice-close').length).toEqual(0);

});

it('should toggle allow-clear directive', function () {
Expand All @@ -636,6 +639,31 @@ describe('ui-select tests', function () {
expect(el.find('.select2-search-choice-close').length).toEqual(1);
});

it('should clear selection (with object as source)', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you maybe make use of createUISelect and /or createUIMultipleSelect function in the tests?

Copy link
Author

@jrbotros jrbotros Apr 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The main should clear selection test above still uses createUISelect, but I didn't see a way to test the "object as source" syntax without compiling the template manually (I used the should bind model correctly (with object as source) test as a template). Any ideas how to make this cleaner?

Copy link
Contributor

@Jefiozie Jefiozie Apr 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

ah missed that yeah hmm.. maybe we could make a new function createUiSelectObjectSource or some other good name where the template is build and can be used in the future?

Copy link
Author

Choose a reason for hiding this comment

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

Let's get this landed and open another issue for doing some test cleanup? Happy to take a stab at it in a few days (probably over the weekend)!

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it is best I want to do some major refactoring to this repo, for example it needs to have seperate directives for tagging etc.

var el = compileTemplate(
'<ui-select ng-model="selection.selected" theme="select2"> \
<ui-select-match placeholder="Pick one..." allow-clear="true">{{$select.selected.value.name}}</ui-select-match> \
<ui-select-choices repeat="person.value.name as (key,person) in peopleObj | filter: $select.search"> \
<div ng-bind-html="person.value.name | highlight: $select.search"></div> \
<div ng-bind-html="person.value.email | highlight: $select.search"></div> \
</ui-select-choices> \
</ui-select>'
);
var $select = el.scope().$select;

clickItem(el, 'Samantha');
expect(scope.selection.selected).toEqual('Samantha');

// allowClear should be true.
expect($select.allowClear).toEqual(true);

// Trigger clear.
el.find('.select2-search-choice-close').click();
expect(scope.selection.selected).toEqual(null);

// If there is no selection the X icon should be gone.
expect(el.find('.select2-search-choice-close').length).toEqual(0);
});

it('should pass tabindex to focusser', function () {
var el = createUiSelect({tabindex: 5});
Expand Down