Skip to content

Commit 57df72f

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 71b7b15 commit 57df72f

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
@@ -32,7 +32,7 @@ import {
3232
MockNgZone,
3333
} from '@angular/cdk/testing/private';
3434
import {Subject} from 'rxjs';
35-
import {ScrollDispatcher} from '@angular/cdk/scrolling';
35+
import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling';
3636
import {FocusMonitor} from '@angular/cdk/a11y';
3737
import {
3838
MAT_MENU_SCROLL_STRATEGY,
@@ -49,6 +49,7 @@ describe('MDC-based MatMenu', () => {
4949
let overlayContainer: OverlayContainer;
5050
let overlayContainerElement: HTMLElement;
5151
let focusMonitor: FocusMonitor;
52+
let viewportRuler: ViewportRuler;
5253

5354
function createComponent<T>(component: Type<T>,
5455
providers: Provider[] = [],
@@ -59,11 +60,13 @@ describe('MDC-based MatMenu', () => {
5960
providers
6061
}).compileComponents();
6162

62-
inject([OverlayContainer, FocusMonitor], (oc: OverlayContainer, fm: FocusMonitor) => {
63-
overlayContainer = oc;
64-
overlayContainerElement = oc.getContainerElement();
65-
focusMonitor = fm;
66-
})();
63+
inject([OverlayContainer, FocusMonitor, ViewportRuler],
64+
(oc: OverlayContainer, fm: FocusMonitor, vr: ViewportRuler) => {
65+
overlayContainer = oc;
66+
overlayContainerElement = oc.getContainerElement();
67+
focusMonitor = fm;
68+
viewportRuler = vr;
69+
})();
6770

6871
return TestBed.createComponent<T>(component);
6972
}
@@ -1014,6 +1017,28 @@ describe('MDC-based MatMenu', () => {
10141017
.toBe(false);
10151018
});
10161019

1020+
it('should keep the panel in the viewport when more items are added while open', () => {
1021+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
1022+
fixture.detectChanges();
1023+
1024+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
1025+
triggerEl.style.position = 'absolute';
1026+
triggerEl.style.left = '200px';
1027+
triggerEl.style.bottom = '300px';
1028+
triggerEl.click();
1029+
fixture.detectChanges();
1030+
1031+
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
1032+
const viewportHeight = viewportRuler.getViewportSize().height;
1033+
let panelRect = panel.getBoundingClientRect();
1034+
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
1035+
1036+
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
1037+
fixture.detectChanges();
1038+
panelRect = panel.getBoundingClientRect();
1039+
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
1040+
});
1041+
10171042
describe('lazy rendering', () => {
10181043
it('should be able to render the menu content lazily', fakeAsync(() => {
10191044
const fixture = createComponent(SimpleLazyMenu);

src/material/menu/menu-trigger.ts

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

249249
const overlayRef = this._createOverlay();
250250
const overlayConfig = overlayRef.getConfig();
251+
const positionStrategy = overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy;
251252

252-
this._setPosition(overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy);
253+
this._setPosition(positionStrategy);
253254
overlayConfig.hasBackdrop = this.menu.hasBackdrop == null ? !this.triggersSubmenu() :
254255
this.menu.hasBackdrop;
255256
overlayRef.attach(this._getPortal());
@@ -263,6 +264,12 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
263264

264265
if (this.menu instanceof _MatMenuBase) {
265266
this.menu._startAnimation();
267+
this.menu._directDescendantItems.changes.pipe(takeUntil(this.menu.close)).subscribe(() => {
268+
// Re-adjust the position without locking when the amount of items
269+
// changes so that the overlay is allowed to pick a new optimal position.
270+
positionStrategy.withLockedPosition(false).reapplyLastPosition();
271+
positionStrategy.withLockedPosition(true);
272+
});
266273
}
267274
}
268275

src/material/menu/menu.spec.ts

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {FocusMonitor} from '@angular/cdk/a11y';
22
import {Direction, Directionality} from '@angular/cdk/bidi';
33
import {DOWN_ARROW, END, ESCAPE, HOME, LEFT_ARROW, RIGHT_ARROW, TAB} from '@angular/cdk/keycodes';
44
import {Overlay, OverlayContainer} from '@angular/cdk/overlay';
5-
import {ScrollDispatcher} from '@angular/cdk/scrolling';
5+
import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling';
66
import {
77
createKeyboardEvent,
88
createMouseEvent,
@@ -50,6 +50,7 @@ describe('MatMenu', () => {
5050
let overlayContainer: OverlayContainer;
5151
let overlayContainerElement: HTMLElement;
5252
let focusMonitor: FocusMonitor;
53+
let viewportRuler: ViewportRuler;
5354

5455
function createComponent<T>(component: Type<T>,
5556
providers: Provider[] = [],
@@ -60,11 +61,13 @@ describe('MatMenu', () => {
6061
providers
6162
}).compileComponents();
6263

63-
inject([OverlayContainer, FocusMonitor], (oc: OverlayContainer, fm: FocusMonitor) => {
64-
overlayContainer = oc;
65-
overlayContainerElement = oc.getContainerElement();
66-
focusMonitor = fm;
67-
})();
64+
inject([OverlayContainer, FocusMonitor, ViewportRuler],
65+
(oc: OverlayContainer, fm: FocusMonitor, vr: ViewportRuler) => {
66+
overlayContainer = oc;
67+
overlayContainerElement = oc.getContainerElement();
68+
focusMonitor = fm;
69+
viewportRuler = vr;
70+
})();
6871

6972
return TestBed.createComponent<T>(component);
7073
}
@@ -957,6 +960,28 @@ describe('MatMenu', () => {
957960
.toBe(false);
958961
});
959962

963+
it('should keep the panel in the viewport when more items are added while open', () => {
964+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
965+
fixture.detectChanges();
966+
967+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
968+
triggerEl.style.position = 'absolute';
969+
triggerEl.style.left = '200px';
970+
triggerEl.style.bottom = '300px';
971+
triggerEl.click();
972+
fixture.detectChanges();
973+
974+
const panel = overlayContainerElement.querySelector('.mat-menu-panel')!;
975+
const viewportHeight = viewportRuler.getViewportSize().height;
976+
let panelRect = panel.getBoundingClientRect();
977+
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
978+
979+
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
980+
fixture.detectChanges();
981+
panelRect = panel.getBoundingClientRect();
982+
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
983+
});
984+
960985
describe('lazy rendering', () => {
961986
it('should be able to render the menu content lazily', fakeAsync(() => {
962987
const fixture = createComponent(SimpleLazyMenu);

src/material/menu/menu.ts

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

108108
/** Only the direct descendant menu items. */
109-
private _directDescendantItems = new QueryList<MatMenuItem>();
109+
_directDescendantItems = new QueryList<MatMenuItem>();
110110

111111
/** Subscription to tab events on the menu panel */
112112
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
@@ -4,6 +4,7 @@ export declare class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatM
44
_classList: {
55
[key: string]: boolean;
66
};
7+
_directDescendantItems: QueryList<MatMenuItem>;
78
_isAnimating: boolean;
89
_panelAnimationState: 'void' | 'enter';
910
ariaDescribedby: string;

0 commit comments

Comments
 (0)