From d7fe423a3e3f209757792edd5a21a24629850661 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 1 Feb 2022 16:43:35 +0100 Subject: [PATCH] fix(material/menu): adjust overlay size when amount of items changes (#21457) Currently we lock the menu into a position after it is opened so that it doesn't jump around when the user scrolls, but this means that if the amount of items changes, it might not be the optimal position anymore. These changes add some code to re-calculate the position if the amount of items changes. Fixes #21456. (cherry picked from commit a74d92e33df365558eaf54661db32de379d68885) --- .../flexible-connected-position-strategy.ts | 12 ++++++--- .../mdc-menu/menu.spec.ts | 26 ++++++++++++++++++- src/material/menu/menu-trigger.ts | 9 ++++++- src/material/menu/menu.spec.ts | 26 ++++++++++++++++++- src/material/menu/menu.ts | 2 +- tools/public_api_guard/material/menu.md | 1 + 6 files changed, 69 insertions(+), 7 deletions(-) diff --git a/src/cdk/overlay/position/flexible-connected-position-strategy.ts b/src/cdk/overlay/position/flexible-connected-position-strategy.ts index df943e0d39cf..33bd7193fb74 100644 --- a/src/cdk/overlay/position/flexible-connected-position-strategy.ts +++ b/src/cdk/overlay/position/flexible-connected-position-strategy.ts @@ -360,16 +360,22 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy { * allows one to re-align the panel without changing the orientation of the panel. */ reapplyLastPosition(): void { - if (!this._isDisposed && (!this._platform || this._platform.isBrowser)) { + if (this._isDisposed || !this._platform.isBrowser) { + return; + } + + const lastPosition = this._lastPosition; + + if (lastPosition) { this._originRect = this._getOriginRect(); this._overlayRect = this._pane.getBoundingClientRect(); this._viewportRect = this._getNarrowedViewportRect(); this._containerRect = this._overlayContainer.getContainerElement().getBoundingClientRect(); - const lastPosition = this._lastPosition || this._preferredPositions[0]; const originPoint = this._getOriginPoint(this._originRect, this._containerRect, lastPosition); - this._applyPosition(lastPosition, originPoint); + } else { + this.apply(); } } diff --git a/src/material-experimental/mdc-menu/menu.spec.ts b/src/material-experimental/mdc-menu/menu.spec.ts index 3143f9a86a3e..de7527db3587 100644 --- a/src/material-experimental/mdc-menu/menu.spec.ts +++ b/src/material-experimental/mdc-menu/menu.spec.ts @@ -41,7 +41,7 @@ import { MockNgZone, } from '../../cdk/testing/private'; import {Subject} from 'rxjs'; -import {ScrollDispatcher} from '@angular/cdk/scrolling'; +import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling'; import {FocusMonitor} from '@angular/cdk/a11y'; import { MAT_MENU_SCROLL_STRATEGY, @@ -57,6 +57,7 @@ const MENU_PANEL_TOP_PADDING = 8; describe('MDC-based MatMenu', () => { let overlayContainerElement: HTMLElement; let focusMonitor: FocusMonitor; + let viewportRuler: ViewportRuler; function createComponent( component: Type, @@ -71,6 +72,7 @@ describe('MDC-based MatMenu', () => { overlayContainerElement = TestBed.inject(OverlayContainer).getContainerElement(); focusMonitor = TestBed.inject(FocusMonitor); + viewportRuler = TestBed.inject(ViewportRuler); const fixture = TestBed.createComponent(component); window.scroll(0, 0); return fixture; @@ -1142,6 +1144,28 @@ describe('MDC-based MatMenu', () => { expect(panel.classList).toContain('mat-menu-after'); })); + it('should keep the panel in the viewport when more items are added while open', () => { + const fixture = createComponent(SimpleMenu, [], [FakeIcon]); + fixture.detectChanges(); + + const triggerEl = fixture.componentInstance.triggerEl.nativeElement; + triggerEl.style.position = 'absolute'; + triggerEl.style.left = '200px'; + triggerEl.style.bottom = '300px'; + triggerEl.click(); + fixture.detectChanges(); + + const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!; + const viewportHeight = viewportRuler.getViewportSize().height; + let panelRect = panel.getBoundingClientRect(); + expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight); + + fixture.componentInstance.extraItems = new Array(50).fill('Hello there'); + fixture.detectChanges(); + panelRect = panel.getBoundingClientRect(); + expect(Math.floor(panelRect.bottom)).toBe(viewportHeight); + }); + describe('lazy rendering', () => { it('should be able to render the menu content lazily', fakeAsync(() => { const fixture = createComponent(SimpleLazyMenu); diff --git a/src/material/menu/menu-trigger.ts b/src/material/menu/menu-trigger.ts index d51c42715890..3f4f1c42e8bf 100644 --- a/src/material/menu/menu-trigger.ts +++ b/src/material/menu/menu-trigger.ts @@ -278,8 +278,9 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy const overlayRef = this._createOverlay(); const overlayConfig = overlayRef.getConfig(); + const positionStrategy = overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy; - this._setPosition(overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy); + this._setPosition(positionStrategy); overlayConfig.hasBackdrop = this.menu.hasBackdrop == null ? !this.triggersSubmenu() : this.menu.hasBackdrop; overlayRef.attach(this._getPortal()); @@ -293,6 +294,12 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy if (this.menu instanceof _MatMenuBase) { this.menu._startAnimation(); + this.menu._directDescendantItems.changes.pipe(takeUntil(this.menu.close)).subscribe(() => { + // Re-adjust the position without locking when the amount of items + // changes so that the overlay is allowed to pick a new optimal position. + positionStrategy.withLockedPosition(false).reapplyLastPosition(); + positionStrategy.withLockedPosition(true); + }); } } diff --git a/src/material/menu/menu.spec.ts b/src/material/menu/menu.spec.ts index 1adb1fd42934..8cf46cdd46f1 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -11,7 +11,7 @@ import { TAB, } from '@angular/cdk/keycodes'; import {Overlay, OverlayContainer} from '@angular/cdk/overlay'; -import {ScrollDispatcher} from '@angular/cdk/scrolling'; +import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling'; import { createKeyboardEvent, createMouseEvent, @@ -57,6 +57,7 @@ import {MAT_MENU_SCROLL_STRATEGY, MENU_PANEL_TOP_PADDING} from './menu-trigger'; describe('MatMenu', () => { let overlayContainerElement: HTMLElement; let focusMonitor: FocusMonitor; + let viewportRuler: ViewportRuler; function createComponent( component: Type, @@ -71,6 +72,7 @@ describe('MatMenu', () => { overlayContainerElement = TestBed.inject(OverlayContainer).getContainerElement(); focusMonitor = TestBed.inject(FocusMonitor); + viewportRuler = TestBed.inject(ViewportRuler); const fixture = TestBed.createComponent(component); window.scroll(0, 0); return fixture; @@ -1137,6 +1139,28 @@ describe('MatMenu', () => { expect(panel.classList).toContain('mat-menu-after'); })); + it('should keep the panel in the viewport when more items are added while open', () => { + const fixture = createComponent(SimpleMenu, [], [FakeIcon]); + fixture.detectChanges(); + + const triggerEl = fixture.componentInstance.triggerEl.nativeElement; + triggerEl.style.position = 'absolute'; + triggerEl.style.left = '200px'; + triggerEl.style.bottom = '300px'; + triggerEl.click(); + fixture.detectChanges(); + + const panel = overlayContainerElement.querySelector('.mat-menu-panel')!; + const viewportHeight = viewportRuler.getViewportSize().height; + let panelRect = panel.getBoundingClientRect(); + expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight); + + fixture.componentInstance.extraItems = new Array(50).fill('Hello there'); + fixture.detectChanges(); + panelRect = panel.getBoundingClientRect(); + expect(Math.floor(panelRect.bottom)).toBe(viewportHeight); + }); + describe('lazy rendering', () => { it('should be able to render the menu content lazily', fakeAsync(() => { const fixture = createComponent(SimpleLazyMenu); diff --git a/src/material/menu/menu.ts b/src/material/menu/menu.ts index 91fb0eff4c01..11e0518ad447 100644 --- a/src/material/menu/menu.ts +++ b/src/material/menu/menu.ts @@ -109,7 +109,7 @@ export class _MatMenuBase @ContentChildren(MatMenuItem, {descendants: true}) _allItems: QueryList; /** Only the direct descendant menu items. */ - private _directDescendantItems = new QueryList(); + _directDescendantItems = new QueryList(); /** Subscription to tab events on the menu panel */ private _tabSubscription = Subscription.EMPTY; diff --git a/tools/public_api_guard/material/menu.md b/tools/public_api_guard/material/menu.md index da6acf65fbff..f7e52054035b 100644 --- a/tools/public_api_guard/material/menu.md +++ b/tools/public_api_guard/material/menu.md @@ -110,6 +110,7 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel // @deprecated readonly close: EventEmitter; readonly closed: EventEmitter; + _directDescendantItems: QueryList; direction: Direction; // (undocumented) protected _elevationPrefix: string;