From 6965f95e482ee0132de050e565d5256fea90ec17 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 27 Feb 2022 12:51:05 +0100 Subject: [PATCH] fix(material/menu): account for menu padding different from the default 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. --- src/material-experimental/mdc-menu/menu.scss | 7 +++- .../mdc-menu/menu.spec.ts | 36 +++++++++++++---- src/material/menu/menu-trigger.ts | 24 ++++++++++- src/material/menu/menu.scss | 4 ++ src/material/menu/menu.spec.ts | 40 ++++++++++++++----- tools/public_api_guard/material/menu.md | 2 +- 6 files changed, 92 insertions(+), 21 deletions(-) diff --git a/src/material-experimental/mdc-menu/menu.scss b/src/material-experimental/mdc-menu/menu.scss index ea8f3fc64337..bca9187692d9 100644 --- a/src/material-experimental/mdc-menu/menu.scss +++ b/src/material-experimental/mdc-menu/menu.scss @@ -35,7 +35,12 @@ mat-menu { @include menu-common.base; // The CDK positioning uses flexbox to anchor the element, whereas MDC uses `position: absolute`. - position: static; + // Furthermore, the relative position ensures that the `offsetParent` is the menu, rather than + // the overlay. This comes into play when we measure the `offsetTop` of a menu item. + position: relative; + + // MDC sets this to `inline-block` which messes up tests. + display: block; } .mat-mdc-menu-item { diff --git a/src/material-experimental/mdc-menu/menu.spec.ts b/src/material-experimental/mdc-menu/menu.spec.ts index ea73313c0815..44b959838815 100644 --- a/src/material-experimental/mdc-menu/menu.spec.ts +++ b/src/material-experimental/mdc-menu/menu.spec.ts @@ -2137,7 +2137,7 @@ describe('MDC-based MatMenu', () => { compileTestComponent(); instance.rootTriggerEl.nativeElement.style.position = 'fixed'; instance.rootTriggerEl.nativeElement.style.left = '50px'; - instance.rootTriggerEl.nativeElement.style.top = '50px'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; instance.rootTrigger.openMenu(); fixture.detectChanges(); tick(500); @@ -2147,7 +2147,7 @@ describe('MDC-based MatMenu', () => { tick(500); const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect(); - const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect(); + const panelRect = overlay.querySelectorAll('.mat-mdc-menu-panel')[1].getBoundingClientRect(); expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left)); expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING); @@ -2157,7 +2157,7 @@ describe('MDC-based MatMenu', () => { compileTestComponent(); instance.rootTriggerEl.nativeElement.style.position = 'fixed'; instance.rootTriggerEl.nativeElement.style.right = '10px'; - instance.rootTriggerEl.nativeElement.style.top = '50%'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; instance.rootTrigger.openMenu(); fixture.detectChanges(); tick(500); @@ -2167,7 +2167,7 @@ describe('MDC-based MatMenu', () => { tick(500); const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect(); - const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect(); + const panelRect = overlay.querySelectorAll('.mat-mdc-menu-panel')[1].getBoundingClientRect(); expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right)); expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING); @@ -2177,7 +2177,7 @@ describe('MDC-based MatMenu', () => { compileTestComponent('rtl'); instance.rootTriggerEl.nativeElement.style.position = 'fixed'; instance.rootTriggerEl.nativeElement.style.left = '50%'; - instance.rootTriggerEl.nativeElement.style.top = '50%'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; instance.rootTrigger.openMenu(); fixture.detectChanges(); tick(500); @@ -2187,7 +2187,7 @@ describe('MDC-based MatMenu', () => { tick(500); const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect(); - const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect(); + const panelRect = overlay.querySelectorAll('.mat-mdc-menu-panel')[1].getBoundingClientRect(); expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right)); expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING); @@ -2197,7 +2197,7 @@ describe('MDC-based MatMenu', () => { compileTestComponent('rtl'); instance.rootTriggerEl.nativeElement.style.position = 'fixed'; instance.rootTriggerEl.nativeElement.style.left = '10px'; - instance.rootTriggerEl.nativeElement.style.top = '50%'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; instance.rootTrigger.openMenu(); fixture.detectChanges(); tick(500); @@ -2207,12 +2207,32 @@ describe('MDC-based MatMenu', () => { tick(500); const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect(); - const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect(); + const panelRect = overlay.querySelectorAll('.mat-mdc-menu-panel')[1].getBoundingClientRect(); expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left)); 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 = '10px'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; + 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('.mat-mdc-menu-panel')[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(); diff --git a/src/material/menu/menu-trigger.ts b/src/material/menu/menu-trigger.ts index 7c4bbdb859de..8e7004bb5209 100644 --- a/src/material/menu/menu-trigger.ts +++ b/src/material/menu/menu-trigger.ts @@ -65,7 +65,11 @@ export const MAT_MENU_SCROLL_STRATEGY_FACTORY_PROVIDER = { useFactory: MAT_MENU_SCROLL_STRATEGY_FACTORY, }; -/** Default top padding of the menu panel. */ +/** + * Default top padding of the menu panel. + * @deprecated No longer being used. Will be removed. + * @breaking-change 15.0.0 + */ export const MENU_PANEL_TOP_PADDING = 8; /** Options for binding a passive event listener. */ @@ -98,6 +102,12 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy */ private _parentMaterialMenu: _MatMenuBase | undefined; + /** + * 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. @@ -511,7 +521,17 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy // to the edges of the trigger, instead of overlapping it. overlayFallbackX = originX = menu.xPosition === 'before' ? 'start' : 'end'; originFallbackX = overlayX = originX === 'end' ? 'start' : 'end'; - offsetY = overlayY === 'bottom' ? MENU_PANEL_TOP_PADDING : -MENU_PANEL_TOP_PADDING; + + if (this._parentMaterialMenu) { + if (this._parentInnerPadding == null) { + const firstItem = this._parentMaterialMenu.items.first; + this._parentInnerPadding = firstItem ? firstItem._getHostElement().offsetTop : 0; + } + + offsetY = overlayY === 'bottom' ? this._parentInnerPadding : -this._parentInnerPadding; + } + + console.log('calculated offset', offsetY); } else if (!menu.overlapTrigger) { originY = overlayY === 'top' ? 'bottom' : 'top'; originFallbackY = overlayFallbackY === 'top' ? 'bottom' : 'top'; diff --git a/src/material/menu/menu.scss b/src/material/menu/menu.scss index 353986269014..bbb3c57510d6 100644 --- a/src/material/menu/menu.scss +++ b/src/material/menu/menu.scss @@ -22,6 +22,10 @@ mat-menu { // collapse it to zero when they scroll away. min-height: menu-common.$item-height + $vertical-padding * 2; + // Nothing in CSS depends upon this, but we need it so that the `offsetParent` in JS is the + // menu rather than the overlay panel. This comes into play when we read the `offsetTop`. + position: relative; + // Prevent users from interacting with the panel while it's animating. Note that // people won't be able to click through it, because the overlay pane will catch the click. // This fixes the following issues: diff --git a/src/material/menu/menu.spec.ts b/src/material/menu/menu.spec.ts index 77c21ca8bd32..8f7e52cf46c2 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -52,7 +52,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 overlayContainerElement: HTMLElement; @@ -2128,7 +2130,7 @@ describe('MatMenu', () => { compileTestComponent(); instance.rootTriggerEl.nativeElement.style.position = 'fixed'; instance.rootTriggerEl.nativeElement.style.left = '50px'; - instance.rootTriggerEl.nativeElement.style.top = '50px'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; instance.rootTrigger.openMenu(); fixture.detectChanges(); tick(500); @@ -2138,7 +2140,7 @@ describe('MatMenu', () => { tick(500); const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect(); - const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect(); + const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect(); expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left)); expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING); @@ -2148,7 +2150,7 @@ describe('MatMenu', () => { compileTestComponent(); instance.rootTriggerEl.nativeElement.style.position = 'fixed'; instance.rootTriggerEl.nativeElement.style.right = '10px'; - instance.rootTriggerEl.nativeElement.style.top = '50%'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; instance.rootTrigger.openMenu(); fixture.detectChanges(); tick(500); @@ -2158,7 +2160,7 @@ describe('MatMenu', () => { tick(500); const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect(); - const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect(); + const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect(); expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right)); expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING); @@ -2168,7 +2170,7 @@ describe('MatMenu', () => { compileTestComponent('rtl'); instance.rootTriggerEl.nativeElement.style.position = 'fixed'; instance.rootTriggerEl.nativeElement.style.left = '50%'; - instance.rootTriggerEl.nativeElement.style.top = '50%'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; instance.rootTrigger.openMenu(); fixture.detectChanges(); tick(500); @@ -2178,7 +2180,7 @@ describe('MatMenu', () => { tick(500); const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect(); - const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect(); + const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect(); expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right)); expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING); @@ -2188,7 +2190,7 @@ describe('MatMenu', () => { compileTestComponent('rtl'); instance.rootTriggerEl.nativeElement.style.position = 'fixed'; instance.rootTriggerEl.nativeElement.style.left = '10px'; - instance.rootTriggerEl.nativeElement.style.top = '50%'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; instance.rootTrigger.openMenu(); fixture.detectChanges(); tick(500); @@ -2198,12 +2200,32 @@ describe('MatMenu', () => { tick(500); const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect(); - const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect(); + const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect(); expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left)); 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 = '10px'; + instance.rootTriggerEl.nativeElement.style.top = '200px'; + 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('.mat-menu-panel')[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(); diff --git a/tools/public_api_guard/material/menu.md b/tools/public_api_guard/material/menu.md index 264db2cc79f1..9ad5863217a9 100644 --- a/tools/public_api_guard/material/menu.md +++ b/tools/public_api_guard/material/menu.md @@ -321,7 +321,7 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy static ɵfac: i0.ɵɵFactoryDeclaration<_MatMenuTriggerBase, [null, null, null, null, { optional: true; }, { optional: true; self: true; }, { optional: true; }, null, null]>; } -// @public +// @public @deprecated const MENU_PANEL_TOP_PADDING = 8; // @public