Skip to content

Commit 5fb0dc6

Browse files
authored
fix(menu): mark lazy menu content as dirty before attach (#16506)
This fixes a Google presubmit issue where the menu's `@ContentChildren` aren't updated after attaching lazy `MatMenuContent`.
1 parent 74794d4 commit 5fb0dc6

File tree

3 files changed

+90
-35
lines changed

3 files changed

+90
-35
lines changed

src/material/menu/menu-content.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,19 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import {DomPortalOutlet, TemplatePortal} from '@angular/cdk/portal';
10+
import {DOCUMENT} from '@angular/common';
911
import {
10-
Directive,
11-
TemplateRef,
12-
ComponentFactoryResolver,
1312
ApplicationRef,
14-
Injector,
15-
ViewContainerRef,
13+
ChangeDetectorRef,
14+
ComponentFactoryResolver,
15+
Directive,
1616
Inject,
17+
Injector,
1718
OnDestroy,
19+
TemplateRef,
20+
ViewContainerRef,
1821
} from '@angular/core';
19-
import {TemplatePortal, DomPortalOutlet} from '@angular/cdk/portal';
20-
import {DOCUMENT} from '@angular/common';
2122
import {Subject} from 'rxjs';
2223

2324
/**
@@ -39,7 +40,8 @@ export class MatMenuContent implements OnDestroy {
3940
private _appRef: ApplicationRef,
4041
private _injector: Injector,
4142
private _viewContainerRef: ViewContainerRef,
42-
@Inject(DOCUMENT) private _document: any) {}
43+
@Inject(DOCUMENT) private _document: any,
44+
private _changeDetectorRef?: ChangeDetectorRef) {}
4345

4446
/**
4547
* Attaches the content with a particular context.
@@ -63,6 +65,17 @@ export class MatMenuContent implements OnDestroy {
6365
// own `OverlayRef` panel), we have to re-insert the host element every time, otherwise we
6466
// risk it staying attached to a pane that's no longer in the DOM.
6567
element.parentNode!.insertBefore(this._outlet.outletElement, element);
68+
69+
// When `MatMenuContent` is used in an `OnPush` component, the insertion of the menu
70+
// content via `createEmbeddedView` does not cause the content to be seen as "dirty"
71+
// by Angular. This causes the `@ContentChildren` for menu items within the menu to
72+
// not be updated by Angular. By explicitly marking for check here, we tell Angular that
73+
// it needs to check for new menu items and update the `@ContentChild` in `MatMenu`.
74+
// @breaking-change 9.0.0 Make change detector ref required
75+
if (this._changeDetectorRef) {
76+
this._changeDetectorRef.markForCheck();
77+
}
78+
6679
this._portal.attach(this._outlet, context);
6780
this._attached.next();
6881
}

src/material/menu/menu.spec.ts

Lines changed: 68 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,49 @@
1-
import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
2-
import {By} from '@angular/platform-browser';
3-
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
1+
import {FocusMonitor} from '@angular/cdk/a11y';
2+
import {Direction, Directionality} from '@angular/cdk/bidi';
3+
import {DOWN_ARROW, END, ESCAPE, HOME, LEFT_ARROW, RIGHT_ARROW, TAB} from '@angular/cdk/keycodes';
4+
import {Overlay, OverlayContainer} from '@angular/cdk/overlay';
5+
import {ScrollDispatcher} from '@angular/cdk/scrolling';
6+
import {
7+
createKeyboardEvent,
8+
createMouseEvent,
9+
dispatchEvent,
10+
dispatchFakeEvent,
11+
dispatchKeyboardEvent,
12+
dispatchMouseEvent,
13+
MockNgZone,
14+
patchElementFocus,
15+
} from '@angular/cdk/testing';
416
import {
17+
ChangeDetectionStrategy,
518
Component,
619
ElementRef,
720
EventEmitter,
821
Input,
9-
Output,
1022
NgZone,
23+
Output,
24+
Provider,
25+
QueryList,
1126
TemplateRef,
27+
Type,
1228
ViewChild,
1329
ViewChildren,
14-
QueryList,
15-
Type,
16-
Provider,
1730
} from '@angular/core';
18-
import {Direction, Directionality} from '@angular/cdk/bidi';
19-
import {OverlayContainer, Overlay} from '@angular/cdk/overlay';
20-
import {ESCAPE, LEFT_ARROW, RIGHT_ARROW, DOWN_ARROW, TAB, HOME, END} from '@angular/cdk/keycodes';
31+
import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
32+
import {MatRipple} from '@angular/material/core';
33+
import {By} from '@angular/platform-browser';
34+
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
35+
import {Subject} from 'rxjs';
2136
import {
2237
MAT_MENU_DEFAULT_OPTIONS,
2338
MatMenu,
39+
MatMenuItem,
2440
MatMenuModule,
2541
MatMenuPanel,
2642
MatMenuTrigger,
2743
MenuPositionX,
2844
MenuPositionY,
29-
MatMenuItem,
3045
} from './index';
31-
import {MENU_PANEL_TOP_PADDING, MAT_MENU_SCROLL_STRATEGY} from './menu-trigger';
32-
import {MatRipple} from '@angular/material/core';
33-
import {
34-
dispatchKeyboardEvent,
35-
dispatchMouseEvent,
36-
dispatchEvent,
37-
createKeyboardEvent,
38-
createMouseEvent,
39-
dispatchFakeEvent,
40-
patchElementFocus,
41-
MockNgZone,
42-
} from '@angular/cdk/testing';
43-
import {Subject} from 'rxjs';
44-
import {ScrollDispatcher} from '@angular/cdk/scrolling';
45-
import {FocusMonitor} from '@angular/cdk/a11y';
46+
import {MAT_MENU_SCROLL_STRATEGY, MENU_PANEL_TOP_PADDING} from './menu-trigger';
4647

4748

4849
describe('MatMenu', () => {
@@ -892,6 +893,25 @@ describe('MatMenu', () => {
892893
expect(document.activeElement).toBe(items[1], 'Expected second item to be focused');
893894
flush();
894895
}));
896+
897+
it('should open submenus when the menu is inside an OnPush component', fakeAsync(() => {
898+
const fixture = createComponent(LazyMenuWithOnPush);
899+
fixture.detectChanges();
900+
901+
// Open the top-level menu
902+
fixture.componentInstance.rootTrigger.nativeElement.click();
903+
fixture.detectChanges();
904+
flush();
905+
906+
// Dispatch a `mouseenter` on the menu item to open the submenu.
907+
// This will only work if the top-level menu is aware the this menu item exists.
908+
dispatchMouseEvent(fixture.componentInstance.menuItemWithSubmenu.nativeElement, 'mouseenter');
909+
fixture.detectChanges();
910+
flush();
911+
912+
expect(overlayContainerElement.querySelectorAll('.mat-menu-item').length)
913+
.toBe(2, 'Expected two open menus');
914+
}));
895915
});
896916

897917
describe('positions', () => {
@@ -2373,3 +2393,25 @@ class SimpleMenuWithRepeater {
23732393
items = ['Pizza', 'Pasta'];
23742394
}
23752395

2396+
@Component({
2397+
template: `
2398+
<button [matMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
2399+
2400+
<mat-menu #menu="matMenu">
2401+
<ng-template matMenuContent>
2402+
<button [matMenuTriggerFor]="menu2" mat-menu-item #menuItem>Item</button>
2403+
</ng-template>
2404+
</mat-menu>
2405+
2406+
<mat-menu #menu2="matMenu">
2407+
<ng-template matMenuContent>
2408+
<button mat-menu-item #subMenuItem>Sub item</button>
2409+
</ng-template>
2410+
</mat-menu>
2411+
`,
2412+
changeDetection: ChangeDetectionStrategy.OnPush,
2413+
})
2414+
class LazyMenuWithOnPush {
2415+
@ViewChild('triggerEl', {static: false, read: ElementRef}) rootTrigger: ElementRef;
2416+
@ViewChild('menuItem', {static: false, read: ElementRef}) menuItemWithSubmenu: ElementRef;
2417+
}

tools/public_api_guard/material/menu.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export declare const matMenuAnimations: {
6363

6464
export declare class MatMenuContent implements OnDestroy {
6565
_attached: Subject<void>;
66-
constructor(_template: TemplateRef<any>, _componentFactoryResolver: ComponentFactoryResolver, _appRef: ApplicationRef, _injector: Injector, _viewContainerRef: ViewContainerRef, _document: any);
66+
constructor(_template: TemplateRef<any>, _componentFactoryResolver: ComponentFactoryResolver, _appRef: ApplicationRef, _injector: Injector, _viewContainerRef: ViewContainerRef, _document: any, _changeDetectorRef?: ChangeDetectorRef | undefined);
6767
attach(context?: any): void;
6868
detach(): void;
6969
ngOnDestroy(): void;

0 commit comments

Comments
 (0)