Skip to content

fix(material/menu): position classes not updated when window is resized #24385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/material-experimental/mdc-menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
14 changes: 13 additions & 1 deletion src/material-experimental/mdc-menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import {Overlay, ScrollStrategy} from '@angular/cdk/overlay';
import {
ChangeDetectionStrategy,
ChangeDetectorRef,
Component,
ElementRef,
Inject,
Expand Down Expand Up @@ -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<HTMLElement>,
ngZone: NgZone,
defaultOptions: MatMenuDefaultOptions,
);

constructor(
_elementRef: ElementRef<HTMLElement>,
_ngZone: NgZone,
@Inject(MAT_MENU_DEFAULT_OPTIONS) _defaultOptions: MatMenuDefaultOptions,
changeDetectorRef?: ChangeDetectorRef,
) {
super(_elementRef, _ngZone, _defaultOptions);
super(_elementRef, _ngZone, _defaultOptions, changeDetectorRef);
}
}
26 changes: 25 additions & 1 deletion src/material/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
Inject,
InjectionToken,
Input,
NgZone,
OnDestroy,
Optional,
Output,
Expand Down Expand Up @@ -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<HTMLElement>,
viewContainerRef: ViewContainerRef,
scrollStrategy: any,
parentMenu: MatMenuPanel,
menuItemInstance: MatMenuItem,
dir: Directionality,
focusMonitor: FocusMonitor,
);

constructor(
private _overlay: Overlay,
private _element: ElementRef<HTMLElement>,
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
});
}
}
Expand Down
23 changes: 23 additions & 0 deletions src/material/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crisbeto This test does somehow not cover the new logic as it passes even without the changes in code.

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';
Expand Down
19 changes: 18 additions & 1 deletion src/material/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -272,6 +273,8 @@ export class _MatMenuBase
private _elementRef: ElementRef<HTMLElement>,
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() {
Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -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<HTMLElement>,
ngZone: NgZone,
defaultOptions: MatMenuDefaultOptions,
);

constructor(
elementRef: ElementRef<HTMLElement>,
ngZone: NgZone,
@Inject(MAT_MENU_DEFAULT_OPTIONS) defaultOptions: MatMenuDefaultOptions,
changeDetectorRef?: ChangeDetectorRef,
) {
super(elementRef, ngZone, defaultOptions);
super(elementRef, ngZone, defaultOptions, changeDetectorRef);
}
}
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 @@ -71,6 +71,7 @@ const MAT_MENU_SCROLL_STRATEGY_FACTORY_PROVIDER: {

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

// @public
export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnInit, OnDestroy {
constructor(_elementRef: ElementRef<HTMLElement>, _ngZone: NgZone, _defaultOptions: MatMenuDefaultOptions);
constructor(_elementRef: ElementRef<HTMLElement>, _ngZone: NgZone, _defaultOptions: MatMenuDefaultOptions, _changeDetectorRef?: ChangeDetectorRef | undefined);
// (undocumented)
addItem(_item: MatMenuItem): void;
_allItems: QueryList<MatMenuItem>;
Expand Down Expand Up @@ -282,6 +283,8 @@ export class MatMenuTrigger extends _MatMenuTriggerBase {
export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy {
// @deprecated
constructor(overlay: Overlay, element: ElementRef<HTMLElement>, viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, menuItemInstance: MatMenuItem, dir: Directionality, focusMonitor?: FocusMonitor | null);
// @deprecated
constructor(overlay: Overlay, element: ElementRef<HTMLElement>, viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, menuItemInstance: MatMenuItem, dir: Directionality, focusMonitor: FocusMonitor);
closeMenu(): void;
// @deprecated (undocumented)
get _deprecatedMatMenuTriggerFor(): MatMenuPanel;
Expand Down Expand Up @@ -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
Expand Down