From 6544b996cd7ebe85b5c836cc5d8edee4b3c31f3b Mon Sep 17 00:00:00 2001 From: Andy Chrzaszcz Date: Mon, 3 Aug 2020 09:17:30 -0400 Subject: [PATCH] refactor(cdk-experimental/menu): move listeners from host object to host listener decorator Some of the Cdk directives are expected to be extended by some subclasses (e.g. material). In Ivy the host metadata will be merged whereas in ViewEngine it will be overridden forcing duplication of listeners in the host object on the subclass which results in double listeners in Ivy. Moving to @HostListener prevents having to duplicate them in subclasses. --- src/cdk-experimental/menu/menu-bar.ts | 19 ++++++++++++++++--- .../menu/menu-item-checkbox.ts | 8 ++++++-- src/cdk-experimental/menu/menu-item-radio.ts | 17 +++++++++++++++-- src/cdk-experimental/menu/menu.ts | 7 ++++++- 4 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/cdk-experimental/menu/menu-bar.ts b/src/cdk-experimental/menu/menu-bar.ts index e414ff6937ee..6d6a6ea939b2 100644 --- a/src/cdk-experimental/menu/menu-bar.ts +++ b/src/cdk-experimental/menu/menu-bar.ts @@ -15,6 +15,7 @@ import { OnDestroy, Optional, NgZone, + HostListener, } from '@angular/core'; import {Directionality} from '@angular/cdk/bidi'; import {FocusKeyManager, FocusOrigin} from '@angular/cdk/a11y'; @@ -46,9 +47,6 @@ function isMenuElement(target: Element) { selector: '[cdkMenuBar]', exportAs: 'cdkMenuBar', host: { - '(keydown)': '_handleKeyEvent($event)', - '(document:click)': '_closeOnBackgroundClick($event)', - '(focus)': 'focusFirstItem()', 'role': 'menubar', 'class': 'cdk-menu-bar', 'tabindex': '0', @@ -100,6 +98,11 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit, this._subscribeToMouseManager(); } + // In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order + // to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we + // can move this back into `host`. + // tslint:disable:no-host-decorator-in-concrete + @HostListener('focus') /** Place focus on the first MenuItem in the menu and set the focus origin. */ focusFirstItem(focusOrigin: FocusOrigin = 'program') { this._keyManager.setFocusOrigin(focusOrigin); @@ -112,6 +115,11 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit, this._keyManager.setLastItemActive(); } + // In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order + // to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we + // can move this back into `host`. + // tslint:disable:no-host-decorator-in-concrete + @HostListener('keydown', ['$event']) /** * Handle keyboard events, specifically changing the focused element and/or toggling the active * items menu. @@ -248,6 +256,11 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit, return this.orientation === 'horizontal'; } + // In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order + // to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we + // can move this back into `host`. + // tslint:disable:no-host-decorator-in-concrete + @HostListener('document:click', ['$event']) /** Close any open submenu if there was a click event which occurred outside the menu stack. */ _closeOnBackgroundClick(event: MouseEvent) { if (this._hasOpenSubmenu()) { diff --git a/src/cdk-experimental/menu/menu-item-checkbox.ts b/src/cdk-experimental/menu/menu-item-checkbox.ts index 050b7e3774bc..26d17d468e3a 100644 --- a/src/cdk-experimental/menu/menu-item-checkbox.ts +++ b/src/cdk-experimental/menu/menu-item-checkbox.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive} from '@angular/core'; +import {Directive, HostListener} from '@angular/core'; import {CdkMenuItemSelectable} from './menu-item-selectable'; import {CdkMenuItem} from './menu-item'; @@ -18,7 +18,6 @@ import {CdkMenuItem} from './menu-item'; selector: '[cdkMenuItemCheckbox]', exportAs: 'cdkMenuItemCheckbox', host: { - '(click)': 'trigger()', 'type': 'button', 'role': 'menuitemcheckbox', '[attr.aria-checked]': 'checked || null', @@ -30,6 +29,11 @@ import {CdkMenuItem} from './menu-item'; ], }) export class CdkMenuItemCheckbox extends CdkMenuItemSelectable { + // In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order + // to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we + // can move this back into `host`. + // tslint:disable:no-host-decorator-in-concrete + @HostListener('click') trigger() { super.trigger(); diff --git a/src/cdk-experimental/menu/menu-item-radio.ts b/src/cdk-experimental/menu/menu-item-radio.ts index a3baf58d836e..79d345552cc3 100644 --- a/src/cdk-experimental/menu/menu-item-radio.ts +++ b/src/cdk-experimental/menu/menu-item-radio.ts @@ -6,7 +6,16 @@ * found in the LICENSE file at https://angular.io/license */ import {UniqueSelectionDispatcher} from '@angular/cdk/collections'; -import {Directive, OnDestroy, ElementRef, Self, Optional, Inject, NgZone} from '@angular/core'; +import { + Directive, + OnDestroy, + ElementRef, + Self, + Optional, + Inject, + NgZone, + HostListener, +} from '@angular/core'; import {Directionality} from '@angular/cdk/bidi'; import {CdkMenuItemSelectable} from './menu-item-selectable'; import {CdkMenuItem} from './menu-item'; @@ -22,7 +31,6 @@ import {CDK_MENU, Menu} from './menu-interface'; selector: '[cdkMenuItemRadio]', exportAs: 'cdkMenuItemRadio', host: { - '(click)': 'trigger()', 'type': 'button', 'role': 'menuitemradio', '[attr.aria-checked]': 'checked || null', @@ -60,6 +68,11 @@ export class CdkMenuItemRadio extends CdkMenuItemSelectable implements OnDestroy ); } + // In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order + // to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we + // can move this back into `host`. + // tslint:disable:no-host-decorator-in-concrete + @HostListener('click') /** Toggles the checked state of the radio-button. */ trigger() { super.trigger(); diff --git a/src/cdk-experimental/menu/menu.ts b/src/cdk-experimental/menu/menu.ts index 84589dc9641b..4b21f20d7e70 100644 --- a/src/cdk-experimental/menu/menu.ts +++ b/src/cdk-experimental/menu/menu.ts @@ -18,6 +18,7 @@ import { Optional, OnInit, NgZone, + HostListener, } from '@angular/core'; import {FocusKeyManager, FocusOrigin} from '@angular/cdk/a11y'; import { @@ -51,7 +52,6 @@ import {getItemPointerEntries} from './item-pointer-entries'; selector: '[cdkMenu]', exportAs: 'cdkMenu', host: { - '(keydown)': '_handleKeyEvent($event)', 'role': 'menu', 'class': 'cdk-menu', '[attr.aria-orientation]': 'orientation', @@ -136,6 +136,11 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI this._keyManager.setLastItemActive(); } + // In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order + // to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we + // can move this back into `host`. + // tslint:disable:no-host-decorator-in-concrete + @HostListener('keydown', ['$event']) /** Handle keyboard events for the Menu. */ _handleKeyEvent(event: KeyboardEvent) { const keyManager = this._keyManager;