Skip to content

Commit f868e33

Browse files
crisbetocrisbeto
authored
fix(material/menu): position classes not update when window is resized (#24385)
Relates to internal bug b/218602349. We were calling `MatMenu.setPositionClasses` in order to set the classes if the position changes, but the call was outside of the `NgZone` so nothing was happening. These changes bring the call back into the `NgZone`.
1 parent 599d1b4 commit f868e33

File tree

6 files changed

+107
-5
lines changed

6 files changed

+107
-5
lines changed

‎src/material-experimental/mdc-menu/menu.spec.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,6 +1331,29 @@ describe('MDC-based MatMenu', () => {
13311331
expect(panel.classList).not.toContain('mat-menu-above');
13321332
}));
13331333

1334+
it('should update the position classes if the window is resized', fakeAsync(() => {
1335+
trigger.style.position = 'fixed';
1336+
trigger.style.top = '300px';
1337+
fixture.componentInstance.yPosition = 'above';
1338+
fixture.componentInstance.trigger.openMenu();
1339+
fixture.detectChanges();
1340+
tick(500);
1341+
1342+
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel') as HTMLElement;
1343+
1344+
expect(panel.classList).toContain('mat-menu-above');
1345+
expect(panel.classList).not.toContain('mat-menu-below');
1346+
1347+
trigger.style.top = '0';
1348+
dispatchFakeEvent(window, 'resize');
1349+
fixture.detectChanges();
1350+
tick(500);
1351+
fixture.detectChanges();
1352+
1353+
expect(panel.classList).not.toContain('mat-menu-above');
1354+
expect(panel.classList).toContain('mat-menu-below');
1355+
}));
1356+
13341357
it('should be able to update the position after the first open', fakeAsync(() => {
13351358
trigger.style.position = 'fixed';
13361359
trigger.style.top = '200px';

‎src/material-experimental/mdc-menu/menu.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import {Overlay, ScrollStrategy} from '@angular/cdk/overlay';
1010
import {
1111
ChangeDetectionStrategy,
12+
ChangeDetectorRef,
1213
Component,
1314
ElementRef,
1415
Inject,
@@ -56,11 +57,22 @@ export class MatMenu extends _MatMenuBase {
5657
protected override _elevationPrefix = 'mat-mdc-elevation-z';
5758
protected override _baseElevation = 8;
5859

60+
/*
61+
* @deprecated `changeDetectorRef` parameter will become a required parameter.
62+
* @breaking-change 15.0.0
63+
*/
64+
constructor(
65+
elementRef: ElementRef<HTMLElement>,
66+
ngZone: NgZone,
67+
defaultOptions: MatMenuDefaultOptions,
68+
);
69+
5970
constructor(
6071
_elementRef: ElementRef<HTMLElement>,
6172
_ngZone: NgZone,
6273
@Inject(MAT_MENU_DEFAULT_OPTIONS) _defaultOptions: MatMenuDefaultOptions,
74+
changeDetectorRef?: ChangeDetectorRef,
6375
) {
64-
super(_elementRef, _ngZone, _defaultOptions);
76+
super(_elementRef, _ngZone, _defaultOptions, changeDetectorRef);
6577
}
6678
}

‎src/material/menu/menu-trigger.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
Inject,
3333
InjectionToken,
3434
Input,
35+
NgZone,
3536
OnDestroy,
3637
Optional,
3738
Output,
@@ -200,6 +201,21 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
200201
focusMonitor?: FocusMonitor | null,
201202
);
202203

204+
/**
205+
* @deprecated `ngZone` will become a required parameter.
206+
* @breaking-change 15.0.0
207+
*/
208+
constructor(
209+
overlay: Overlay,
210+
element: ElementRef<HTMLElement>,
211+
viewContainerRef: ViewContainerRef,
212+
scrollStrategy: any,
213+
parentMenu: MatMenuPanel,
214+
menuItemInstance: MatMenuItem,
215+
dir: Directionality,
216+
focusMonitor: FocusMonitor,
217+
);
218+
203219
constructor(
204220
private _overlay: Overlay,
205221
private _element: ElementRef<HTMLElement>,
@@ -211,6 +227,7 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
211227
@Optional() @Self() private _menuItemInstance: MatMenuItem,
212228
@Optional() private _dir: Directionality,
213229
private _focusMonitor: FocusMonitor | null,
230+
private _ngZone?: NgZone,
214231
) {
215232
this._scrollStrategy = scrollStrategy;
216233
this._parentMaterialMenu = parentMenu instanceof _MatMenuBase ? parentMenu : undefined;
@@ -472,7 +489,14 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
472489
const posX: MenuPositionX = change.connectionPair.overlayX === 'start' ? 'after' : 'before';
473490
const posY: MenuPositionY = change.connectionPair.overlayY === 'top' ? 'below' : 'above';
474491

475-
this.menu.setPositionClasses!(posX, posY);
492+
// @breaking-change 15.0.0 Remove null check for `ngZone`.
493+
// `positionChanges` fires outside of the `ngZone` and `setPositionClasses` might be
494+
// updating something in the view so we need to bring it back in.
495+
if (this._ngZone) {
496+
this._ngZone.run(() => this.menu.setPositionClasses!(posX, posY));
497+
} else {
498+
this.menu.setPositionClasses!(posX, posY);
499+
}
476500
});
477501
}
478502
}

‎src/material/menu/menu.spec.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,6 +1326,29 @@ describe('MatMenu', () => {
13261326
expect(panel.classList).not.toContain('mat-menu-above');
13271327
}));
13281328

1329+
it('should update the position classes if the window is resized', fakeAsync(() => {
1330+
trigger.style.position = 'fixed';
1331+
trigger.style.top = '300px';
1332+
fixture.componentInstance.yPosition = 'above';
1333+
fixture.componentInstance.trigger.openMenu();
1334+
fixture.detectChanges();
1335+
tick(500);
1336+
1337+
const panel = overlayContainerElement.querySelector('.mat-menu-panel') as HTMLElement;
1338+
1339+
expect(panel.classList).toContain('mat-menu-above');
1340+
expect(panel.classList).not.toContain('mat-menu-below');
1341+
1342+
trigger.style.top = '0';
1343+
dispatchFakeEvent(window, 'resize');
1344+
fixture.detectChanges();
1345+
tick(500);
1346+
fixture.detectChanges();
1347+
1348+
expect(panel.classList).not.toContain('mat-menu-above');
1349+
expect(panel.classList).toContain('mat-menu-below');
1350+
}));
1351+
13291352
it('should be able to update the position after the first open', fakeAsync(() => {
13301353
trigger.style.position = 'fixed';
13311354
trigger.style.top = '200px';

‎src/material/menu/menu.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
ViewChild,
3838
ViewEncapsulation,
3939
OnInit,
40+
ChangeDetectorRef,
4041
} from '@angular/core';
4142
import {merge, Observable, Subject, Subscription} from 'rxjs';
4243
import {startWith, switchMap, take} from 'rxjs/operators';
@@ -272,6 +273,8 @@ export class _MatMenuBase
272273
private _elementRef: ElementRef<HTMLElement>,
273274
private _ngZone: NgZone,
274275
@Inject(MAT_MENU_DEFAULT_OPTIONS) private _defaultOptions: MatMenuDefaultOptions,
276+
// @breaking-change 15.0.0 `_changeDetectorRef` to become a required parameter.
277+
private _changeDetectorRef?: ChangeDetectorRef,
275278
) {}
276279

277280
ngOnInit() {
@@ -452,6 +455,9 @@ export class _MatMenuBase
452455
classes['mat-menu-after'] = posX === 'after';
453456
classes['mat-menu-above'] = posY === 'above';
454457
classes['mat-menu-below'] = posY === 'below';
458+
459+
// @breaking-change 15.0.0 Remove null check for `_changeDetectorRef`.
460+
this._changeDetectorRef?.markForCheck();
455461
}
456462

457463
/** Starts the enter animation. */
@@ -522,11 +528,22 @@ export class MatMenu extends _MatMenuBase {
522528
protected override _elevationPrefix = 'mat-elevation-z';
523529
protected override _baseElevation = 4;
524530

531+
/**
532+
* @deprecated `changeDetectorRef` parameter will become a required parameter.
533+
* @breaking-change 15.0.0
534+
*/
535+
constructor(
536+
elementRef: ElementRef<HTMLElement>,
537+
ngZone: NgZone,
538+
defaultOptions: MatMenuDefaultOptions,
539+
);
540+
525541
constructor(
526542
elementRef: ElementRef<HTMLElement>,
527543
ngZone: NgZone,
528544
@Inject(MAT_MENU_DEFAULT_OPTIONS) defaultOptions: MatMenuDefaultOptions,
545+
changeDetectorRef?: ChangeDetectorRef,
529546
) {
530-
super(elementRef, ngZone, defaultOptions);
547+
super(elementRef, ngZone, defaultOptions, changeDetectorRef);
531548
}
532549
}

‎tools/public_api_guard/material/menu.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ const MAT_MENU_SCROLL_STRATEGY_FACTORY_PROVIDER: {
7171

7272
// @public
7373
export class MatMenu extends _MatMenuBase {
74+
// @deprecated
7475
constructor(elementRef: ElementRef<HTMLElement>, ngZone: NgZone, defaultOptions: MatMenuDefaultOptions);
7576
// (undocumented)
7677
protected _baseElevation: number;
@@ -90,7 +91,7 @@ export const matMenuAnimations: {
9091

9192
// @public
9293
export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnInit, OnDestroy {
93-
constructor(_elementRef: ElementRef<HTMLElement>, _ngZone: NgZone, _defaultOptions: MatMenuDefaultOptions);
94+
constructor(_elementRef: ElementRef<HTMLElement>, _ngZone: NgZone, _defaultOptions: MatMenuDefaultOptions, _changeDetectorRef?: ChangeDetectorRef | undefined);
9495
// (undocumented)
9596
addItem(_item: MatMenuItem): void;
9697
_allItems: QueryList<MatMenuItem>;
@@ -282,6 +283,8 @@ export class MatMenuTrigger extends _MatMenuTriggerBase {
282283
export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy {
283284
// @deprecated
284285
constructor(overlay: Overlay, element: ElementRef<HTMLElement>, viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, menuItemInstance: MatMenuItem, dir: Directionality, focusMonitor?: FocusMonitor | null);
286+
// @deprecated
287+
constructor(overlay: Overlay, element: ElementRef<HTMLElement>, viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, menuItemInstance: MatMenuItem, dir: Directionality, focusMonitor: FocusMonitor);
285288
closeMenu(): void;
286289
// @deprecated (undocumented)
287290
get _deprecatedMatMenuTriggerFor(): MatMenuPanel;
@@ -315,7 +318,7 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
315318
// (undocumented)
316319
static ɵdir: i0.ɵɵDirectiveDeclaration<_MatMenuTriggerBase, never, never, { "_deprecatedMatMenuTriggerFor": "mat-menu-trigger-for"; "menu": "matMenuTriggerFor"; "menuData": "matMenuTriggerData"; "restoreFocus": "matMenuTriggerRestoreFocus"; }, { "menuOpened": "menuOpened"; "onMenuOpen": "onMenuOpen"; "menuClosed": "menuClosed"; "onMenuClose": "onMenuClose"; }, never>;
317320
// (undocumented)
318-
static ɵfac: i0.ɵɵFactoryDeclaration<_MatMenuTriggerBase, [null, null, null, null, { optional: true; }, { optional: true; self: true; }, { optional: true; }, null]>;
321+
static ɵfac: i0.ɵɵFactoryDeclaration<_MatMenuTriggerBase, [null, null, null, null, { optional: true; }, { optional: true; self: true; }, { optional: true; }, null, null]>;
319322
}
320323

321324
// @public

0 commit comments

Comments
 (0)