From f868e33a5a13f32ffad6e3ee50460e3ca1b7f82b Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 14 Feb 2022 17:46:55 +0100 Subject: [PATCH] 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`. --- .../mdc-menu/menu.spec.ts | 23 ++++++++++++++++ src/material-experimental/mdc-menu/menu.ts | 14 +++++++++- src/material/menu/menu-trigger.ts | 26 ++++++++++++++++++- src/material/menu/menu.spec.ts | 23 ++++++++++++++++ src/material/menu/menu.ts | 19 +++++++++++++- tools/public_api_guard/material/menu.md | 7 +++-- 6 files changed, 107 insertions(+), 5 deletions(-) diff --git a/src/material-experimental/mdc-menu/menu.spec.ts b/src/material-experimental/mdc-menu/menu.spec.ts index de7527db3587..36023c0fb6b0 100644 --- a/src/material-experimental/mdc-menu/menu.spec.ts +++ b/src/material-experimental/mdc-menu/menu.spec.ts @@ -1331,6 +1331,29 @@ describe('MDC-based MatMenu', () => { expect(panel.classList).not.toContain('mat-menu-above'); })); + it('should update the position classes if the window is resized', fakeAsync(() => { + trigger.style.position = 'fixed'; + trigger.style.top = '300px'; + fixture.componentInstance.yPosition = 'above'; + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + tick(500); + + const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel') as HTMLElement; + + expect(panel.classList).toContain('mat-menu-above'); + expect(panel.classList).not.toContain('mat-menu-below'); + + trigger.style.top = '0'; + dispatchFakeEvent(window, 'resize'); + fixture.detectChanges(); + tick(500); + fixture.detectChanges(); + + expect(panel.classList).not.toContain('mat-menu-above'); + expect(panel.classList).toContain('mat-menu-below'); + })); + it('should be able to update the position after the first open', fakeAsync(() => { trigger.style.position = 'fixed'; trigger.style.top = '200px'; diff --git a/src/material-experimental/mdc-menu/menu.ts b/src/material-experimental/mdc-menu/menu.ts index 4c5278c60c21..2a1baf53faef 100644 --- a/src/material-experimental/mdc-menu/menu.ts +++ b/src/material-experimental/mdc-menu/menu.ts @@ -9,6 +9,7 @@ import {Overlay, ScrollStrategy} from '@angular/cdk/overlay'; import { ChangeDetectionStrategy, + ChangeDetectorRef, Component, ElementRef, Inject, @@ -56,11 +57,22 @@ export class MatMenu extends _MatMenuBase { protected override _elevationPrefix = 'mat-mdc-elevation-z'; protected override _baseElevation = 8; + /* + * @deprecated `changeDetectorRef` parameter will become a required parameter. + * @breaking-change 15.0.0 + */ + constructor( + elementRef: ElementRef, + ngZone: NgZone, + defaultOptions: MatMenuDefaultOptions, + ); + constructor( _elementRef: ElementRef, _ngZone: NgZone, @Inject(MAT_MENU_DEFAULT_OPTIONS) _defaultOptions: MatMenuDefaultOptions, + changeDetectorRef?: ChangeDetectorRef, ) { - super(_elementRef, _ngZone, _defaultOptions); + super(_elementRef, _ngZone, _defaultOptions, changeDetectorRef); } } diff --git a/src/material/menu/menu-trigger.ts b/src/material/menu/menu-trigger.ts index 3f4f1c42e8bf..ff1b4c208f20 100644 --- a/src/material/menu/menu-trigger.ts +++ b/src/material/menu/menu-trigger.ts @@ -32,6 +32,7 @@ import { Inject, InjectionToken, Input, + NgZone, OnDestroy, Optional, Output, @@ -200,6 +201,21 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy focusMonitor?: FocusMonitor | null, ); + /** + * @deprecated `ngZone` will become a required parameter. + * @breaking-change 15.0.0 + */ + constructor( + overlay: Overlay, + element: ElementRef, + viewContainerRef: ViewContainerRef, + scrollStrategy: any, + parentMenu: MatMenuPanel, + menuItemInstance: MatMenuItem, + dir: Directionality, + focusMonitor: FocusMonitor, + ); + constructor( private _overlay: Overlay, private _element: ElementRef, @@ -211,6 +227,7 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy @Optional() @Self() private _menuItemInstance: MatMenuItem, @Optional() private _dir: Directionality, private _focusMonitor: FocusMonitor | null, + private _ngZone?: NgZone, ) { this._scrollStrategy = scrollStrategy; this._parentMaterialMenu = parentMenu instanceof _MatMenuBase ? parentMenu : undefined; @@ -472,7 +489,14 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy const posX: MenuPositionX = change.connectionPair.overlayX === 'start' ? 'after' : 'before'; const posY: MenuPositionY = change.connectionPair.overlayY === 'top' ? 'below' : 'above'; - this.menu.setPositionClasses!(posX, posY); + // @breaking-change 15.0.0 Remove null check for `ngZone`. + // `positionChanges` fires outside of the `ngZone` and `setPositionClasses` might be + // updating something in the view so we need to bring it back in. + if (this._ngZone) { + this._ngZone.run(() => this.menu.setPositionClasses!(posX, posY)); + } else { + this.menu.setPositionClasses!(posX, posY); + } }); } } diff --git a/src/material/menu/menu.spec.ts b/src/material/menu/menu.spec.ts index 8cf46cdd46f1..6fec2b2a6a7b 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -1326,6 +1326,29 @@ describe('MatMenu', () => { expect(panel.classList).not.toContain('mat-menu-above'); })); + it('should update the position classes if the window is resized', fakeAsync(() => { + trigger.style.position = 'fixed'; + trigger.style.top = '300px'; + fixture.componentInstance.yPosition = 'above'; + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + tick(500); + + const panel = overlayContainerElement.querySelector('.mat-menu-panel') as HTMLElement; + + expect(panel.classList).toContain('mat-menu-above'); + expect(panel.classList).not.toContain('mat-menu-below'); + + trigger.style.top = '0'; + dispatchFakeEvent(window, 'resize'); + fixture.detectChanges(); + tick(500); + fixture.detectChanges(); + + expect(panel.classList).not.toContain('mat-menu-above'); + expect(panel.classList).toContain('mat-menu-below'); + })); + it('should be able to update the position after the first open', fakeAsync(() => { trigger.style.position = 'fixed'; trigger.style.top = '200px'; diff --git a/src/material/menu/menu.ts b/src/material/menu/menu.ts index 11e0518ad447..1463c6d7a721 100644 --- a/src/material/menu/menu.ts +++ b/src/material/menu/menu.ts @@ -37,6 +37,7 @@ import { ViewChild, ViewEncapsulation, OnInit, + ChangeDetectorRef, } from '@angular/core'; import {merge, Observable, Subject, Subscription} from 'rxjs'; import {startWith, switchMap, take} from 'rxjs/operators'; @@ -272,6 +273,8 @@ export class _MatMenuBase private _elementRef: ElementRef, private _ngZone: NgZone, @Inject(MAT_MENU_DEFAULT_OPTIONS) private _defaultOptions: MatMenuDefaultOptions, + // @breaking-change 15.0.0 `_changeDetectorRef` to become a required parameter. + private _changeDetectorRef?: ChangeDetectorRef, ) {} ngOnInit() { @@ -452,6 +455,9 @@ export class _MatMenuBase classes['mat-menu-after'] = posX === 'after'; classes['mat-menu-above'] = posY === 'above'; classes['mat-menu-below'] = posY === 'below'; + + // @breaking-change 15.0.0 Remove null check for `_changeDetectorRef`. + this._changeDetectorRef?.markForCheck(); } /** Starts the enter animation. */ @@ -522,11 +528,22 @@ export class MatMenu extends _MatMenuBase { protected override _elevationPrefix = 'mat-elevation-z'; protected override _baseElevation = 4; + /** + * @deprecated `changeDetectorRef` parameter will become a required parameter. + * @breaking-change 15.0.0 + */ + constructor( + elementRef: ElementRef, + ngZone: NgZone, + defaultOptions: MatMenuDefaultOptions, + ); + constructor( elementRef: ElementRef, ngZone: NgZone, @Inject(MAT_MENU_DEFAULT_OPTIONS) defaultOptions: MatMenuDefaultOptions, + changeDetectorRef?: ChangeDetectorRef, ) { - super(elementRef, ngZone, defaultOptions); + super(elementRef, ngZone, defaultOptions, changeDetectorRef); } } diff --git a/tools/public_api_guard/material/menu.md b/tools/public_api_guard/material/menu.md index f7e52054035b..8b887138a04a 100644 --- a/tools/public_api_guard/material/menu.md +++ b/tools/public_api_guard/material/menu.md @@ -71,6 +71,7 @@ const MAT_MENU_SCROLL_STRATEGY_FACTORY_PROVIDER: { // @public export class MatMenu extends _MatMenuBase { + // @deprecated constructor(elementRef: ElementRef, ngZone: NgZone, defaultOptions: MatMenuDefaultOptions); // (undocumented) protected _baseElevation: number; @@ -90,7 +91,7 @@ export const matMenuAnimations: { // @public export class _MatMenuBase implements AfterContentInit, MatMenuPanel, OnInit, OnDestroy { - constructor(_elementRef: ElementRef, _ngZone: NgZone, _defaultOptions: MatMenuDefaultOptions); + constructor(_elementRef: ElementRef, _ngZone: NgZone, _defaultOptions: MatMenuDefaultOptions, _changeDetectorRef?: ChangeDetectorRef | undefined); // (undocumented) addItem(_item: MatMenuItem): void; _allItems: QueryList; @@ -282,6 +283,8 @@ export class MatMenuTrigger extends _MatMenuTriggerBase { export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy { // @deprecated constructor(overlay: Overlay, element: ElementRef, viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, menuItemInstance: MatMenuItem, dir: Directionality, focusMonitor?: FocusMonitor | null); + // @deprecated + constructor(overlay: Overlay, element: ElementRef, viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, menuItemInstance: MatMenuItem, dir: Directionality, focusMonitor: FocusMonitor); closeMenu(): void; // @deprecated (undocumented) get _deprecatedMatMenuTriggerFor(): MatMenuPanel; @@ -315,7 +318,7 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy // (undocumented) 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>; // (undocumented) - static ɵfac: i0.ɵɵFactoryDeclaration<_MatMenuTriggerBase, [null, null, null, null, { optional: true; }, { optional: true; self: true; }, { optional: true; }, null]>; + static ɵfac: i0.ɵɵFactoryDeclaration<_MatMenuTriggerBase, [null, null, null, null, { optional: true; }, { optional: true; self: true; }, { optional: true; }, null, null]>; } // @public