Skip to content

Commit bcc79b0

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 0f9c1e6 commit bcc79b0

File tree

5 files changed

+72
-14
lines changed

5 files changed

+72
-14
lines changed

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

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import {
4141
MockNgZone,
4242
} from '@angular/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[] = [],
@@ -68,11 +69,13 @@ describe('MDC-based MatMenu', () => {
6869
providers
6970
}).compileComponents();
7071

71-
inject([OverlayContainer, FocusMonitor], (oc: OverlayContainer, fm: FocusMonitor) => {
72-
overlayContainer = oc;
73-
overlayContainerElement = oc.getContainerElement();
74-
focusMonitor = fm;
75-
})();
72+
inject([OverlayContainer, FocusMonitor, ViewportRuler],
73+
(oc: OverlayContainer, fm: FocusMonitor, vr: ViewportRuler) => {
74+
overlayContainer = oc;
75+
overlayContainerElement = oc.getContainerElement();
76+
focusMonitor = fm;
77+
viewportRuler = vr;
78+
})();
7679

7780
return TestBed.createComponent<T>(component);
7881
}
@@ -1082,6 +1085,28 @@ describe('MDC-based MatMenu', () => {
10821085
expect(document.activeElement).toBe(items[3], 'Expected fourth item to be focused');
10831086
}));
10841087

1088+
it('should keep the panel in the viewport when more items are added while open', () => {
1089+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
1090+
fixture.detectChanges();
1091+
1092+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
1093+
triggerEl.style.position = 'absolute';
1094+
triggerEl.style.left = '200px';
1095+
triggerEl.style.bottom = '300px';
1096+
triggerEl.click();
1097+
fixture.detectChanges();
1098+
1099+
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
1100+
const viewportHeight = viewportRuler.getViewportSize().height;
1101+
let panelRect = panel.getBoundingClientRect();
1102+
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
1103+
1104+
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
1105+
fixture.detectChanges();
1106+
panelRect = panel.getBoundingClientRect();
1107+
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
1108+
});
1109+
10851110
describe('lazy rendering', () => {
10861111
it('should be able to render the menu content lazily', fakeAsync(() => {
10871112
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: 31 additions & 6 deletions
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[] = [],
@@ -69,11 +70,13 @@ describe('MatMenu', () => {
6970
providers
7071
}).compileComponents();
7172

72-
inject([OverlayContainer, FocusMonitor], (oc: OverlayContainer, fm: FocusMonitor) => {
73-
overlayContainer = oc;
74-
overlayContainerElement = oc.getContainerElement();
75-
focusMonitor = fm;
76-
})();
73+
inject([OverlayContainer, FocusMonitor, ViewportRuler],
74+
(oc: OverlayContainer, fm: FocusMonitor, vr: ViewportRuler) => {
75+
overlayContainer = oc;
76+
overlayContainerElement = oc.getContainerElement();
77+
focusMonitor = fm;
78+
viewportRuler = vr;
79+
})();
7780

7881
return TestBed.createComponent<T>(component);
7982
}
@@ -1002,6 +1005,28 @@ describe('MatMenu', () => {
10021005
expect(document.activeElement).toBe(items[3], 'Expected fourth item to be focused');
10031006
}));
10041007

1008+
it('should keep the panel in the viewport when more items are added while open', () => {
1009+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
1010+
fixture.detectChanges();
1011+
1012+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
1013+
triggerEl.style.position = 'absolute';
1014+
triggerEl.style.left = '200px';
1015+
triggerEl.style.bottom = '300px';
1016+
triggerEl.click();
1017+
fixture.detectChanges();
1018+
1019+
const panel = overlayContainerElement.querySelector('.mat-menu-panel')!;
1020+
const viewportHeight = viewportRuler.getViewportSize().height;
1021+
let panelRect = panel.getBoundingClientRect();
1022+
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
1023+
1024+
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
1025+
fixture.detectChanges();
1026+
panelRect = panel.getBoundingClientRect();
1027+
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
1028+
});
1029+
10051030
describe('lazy rendering', () => {
10061031
it('should be able to render the menu content lazily', fakeAsync(() => {
10071032
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;

tools/public_api_guard/material/menu.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export declare class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatM
55
_classList: {
66
[key: string]: boolean;
77
};
8+
_directDescendantItems: QueryList<MatMenuItem>;
89
protected _elevationPrefix: string;
910
_isAnimating: boolean;
1011
_panelAnimationState: 'void' | 'enter';

0 commit comments

Comments
 (0)