Skip to content

Commit

Permalink
fix(dropdown): stop esc keydown event
Browse files Browse the repository at this point in the history 
Fixes angular-ui#5778
Closes angular-ui#5787

BREAKING CHANGE: Stops propagation of keydown event when escape key is pressed. Removes keydown event from the document and moves it to the dropdown element.
  • Loading branch information
@deeg
deeg committed Apr 12, 2016
1 parent bba3f27 commit c78b734
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 27 deletions.
13 changes: 7 additions & 6 deletions src/dropdown/dropdown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
.service('uibDropdownService', ['$document', '$rootScope', function($document, $rootScope) {
var openScope = null;

this.open = function(dropdownScope) {
this.open = function(dropdownScope, element) {
if (!openScope) {
$document.on('click', closeDropdown);
$document.on('keydown', keybindFilter);
element.on('keydown', keybindFilter);
}

if (openScope && openScope !== dropdownScope) {
Expand All @@ -21,11 +21,11 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
openScope = dropdownScope;
};

this.close = function(dropdownScope) {
this.close = function(dropdownScope, element) {
if (openScope === dropdownScope) {
openScope = null;
$document.off('click', closeDropdown);
$document.off('keydown', keybindFilter);
element.off('keydown', keybindFilter);
}
};

Expand Down Expand Up @@ -58,6 +58,7 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])

var keybindFilter = function(evt) {
if (evt.which === 27) {
evt.stopPropagation();
openScope.focusToggleElement();
closeDropdown();
} else if (openScope.isKeynavEnabled() && [38, 40].indexOf(evt.which) !== -1 && openScope.isOpen) {
Expand Down Expand Up @@ -249,7 +250,7 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
}

scope.focusToggleElement();
uibDropdownService.open(scope);
uibDropdownService.open(scope, $element);
} else {
if (self.dropdownMenuTemplateUrl) {
if (templateScope) {
Expand All @@ -260,7 +261,7 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.position'])
self.dropdownMenu = newEl;
}

uibDropdownService.close(scope);
uibDropdownService.close(scope, $element);
self.selectedOption = null;
}

Expand Down
46 changes: 25 additions & 21 deletions src/dropdown/test/dropdown.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ describe('uib-dropdown', function() {

var triggerKeyDown = function (element, keyCode) {
var e = $.Event('keydown');
spyOn(e, "stopPropagation");
e.which = keyCode;
element.trigger(e);
return e;
};

describe('basic', function() {
Expand Down Expand Up @@ -68,14 +70,15 @@ describe('uib-dropdown', function() {
it('should close on escape key & focus toggle element', function() {
$document.find('body').append(element);
clickDropdownToggle();
triggerKeyDown($document, 27);
var event = triggerKeyDown(element, 27);
expect(element).not.toHaveClass(dropdownConfig.openClass);
expect(element.find('a')).toHaveFocus();
expect(event.stopPropagation).toHaveBeenCalled();
});

it('should not close on backspace key', function() {
clickDropdownToggle();
triggerKeyDown($document, 8);
triggerKeyDown(element, 8);
expect(element).toHaveClass(dropdownConfig.openClass);
});

Expand Down Expand Up @@ -470,7 +473,7 @@ describe('uib-dropdown', function() {
element = dropdown('disabled');
$document.find('body').append(element);
clickDropdownToggle();
triggerKeyDown($document, 27);
triggerKeyDown(element, 27);
expect(element).not.toHaveClass(dropdownConfig.openClass);
expect(element.find('a')).toHaveFocus();
});
Expand Down Expand Up @@ -524,7 +527,7 @@ describe('uib-dropdown', function() {
it('should focus first list element when down arrow pressed', function() {
$document.find('body').append(element);
clickDropdownToggle();
triggerKeyDown($document, 40);
triggerKeyDown(element, 40);

expect(element).toHaveClass(dropdownConfig.openClass);
var optionEl = element.find('ul').eq(0).find('a').eq(0);
Expand All @@ -533,7 +536,7 @@ describe('uib-dropdown', function() {

it('should not focus first list element when down arrow pressed if closed', function() {
$document.find('body').append(element);
triggerKeyDown($document, 40);
triggerKeyDown(element, 40);

expect(element).not.toHaveClass(dropdownConfig.openClass);
var focusEl = element.find('ul').eq(0).find('a').eq(0);
Expand All @@ -543,8 +546,8 @@ describe('uib-dropdown', function() {
it('should focus second list element when down arrow pressed twice', function() {
$document.find('body').append(element);
clickDropdownToggle();
triggerKeyDown($document, 40);
triggerKeyDown($document, 40);
triggerKeyDown(element, 40);
triggerKeyDown(element, 40);

expect(element).toHaveClass(dropdownConfig.openClass);
var focusEl = element.find('ul').eq(0).find('a').eq(1);
Expand All @@ -556,15 +559,15 @@ describe('uib-dropdown', function() {
clickDropdownToggle();
expect(element).toHaveClass(dropdownConfig.openClass);

triggerKeyDown($document, 38);
triggerKeyDown(element, 38);
var focusEl = element.find('ul').eq(0).find('a').eq(0);
expect(focusEl).not.toHaveFocus();
});

it('should focus last list element when up arrow pressed after dropdown toggled', function() {
$document.find('body').append(element);
clickDropdownToggle();
triggerKeyDown($document, 38);
triggerKeyDown(element, 38);

expect(element).toHaveClass(dropdownConfig.openClass);
var focusEl = element.find('ul').eq(0).find('a').eq(1);
Expand All @@ -574,7 +577,7 @@ describe('uib-dropdown', function() {
it('should not change focus when other keys are pressed', function() {
$document.find('body').append(element);
clickDropdownToggle();
triggerKeyDown($document, 37);
triggerKeyDown(element, 37);

expect(element).toHaveClass(dropdownConfig.openClass);
var focusEl = element.find('ul').eq(0).find('a');
Expand All @@ -585,10 +588,10 @@ describe('uib-dropdown', function() {
it('should focus first list element when down arrow pressed 2x and up pressed 1x', function() {
$document.find('body').append(element);
clickDropdownToggle();
triggerKeyDown($document, 40);
triggerKeyDown($document, 40);
triggerKeyDown(element, 40);
triggerKeyDown(element, 40);

triggerKeyDown($document, 38);
triggerKeyDown(element, 38);

expect(element).toHaveClass(dropdownConfig.openClass);
var focusEl = element.find('ul').eq(0).find('a').eq(0);
Expand All @@ -598,14 +601,14 @@ describe('uib-dropdown', function() {
it('should stay focused on final list element if down pressed at list end', function() {
$document.find('body').append(element);
clickDropdownToggle();
triggerKeyDown($document, 40);
triggerKeyDown($document, 40);
triggerKeyDown(element, 40);
triggerKeyDown(element, 40);

expect(element).toHaveClass(dropdownConfig.openClass);
var focusEl = element.find('ul').eq(0).find('a').eq(1);
expect(focusEl).toHaveFocus();

triggerKeyDown($document, 40);
triggerKeyDown(element, 40);
expect(focusEl).toHaveFocus();
});

Expand All @@ -614,13 +617,13 @@ describe('uib-dropdown', function() {
$document.find('body').append(element);
clickDropdownToggle();

triggerKeyDown($document, 40);
triggerKeyDown(element, 40);

expect(element).toHaveClass(dropdownConfig.openClass);
var focusEl = element.find('ul').eq(0).find('a').eq(0);
expect(focusEl).toHaveFocus();

triggerKeyDown($document, 27);
triggerKeyDown(element, 27);
expect(element).not.toHaveClass(dropdownConfig.openClass);
});

Expand All @@ -636,7 +639,7 @@ describe('uib-dropdown', function() {
it('should focus first list element when down arrow pressed', function() {
clickDropdownToggle();

triggerKeyDown($document, 40);
triggerKeyDown(element, 40);

var dropdownMenu = $document.find('#dropdown-menu');

Expand All @@ -647,8 +650,9 @@ describe('uib-dropdown', function() {

it('should focus second list element when down arrow pressed twice', function() {
clickDropdownToggle();
triggerKeyDown($document, 40);
triggerKeyDown($document, 40);
triggerKeyDown(element, 40);
triggerKeyDown(element, 40);
triggerKeyDown(element, 40);

var dropdownMenu = $document.find('#dropdown-menu');

Expand Down

0 comments on commit c78b734

Please sign in to comment.