From 3d9089fac25b8674ee635c6167ae64e1bccc03b0 Mon Sep 17 00:00:00 2001 From: Max Fierke Date: Tue, 24 Mar 2015 16:30:40 -0500 Subject: [PATCH] fix(dropdown): Fix $digest:inprog on dropdown dismissal Make $apply first check if $rootScope is in $digest cycle before executing Closes #3274 --- src/dropdown/dropdown.js | 53 +------------- src/dropdown/test/dropdown.spec.js | 114 +---------------------------- 2 files changed, 6 insertions(+), 161 deletions(-) diff --git a/src/dropdown/dropdown.js b/src/dropdown/dropdown.js index 8f03186160..14c255a13b 100644 --- a/src/dropdown/dropdown.js +++ b/src/dropdown/dropdown.js @@ -1,4 +1,4 @@ -angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position']) +angular.module('ui.bootstrap.dropdown', []) .constant('dropdownConfig', { openClass: 'open' @@ -33,18 +33,11 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position']) // unbound this event handler. So check openScope before proceeding. if (!openScope) { return; } - if( evt && openScope.getAutoClose() === 'disabled' ) { return ; } - var toggleElement = openScope.getToggleElement(); if ( evt && toggleElement && toggleElement[0].contains(evt.target) ) { return; } - var $element = openScope.getElement(); - if( evt && openScope.getAutoClose() === 'outsideClick' && $element && $element[0].contains(evt.target) ) { - return; - } - openScope.isOpen = false; if (!$rootScope.$$phase) { @@ -60,14 +53,13 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position']) }; }]) -.controller('DropdownController', ['$scope', '$attrs', '$parse', 'dropdownConfig', 'dropdownService', '$animate', '$position', '$document', function($scope, $attrs, $parse, dropdownConfig, dropdownService, $animate, $position, $document) { +.controller('DropdownController', ['$scope', '$attrs', '$parse', 'dropdownConfig', 'dropdownService', '$animate', function($scope, $attrs, $parse, dropdownConfig, dropdownService, $animate) { var self = this, scope = $scope.$new(), // create a child scope so we are not polluting original one openClass = dropdownConfig.openClass, getIsOpen, setIsOpen = angular.noop, - toggleInvoker = $attrs.onToggle ? $parse($attrs.onToggle) : angular.noop, - appendToBody = false; + toggleInvoker = $attrs.onToggle ? $parse($attrs.onToggle) : angular.noop; this.init = function( element ) { self.$element = element; @@ -80,15 +72,6 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position']) scope.isOpen = !!value; }); } - - appendToBody = angular.isDefined($attrs.dropdownAppendToBody); - - if ( appendToBody && self.dropdownMenu ) { - $document.find('body').append( self.dropdownMenu ); - element.on('$destroy', function handleDestroyEvent() { - self.dropdownMenu.remove(); - }); - } }; this.toggle = function( open ) { @@ -104,14 +87,6 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position']) return self.toggleElement; }; - scope.getAutoClose = function() { - return $attrs.autoClose || 'always'; //or 'outsideClick' or 'disabled' - }; - - scope.getElement = function() { - return self.$element; - }; - scope.focusToggleElement = function() { if ( self.toggleElement ) { self.toggleElement[0].focus(); @@ -119,15 +94,6 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position']) }; scope.$watch('isOpen', function( isOpen, wasOpen ) { - if ( appendToBody && self.dropdownMenu ) { - var pos = $position.positionElements(self.$element, self.dropdownMenu, 'bottom-left', true); - self.dropdownMenu.css({ - top: pos.top + 'px', - left: pos.left + 'px', - display: isOpen ? 'block' : 'none' - }); - } - $animate[isOpen ? 'addClass' : 'removeClass'](self.$element, openClass); if ( isOpen ) { @@ -161,19 +127,6 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position']) }; }) -.directive('dropdownMenu', function() { - return { - restrict: 'AC', - require: '?^dropdown', - link: function(scope, element, attrs, dropdownCtrl) { - if ( !dropdownCtrl ) { - return; - } - dropdownCtrl.dropdownMenu = element; - } - }; -}) - .directive('dropdownToggle', function() { return { require: '?^dropdown', diff --git a/src/dropdown/test/dropdown.spec.js b/src/dropdown/test/dropdown.spec.js index 8438aca550..c1c46a85ee 100644 --- a/src/dropdown/test/dropdown.spec.js +++ b/src/dropdown/test/dropdown.spec.js @@ -9,10 +9,6 @@ describe('dropdownToggle', function() { $document = _$document_; })); - afterEach(function() { - element.remove(); - }); - var clickDropdownToggle = function(elm) { elm = elm || element; elm.find('a[dropdown-toggle]').click(); @@ -54,6 +50,7 @@ describe('dropdownToggle', function() { var optionEl = element.find('ul > li').eq(0).find('a').eq(0); optionEl.click(); expect(element.hasClass('open')).toBe(false); + element.remove(); }); it('should close on document click', function() { @@ -69,6 +66,7 @@ describe('dropdownToggle', function() { triggerKeyDown($document, 27); expect(element.hasClass('open')).toBe(false); expect(isFocused(element.find('a'))).toBe(true); + element.remove(); }); it('should not close on backspace key', function() { @@ -182,26 +180,6 @@ describe('dropdownToggle', function() { }); }); - describe('using dropdown-append-to-body', function() { - function dropdown() { - return $compile('
    • ')($rootScope); - } - - beforeEach(function() { - element = dropdown(); - }); - - it('adds the menu to the body', function() { - expect($document.find('#dropdown-menu').parent()[0]).toBe($document.find('body')[0]); - }); - - it('removes the menu when the dropdown is removed', function() { - element.remove(); - $rootScope.$digest(); - expect($document.find('#dropdown-menu').length).toEqual(0); - }); - }); - describe('integration with $location URL rewriting', function() { function dropdown() { @@ -278,6 +256,7 @@ describe('dropdownToggle', function() { $rootScope.isopen = true; $rootScope.$digest(); expect(isFocused(element.find('a'))).toBe(true); + element.remove(); }); }); @@ -345,91 +324,4 @@ describe('dropdownToggle', function() { expect($rootScope.toggleHandler).toHaveBeenCalledWith(false); }); }); - - describe('`auto-close` option', function() { - function dropdown(autoClose) { - return $compile('
    • ')($rootScope); - } - - it('should close on document click if no auto-close is specified', function() { - element = dropdown(); - clickDropdownToggle(); - expect(element.hasClass('open')).toBe(true); - $document.click(); - expect(element.hasClass('open')).toBe(false); - }); - - it('should close on document click if empty auto-close is specified', function() { - element = dropdown(''); - clickDropdownToggle(); - expect(element.hasClass('open')).toBe(true); - $document.click(); - expect(element.hasClass('open')).toBe(false); - }); - - it('auto-close="disabled"', function() { - element = dropdown('disabled'); - clickDropdownToggle(); - expect(element.hasClass('open')).toBe(true); - $document.click(); - expect(element.hasClass('open')).toBe(true); - }); - - it('auto-close="outsideClick"', function() { - element = dropdown('outsideClick'); - clickDropdownToggle(); - expect(element.hasClass('open')).toBe(true); - element.find('ul li a').click(); - expect(element.hasClass('open')).toBe(true); - $document.click(); - expect(element.hasClass('open')).toBe(false); - }); - - it('control with is-open', function() { - $rootScope.isopen = true; - element = $compile('
    • ')($rootScope); - $rootScope.$digest(); - - expect(element.hasClass('open')).toBe(true); - //should remain open - $document.click(); - expect(element.hasClass('open')).toBe(true); - //now should close - $rootScope.isopen = false; - $rootScope.$digest(); - expect(element.hasClass('open')).toBe(false); - }); - - it('should close anyway if toggle is clicked', function() { - element = dropdown('disabled'); - clickDropdownToggle(); - expect(element.hasClass('open')).toBe(true); - clickDropdownToggle(); - expect(element.hasClass('open')).toBe(false); - }); - - it('should close anyway if esc is pressed', function() { - element = dropdown('disabled'); - $document.find('body').append(element); - clickDropdownToggle(); - triggerKeyDown($document, 27); - expect(element.hasClass('open')).toBe(false); - expect(isFocused(element.find('a'))).toBe(true); - }); - - it('should close anyway if another dropdown is opened', function() { - var elm1 = dropdown('disabled'); - var elm2 = dropdown(); - expect(elm1.hasClass('open')).toBe(false); - expect(elm2.hasClass('open')).toBe(false); - clickDropdownToggle(elm1); - expect(elm1.hasClass('open')).toBe(true); - expect(elm2.hasClass('open')).toBe(false); - clickDropdownToggle(elm2); - expect(elm1.hasClass('open')).toBe(false); - expect(elm2.hasClass('open')).toBe(true); - }); - }); });