Skip to content

Commit fb6d88f

Browse files
committed
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 03485cd commit fb6d88f

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,
@@ -57,6 +57,7 @@ const MENU_PANEL_TOP_PADDING = 8;
5757
describe('MDC-based MatMenu', () => {
5858
let overlayContainerElement: HTMLElement;
5959
let focusMonitor: FocusMonitor;
60+
let viewportRuler: ViewportRuler;
6061

6162
function createComponent<T>(
6263
component: Type<T>,
@@ -71,6 +72,7 @@ describe('MDC-based MatMenu', () => {
7172

7273
overlayContainerElement = TestBed.inject(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;
@@ -1140,6 +1142,28 @@ describe('MDC-based MatMenu', () => {
11401142
expect(panel.classList).toContain('mat-menu-after');
11411143
}));
11421144

1145+
it('should keep the panel in the viewport when more items are added while open', () => {
1146+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
1147+
fixture.detectChanges();
1148+
1149+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
1150+
triggerEl.style.position = 'absolute';
1151+
triggerEl.style.left = '200px';
1152+
triggerEl.style.bottom = '300px';
1153+
triggerEl.click();
1154+
fixture.detectChanges();
1155+
1156+
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
1157+
const viewportHeight = viewportRuler.getViewportSize().height;
1158+
let panelRect = panel.getBoundingClientRect();
1159+
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
1160+
1161+
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
1162+
fixture.detectChanges();
1163+
panelRect = panel.getBoundingClientRect();
1164+
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
1165+
});
1166+
11431167
describe('lazy rendering', () => {
11441168
it('should be able to render the menu content lazily', fakeAsync(() => {
11451169
const fixture = createComponent(SimpleLazyMenu);

src/material/menu/menu-trigger.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,9 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
265265

266266
const overlayRef = this._createOverlay();
267267
const overlayConfig = overlayRef.getConfig();
268+
const positionStrategy = overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy;
268269

269-
this._setPosition(overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy);
270+
this._setPosition(positionStrategy);
270271
overlayConfig.hasBackdrop =
271272
this.menu.hasBackdrop == null ? !this.triggersSubmenu() : this.menu.hasBackdrop;
272273
overlayRef.attach(this._getPortal());
@@ -280,6 +281,12 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
280281

281282
if (this.menu instanceof _MatMenuBase) {
282283
this.menu._startAnimation();
284+
this.menu._directDescendantItems.changes.pipe(takeUntil(this.menu.close)).subscribe(() => {
285+
// Re-adjust the position without locking when the amount of items
286+
// changes so that the overlay is allowed to pick a new optimal position.
287+
positionStrategy.withLockedPosition(false).reapplyLastPosition();
288+
positionStrategy.withLockedPosition(true);
289+
});
283290
}
284291
}
285292

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,
@@ -57,6 +57,7 @@ import {MAT_MENU_SCROLL_STRATEGY, MENU_PANEL_TOP_PADDING} from './menu-trigger';
5757
describe('MatMenu', () => {
5858
let overlayContainerElement: HTMLElement;
5959
let focusMonitor: FocusMonitor;
60+
let viewportRuler: ViewportRuler;
6061

6162
function createComponent<T>(
6263
component: Type<T>,
@@ -71,6 +72,7 @@ describe('MatMenu', () => {
7172

7273
overlayContainerElement = TestBed.inject(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;
@@ -1135,6 +1137,28 @@ describe('MatMenu', () => {
11351137
expect(panel.classList).toContain('mat-menu-after');
11361138
}));
11371139

1140+
it('should keep the panel in the viewport when more items are added while open', () => {
1141+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
1142+
fixture.detectChanges();
1143+
1144+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
1145+
triggerEl.style.position = 'absolute';
1146+
triggerEl.style.left = '200px';
1147+
triggerEl.style.bottom = '300px';
1148+
triggerEl.click();
1149+
fixture.detectChanges();
1150+
1151+
const panel = overlayContainerElement.querySelector('.mat-menu-panel')!;
1152+
const viewportHeight = viewportRuler.getViewportSize().height;
1153+
let panelRect = panel.getBoundingClientRect();
1154+
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
1155+
1156+
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
1157+
fixture.detectChanges();
1158+
panelRect = panel.getBoundingClientRect();
1159+
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
1160+
});
1161+
11381162
describe('lazy rendering', () => {
11391163
it('should be able to render the menu content lazily', fakeAsync(() => {
11401164
const fixture = createComponent(SimpleLazyMenu);

src/material/menu/menu.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export class _MatMenuBase
109109
@ContentChildren(MatMenuItem, {descendants: true}) _allItems: QueryList<MatMenuItem>;
110110

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

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

0 commit comments

Comments
 (0)