fix(dropdown): execute user (click) handlers on Enter and Space (#3573)

Fixes #3570

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,

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}]},

e2e-app/src/app/dropdown/click/dropdown-click.component.ts

@@ -0,0 +1,32 @@
+import {Component} from '@angular/core';
+
+@Component({
+  template: `
+    <form>
+      <div class="form-group form-inline">
+        <input type="text" class="form-control mr-1" style="width: 100px" placeholder="before" id="before"/>
+
+        <div class="input-group">
+          <div ngbDropdown class="d-inline-block mr-3">
+            <button class="btn btn-outline-secondary" id="dropdown" ngbDropdownToggle>Choose one</button>
+            <div ngbDropdownMenu aria-labelledby="dropdown">
+              <a href ngbDropdownItem (keydown.enter)="enterKey = true" (click)="enterClick = true; $event.preventDefault()">Enter</a>
+              <button ngbDropdownItem (keyup.space)="spaceKey = true" (click)="spaceClick = true">Space</button>
+            </div>
+          </div>
+        </div>
+
+        <span id="space-click" *ngIf="spaceClick" class="ml-3">SPACE-CLICK</span>
+        <span id="enter-click" *ngIf="enterClick" class="ml-3">ENTER-CLICK</span>
+        <span id="enter-key" *ngIf="enterKey" class="ml-3">ENTER-KEY</span>
+        <span id="space-key" *ngIf="spaceKey" class="ml-3">SPACE-KEY</span>
+      </div>
+    </form>
+  `
+})
+export class DropdownClickComponent {
+  enterClick = false;
+  spaceClick = false;
+  enterKey = false;
+  spaceKey = false;
+}

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`);
+  });
+
+});

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'); }
+}

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;
     }