Skip to content

Commit

Permalink
fix(material/menu): item highlighted state not updating in time when …
Browse files Browse the repository at this point in the history
…using lazy content (#23185)

`matMenuContent` is declared as an `ng-template` and stamped out inside the menu which means that its change detection tree is actually in the declaration place, rather than the insertion place. This can result in the `highlighted` state not being updated when inside an `OnPush` component.

Fixes #23175.

(cherry picked from commit 2a2cd9c)
  • Loading branch information
crisbeto authored and mmalerba committed Jul 27, 2021
1 parent af20bfc commit 6a3230e
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 6 deletions.
6 changes: 4 additions & 2 deletions src/material-experimental/mdc-menu/menu-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
Inject,
ElementRef,
Optional,
ChangeDetectorRef,
} from '@angular/core';
import {
MAT_RIPPLE_GLOBAL_OPTIONS,
Expand Down Expand Up @@ -62,8 +63,9 @@ export class MatMenuItem extends BaseMatMenuItem {
@Inject(MAT_MENU_PANEL) @Optional() parentMenu?: MatMenuPanel<unknown>,
@Optional() @Inject(MAT_RIPPLE_GLOBAL_OPTIONS)
globalRippleOptions?: RippleGlobalOptions,
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string) {
super(elementRef, document, focusMonitor, parentMenu);
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string,
changeDetectorRef?: ChangeDetectorRef) {
super(elementRef, document, focusMonitor, parentMenu, changeDetectorRef);

this._noopAnimations = animationMode === 'NoopAnimations';
this._rippleAnimation = globalRippleOptions?.animation || {
Expand Down
17 changes: 16 additions & 1 deletion src/material/menu/menu-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
Input,
HostListener,
AfterViewInit,
ChangeDetectorRef,
} from '@angular/core';
import {
CanDisable,
Expand Down Expand Up @@ -81,7 +82,12 @@ export class MatMenuItem extends _MatMenuItemBase
*/
@Inject(DOCUMENT) _document?: any,
private _focusMonitor?: FocusMonitor,
@Inject(MAT_MENU_PANEL) @Optional() public _parentMenu?: MatMenuPanel<MatMenuItem>) {
@Inject(MAT_MENU_PANEL) @Optional() public _parentMenu?: MatMenuPanel<MatMenuItem>,
/**
* @deprecated `_changeDetectorRef` to become a required parameter.
* @breaking-change 14.0.0
*/
private _changeDetectorRef?: ChangeDetectorRef) {

// @breaking-change 8.0.0 make `_focusMonitor` and `document` required params.
super();
Expand Down Expand Up @@ -173,6 +179,15 @@ export class MatMenuItem extends _MatMenuItemBase
return clone.textContent?.trim() || '';
}

_setHighlighted(isHighlighted: boolean) {
// We need to mark this for check for the case where the content is coming from a
// `matMenuContent` whose change detection tree is at the declaration position,
// not the insertion position. See #23175.
// @breaking-change 14.0.0 Remove null check for `_changeDetectorRef`.
this._highlighted = isHighlighted;
this._changeDetectorRef?.markForCheck();
}

static ngAcceptInputType_disabled: BooleanInput;
static ngAcceptInputType_disableRipple: BooleanInput;
}
2 changes: 1 addition & 1 deletion src/material/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
this._menuOpen ? this.menuOpened.emit() : this.menuClosed.emit();

if (this.triggersSubmenu()) {
this._menuItemInstance._highlighted = isOpen;
this._menuItemInstance._setHighlighted(isOpen);
}
}

Expand Down
7 changes: 5 additions & 2 deletions tools/public_api_guard/material/menu.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ export class _MatMenuDirectivesModule {
// @public
export class MatMenuItem extends _MatMenuItemBase implements FocusableOption, CanDisable, CanDisableRipple, AfterViewInit, OnDestroy {
constructor(_elementRef: ElementRef<HTMLElement>,
_document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel<MatMenuItem> | undefined);
_document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel<MatMenuItem> | undefined,
_changeDetectorRef?: ChangeDetectorRef | undefined);
_checkDisabled(event: Event): void;
focus(origin?: FocusOrigin, options?: FocusOptions): void;
readonly _focused: Subject<MatMenuItem>;
Expand All @@ -219,11 +220,13 @@ export class MatMenuItem extends _MatMenuItemBase implements FocusableOption, Ca
// (undocumented)
_parentMenu?: MatMenuPanel<MatMenuItem> | undefined;
role: 'menuitem' | 'menuitemradio' | 'menuitemcheckbox';
// (undocumented)
_setHighlighted(isHighlighted: boolean): void;
_triggersSubmenu: boolean;
// (undocumented)
static ɵcmp: i0.ɵɵComponentDeclaration<MatMenuItem, "[mat-menu-item]", ["matMenuItem"], { "disabled": "disabled"; "disableRipple": "disableRipple"; "role": "role"; }, {}, never, ["*"]>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<MatMenuItem, [null, null, null, { optional: true; }]>;
static ɵfac: i0.ɵɵFactoryDeclaration<MatMenuItem, [null, null, null, { optional: true; }, null]>;
}

// @public (undocumented)
Expand Down

0 comments on commit 6a3230e

Please sign in to comment.