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 6132fabc54b6..0a068ef62be9 100644 --- a/src/material-experimental/mdc-menu/menu.scss +++ b/src/material-experimental/mdc-menu/menu.scss @@ -61,12 +61,6 @@ mat-menu { 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; } @@ -98,6 +92,11 @@ mat-menu { } } +.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 menu-common.item-submenu-trigger(mdc-list.$deprecated-side-padding); diff --git a/src/material-experimental/mdc-menu/menu.spec.ts b/src/material-experimental/mdc-menu/menu.spec.ts index 6705bfbc63b9..abf2c5a98b3a 100644 --- a/src/material-experimental/mdc-menu/menu.spec.ts +++ b/src/material-experimental/mdc-menu/menu.spec.ts @@ -590,6 +590,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 79df14589031..71ecb5aa8327 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; + /** Stream that emits when the menu item is hovered. */ readonly _hovered: Subject = new Subject(); @@ -83,9 +88,11 @@ export class MatMenuItem extends _MatMenuItemMixinBase */ @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 @@ -175,6 +180,26 @@ export class MatMenuItem extends _MatMenuItemMixinBase return clone.textContent?.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 a8c086cdd7ab..21e9685f5a95 100644 --- a/src/material/menu/menu.scss +++ b/src/material/menu/menu.scss @@ -49,15 +49,6 @@ mat-menu { @include menu-common.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 a11y.high-contrast(active, off) { $outline-width: 1px; diff --git a/src/material/menu/menu.spec.ts b/src/material/menu/menu.spec.ts index 72eb5c62abcc..e8faed120779 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -612,6 +612,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 c53ffb5d92d3..0f0f30836d1f 100644 --- a/tools/public_api_guard/material/menu.d.ts +++ b/tools/public_api_guard/material/menu.d.ts @@ -99,6 +99,7 @@ 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; @@ -106,8 +107,7 @@ export declare class MatMenuItem extends _MatMenuItemMixinBase implements Focusa _triggersSubmenu: boolean; role: 'menuitem' | 'menuitemradio' | 'menuitemcheckbox'; constructor(_elementRef: ElementRef, - _document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel | undefined); - _checkDisabled(event: Event): void; + _document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel | undefined, _ngZone?: NgZone | undefined); _getHostElement(): HTMLElement; _getTabIndex(): string; _handleMouseEnter(): void; @@ -118,7 +118,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 {