From f845951edfebd1a4b392c9a89b5bd0fadb3380ed Mon Sep 17 00:00:00 2001 From: Max Okorokov Date: Fri, 7 Feb 2020 11:44:33 +0100 Subject: [PATCH] fix(dropdown): execute user (click) handlers on Enter and Space (#3573) Fixes #3570 --- e2e-app/src/app/app.module.ts | 2 + e2e-app/src/app/app.routing.ts | 5 ++- .../click/dropdown-click.component.ts | 32 ++++++++++++++ .../dropdown/click/dropdown-click.e2e-spec.ts | 43 +++++++++++++++++++ .../app/dropdown/click/dropdown-click.po.ts | 26 +++++++++++ src/dropdown/dropdown.ts | 15 ++++--- 6 files changed, 116 insertions(+), 7 deletions(-) create mode 100644 e2e-app/src/app/dropdown/click/dropdown-click.component.ts create mode 100644 e2e-app/src/app/dropdown/click/dropdown-click.e2e-spec.ts create mode 100644 e2e-app/src/app/dropdown/click/dropdown-click.po.ts diff --git a/e2e-app/src/app/app.module.ts b/e2e-app/src/app/app.module.ts index a6bf2b9d27..9555019d5a 100644 --- a/e2e-app/src/app/app.module.ts +++ b/e2e-app/src/app/app.module.ts @@ -12,6 +12,7 @@ import {DatepickerAutoCloseComponent} from './datepicker/autoclose/datepicker-au import {DatepickerFocusComponent} from './datepicker/focus/datepicker-focus.component'; import {DatepickerMultipleComponent} from './datepicker/multiple/datepicker-multiple.component'; import {DropdownAutoCloseComponent} from './dropdown/autoclose/dropdown-autoclose.component'; +import {DropdownClickComponent} from './dropdown/click/dropdown-click.component'; import {DropdownFocusComponent} from './dropdown/focus/dropdown-focus.component'; import {DropdownPositionComponent} from './dropdown/position/dropdown-position.component'; import {ModalAutoCloseComponent} from './modal/autoclose/modal-autoclose.component'; @@ -39,6 +40,7 @@ import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-val DatepickerMultipleComponent, DropdownAutoCloseComponent, DropdownFocusComponent, + DropdownClickComponent, DropdownPositionComponent, ModalAutoCloseComponent, ModalFocusComponent, diff --git a/e2e-app/src/app/app.routing.ts b/e2e-app/src/app/app.routing.ts index 6d867fc8a3..00bc6b1b13 100644 --- a/e2e-app/src/app/app.routing.ts +++ b/e2e-app/src/app/app.routing.ts @@ -5,6 +5,7 @@ import {DatepickerAutoCloseComponent} from './datepicker/autoclose/datepicker-au import {DatepickerFocusComponent} from './datepicker/focus/datepicker-focus.component'; import {DatepickerMultipleComponent} from './datepicker/multiple/datepicker-multiple.component'; import {DropdownAutoCloseComponent} from './dropdown/autoclose/dropdown-autoclose.component'; +import {DropdownClickComponent} from './dropdown/click/dropdown-click.component'; import {DropdownFocusComponent} from './dropdown/focus/dropdown-focus.component'; import {DropdownPositionComponent} from './dropdown/position/dropdown-position.component'; import {ModalAutoCloseComponent} from './modal/autoclose/modal-autoclose.component'; @@ -45,8 +46,8 @@ export const routes: Routes = [ { path: 'dropdown', children: [ - {path: 'autoclose', component: DropdownAutoCloseComponent}, {path: 'focus', component: DropdownFocusComponent}, - {path: 'position', component: DropdownPositionComponent} + {path: 'autoclose', component: DropdownAutoCloseComponent}, {path: 'click', component: DropdownClickComponent}, + {path: 'focus', component: DropdownFocusComponent}, {path: 'position', component: DropdownPositionComponent} ] }, {path: 'popover', children: [{path: 'autoclose', component: PopoverAutocloseComponent}]}, diff --git a/e2e-app/src/app/dropdown/click/dropdown-click.component.ts b/e2e-app/src/app/dropdown/click/dropdown-click.component.ts new file mode 100644 index 0000000000..5a0330a6bf --- /dev/null +++ b/e2e-app/src/app/dropdown/click/dropdown-click.component.ts @@ -0,0 +1,32 @@ +import {Component} from '@angular/core'; + +@Component({ + template: ` +
+
+ + +
+
+ +
+ Enter + +
+
+
+ + SPACE-CLICK + ENTER-CLICK + ENTER-KEY + SPACE-KEY +
+
+ ` +}) +export class DropdownClickComponent { + enterClick = false; + spaceClick = false; + enterKey = false; + spaceKey = false; +} diff --git a/e2e-app/src/app/dropdown/click/dropdown-click.e2e-spec.ts b/e2e-app/src/app/dropdown/click/dropdown-click.e2e-spec.ts new file mode 100644 index 0000000000..db64a8224e --- /dev/null +++ b/e2e-app/src/app/dropdown/click/dropdown-click.e2e-spec.ts @@ -0,0 +1,43 @@ +import {browser, ElementFinder, Key, protractor} from 'protractor'; +import {openUrl, sendKey} from '../../tools.po'; +import {DropdownClickPage} from './dropdown-click.po'; + +describe(`Dropdown user (click) handler`, () => { + + let page: DropdownClickPage; + let dropdown: ElementFinder; + + beforeAll(() => page = new DropdownClickPage()); + + beforeEach(async() => { + await openUrl('dropdown/click'); + dropdown = page.getDropdown(); + }); + + it(`should call user (click) handler on 'Enter'`, async() => { + await page.toggleItem(0); + + await sendKey(Key.ENTER); + expect(await page.isOpened(dropdown)).toBeFalsy(`Dropdown should be hidden on Enter`); + browser.wait( + protractor.ExpectedConditions.presenceOf(page.getEnterClickLabel()), 0, + `User click handler should have been called`); + browser.wait( + protractor.ExpectedConditions.presenceOf(page.getEnterKeyLabel()), 0, + `User keydown.enter handler should have been called`); + }); + + it(`should call user (click) handler on 'Space'`, async() => { + await page.toggleItem(1); + + await sendKey(Key.SPACE); + expect(await page.isOpened(dropdown)).toBeFalsy(`Dropdown should be hidden on Space`); + browser.wait( + protractor.ExpectedConditions.presenceOf(page.getSpaceClickLabel()), 0, + `User click handler should have been called`); + browser.wait( + protractor.ExpectedConditions.presenceOf(page.getSpaceKeyLabel()), 0, + `User keyup.space handler should have been called`); + }); + +}); diff --git a/e2e-app/src/app/dropdown/click/dropdown-click.po.ts b/e2e-app/src/app/dropdown/click/dropdown-click.po.ts new file mode 100644 index 0000000000..79c34dac10 --- /dev/null +++ b/e2e-app/src/app/dropdown/click/dropdown-click.po.ts @@ -0,0 +1,26 @@ +import {$, $$, Key} from 'protractor'; +import {DropdownPage} from '../dropdown.po'; +import {expectFocused, sendKey} from '../../tools.po'; + +export class DropdownClickPage extends DropdownPage { + async toggleItem(index: number) { + await $('#before').click(); + await sendKey(Key.TAB); + await expectFocused(this.getDropdownToggle(), `dropdown should be focused`); + for (let i = 0; i <= index; ++i) { + await sendKey(Key.ARROW_DOWN); + } + await sendKey(Key.TAB); + await expectFocused(this.getDropdownItem(index), `Item should be focused`); + } + + getDropdownItem(index: number) { return $$(`[ngbDropdownItem]`).get(index); } + + getSpaceClickLabel() { return $('#space-click'); } + + getEnterClickLabel() { return $('#enter-click'); } + + getEnterKeyLabel() { return $('#enter-key'); } + + getSpaceKeyLabel() { return $('#space-key'); } +} diff --git a/src/dropdown/dropdown.ts b/src/dropdown/dropdown.ts index 1e9633e582..4e7706260b 100644 --- a/src/dropdown/dropdown.ts +++ b/src/dropdown/dropdown.ts @@ -18,7 +18,7 @@ import { Optional } from '@angular/core'; import {DOCUMENT} from '@angular/common'; -import {Subject, Subscription} from 'rxjs'; +import {fromEvent, Subject, Subscription} from 'rxjs'; import {take} from 'rxjs/operators'; import {Placement, PlacementArray, positionElements} from '../util/positioning'; @@ -290,14 +290,16 @@ export class NgbDropdown implements AfterContentInit, OnDestroy { let position = -1; let isEventFromItems = false; + let itemElement: HTMLElement = null; const isEventFromToggle = this._isEventFromToggle(event); if (!isEventFromToggle && itemElements.length) { - itemElements.forEach((itemElement, index) => { - if (itemElement.contains(event.target as HTMLElement)) { + itemElements.forEach((item, index) => { + if (item.contains(event.target as HTMLElement)) { isEventFromItems = true; + itemElement = item; } - if (itemElement === this._document.activeElement) { + if (item === this._document.activeElement) { position = index; } }); @@ -306,7 +308,10 @@ export class NgbDropdown implements AfterContentInit, OnDestroy { // closing on Enter / Space if (key === Key.Space || key === Key.Enter) { if (isEventFromItems && (this.autoClose === true || this.autoClose === 'inside')) { - this.close(); + // Item is either a button or a link, so click will be triggered by the browser on Enter or Space. + // So we have to register a one-time click handler that will fire after any user defined click handlers + // to close the dropdown + fromEvent(itemElement, 'click').pipe(take(1)).subscribe(() => this.close()); } return; }