Skip to content

Commit

Permalink
fix(menu): account for menu padding different from the default
Browse files Browse the repository at this point in the history
Currently we use the hardcoded value of the menu padding to offset the panel so the two menu triggers align. Since the value is in TS, the logic breaks down if the user has set their own padding. These changes add some logic to compute the padding the first time a sub-menu is opened.

Fixes #16167.
  • Loading branch information
crisbeto authored and jelbourn committed Sep 6, 2019
1 parent d3f63a3 commit eeecead
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 5 deletions.
20 changes: 20 additions & 0 deletions src/material-experimental/mdc-menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1713,6 +1713,26 @@ describe('MatMenu', () => {
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
}));

it('should account for custom padding when offsetting the sub-menu', () => {
compileTestComponent();
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
instance.rootTriggerEl.nativeElement.style.left = '75px';
instance.rootTriggerEl.nativeElement.style.top = '75px';
instance.rootTrigger.openMenu();
fixture.detectChanges();

// Change the padding to something different from the default.
(overlay.querySelector('.mat-mdc-menu-content') as HTMLElement).style.padding = '15px 0';

instance.levelOneTrigger.openMenu();
fixture.detectChanges();

const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();

expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + 15);
});

it('should close all of the menus when an item is clicked', fakeAsync(() => {
compileTestComponent();
instance.rootTriggerEl.nativeElement.click();
Expand Down
18 changes: 14 additions & 4 deletions src/material/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ export const MAT_MENU_SCROLL_STRATEGY_FACTORY_PROVIDER = {
useFactory: MAT_MENU_SCROLL_STRATEGY_FACTORY,
};

/** Default top padding of the menu panel. */
export const MENU_PANEL_TOP_PADDING = 8;

/** Options for binding a passive event listener. */
const passiveEventListenerOptions = normalizePassiveListenerOptions({passive: true});

Expand Down Expand Up @@ -91,6 +88,12 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
private _menuCloseSubscription = Subscription.EMPTY;
private _scrollStrategy: () => ScrollStrategy;

/**
* Cached value of the padding of the parent menu panel.
* Used to offset sub-menus to compensate for the padding.
*/
private _parentInnerPadding: number | undefined;

/**
* Handles touch start events on the trigger.
* Needs to be an arrow function so we can easily use addEventListener and removeEventListener.
Expand Down Expand Up @@ -450,11 +453,18 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
let offsetY = 0;

if (this.triggersSubmenu()) {
// console.log(this._parentMenu.items.first._getHostElement().offsetTop);
// When the menu is a sub-menu, it should always align itself
// to the edges of the trigger, instead of overlapping it.
overlayFallbackX = originX = this.menu.xPosition === 'before' ? 'start' : 'end';
originFallbackX = overlayX = originX === 'end' ? 'start' : 'end';
offsetY = overlayY === 'bottom' ? MENU_PANEL_TOP_PADDING : -MENU_PANEL_TOP_PADDING;

if (this._parentInnerPadding == null) {
const firstSibling = this._parentMenu.items.first;
this._parentInnerPadding = firstSibling ? firstSibling._getHostElement().offsetTop : 0;
}

offsetY = overlayY === 'bottom' ? this._parentInnerPadding : -this._parentInnerPadding;
} else if (!this.menu.overlapTrigger) {
originY = overlayY === 'top' ? 'bottom' : 'top';
originFallbackY = overlayFallbackY === 'top' ? 'bottom' : 'top';
Expand Down
23 changes: 22 additions & 1 deletion src/material/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ import {
MenuPositionX,
MenuPositionY,
} from './index';
import {MAT_MENU_SCROLL_STRATEGY, MENU_PANEL_TOP_PADDING} from './menu-trigger';
import {MAT_MENU_SCROLL_STRATEGY} from './menu-trigger';

const MENU_PANEL_TOP_PADDING = 8;

describe('MatMenu', () => {
let overlayContainer: OverlayContainer;
Expand Down Expand Up @@ -1722,6 +1723,26 @@ describe('MatMenu', () => {
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
}));

it('should account for custom padding when offsetting the sub-menu', () => {
compileTestComponent();
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
instance.rootTriggerEl.nativeElement.style.left = '75px';
instance.rootTriggerEl.nativeElement.style.top = '75px';
instance.rootTrigger.openMenu();
fixture.detectChanges();

// Change the padding to something different from the default.
(overlay.querySelector('.mat-menu-content') as HTMLElement).style.padding = '15px 0';

instance.levelOneTrigger.openMenu();
fixture.detectChanges();

const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();

expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + 15);
});

it('should close all of the menus when an item is clicked', fakeAsync(() => {
compileTestComponent();
instance.rootTriggerEl.nativeElement.click();
Expand Down

0 comments on commit eeecead

Please sign in to comment.