From 57df72f3292c80144f7911c8c00050a7481e0de1 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 28 Dec 2020 13:04:36 +0200 Subject: [PATCH] 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. --- .../mdc-menu/menu.spec.ts | 37 ++++++++++++++++--- src/material/menu/menu-trigger.ts | 9 ++++- src/material/menu/menu.spec.ts | 37 ++++++++++++++++--- src/material/menu/menu.ts | 2 +- tools/public_api_guard/material/menu.d.ts | 1 + 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/material-experimental/mdc-menu/menu.spec.ts b/src/material-experimental/mdc-menu/menu.spec.ts index ba04404b949b..f371f990bed1 100644 --- a/src/material-experimental/mdc-menu/menu.spec.ts +++ b/src/material-experimental/mdc-menu/menu.spec.ts @@ -32,7 +32,7 @@ import { MockNgZone, } from '@angular/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, @@ -49,6 +49,7 @@ describe('MDC-based MatMenu', () => { let overlayContainer: OverlayContainer; let overlayContainerElement: HTMLElement; let focusMonitor: FocusMonitor; + let viewportRuler: ViewportRuler; function createComponent(component: Type, providers: Provider[] = [], @@ -59,11 +60,13 @@ describe('MDC-based MatMenu', () => { providers }).compileComponents(); - inject([OverlayContainer, FocusMonitor], (oc: OverlayContainer, fm: FocusMonitor) => { - overlayContainer = oc; - overlayContainerElement = oc.getContainerElement(); - focusMonitor = fm; - })(); + inject([OverlayContainer, FocusMonitor, ViewportRuler], + (oc: OverlayContainer, fm: FocusMonitor, vr: ViewportRuler) => { + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); + focusMonitor = fm; + viewportRuler = vr; + })(); return TestBed.createComponent(component); } @@ -1014,6 +1017,28 @@ describe('MDC-based MatMenu', () => { .toBe(false); }); + 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 06de01f4fb1d..2bb4877d33f0 100644 --- a/src/material/menu/menu-trigger.ts +++ b/src/material/menu/menu-trigger.ts @@ -248,8 +248,9 @@ export class MatMenuTrigger 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()); @@ -263,6 +264,12 @@ export class MatMenuTrigger 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 388005dc6a38..70f07984db4c 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -2,7 +2,7 @@ import {FocusMonitor} from '@angular/cdk/a11y'; import {Direction, Directionality} from '@angular/cdk/bidi'; import {DOWN_ARROW, END, ESCAPE, HOME, LEFT_ARROW, RIGHT_ARROW, 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, @@ -50,6 +50,7 @@ describe('MatMenu', () => { let overlayContainer: OverlayContainer; let overlayContainerElement: HTMLElement; let focusMonitor: FocusMonitor; + let viewportRuler: ViewportRuler; function createComponent(component: Type, providers: Provider[] = [], @@ -60,11 +61,13 @@ describe('MatMenu', () => { providers }).compileComponents(); - inject([OverlayContainer, FocusMonitor], (oc: OverlayContainer, fm: FocusMonitor) => { - overlayContainer = oc; - overlayContainerElement = oc.getContainerElement(); - focusMonitor = fm; - })(); + inject([OverlayContainer, FocusMonitor, ViewportRuler], + (oc: OverlayContainer, fm: FocusMonitor, vr: ViewportRuler) => { + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); + focusMonitor = fm; + viewportRuler = vr; + })(); return TestBed.createComponent(component); } @@ -957,6 +960,28 @@ describe('MatMenu', () => { .toBe(false); }); + 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 b33a4409df47..b160bd0dad36 100644 --- a/src/material/menu/menu.ts +++ b/src/material/menu/menu.ts @@ -106,7 +106,7 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel @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.d.ts b/tools/public_api_guard/material/menu.d.ts index 009ff481e689..8383f91041bc 100644 --- a/tools/public_api_guard/material/menu.d.ts +++ b/tools/public_api_guard/material/menu.d.ts @@ -4,6 +4,7 @@ export declare class _MatMenuBase implements AfterContentInit, MatMenuPanel; _isAnimating: boolean; _panelAnimationState: 'void' | 'enter'; ariaDescribedby: string;