Skip to content

Commit

Permalink
fix(dropdown): Fix $digest:inprog on dropdown dismissal
Browse files Browse the repository at this point in the history 
Make $apply first check if $rootScope is in $digest cycle before executing

Closes angular-ui#3274
  • Loading branch information
@maxfierke @fernando-sendMail
maxfierke authored and fernando-sendMail committed Jul 16, 2015
1 parent 0e2eb38 commit 5e9cf63
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 161 deletions.
53 changes: 3 additions & 50 deletions src/dropdown/dropdown.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
angular.module('ui.bootstrap.dropdown', [])

.constant('dropdownConfig', {
openClass: 'open'
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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 ) {
Expand All @@ -104,30 +87,13 @@ 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();
}
};

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 ) {
Expand Down Expand Up @@ -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',
Expand Down
114 changes: 3 additions & 111 deletions src/dropdown/test/dropdown.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -182,26 +180,6 @@ describe('dropdownToggle', function() {
});
});

describe('using dropdown-append-to-body', function() {
function dropdown() {
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);
}

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() {

Expand Down Expand Up @@ -278,6 +256,7 @@ describe('dropdownToggle', function() {
$rootScope.isopen = true;
$rootScope.$digest();
expect(isFocused(element.find('a'))).toBe(true);
element.remove();
});
});

Expand Down Expand Up @@ -345,91 +324,4 @@ describe('dropdownToggle', function() {
expect($rootScope.toggleHandler).toHaveBeenCalledWith(false);
});
});

describe('`auto-close` option', function() {
function dropdown(autoClose) {
return $compile('<li dropdown ' +
(autoClose === void 0 ? '' : 'auto-close="'+autoClose+'"') +
'><a href dropdown-toggle></a><ul><li><a href>Hello</a></li></ul></li>')($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('<li dropdown is-open="isopen" auto-close="disabled"><a href dropdown-toggle></a><ul><li>Hello</li></ul></li>')($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);
});
});
});

0 comments on commit 5e9cf63

Please sign in to comment.