From 2f10304269479bbe9ebcc4eb54251cfea8de4ca1 Mon Sep 17 00:00:00 2001 From: Reico Cannella Date: Thu, 1 Jun 2017 15:47:55 +0200 Subject: [PATCH 1/2] test: testing if dropdown closes even when propagation of event stops --- test/select.spec.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/select.spec.js b/test/select.spec.js index a23c63f68..a1f66b218 100644 --- a/test/select.spec.js +++ b/test/select.spec.js @@ -740,6 +740,28 @@ describe('ui-select tests', function () { el2.remove(); }); + it('should close an opened select clicking outside with stopPropagation()', function () { + var el1 = createUiSelect(); + var el2 = $('
'); + el1.appendTo(document.body); + el2.appendTo(document.body); + + el2.on('click', function (e) { + e.stopPropagation() + }); + + expect(isDropdownOpened(el1)).toEqual(false); + clickMatch(el1); + expect(isDropdownOpened(el1)).toEqual(true); + + // Using a native dom click() to make sure the test fails when it should. + el2[0].click(); + + expect(isDropdownOpened(el1)).toEqual(false); + el1.remove(); + el2.remove(); + }); + it('should bind model correctly (with object as source)', function () { var el = compileTemplate( ' \ From 6f19d5fb738c75c960a6cfc4eda0bfa6a0769350 Mon Sep 17 00:00:00 2001 From: Reico Cannella Date: Thu, 1 Jun 2017 16:23:59 +0200 Subject: [PATCH 2/2] fix(drop-down): close drop-down regardless of `stopPropagation()` calls As of now, an event-handler on the `document` is responsible for closing the drop-down. A click-event stops bubbling when `stopPropagation()` is called, therefor it breaks the closing of the drop-down. Registering the event-listener in capture phase fixes this bug. --- src/uiSelectDirective.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/uiSelectDirective.js b/src/uiSelectDirective.js index 285cfb4ac..a1199269e 100644 --- a/src/uiSelectDirective.js +++ b/src/uiSelectDirective.js @@ -1,6 +1,6 @@ uis.directive('uiSelect', - ['$document', 'uiSelectConfig', 'uiSelectMinErr', 'uisOffset', '$compile', '$parse', '$timeout', - function($document, uiSelectConfig, uiSelectMinErr, uisOffset, $compile, $parse, $timeout) { + ['$document', 'uiSelectConfig', 'uiSelectMinErr', 'uisOffset', '$compile', '$parse', '$timeout', '$window', + function($document, uiSelectConfig, uiSelectMinErr, uisOffset, $compile, $parse, $timeout, $window) { return { restrict: 'EA', @@ -206,11 +206,15 @@ uis.directive('uiSelect', $select.clickTriggeredSelect = false; } - // See Click everywhere but here event http://stackoverflow.com/questions/12931369 - $document.on('click', onDocumentClick); + // See Click everywhere but here. Similar approach to http://stackoverflow.com/questions/12931369 + // but using the capture phase instead of bubble phase of the event propagation. + // + // Using the capture phase avoids problems that araise when event.stopPropatagion() + // is called before the event reaches the `document`. + $window.document.addEventListener('click', onDocumentClick, true); scope.$on('$destroy', function() { - $document.off('click', onDocumentClick); + $window.document.removeEventListener('click', onDocumentClick, true); }); // Move transcluded elements to their correct position in main template