Skip to content

Commit 3707aed

Browse files
maxfierkefernando-sendMail
maxfierke
authored and
fernando-sendMail
committed
fix(dropdown): Fix $digest:inprog on dropdown dismissal
Make $apply first check if $rootScope is in $digest cycle before executing Closes angular-ui#3274
1 parent 90a616a commit 3707aed

File tree

2 files changed

+5
-161
lines changed

2 files changed

+5
-161
lines changed

Diff for: ‎src/dropdown/dropdown.js

+3-50
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
1+
angular.module('ui.bootstrap.dropdown', [])
22

33
.constant('dropdownConfig', {
44
openClass: 'open'
@@ -33,18 +33,11 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
3333
// unbound this event handler. So check openScope before proceeding.
3434
if (!openScope) { return; }
3535

36-
if( evt && openScope.getAutoClose() === 'disabled' ) { return ; }
37-
3836
var toggleElement = openScope.getToggleElement();
3937
if ( evt && toggleElement && toggleElement[0].contains(evt.target) ) {
4038
return;
4139
}
4240

43-
var $element = openScope.getElement();
44-
if( evt && openScope.getAutoClose() === 'outsideClick' && $element && $element[0].contains(evt.target) ) {
45-
return;
46-
}
47-
4841
openScope.isOpen = false;
4942

5043
if (!$rootScope.$$phase) {
@@ -60,14 +53,13 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
6053
};
6154
}])
6255

63-
.controller('DropdownController', ['$scope', '$attrs', '$parse', 'dropdownConfig', 'dropdownService', '$animate', '$position', '$document', function($scope, $attrs, $parse, dropdownConfig, dropdownService, $animate, $position, $document) {
56+
.controller('DropdownController', ['$scope', '$attrs', '$parse', 'dropdownConfig', 'dropdownService', '$animate', function($scope, $attrs, $parse, dropdownConfig, dropdownService, $animate) {
6457
var self = this,
6558
scope = $scope.$new(), // create a child scope so we are not polluting original one
6659
openClass = dropdownConfig.openClass,
6760
getIsOpen,
6861
setIsOpen = angular.noop,
69-
toggleInvoker = $attrs.onToggle ? $parse($attrs.onToggle) : angular.noop,
70-
appendToBody = false;
62+
toggleInvoker = $attrs.onToggle ? $parse($attrs.onToggle) : angular.noop;
7163

7264
this.init = function( element ) {
7365
self.$element = element;
@@ -80,15 +72,6 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
8072
scope.isOpen = !!value;
8173
});
8274
}
83-
84-
appendToBody = angular.isDefined($attrs.dropdownAppendToBody);
85-
86-
if ( appendToBody && self.dropdownMenu ) {
87-
$document.find('body').append( self.dropdownMenu );
88-
element.on('$destroy', function handleDestroyEvent() {
89-
self.dropdownMenu.remove();
90-
});
91-
}
9275
};
9376

9477
this.toggle = function( open ) {
@@ -104,30 +87,13 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
10487
return self.toggleElement;
10588
};
10689

107-
scope.getAutoClose = function() {
108-
return $attrs.autoClose || 'always'; //or 'outsideClick' or 'disabled'
109-
};
110-
111-
scope.getElement = function() {
112-
return self.$element;
113-
};
114-
11590
scope.focusToggleElement = function() {
11691
if ( self.toggleElement ) {
11792
self.toggleElement[0].focus();
11893
}
11994
};
12095

12196
scope.$watch('isOpen', function( isOpen, wasOpen ) {
122-
if ( appendToBody && self.dropdownMenu ) {
123-
var pos = $position.positionElements(self.$element, self.dropdownMenu, 'bottom-left', true);
124-
self.dropdownMenu.css({
125-
top: pos.top + 'px',
126-
left: pos.left + 'px',
127-
display: isOpen ? 'block' : 'none'
128-
});
129-
}
130-
13197
$animate[isOpen ? 'addClass' : 'removeClass'](self.$element, openClass);
13298

13399
if ( isOpen ) {
@@ -161,19 +127,6 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
161127
};
162128
})
163129

164-
.directive('dropdownMenu', function() {
165-
return {
166-
restrict: 'AC',
167-
require: '?^dropdown',
168-
link: function(scope, element, attrs, dropdownCtrl) {
169-
if ( !dropdownCtrl ) {
170-
return;
171-
}
172-
dropdownCtrl.dropdownMenu = element;
173-
}
174-
};
175-
})
176-
177130
.directive('dropdownToggle', function() {
178131
return {
179132
require: '?^dropdown',

Diff for: ‎src/dropdown/test/dropdown.spec.js

+2-111
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@ describe('dropdownToggle', function() {
99
$document = _$document_;
1010
}));
1111

12-
afterEach(function() {
13-
element.remove();
14-
});
15-
1612
var clickDropdownToggle = function(elm) {
1713
elm = elm || element;
1814
elm.find('a[dropdown-toggle]').click();
@@ -54,6 +50,7 @@ describe('dropdownToggle', function() {
5450
var optionEl = element.find('ul > li').eq(0).find('a').eq(0);
5551
optionEl.click();
5652
expect(element.hasClass('open')).toBe(false);
53+
element.remove();
5754
});
5855

5956
it('should close on document click', function() {
@@ -69,6 +66,7 @@ describe('dropdownToggle', function() {
6966
triggerKeyDown($document, 27);
7067
expect(element.hasClass('open')).toBe(false);
7168
expect(isFocused(element.find('a'))).toBe(true);
69+
element.remove();
7270
});
7371

7472
it('should not close on backspace key', function() {
@@ -182,26 +180,6 @@ describe('dropdownToggle', function() {
182180
});
183181
});
184182

185-
describe('using dropdown-append-to-body', function() {
186-
function dropdown() {
187-
return $compile('<li dropdown dropdown-append-to-body><a href dropdown-toggle></a><ul class="dropdown-menu" id="dropdown-menu"><li><a href>Hello On Body</a></li></ul></li>')($rootScope);
188-
}
189-
190-
beforeEach(function() {
191-
element = dropdown();
192-
});
193-
194-
it('adds the menu to the body', function() {
195-
expect($document.find('#dropdown-menu').parent()[0]).toBe($document.find('body')[0]);
196-
});
197-
198-
it('removes the menu when the dropdown is removed', function() {
199-
element.remove();
200-
$rootScope.$digest();
201-
expect($document.find('#dropdown-menu').length).toEqual(0);
202-
});
203-
});
204-
205183
describe('integration with $location URL rewriting', function() {
206184
function dropdown() {
207185

@@ -345,91 +323,4 @@ describe('dropdownToggle', function() {
345323
expect($rootScope.toggleHandler).toHaveBeenCalledWith(false);
346324
});
347325
});
348-
349-
describe('`auto-close` option', function() {
350-
function dropdown(autoClose) {
351-
return $compile('<li dropdown ' +
352-
(autoClose === void 0 ? '' : 'auto-close="'+autoClose+'"') +
353-
'><a href dropdown-toggle></a><ul><li><a href>Hello</a></li></ul></li>')($rootScope);
354-
}
355-
356-
it('should close on document click if no auto-close is specified', function() {
357-
element = dropdown();
358-
clickDropdownToggle();
359-
expect(element.hasClass('open')).toBe(true);
360-
$document.click();
361-
expect(element.hasClass('open')).toBe(false);
362-
});
363-
364-
it('should close on document click if empty auto-close is specified', function() {
365-
element = dropdown('');
366-
clickDropdownToggle();
367-
expect(element.hasClass('open')).toBe(true);
368-
$document.click();
369-
expect(element.hasClass('open')).toBe(false);
370-
});
371-
372-
it('auto-close="disabled"', function() {
373-
element = dropdown('disabled');
374-
clickDropdownToggle();
375-
expect(element.hasClass('open')).toBe(true);
376-
$document.click();
377-
expect(element.hasClass('open')).toBe(true);
378-
});
379-
380-
it('auto-close="outsideClick"', function() {
381-
element = dropdown('outsideClick');
382-
clickDropdownToggle();
383-
expect(element.hasClass('open')).toBe(true);
384-
element.find('ul li a').click();
385-
expect(element.hasClass('open')).toBe(true);
386-
$document.click();
387-
expect(element.hasClass('open')).toBe(false);
388-
});
389-
390-
it('control with is-open', function() {
391-
$rootScope.isopen = true;
392-
element = $compile('<li dropdown is-open="isopen" auto-close="disabled"><a href dropdown-toggle></a><ul><li>Hello</li></ul></li>')($rootScope);
393-
$rootScope.$digest();
394-
395-
expect(element.hasClass('open')).toBe(true);
396-
//should remain open
397-
$document.click();
398-
expect(element.hasClass('open')).toBe(true);
399-
//now should close
400-
$rootScope.isopen = false;
401-
$rootScope.$digest();
402-
expect(element.hasClass('open')).toBe(false);
403-
});
404-
405-
it('should close anyway if toggle is clicked', function() {
406-
element = dropdown('disabled');
407-
clickDropdownToggle();
408-
expect(element.hasClass('open')).toBe(true);
409-
clickDropdownToggle();
410-
expect(element.hasClass('open')).toBe(false);
411-
});
412-
413-
it('should close anyway if esc is pressed', function() {
414-
element = dropdown('disabled');
415-
$document.find('body').append(element);
416-
clickDropdownToggle();
417-
triggerKeyDown($document, 27);
418-
expect(element.hasClass('open')).toBe(false);
419-
expect(isFocused(element.find('a'))).toBe(true);
420-
});
421-
422-
it('should close anyway if another dropdown is opened', function() {
423-
var elm1 = dropdown('disabled');
424-
var elm2 = dropdown();
425-
expect(elm1.hasClass('open')).toBe(false);
426-
expect(elm2.hasClass('open')).toBe(false);
427-
clickDropdownToggle(elm1);
428-
expect(elm1.hasClass('open')).toBe(true);
429-
expect(elm2.hasClass('open')).toBe(false);
430-
clickDropdownToggle(elm2);
431-
expect(elm1.hasClass('open')).toBe(false);
432-
expect(elm2.hasClass('open')).toBe(true);
433-
});
434-
});
435326
});

0 commit comments

Comments
 (0)