Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(material/menu): adjust overlay size when amount of items changes #21457

Merged
merged 1 commit into from
Feb 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/cdk/overlay/position/flexible-connected-position-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,16 +360,22 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
* allows one to re-align the panel without changing the orientation of the panel.
*/
reapplyLastPosition(): void {
if (!this._isDisposed && (!this._platform || this._platform.isBrowser)) {
if (this._isDisposed || !this._platform.isBrowser) {
return;
}

const lastPosition = this._lastPosition;

if (lastPosition) {
this._originRect = this._getOriginRect();
this._overlayRect = this._pane.getBoundingClientRect();
this._viewportRect = this._getNarrowedViewportRect();
this._containerRect = this._overlayContainer.getContainerElement().getBoundingClientRect();

const lastPosition = this._lastPosition || this._preferredPositions[0];
const originPoint = this._getOriginPoint(this._originRect, this._containerRect, lastPosition);

this._applyPosition(lastPosition, originPoint);
} else {
this.apply();
}
}

Expand Down
26 changes: 25 additions & 1 deletion src/material-experimental/mdc-menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
MockNgZone,
} from '../../cdk/testing/private';
import {Subject} from 'rxjs';
import {ScrollDispatcher} from '@angular/cdk/scrolling';
import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling';
import {FocusMonitor} from '@angular/cdk/a11y';
import {
MAT_MENU_SCROLL_STRATEGY,
Expand All @@ -57,6 +57,7 @@ const MENU_PANEL_TOP_PADDING = 8;
describe('MDC-based MatMenu', () => {
let overlayContainerElement: HTMLElement;
let focusMonitor: FocusMonitor;
let viewportRuler: ViewportRuler;

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

overlayContainerElement = TestBed.inject(OverlayContainer).getContainerElement();
focusMonitor = TestBed.inject(FocusMonitor);
viewportRuler = TestBed.inject(ViewportRuler);
const fixture = TestBed.createComponent<T>(component);
window.scroll(0, 0);
return fixture;
Expand Down Expand Up @@ -1142,6 +1144,28 @@ describe('MDC-based MatMenu', () => {
expect(panel.classList).toContain('mat-menu-after');
}));

it('should keep the panel in the viewport when more items are added while open', () => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();

const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
triggerEl.style.position = 'absolute';
triggerEl.style.left = '200px';
triggerEl.style.bottom = '300px';
triggerEl.click();
fixture.detectChanges();

const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
const viewportHeight = viewportRuler.getViewportSize().height;
let panelRect = panel.getBoundingClientRect();
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);

fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
fixture.detectChanges();
panelRect = panel.getBoundingClientRect();
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
});

describe('lazy rendering', () => {
it('should be able to render the menu content lazily', fakeAsync(() => {
const fixture = createComponent(SimpleLazyMenu);
Expand Down
9 changes: 8 additions & 1 deletion src/material/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,9 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy

const overlayRef = this._createOverlay();
const overlayConfig = overlayRef.getConfig();
const positionStrategy = overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy;

this._setPosition(overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy);
this._setPosition(positionStrategy);
overlayConfig.hasBackdrop =
this.menu.hasBackdrop == null ? !this.triggersSubmenu() : this.menu.hasBackdrop;
overlayRef.attach(this._getPortal());
Expand All @@ -293,6 +294,12 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy

if (this.menu instanceof _MatMenuBase) {
this.menu._startAnimation();
this.menu._directDescendantItems.changes.pipe(takeUntil(this.menu.close)).subscribe(() => {
// Re-adjust the position without locking when the amount of items
// changes so that the overlay is allowed to pick a new optimal position.
positionStrategy.withLockedPosition(false).reapplyLastPosition();
positionStrategy.withLockedPosition(true);
});
}
}

Expand Down
26 changes: 25 additions & 1 deletion src/material/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
TAB,
} from '@angular/cdk/keycodes';
import {Overlay, OverlayContainer} from '@angular/cdk/overlay';
import {ScrollDispatcher} from '@angular/cdk/scrolling';
import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling';
import {
createKeyboardEvent,
createMouseEvent,
Expand Down Expand Up @@ -57,6 +57,7 @@ import {MAT_MENU_SCROLL_STRATEGY, MENU_PANEL_TOP_PADDING} from './menu-trigger';
describe('MatMenu', () => {
let overlayContainerElement: HTMLElement;
let focusMonitor: FocusMonitor;
let viewportRuler: ViewportRuler;

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

overlayContainerElement = TestBed.inject(OverlayContainer).getContainerElement();
focusMonitor = TestBed.inject(FocusMonitor);
viewportRuler = TestBed.inject(ViewportRuler);
const fixture = TestBed.createComponent<T>(component);
window.scroll(0, 0);
return fixture;
Expand Down Expand Up @@ -1137,6 +1139,28 @@ describe('MatMenu', () => {
expect(panel.classList).toContain('mat-menu-after');
}));

it('should keep the panel in the viewport when more items are added while open', () => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();

const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
triggerEl.style.position = 'absolute';
triggerEl.style.left = '200px';
triggerEl.style.bottom = '300px';
triggerEl.click();
fixture.detectChanges();

const panel = overlayContainerElement.querySelector('.mat-menu-panel')!;
const viewportHeight = viewportRuler.getViewportSize().height;
let panelRect = panel.getBoundingClientRect();
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);

fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
fixture.detectChanges();
panelRect = panel.getBoundingClientRect();
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
});

describe('lazy rendering', () => {
it('should be able to render the menu content lazily', fakeAsync(() => {
const fixture = createComponent(SimpleLazyMenu);
Expand Down
2 changes: 1 addition & 1 deletion src/material/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class _MatMenuBase
@ContentChildren(MatMenuItem, {descendants: true}) _allItems: QueryList<MatMenuItem>;

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

/** Subscription to tab events on the menu panel */
private _tabSubscription = Subscription.EMPTY;
Expand Down
1 change: 1 addition & 0 deletions tools/public_api_guard/material/menu.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
// @deprecated
readonly close: EventEmitter<MenuCloseReason>;
readonly closed: EventEmitter<MenuCloseReason>;
_directDescendantItems: QueryList<MatMenuItem>;
direction: Direction;
// (undocumented)
protected _elevationPrefix: string;
Expand Down