Skip to content

Commit c02a6d6

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 51f5924 commit c02a6d6

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
}
@@ -1059,6 +1062,28 @@ describe('MDC-based MatMenu', () => {
10591062
expect(document.activeElement).toBe(items[3], 'Expected fourth item to be focused');
10601063
}));
10611064

1065+
it('should keep the panel in the viewport when more items are added while open', () => {
1066+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
1067+
fixture.detectChanges();
1068+
1069+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
1070+
triggerEl.style.position = 'absolute';
1071+
triggerEl.style.left = '200px';
1072+
triggerEl.style.bottom = '300px';
1073+
triggerEl.click();
1074+
fixture.detectChanges();
1075+
1076+
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
1077+
const viewportHeight = viewportRuler.getViewportSize().height;
1078+
let panelRect = panel.getBoundingClientRect();
1079+
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
1080+
1081+
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
1082+
fixture.detectChanges();
1083+
panelRect = panel.getBoundingClientRect();
1084+
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
1085+
});
1086+
10621087
describe('lazy rendering', () => {
10631088
it('should be able to render the menu content lazily', fakeAsync(() => {
10641089
const fixture = createComponent(SimpleLazyMenu);

src/material/menu/menu-trigger.ts

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

248248
const overlayRef = this._createOverlay();
249249
const overlayConfig = overlayRef.getConfig();
250+
const positionStrategy = overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy;
250251

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

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

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
@@ -111,7 +111,7 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
111111
@ContentChildren(MatMenuItem, {descendants: true}) _allItems: QueryList<MatMenuItem>;
112112

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

116116
/** Subscription to tab events on the menu panel */
117117
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)