diff --git a/src/material-experimental/mdc-menu/menu-item.html b/src/material-experimental/mdc-menu/menu-item.html index 7e5412addf51..67a9f3ff0304 100644 --- a/src/material-experimental/mdc-menu/menu-item.html +++ b/src/material-experimental/mdc-menu/menu-item.html @@ -1,5 +1,5 @@ - +
diff --git a/src/material-experimental/mdc-menu/menu.scss b/src/material-experimental/mdc-menu/menu.scss index da8a1a353ce3..f1665bde644e 100644 --- a/src/material-experimental/mdc-menu/menu.scss +++ b/src/material-experimental/mdc-menu/menu.scss @@ -60,12 +60,6 @@ mat-menu { min-height: map.get($height-config, default); &[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; // This is the same as `mdc-list-mixins.list-disabled-opacity` which @@ -107,6 +101,11 @@ mat-menu { } } +.mat-mdc-menu-item-content { + display: flex; + align-items: center; +} + .mat-mdc-menu-item-submenu-trigger { @include menu-common.item-submenu-trigger(mdc-list-variables.$side-padding); } diff --git a/src/material-experimental/mdc-menu/menu.spec.ts b/src/material-experimental/mdc-menu/menu.spec.ts index bd8ed69d12f0..da3a5ae90c47 100644 --- a/src/material-experimental/mdc-menu/menu.spec.ts +++ b/src/material-experimental/mdc-menu/menu.spec.ts @@ -645,6 +645,23 @@ describe('MDC-based MatMenu', () => { expect(items.every(item => item.getAttribute('role') === 'menuitemcheckbox')).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 not change focus origin if origin not specified for menu items', fakeAsync(() => { const fixture = createComponent(MenuWithCheckboxItems); fixture.detectChanges(); diff --git a/src/material/menu/menu-item.html b/src/material/menu/menu-item.html index 8073c518f777..7875eff79295 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 63bb1ce25852..4cb56684fe9d 100644 --- a/src/material/menu/menu-item.ts +++ b/src/material/menu/menu-item.ts @@ -16,8 +16,10 @@ import { Inject, Optional, Input, + NgZone, AfterViewInit, ChangeDetectorRef, + ViewChild, } from '@angular/core'; import { CanDisable, @@ -63,6 +65,9 @@ export class MatMenuItem /** 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(); @@ -84,6 +89,7 @@ export class MatMenuItem @Inject(DOCUMENT) _document?: any, private _focusMonitor?: FocusMonitor, @Inject(MAT_MENU_PANEL) @Optional() public _parentMenu?: MatMenuPanel, + private _ngZone?: NgZone, /** * @deprecated `_changeDetectorRef` to become a required parameter. * @breaking-change 14.0.0 @@ -91,6 +97,7 @@ export class MatMenuItem private _changeDetectorRef?: ChangeDetectorRef, ) { // @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) { @@ -116,6 +123,13 @@ export class MatMenuItem // 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() { @@ -127,6 +141,11 @@ export class MatMenuItem 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(); } @@ -141,14 +160,6 @@ export class MatMenuItem return this._elementRef.nativeElement; } - /** Prevents the default element actions if it is disabled. */ - _checkDisabled(event: Event): void { - if (this.disabled) { - event.preventDefault(); - event.stopPropagation(); - } - } - /** Emits to the hover stream. */ _handleMouseEnter() { this._hovered.next(this); @@ -175,4 +186,24 @@ export class MatMenuItem this._highlighted = isHighlighted; this._changeDetectorRef?.markForCheck(); } + + /** 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(); + } + }; } diff --git a/src/material/menu/menu.scss b/src/material/menu/menu.scss index 353986269014..afb246446e69 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 0848e63c9a29..eed23f367855 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -644,6 +644,23 @@ describe('MatMenu', () => { expect(items.every(item => item.getAttribute('role') === 'menuitemcheckbox')).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 not change focus origin if origin not specified for menu items', fakeAsync(() => { const fixture = createComponent(MenuWithCheckboxItems); fixture.detectChanges(); diff --git a/tools/public_api_guard/material/menu.md b/tools/public_api_guard/material/menu.md index 1eccbea708cc..5f6698eb8d73 100644 --- a/tools/public_api_guard/material/menu.md +++ b/tools/public_api_guard/material/menu.md @@ -192,9 +192,9 @@ export interface MatMenuDefaultOptions { // @public export class MatMenuItem extends _MatMenuItemBase implements FocusableOption, CanDisable, CanDisableRipple, AfterViewInit, OnDestroy { constructor(_elementRef: ElementRef, - _document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel | undefined, + _document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel | undefined, _ngZone?: NgZone | undefined, _changeDetectorRef?: ChangeDetectorRef | undefined); - _checkDisabled(event: Event): void; + _content: ElementRef | undefined; focus(origin?: FocusOrigin, options?: FocusOptions): void; readonly _focused: Subject; _getHostElement(): HTMLElement; @@ -216,7 +216,7 @@ export class MatMenuItem extends _MatMenuItemBase implements FocusableOption, Ca // (undocumented) static ɵcmp: i0.ɵɵComponentDeclaration; // (undocumented) - static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵfac: i0.ɵɵFactoryDeclaration; } // @public (undocumented)