Skip to content

Commit

Permalink
fix(dropdown): restore space bar and enter key item selection (#3926)
Browse files Browse the repository at this point in the history
  • Loading branch information
@emyarod @asudoh
emyarod authored and asudoh committed Sep 7, 2019
1 parent ba46431 commit fc22773
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 10 deletions.
33 changes: 27 additions & 6 deletions packages/components/src/components/dropdown/dropdown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,22 @@ class Dropdown extends mixin(
this.navigate(direction);
event.preventDefault(); // Prevents up/down keys from scrolling container
} else {
// get selected item
// in v10.0, the anchor elements fire click events on Enter keypress when a dropdown item is selected
// in v10.5 (#3586), focus is no longer placed on the dropdown items and is instead kept fixed on the ul menu
// so we need to manually call getCurrentNavigation and select the item
const item = this.getCurrentNavigation();
if (
item &&
isOpen &&
(event.which === 13 || event.which === 32) &&
!this.element.ownerDocument.activeElement.matches(
this.options.selectorItem
)
) {
event.preventDefault();
this.select(item);
}
this._toggle(event);
}
}
Expand Down Expand Up @@ -101,8 +117,8 @@ class Dropdown extends mixin(
// User presses down arrow
(event.which === 40 &&
!event.target.matches(this.options.selectorItem)) ||
// User presses space or enter and the trigger is not a button
(!triggerNode &&
// User presses space or enter and the trigger is not a button OR event is not fired by trigger
((!triggerNode || !triggerNode.contains(event.target)) &&
[13, 32].indexOf(event.which) >= 0 &&
!event.target.matches(this.options.selectorItem)) ||
// User presses esc
Expand Down Expand Up @@ -158,15 +174,20 @@ class Dropdown extends mixin(
}
} else if (changedState && (isOfSelf || actions.remove)) {
// toggled close
(triggerNode || this.element).focus();
// timer is used to call focus AFTER the click event on
// trigger button (which is caused by keypress e.g. during keyboard navigation)
setTimeout(() => (triggerNode || this.element).focus(), 0);
if (triggerNode) {
triggerNode.setAttribute('aria-expanded', 'false');
}
if (listNode) {
listNode.removeAttribute('aria-activedescendant');
this.element
.querySelector(this.options.selectorItemFocused)
.classList.remove(this.options.classFocused);
const focusedItem = this.element.querySelector(
this.options.selectorItemFocused
);
if (focusedItem) {
focusedItem.classList.remove(this.options.classFocused);
}
}
}

Expand Down
98 changes: 94 additions & 4 deletions packages/components/tests/spec/dropdown_spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { delay } from 'bluebird'; // For testing on browsers not supporting Promise
import EventManager from '../utils/event-manager';
import Dropdown from '../../src/components/dropdown/dropdown';

Expand Down Expand Up @@ -96,7 +97,7 @@ describe('Dropdown', function() {
expect(element.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

it('Should close dropdown with enter key', function() {
it('Should close dropdown with enter key', async function() {
spyOn(element, 'focus');
element.classList.add('bx--dropdown--open');
element.dispatchEvent(
Expand All @@ -106,6 +107,7 @@ describe('Dropdown', function() {
element.classList.contains('bx--dropdown--open'),
'Open state'
).toBe(false);
await delay(0);
expect(element.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

Expand All @@ -121,7 +123,7 @@ describe('Dropdown', function() {
expect(element.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

it('Should close dropdown with space key', function() {
it('Should close dropdown with space key', async function() {
spyOn(element, 'focus');
element.classList.add('bx--dropdown--open');
element.dispatchEvent(
Expand All @@ -131,6 +133,7 @@ describe('Dropdown', function() {
element.classList.contains('bx--dropdown--open'),
'Open state'
).toBe(false);
await delay(0);
expect(element.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

Expand All @@ -149,7 +152,7 @@ describe('Dropdown', function() {
expect(element.focus, 'Focus requested').not.toHaveBeenCalled();
});

it('Should close dropdown with ESC key', function() {
it('Should close dropdown with ESC key', async function() {
spyOn(element, 'focus');
element.classList.add('bx--dropdown--open');
element.dispatchEvent(
Expand All @@ -159,10 +162,11 @@ describe('Dropdown', function() {
element.classList.contains('bx--dropdown--open'),
'Open state'
).toBe(false);
await delay(0);
expect(element.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

it('Should close dropdown with ESC key on an item', function() {
it('Should close dropdown with ESC key on an item', async function() {
spyOn(element, 'focus');
element.classList.add('bx--dropdown--open');
itemNode.dispatchEvent(
Expand All @@ -174,6 +178,7 @@ describe('Dropdown', function() {
element.classList.contains('bx--dropdown--open'),
'Open state'
).toBe(false);
await delay(0);
expect(element.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

Expand Down Expand Up @@ -271,6 +276,91 @@ describe('Dropdown', function() {
expect(itemNode.classList.contains('bx--dropdown--focused')).toBe(false);
});

it('Should open dropdown with enter key', function() {
spyOn(list, 'focus');
element.dispatchEvent(
Object.assign(new CustomEvent('keydown'), { which: 13 })
);
expect(element.classList.contains('bx--dropdown--open')).toBe(true);
expect(list.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

it('Should close dropdown with enter key', async function() {
spyOn(trigger, 'focus');
element.classList.add('bx--dropdown--open');
element.dispatchEvent(
Object.assign(new CustomEvent('keydown'), { which: 13 })
);
expect(element.classList.contains('bx--dropdown--open')).toBe(false);
await delay(0);
expect(trigger.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

it('Should open dropdown with space key', function() {
spyOn(list, 'focus');
element.dispatchEvent(
Object.assign(new CustomEvent('keydown'), { which: 32 })
);
expect(element.classList.contains('bx--dropdown--open')).toBe(true);
expect(list.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

it('Should close dropdown with space key', async function() {
spyOn(trigger, 'focus');
element.classList.add('bx--dropdown--open');
element.dispatchEvent(
Object.assign(new CustomEvent('keydown'), { which: 32 })
);
expect(element.classList.contains('bx--dropdown--open')).toBe(false);
await delay(0);
expect(trigger.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

it('Should not close dropdown with space key on an item', function() {
spyOn(element, 'focus');
element.classList.add('bx--dropdown--open');
itemNode.dispatchEvent(
Object.assign(new CustomEvent('keydown', { bubbles: true }), {
which: 32,
})
);
expect(element.classList.contains('bx--dropdown--open')).toBe(true);
expect(element.focus, 'Focus requested').not.toHaveBeenCalled();
});

it('Should close dropdown with ESC key', async function() {
spyOn(trigger, 'focus');
element.classList.add('bx--dropdown--open');
element.dispatchEvent(
Object.assign(new CustomEvent('keydown'), { which: 27 })
);
expect(element.classList.contains('bx--dropdown--open')).toBe(false);
await delay(0);
expect(trigger.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

it('Should close dropdown with ESC key on an item', async function() {
spyOn(trigger, 'focus');
element.classList.add('bx--dropdown--open');
itemNode.dispatchEvent(
Object.assign(new CustomEvent('keydown', { bubbles: true }), {
which: 27,
})
);
expect(element.classList.contains('bx--dropdown--open')).toBe(false);
await delay(0);
expect(trigger.focus, 'Focus requested').toHaveBeenCalledTimes(1);
});

it('Should not open dropdown with ESC key', function() {
spyOn(element, 'focus');
element.dispatchEvent(
Object.assign(new CustomEvent('keydown'), { which: 27 })
);
expect(element.classList.contains('bx--dropdown--open')).toBe(false);
expect(element.focus, 'Focus requested').not.toHaveBeenCalled();
});

afterEach(function() {
element.classList.remove('bx--dropdown--disabled');
element.classList.remove('bx--dropdown--open');
Expand Down

0 comments on commit fc22773

Please sign in to comment.