fix(material/menu): adjust overlay size when amount of items changes

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.

Author: crisbeto

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<T>(
     component: Type<T>,
@@ -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<T>(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);

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);
+      });
     }
   }
 

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<T>(
     component: Type<T>,
@@ -71,6 +72,7 @@ describe('MatMenu', () => {
 
     overlayContainerElement = TestBed.inject(OverlayContainer).getContainerElement();
     focusMonitor = TestBed.inject(FocusMonitor);
+    viewportRuler = TestBed.inject(ViewportRuler);
     const fixture = TestBed.createComponent<T>(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);

src/material/menu/menu.ts

@@ -109,7 +109,7 @@ export class _MatMenuBase
   @ContentChildren(MatMenuItem, {descendants: true}) _allItems: QueryList<MatMenuItem>;
 
   /** Only the direct descendant menu items. */
-  private _directDescendantItems = new QueryList<MatMenuItem>();
+  _directDescendantItems = new QueryList<MatMenuItem>();
 
   /** Subscription to tab events on the menu panel */
   private _tabSubscription = Subscription.EMPTY;
@@ -279,7 +279,9 @@ export class _MatMenuBase
   }
 
   ngAfterContentInit() {
-    this._updateDirectDescendants();
+    this._updateDirectDescendants(this._allItems, false);
+    this._allItems.changes.subscribe(items => this._updateDirectDescendants(items, true));
+
     this._keyManager = new FocusKeyManager(this._directDescendantItems)
       .withWrap()
       .withTypeAhead()
@@ -487,18 +489,15 @@ export class _MatMenuBase
   }
 
   /**
-   * Sets up a stream that will keep track of any newly-added menu items and will update the list
-   * of direct descendants. We collect the descendants this way, because `_allItems` can include
-   * items that are part of child menus, and using a custom way of registering items is unreliable
-   * when it comes to maintaining the item order.
+   * Updates the list of direct descendant items by filtering the
+   * items from a list who belong to this menu instance.
    */
-  private _updateDirectDescendants() {
-    this._allItems.changes
-      .pipe(startWith(this._allItems))
-      .subscribe((items: QueryList<MatMenuItem>) => {
-        this._directDescendantItems.reset(items.filter(item => item._parentMenu === this));
-        this._directDescendantItems.notifyOnChanges();
-      });
+  private _updateDirectDescendants(items: QueryList<MatMenuItem>, shouldNotify: boolean) {
+    this._directDescendantItems.reset(items.filter(item => item._parentMenu === this));
+
+    if (shouldNotify) {
+      this._directDescendantItems.notifyOnChanges();
+    }
   }
 }
 

tools/public_api_guard/material/menu.md

@@ -110,6 +110,7 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
     // @deprecated
     readonly close: EventEmitter<MenuCloseReason>;
     readonly closed: EventEmitter<MenuCloseReason>;
+    _directDescendantItems: QueryList<MatMenuItem>;
     direction: Direction;
     // (undocumented)
     protected _elevationPrefix: string;