From 453294fa3722754bf2251fe7c85d06736e4ed22e Mon Sep 17 00:00:00 2001 From: Jonathan Nelson Date: Wed, 8 Jul 2015 17:30:24 -0400 Subject: [PATCH 1/5] add onBeforeSelect callback Adds an onBeforeSelect callback to allow intercepting a selection. Ensures access to both the prospective new selection and the current selection. --- src/uiSelectController.js | 47 ++++++++---- src/uiSelectDirective.js | 1 + test/select.spec.js | 154 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 188 insertions(+), 14 deletions(-) diff --git a/src/uiSelectController.js b/src/uiSelectController.js index c0a1a94ac..e74bd8e48 100644 --- a/src/uiSelectController.js +++ b/src/uiSelectController.js @@ -5,8 +5,8 @@ * put as much logic in the controller (instead of the link functions) as possible so it can be easily tested. */ uis.controller('uiSelectCtrl', - ['$scope', '$element', '$timeout', '$filter', 'uisRepeatParser', 'uiSelectMinErr', 'uiSelectConfig', - function($scope, $element, $timeout, $filter, RepeatParser, uiSelectMinErr, uiSelectConfig) { + ['$scope', '$element', '$timeout', '$filter', '$q', 'uisRepeatParser', 'uiSelectMinErr', 'uiSelectConfig', + function($scope, $element, $timeout, $filter, $q, RepeatParser, uiSelectMinErr, uiSelectConfig) { var ctrl = this; @@ -284,24 +284,43 @@ uis.controller('uiSelectCtrl', } } - $scope.$broadcast('uis:select', item); + var completeSelection = function() { + $scope.$broadcast('uis:select', item); + + $timeout(function(){ + ctrl.onSelectCallback($scope, callbackContext); + }); + + if (ctrl.closeOnSelect) { + ctrl.close(skipFocusser); + } + if ($event && $event.type === 'click') { + ctrl.clickTriggeredSelect = true; + } + }; var locals = {}; locals[ctrl.parserResult.itemName] = item; - $timeout(function(){ - ctrl.onSelectCallback($scope, { - $item: item, - $model: ctrl.parserResult.modelMapper($scope, locals) - }); - }); + var callbackContext = { + $item: item, + $model: ctrl.parserResult.modelMapper($scope, locals) + }; - if (ctrl.closeOnSelect) { - ctrl.close(skipFocusser); - } - if ($event && $event.type === 'click') { - ctrl.clickTriggeredSelect = true; + var onBeforeSelectResult = ctrl.onBeforeSelectCallback($scope, callbackContext); + + if (angular.isDefined(onBeforeSelectResult)) { + if (!onBeforeSelectResult) { + return; // abort the selection in case of deliberate falsey result + } else if (angular.isDefined(onBeforeSelectResult.promise)) { + onBeforeSelectResult.promise.then(completeSelection); + } else { + completeSelection(); + } + } else { + completeSelection(); } + } } }; diff --git a/src/uiSelectDirective.js b/src/uiSelectDirective.js index fc6cc4138..4af6acb7f 100644 --- a/src/uiSelectDirective.js +++ b/src/uiSelectDirective.js @@ -41,6 +41,7 @@ uis.directive('uiSelect', } }(); + $select.onBeforeSelectCallback = $parse(attrs.onBeforeSelect); $select.onSelectCallback = $parse(attrs.onSelect); $select.onRemoveCallback = $parse(attrs.onRemove); diff --git a/test/select.spec.js b/test/select.spec.js index 2ba86fb2a..3875f2181 100644 --- a/test/select.spec.js +++ b/test/select.spec.js @@ -959,6 +959,160 @@ describe('ui-select tests', function() { }); + it('should invoke before-select callback before select callback synchronously', function () { + + var order = []; + scope.onBeforeSelectFn = function ($item, $model, $label) { + order.push('onBeforeSelectFn'); + }; + scope.onSelectFn = function ($item, $model, $label) { + order.push('onSelectFn'); + }; + var el = compileTemplate( + ' \ + {{$select.selected.name}} \ + \ +
\ +
\ + \ + ' + ); + + clickItem(el, 'Samantha'); + $timeout.flush(); + + expect(order[0]).toEqual('onBeforeSelectFn'); + expect(order[1]).toEqual('onSelectFn'); + + }); + + it('should invoke before-select callback before select callback when promised', inject(function ($q) { + + var order = []; + scope.onBeforeSelectFn = function ($item, $model, $label) { + var deferred = $q.defer(); + $timeout(function () { + order.push('onBeforeSelectFn'); + deferred.resolve(order); + }, 50); + return deferred; + }; + scope.onSelectFn = function ($item, $model, $label) { + order.push('onSelectFn'); + }; + var el = compileTemplate( + ' \ + {{$select.selected.name}} \ + \ +
\ +
\ + \ + ' + ); + + clickItem(el, 'Samantha'); + $timeout.flush(); + + expect(order[0]).toEqual('onBeforeSelectFn'); + expect(order[1]).toEqual('onSelectFn'); + + })); + + it('should abort selection if before-select callback returns falsy', function () { + + scope.onBeforeSelectFn = function ($item, $model, $label) { + return false; + }; + scope.onSelectFn = function ($item, $model, $label) { + scope.$item = $item; + scope.$model = $model; + }; + var el = compileTemplate( + ' \ + {{$select.selected.name}} \ + \ +
\ +
\ + \ + ' + ); + + expect(scope.$item).toBeFalsy(); + expect(scope.$model).toBeFalsy(); + + clickItem(el, 'Samantha'); + $timeout.flush(); + + expect(scope.$item).toBeFalsy(); + expect(scope.$model).toBeFalsy(); + + }); + + it('should abort selection if before-select callback rejects promise', inject(function ($q) { + + scope.onBeforeSelectFn = function ($item, $model, $label) { + var deferred = $q.defer(); + $timeout(function () { + deferred.reject(); + }, 50); + return deferred; + }; + scope.onSelectFn = function ($item, $model, $label) { + scope.$item = $item; + scope.$model = $model; + }; + var el = compileTemplate( + ' \ + {{$select.selected.name}} \ + \ +
\ +
\ + \ + ' + ); + + expect(scope.$item).toBeFalsy(); + expect(scope.$model).toBeFalsy(); + + clickItem(el, 'Samantha'); + $timeout.flush(); + + expect(scope.$item).toBeFalsy(); + expect(scope.$model).toBeFalsy(); + + })); + + it('should keep reference to current selection and incoming selection within before-select callback', function () { + + var currentSelection, incomingSelection; + scope.onBeforeSelectFn = function ($item, $model, $label) { + incomingSelection = $item; + currentSelection = scope.selection.selected; + }; + var el = compileTemplate( + ' \ + {{$select.selected.name}} \ + \ +
\ +
\ + \ + ' + ); + + clickItem(el, 'Samantha'); + $timeout.flush(); + + expect(currentSelection).toBeFalsy(); + expect(incomingSelection.name).toBe('Samantha'); + + clickItem(el, 'Adam'); + $timeout.flush(); + + expect(currentSelection).toBe('Samantha'); + expect(incomingSelection.name).toBe('Adam'); + + }); + it('should invoke hover callback', function(){ var highlighted; From a62af5bbce31a4062760a3393bbcd5a655980e08 Mon Sep 17 00:00:00 2001 From: Jonathan Nelson Date: Wed, 8 Jul 2015 18:11:42 -0400 Subject: [PATCH 2/5] remove unused on-select attribute --- test/select.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/select.spec.js b/test/select.spec.js index 3875f2181..fd4ae96c9 100644 --- a/test/select.spec.js +++ b/test/select.spec.js @@ -1090,7 +1090,7 @@ describe('ui-select tests', function() { currentSelection = scope.selection.selected; }; var el = compileTemplate( - ' \ + ' \ {{$select.selected.name}} \ \
\ From 326dd7f6d7d1f167022319998f832c7b5f891ebb Mon Sep 17 00:00:00 2001 From: Jonathan Nelson Date: Thu, 9 Jul 2015 13:08:55 -0400 Subject: [PATCH 3/5] remove duration --- test/select.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/select.spec.js b/test/select.spec.js index fd4ae96c9..deecc5453 100644 --- a/test/select.spec.js +++ b/test/select.spec.js @@ -994,7 +994,7 @@ describe('ui-select tests', function() { $timeout(function () { order.push('onBeforeSelectFn'); deferred.resolve(order); - }, 50); + }); return deferred; }; scope.onSelectFn = function ($item, $model, $label) { @@ -1054,7 +1054,7 @@ describe('ui-select tests', function() { var deferred = $q.defer(); $timeout(function () { deferred.reject(); - }, 50); + }); return deferred; }; scope.onSelectFn = function ($item, $model, $label) { From bb8bfd0abd0b660eb4f4e5a58b2a4192f736a3fc Mon Sep 17 00:00:00 2001 From: Jonathan Nelson Date: Thu, 9 Jul 2015 13:11:14 -0400 Subject: [PATCH 4/5] use typical promise-handling convention --- src/uiSelectController.js | 4 ++-- test/select.spec.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/uiSelectController.js b/src/uiSelectController.js index e74bd8e48..c14e4bea3 100644 --- a/src/uiSelectController.js +++ b/src/uiSelectController.js @@ -312,8 +312,8 @@ uis.controller('uiSelectCtrl', if (angular.isDefined(onBeforeSelectResult)) { if (!onBeforeSelectResult) { return; // abort the selection in case of deliberate falsey result - } else if (angular.isDefined(onBeforeSelectResult.promise)) { - onBeforeSelectResult.promise.then(completeSelection); + } else if (angular.isFunction(onBeforeSelectResult.then)) { + onBeforeSelectResult.then(completeSelection); } else { completeSelection(); } diff --git a/test/select.spec.js b/test/select.spec.js index deecc5453..5c47fd383 100644 --- a/test/select.spec.js +++ b/test/select.spec.js @@ -995,7 +995,7 @@ describe('ui-select tests', function() { order.push('onBeforeSelectFn'); deferred.resolve(order); }); - return deferred; + return deferred.promise; }; scope.onSelectFn = function ($item, $model, $label) { order.push('onSelectFn'); @@ -1055,7 +1055,7 @@ describe('ui-select tests', function() { $timeout(function () { deferred.reject(); }); - return deferred; + return deferred.promise; }; scope.onSelectFn = function ($item, $model, $label) { scope.$item = $item; From 284dc8395c73fc43578edd105b9f17ef6d564c7b Mon Sep 17 00:00:00 2001 From: Jonathan Nelson Date: Thu, 9 Jul 2015 13:12:00 -0400 Subject: [PATCH 5/5] add test for promise resolve success --- test/select.spec.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/select.spec.js b/test/select.spec.js index 5c47fd383..0ef4d4394 100644 --- a/test/select.spec.js +++ b/test/select.spec.js @@ -1018,6 +1018,42 @@ describe('ui-select tests', function() { })); + it('should complete on-select if before-select callback promise is resolved', inject(function ($q) { + + scope.onBeforeSelectFn = function ($item, $model, $label) { + var deferred = $q.defer(); + $timeout(function () { + deferred.resolve(); + }); + return deferred.promise; + }; + scope.onSelectFn = function ($item, $model, $label) { + scope.$item = $item; + scope.$model = $model; + }; + var el = compileTemplate( + ' \ + {{$select.selected.name}} \ + \ +
\ +
\ + \ + ' + ); + + expect(scope.$item).toBeFalsy(); + expect(scope.$model).toBeFalsy(); + + clickItem(el, 'Samantha'); + $timeout.flush(); + + expect(scope.selection.selected).toBe('Samantha'); + + expect(scope.$item).toEqual(scope.people[5]); + expect(scope.$model).toEqual('Samantha'); + + })); + it('should abort selection if before-select callback returns falsy', function () { scope.onBeforeSelectFn = function ($item, $model, $label) {