Skip to content

Commit 11be78f

Browse files
crisbetommalerba
authored andcommitted
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.
1 parent 0742bab commit 11be78f

File tree

4 files changed

+59
-4
lines changed

4 files changed

+59
-4
lines changed

src/material-experimental/mdc-menu/menu.spec.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import {
4141
MockNgZone,
4242
} from '../../cdk/testing/private';
4343
import {Subject} from 'rxjs';
44-
import {ScrollDispatcher} from '@angular/cdk/scrolling';
44+
import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling';
4545
import {FocusMonitor} from '@angular/cdk/a11y';
4646
import {
4747
MAT_MENU_SCROLL_STRATEGY,
@@ -58,6 +58,7 @@ describe('MDC-based MatMenu', () => {
5858
let overlayContainer: OverlayContainer;
5959
let overlayContainerElement: HTMLElement;
6060
let focusMonitor: FocusMonitor;
61+
let viewportRuler: ViewportRuler;
6162

6263
function createComponent<T>(component: Type<T>,
6364
providers: Provider[] = [],
@@ -71,6 +72,7 @@ describe('MDC-based MatMenu', () => {
7172
overlayContainer = TestBed.inject(OverlayContainer);
7273
overlayContainerElement = overlayContainer.getContainerElement();
7374
focusMonitor = TestBed.inject(FocusMonitor);
75+
viewportRuler = TestBed.inject(ViewportRuler);
7476
const fixture = TestBed.createComponent<T>(component);
7577
window.scroll(0, 0);
7678
return fixture;
@@ -1115,6 +1117,28 @@ describe('MDC-based MatMenu', () => {
11151117
expect(panel.classList).toContain('mat-menu-after');
11161118
}));
11171119

1120+
it('should keep the panel in the viewport when more items are added while open', () => {
1121+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
1122+
fixture.detectChanges();
1123+
1124+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
1125+
triggerEl.style.position = 'absolute';
1126+
triggerEl.style.left = '200px';
1127+
triggerEl.style.bottom = '300px';
1128+
triggerEl.click();
1129+
fixture.detectChanges();
1130+
1131+
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
1132+
const viewportHeight = viewportRuler.getViewportSize().height;
1133+
let panelRect = panel.getBoundingClientRect();
1134+
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
1135+
1136+
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
1137+
fixture.detectChanges();
1138+
panelRect = panel.getBoundingClientRect();
1139+
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
1140+
});
1141+
11181142
describe('lazy rendering', () => {
11191143
it('should be able to render the menu content lazily', fakeAsync(() => {
11201144
const fixture = createComponent(SimpleLazyMenu);

src/material/menu/menu-trigger.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,9 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
256256

257257
const overlayRef = this._createOverlay();
258258
const overlayConfig = overlayRef.getConfig();
259+
const positionStrategy = overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy;
259260

260-
this._setPosition(overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy);
261+
this._setPosition(positionStrategy);
261262
overlayConfig.hasBackdrop = this.menu.hasBackdrop == null ? !this.triggersSubmenu() :
262263
this.menu.hasBackdrop;
263264
overlayRef.attach(this._getPortal());
@@ -271,6 +272,12 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
271272

272273
if (this.menu instanceof _MatMenuBase) {
273274
this.menu._startAnimation();
275+
this.menu._directDescendantItems.changes.pipe(takeUntil(this.menu.close)).subscribe(() => {
276+
// Re-adjust the position without locking when the amount of items
277+
// changes so that the overlay is allowed to pick a new optimal position.
278+
positionStrategy.withLockedPosition(false).reapplyLastPosition();
279+
positionStrategy.withLockedPosition(true);
280+
});
274281
}
275282
}
276283

src/material/menu/menu.spec.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
TAB,
1212
} from '@angular/cdk/keycodes';
1313
import {Overlay, OverlayContainer} from '@angular/cdk/overlay';
14-
import {ScrollDispatcher} from '@angular/cdk/scrolling';
14+
import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling';
1515
import {
1616
createKeyboardEvent,
1717
createMouseEvent,
@@ -59,6 +59,7 @@ describe('MatMenu', () => {
5959
let overlayContainer: OverlayContainer;
6060
let overlayContainerElement: HTMLElement;
6161
let focusMonitor: FocusMonitor;
62+
let viewportRuler: ViewportRuler;
6263

6364
function createComponent<T>(component: Type<T>,
6465
providers: Provider[] = [],
@@ -72,6 +73,7 @@ describe('MatMenu', () => {
7273
overlayContainer = TestBed.inject(OverlayContainer);
7374
overlayContainerElement = overlayContainer.getContainerElement();
7475
focusMonitor = TestBed.inject(FocusMonitor);
76+
viewportRuler = TestBed.inject(ViewportRuler);
7577
const fixture = TestBed.createComponent<T>(component);
7678
window.scroll(0, 0);
7779
return fixture;
@@ -1115,6 +1117,28 @@ describe('MatMenu', () => {
11151117
expect(panel.classList).toContain('mat-menu-after');
11161118
}));
11171119

1120+
it('should keep the panel in the viewport when more items are added while open', () => {
1121+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
1122+
fixture.detectChanges();
1123+
1124+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
1125+
triggerEl.style.position = 'absolute';
1126+
triggerEl.style.left = '200px';
1127+
triggerEl.style.bottom = '300px';
1128+
triggerEl.click();
1129+
fixture.detectChanges();
1130+
1131+
const panel = overlayContainerElement.querySelector('.mat-menu-panel')!;
1132+
const viewportHeight = viewportRuler.getViewportSize().height;
1133+
let panelRect = panel.getBoundingClientRect();
1134+
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
1135+
1136+
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
1137+
fixture.detectChanges();
1138+
panelRect = panel.getBoundingClientRect();
1139+
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
1140+
});
1141+
11181142
describe('lazy rendering', () => {
11191143
it('should be able to render the menu content lazily', fakeAsync(() => {
11201144
const fixture = createComponent(SimpleLazyMenu);

src/material/menu/menu.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
107107
@ContentChildren(MatMenuItem, {descendants: true}) _allItems: QueryList<MatMenuItem>;
108108

109109
/** Only the direct descendant menu items. */
110-
private _directDescendantItems = new QueryList<MatMenuItem>();
110+
_directDescendantItems = new QueryList<MatMenuItem>();
111111

112112
/** Subscription to tab events on the menu panel */
113113
private _tabSubscription = Subscription.EMPTY;

0 commit comments

Comments
 (0)