diff --git a/src/material-experimental/mdc-menu/menu-item.html b/src/material-experimental/mdc-menu/menu-item.html index 3092e914580e..e3833babd81c 100644 --- a/src/material-experimental/mdc-menu/menu-item.html +++ b/src/material-experimental/mdc-menu/menu-item.html @@ -1,4 +1,4 @@ - +
diff --git a/src/material-experimental/mdc-menu/menu.scss b/src/material-experimental/mdc-menu/menu.scss index ff4e5824dd0f..87b21757a5f3 100644 --- a/src/material-experimental/mdc-menu/menu.scss +++ b/src/material-experimental/mdc-menu/menu.scss @@ -55,12 +55,6 @@ text-decoration: none; &[disabled] { - // Usually every click inside the menu closes it, however some browsers will stop events - // when the user clicks on a disabled item, **except** when the user clicks on a non-disabled - // child node of the disabled button. This is inconsistent because some regions of a disabled - // button will still cause the menu to close and some won't (see #16694). We make the behavior - // more consistent by disabling pointer events and allowing the user to click through. - pointer-events: none; cursor: default; } @@ -86,6 +80,11 @@ } } +.mat-mdc-menu-item-content { + display: flex; + align-items: center; +} + // Renders out a chevron on menu items that trigger a sub-menu. .mat-mdc-menu-item-submenu-trigger { @include mat-menu-item-submenu-trigger($mdc-list-side-padding); diff --git a/src/material-experimental/mdc-menu/menu.spec.ts b/src/material-experimental/mdc-menu/menu.spec.ts index 59c316d51a4f..f1ba38e30075 100644 --- a/src/material-experimental/mdc-menu/menu.spec.ts +++ b/src/material-experimental/mdc-menu/menu.spec.ts @@ -569,6 +569,23 @@ describe('MDC-based MatMenu', () => { expect(items.every(item => item.getAttribute('role') === 'menuitem')).toBe(true); }); + it('should prevent the default action when clicking on a disabled item', () => { + const fixture = createComponent(SimpleMenu, [], [FakeIcon]); + fixture.detectChanges(); + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + + const item = overlayContainerElement.querySelector('.mat-mdc-menu-item[disabled]')!; + const itemEvent = dispatchFakeEvent(item, 'click'); + fixture.detectChanges(); + expect(itemEvent.defaultPrevented).toBe(true); + + const contentWrapper = item.querySelector('span')!; + const wrapperEvent = dispatchFakeEvent(contentWrapper, 'click'); + fixture.detectChanges(); + expect(wrapperEvent.defaultPrevented).toBe(true); + }); + it('should be able to set an alternate role on the menu items', () => { const fixture = createComponent(MenuWithCheckboxItems); fixture.detectChanges(); diff --git a/src/material/menu/menu-item.html b/src/material/menu/menu-item.html index a025ff57a01e..53a7b2c4d003 100644 --- a/src/material/menu/menu-item.html +++ b/src/material/menu/menu-item.html @@ -1,4 +1,4 @@ - +
diff --git a/src/material/menu/menu-item.ts b/src/material/menu/menu-item.ts index 1e7a913906fa..782294ef74c5 100644 --- a/src/material/menu/menu-item.ts +++ b/src/material/menu/menu-item.ts @@ -18,7 +18,9 @@ import { Optional, Input, HostListener, + NgZone, AfterViewInit, + ViewChild, } from '@angular/core'; import { CanDisable, CanDisableCtor, @@ -63,6 +65,9 @@ export class MatMenuItem extends _MatMenuItemMixinBase /** ARIA role for the menu item. */ @Input() role: 'menuitem' | 'menuitemradio' | 'menuitemcheckbox' = 'menuitem'; + /** Reference to the element wrapping the projected content. */ + @ViewChild('content') _content: ElementRef | undefined; + private _document: Document; /** Stream that emits when the menu item is hovered. */ @@ -81,9 +86,11 @@ export class MatMenuItem extends _MatMenuItemMixinBase private _elementRef: ElementRef, @Inject(DOCUMENT) document?: any, private _focusMonitor?: FocusMonitor, - @Inject(MAT_MENU_PANEL) @Optional() public _parentMenu?: MatMenuPanel) { + @Inject(MAT_MENU_PANEL) @Optional() public _parentMenu?: MatMenuPanel, + private _ngZone?: NgZone) { // @breaking-change 8.0.0 make `_focusMonitor` and `document` required params. + // @breaking-change 11.0.0 make `_ngZone` a required parameter. super(); if (_parentMenu && _parentMenu.addItem) { @@ -111,6 +118,13 @@ export class MatMenuItem extends _MatMenuItemMixinBase // mouse or touch interaction. this._focusMonitor.monitor(this._elementRef, false); } + + // @breaking-change 11.0.0 Remove null check for `_ngZone`. + if (this._ngZone) { + this._ngZone.runOutsideAngular(() => this._bindDisabledClickEvents()); + } else { + this._bindDisabledClickEvents(); + } } ngOnDestroy() { @@ -122,6 +136,11 @@ export class MatMenuItem extends _MatMenuItemMixinBase this._parentMenu.removeItem(this); } + this._elementRef.nativeElement.removeEventListener('click', this._preventDisabledClicks); + if (this._content) { + this._content.nativeElement.removeEventListener('click', this._preventDisabledClicks); + } + this._hovered.complete(); this._focused.complete(); } @@ -136,20 +155,6 @@ export class MatMenuItem extends _MatMenuItemMixinBase return this._elementRef.nativeElement; } - /** Prevents the default element actions if it is disabled. */ - // We have to use a `HostListener` here in order to support both Ivy and ViewEngine. - // In Ivy the `host` bindings will be merged when this class is extended, whereas in - // ViewEngine they're overwritten. - // TODO(crisbeto): we move this back into `host` once Ivy is turned on by default. - // tslint:disable-next-line:no-host-decorator-in-concrete - @HostListener('click', ['$event']) - _checkDisabled(event: Event): void { - if (this.disabled) { - event.preventDefault(); - event.stopPropagation(); - } - } - /** Emits to the hover stream. */ // We have to use a `HostListener` here in order to support both Ivy and ViewEngine. // In Ivy the `host` bindings will be merged when this class is extended, whereas in @@ -163,7 +168,8 @@ export class MatMenuItem extends _MatMenuItemMixinBase /** Gets the label to be used when determining whether the option should be focused. */ getLabel(): string { - const element: HTMLElement = this._elementRef.nativeElement; + const element: HTMLElement = this._content ? + this._content.nativeElement : this._elementRef.nativeElement; const textNodeType = this._document ? this._document.TEXT_NODE : 3; let output = ''; @@ -183,6 +189,26 @@ export class MatMenuItem extends _MatMenuItemMixinBase return output.trim(); } + /** Binds the click events that prevent the default actions while disabled. */ + private _bindDisabledClickEvents() { + // We need to bind this event both on the root node and the content wrapper, because browsers + // won't dispatch events on disabled `button` nodes, but they'll still be dispatched if the + // user interacts with a non-disabled child of the button. This means that can get regions + // inside a disabled menu item where clicks land and others where they don't. + this._elementRef.nativeElement.addEventListener('click', this._preventDisabledClicks); + if (this._content) { + this._content.nativeElement.addEventListener('click', this._preventDisabledClicks); + } + } + + /** Prevents the default click action if the menu item is disabled. */ + private _preventDisabledClicks = (event: Event) => { + if (this.disabled) { + event.preventDefault(); + event.stopPropagation(); + } + } + static ngAcceptInputType_disabled: BooleanInput; static ngAcceptInputType_disableRipple: BooleanInput; } diff --git a/src/material/menu/menu.scss b/src/material/menu/menu.scss index afcd241fc5ee..d737530018a8 100644 --- a/src/material/menu/menu.scss +++ b/src/material/menu/menu.scss @@ -44,15 +44,6 @@ $mat-menu-submenu-indicator-size: 10px !default; @include mat-menu-item-base(); position: relative; - &[disabled] { - // Usually every click inside the menu closes it, however some browsers will stop events - // when the user clicks on a disabled item, **except** when the user clicks on a non-disabled - // child node of the disabled button. This is inconsistent because some regions of a disabled - // button will still cause the menu to close and some won't (see #16694). We make the behavior - // more consistent by disabling pointer events and allowing the user to click through. - pointer-events: none; - } - @include cdk-high-contrast(active, off) { &.cdk-program-focused, &.cdk-keyboard-focused, diff --git a/src/material/menu/menu.spec.ts b/src/material/menu/menu.spec.ts index f5b77bb64bed..27cff0456a4f 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -604,6 +604,23 @@ describe('MatMenu', () => { expect(items.every(item => item.getAttribute('role') === 'menuitem')).toBe(true); }); + it('should prevent the default action when clicking on a disabled item', () => { + const fixture = createComponent(SimpleMenu, [], [FakeIcon]); + fixture.detectChanges(); + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + + const item = overlayContainerElement.querySelector('.mat-menu-item[disabled]')!; + const itemEvent = dispatchFakeEvent(item, 'click'); + fixture.detectChanges(); + expect(itemEvent.defaultPrevented).toBe(true); + + const contentWrapper = item.querySelector('span')!; + const wrapperEvent = dispatchFakeEvent(contentWrapper, 'click'); + fixture.detectChanges(); + expect(wrapperEvent.defaultPrevented).toBe(true); + }); + it('should be able to set an alternate role on the menu items', () => { const fixture = createComponent(MenuWithCheckboxItems); fixture.detectChanges(); diff --git a/tools/public_api_guard/material/menu.d.ts b/tools/public_api_guard/material/menu.d.ts index b4167e72f340..a4ee9430fc50 100644 --- a/tools/public_api_guard/material/menu.d.ts +++ b/tools/public_api_guard/material/menu.d.ts @@ -101,14 +101,14 @@ export interface MatMenuDefaultOptions { } export declare class MatMenuItem extends _MatMenuItemMixinBase implements FocusableOption, CanDisable, CanDisableRipple, AfterViewInit, OnDestroy { + _content: ElementRef | undefined; readonly _focused: Subject; _highlighted: boolean; readonly _hovered: Subject; _parentMenu?: MatMenuPanel | undefined; _triggersSubmenu: boolean; role: 'menuitem' | 'menuitemradio' | 'menuitemcheckbox'; - constructor(_elementRef: ElementRef, document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel | undefined); - _checkDisabled(event: Event): void; + constructor(_elementRef: ElementRef, document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel | undefined, _ngZone?: NgZone | undefined); _getHostElement(): HTMLElement; _getTabIndex(): string; _handleMouseEnter(): void; @@ -119,7 +119,7 @@ export declare class MatMenuItem extends _MatMenuItemMixinBase implements Focusa static ngAcceptInputType_disableRipple: BooleanInput; static ngAcceptInputType_disabled: BooleanInput; static ɵcmp: i0.ɵɵComponentDefWithMeta; - static ɵfac: i0.ɵɵFactoryDef; + static ɵfac: i0.ɵɵFactoryDef; } export declare class MatMenuModule {