Skip to content

Commit 4429352

Browse files
crisbetocrisbeto
authored
fix(material/menu): account for menu padding different from the default (#16169)
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.
1 parent c296084 commit 4429352

File tree

6 files changed

+88
-21
lines changed

6 files changed

+88
-21
lines changed

‎src/material-experimental/mdc-menu/menu.scss

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ mat-menu {
3535
@include menu-common.base;
3636

3737
// The CDK positioning uses flexbox to anchor the element, whereas MDC uses `position: absolute`.
38-
position: static;
38+
// Furthermore, the relative position ensures that the `offsetParent` is the menu, rather than
39+
// the overlay. This comes into play when we measure the `offsetTop` of a menu item.
40+
position: relative;
3941
}
4042

4143
.mat-mdc-menu-item {
@@ -56,6 +58,7 @@ mat-menu {
5658
font-size: inherit;
5759
background: none;
5860
text-decoration: none;
61+
margin: 0; // Resolves an issue where buttons have an extra 2px margin on Safari.
5962

6063
// Set the `min-height` here ourselves, instead of going through
6164
// the `mdc-list-one-line-item-density` mixin, because it sets a `height`

‎src/material-experimental/mdc-menu/menu.spec.ts

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2137,7 +2137,7 @@ describe('MDC-based MatMenu', () => {
21372137
compileTestComponent();
21382138
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
21392139
instance.rootTriggerEl.nativeElement.style.left = '50px';
2140-
instance.rootTriggerEl.nativeElement.style.top = '50px';
2140+
instance.rootTriggerEl.nativeElement.style.top = '200px';
21412141
instance.rootTrigger.openMenu();
21422142
fixture.detectChanges();
21432143
tick(500);
@@ -2147,7 +2147,7 @@ describe('MDC-based MatMenu', () => {
21472147
tick(500);
21482148

21492149
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2150-
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
2150+
const panelRect = overlay.querySelectorAll('.mat-mdc-menu-panel')[1].getBoundingClientRect();
21512151

21522152
expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left));
21532153
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
@@ -2157,7 +2157,7 @@ describe('MDC-based MatMenu', () => {
21572157
compileTestComponent();
21582158
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
21592159
instance.rootTriggerEl.nativeElement.style.right = '10px';
2160-
instance.rootTriggerEl.nativeElement.style.top = '50%';
2160+
instance.rootTriggerEl.nativeElement.style.top = '200px';
21612161
instance.rootTrigger.openMenu();
21622162
fixture.detectChanges();
21632163
tick(500);
@@ -2167,7 +2167,7 @@ describe('MDC-based MatMenu', () => {
21672167
tick(500);
21682168

21692169
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2170-
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
2170+
const panelRect = overlay.querySelectorAll('.mat-mdc-menu-panel')[1].getBoundingClientRect();
21712171

21722172
expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right));
21732173
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
@@ -2177,7 +2177,7 @@ describe('MDC-based MatMenu', () => {
21772177
compileTestComponent('rtl');
21782178
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
21792179
instance.rootTriggerEl.nativeElement.style.left = '50%';
2180-
instance.rootTriggerEl.nativeElement.style.top = '50%';
2180+
instance.rootTriggerEl.nativeElement.style.top = '200px';
21812181
instance.rootTrigger.openMenu();
21822182
fixture.detectChanges();
21832183
tick(500);
@@ -2187,7 +2187,7 @@ describe('MDC-based MatMenu', () => {
21872187
tick(500);
21882188

21892189
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2190-
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
2190+
const panelRect = overlay.querySelectorAll('.mat-mdc-menu-panel')[1].getBoundingClientRect();
21912191

21922192
expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right));
21932193
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
@@ -2197,7 +2197,7 @@ describe('MDC-based MatMenu', () => {
21972197
compileTestComponent('rtl');
21982198
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
21992199
instance.rootTriggerEl.nativeElement.style.left = '10px';
2200-
instance.rootTriggerEl.nativeElement.style.top = '50%';
2200+
instance.rootTriggerEl.nativeElement.style.top = '200px';
22012201
instance.rootTrigger.openMenu();
22022202
fixture.detectChanges();
22032203
tick(500);
@@ -2207,12 +2207,32 @@ describe('MDC-based MatMenu', () => {
22072207
tick(500);
22082208

22092209
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2210-
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
2210+
const panelRect = overlay.querySelectorAll('.mat-mdc-menu-panel')[1].getBoundingClientRect();
22112211

22122212
expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left));
22132213
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
22142214
}));
22152215

2216+
it('should account for custom padding when offsetting the sub-menu', () => {
2217+
compileTestComponent();
2218+
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
2219+
instance.rootTriggerEl.nativeElement.style.left = '10px';
2220+
instance.rootTriggerEl.nativeElement.style.top = '200px';
2221+
instance.rootTrigger.openMenu();
2222+
fixture.detectChanges();
2223+
2224+
// Change the padding to something different from the default.
2225+
(overlay.querySelector('.mat-mdc-menu-content') as HTMLElement).style.padding = '15px 0';
2226+
2227+
instance.levelOneTrigger.openMenu();
2228+
fixture.detectChanges();
2229+
2230+
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2231+
const panelRect = overlay.querySelectorAll('.mat-mdc-menu-panel')[1].getBoundingClientRect();
2232+
2233+
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + 15);
2234+
});
2235+
22162236
it('should close all of the menus when an item is clicked', fakeAsync(() => {
22172237
compileTestComponent();
22182238
instance.rootTriggerEl.nativeElement.click();

‎src/material/menu/menu-trigger.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,11 @@ export const MAT_MENU_SCROLL_STRATEGY_FACTORY_PROVIDER = {
6565
useFactory: MAT_MENU_SCROLL_STRATEGY_FACTORY,
6666
};
6767

68-
/** Default top padding of the menu panel. */
68+
/**
69+
* Default top padding of the menu panel.
70+
* @deprecated No longer being used. Will be removed.
71+
* @breaking-change 15.0.0
72+
*/
6973
export const MENU_PANEL_TOP_PADDING = 8;
7074

7175
/** Options for binding a passive event listener. */
@@ -98,6 +102,12 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
98102
*/
99103
private _parentMaterialMenu: _MatMenuBase | undefined;
100104

105+
/**
106+
* Cached value of the padding of the parent menu panel.
107+
* Used to offset sub-menus to compensate for the padding.
108+
*/
109+
private _parentInnerPadding: number | undefined;
110+
101111
/**
102112
* Handles touch start events on the trigger.
103113
* Needs to be an arrow function so we can easily use addEventListener and removeEventListener.
@@ -511,7 +521,15 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
511521
// to the edges of the trigger, instead of overlapping it.
512522
overlayFallbackX = originX = menu.xPosition === 'before' ? 'start' : 'end';
513523
originFallbackX = overlayX = originX === 'end' ? 'start' : 'end';
514-
offsetY = overlayY === 'bottom' ? MENU_PANEL_TOP_PADDING : -MENU_PANEL_TOP_PADDING;
524+
525+
if (this._parentMaterialMenu) {
526+
if (this._parentInnerPadding == null) {
527+
const firstItem = this._parentMaterialMenu.items.first;
528+
this._parentInnerPadding = firstItem ? firstItem._getHostElement().offsetTop : 0;
529+
}
530+
531+
offsetY = overlayY === 'bottom' ? this._parentInnerPadding : -this._parentInnerPadding;
532+
}
515533
} else if (!menu.overlapTrigger) {
516534
originY = overlayY === 'top' ? 'bottom' : 'top';
517535
originFallbackY = overlayFallbackY === 'top' ? 'bottom' : 'top';

‎src/material/menu/menu.scss

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ mat-menu {
2222
// collapse it to zero when they scroll away.
2323
min-height: menu-common.$item-height + $vertical-padding * 2;
2424

25+
// Nothing in CSS depends upon this, but we need it so that the `offsetParent` in JS is the
26+
// menu rather than the overlay panel. This comes into play when we read the `offsetTop`.
27+
position: relative;
28+
2529
// Prevent users from interacting with the panel while it's animating. Note that
2630
// people won't be able to click through it, because the overlay pane will catch the click.
2731
// This fixes the following issues:

‎src/material/menu/menu.spec.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ import {
5252
MenuPositionX,
5353
MenuPositionY,
5454
} from './index';
55-
import {MAT_MENU_SCROLL_STRATEGY, MENU_PANEL_TOP_PADDING} from './menu-trigger';
55+
import {MAT_MENU_SCROLL_STRATEGY} from './menu-trigger';
56+
57+
const MENU_PANEL_TOP_PADDING = 8;
5658

5759
describe('MatMenu', () => {
5860
let overlayContainerElement: HTMLElement;
@@ -2128,7 +2130,7 @@ describe('MatMenu', () => {
21282130
compileTestComponent();
21292131
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
21302132
instance.rootTriggerEl.nativeElement.style.left = '50px';
2131-
instance.rootTriggerEl.nativeElement.style.top = '50px';
2133+
instance.rootTriggerEl.nativeElement.style.top = '200px';
21322134
instance.rootTrigger.openMenu();
21332135
fixture.detectChanges();
21342136
tick(500);
@@ -2138,7 +2140,7 @@ describe('MatMenu', () => {
21382140
tick(500);
21392141

21402142
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2141-
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
2143+
const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect();
21422144

21432145
expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left));
21442146
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
@@ -2148,7 +2150,7 @@ describe('MatMenu', () => {
21482150
compileTestComponent();
21492151
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
21502152
instance.rootTriggerEl.nativeElement.style.right = '10px';
2151-
instance.rootTriggerEl.nativeElement.style.top = '50%';
2153+
instance.rootTriggerEl.nativeElement.style.top = '200px';
21522154
instance.rootTrigger.openMenu();
21532155
fixture.detectChanges();
21542156
tick(500);
@@ -2158,7 +2160,7 @@ describe('MatMenu', () => {
21582160
tick(500);
21592161

21602162
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2161-
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
2163+
const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect();
21622164

21632165
expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right));
21642166
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
@@ -2168,7 +2170,7 @@ describe('MatMenu', () => {
21682170
compileTestComponent('rtl');
21692171
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
21702172
instance.rootTriggerEl.nativeElement.style.left = '50%';
2171-
instance.rootTriggerEl.nativeElement.style.top = '50%';
2173+
instance.rootTriggerEl.nativeElement.style.top = '200px';
21722174
instance.rootTrigger.openMenu();
21732175
fixture.detectChanges();
21742176
tick(500);
@@ -2178,7 +2180,7 @@ describe('MatMenu', () => {
21782180
tick(500);
21792181

21802182
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2181-
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
2183+
const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect();
21822184

21832185
expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right));
21842186
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
@@ -2188,7 +2190,7 @@ describe('MatMenu', () => {
21882190
compileTestComponent('rtl');
21892191
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
21902192
instance.rootTriggerEl.nativeElement.style.left = '10px';
2191-
instance.rootTriggerEl.nativeElement.style.top = '50%';
2193+
instance.rootTriggerEl.nativeElement.style.top = '200px';
21922194
instance.rootTrigger.openMenu();
21932195
fixture.detectChanges();
21942196
tick(500);
@@ -2198,12 +2200,32 @@ describe('MatMenu', () => {
21982200
tick(500);
21992201

22002202
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2201-
const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
2203+
const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect();
22022204

22032205
expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left));
22042206
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
22052207
}));
22062208

2209+
it('should account for custom padding when offsetting the sub-menu', () => {
2210+
compileTestComponent();
2211+
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
2212+
instance.rootTriggerEl.nativeElement.style.left = '10px';
2213+
instance.rootTriggerEl.nativeElement.style.top = '200px';
2214+
instance.rootTrigger.openMenu();
2215+
fixture.detectChanges();
2216+
2217+
// Change the padding to something different from the default.
2218+
(overlay.querySelector('.mat-menu-content') as HTMLElement).style.padding = '15px 0';
2219+
2220+
instance.levelOneTrigger.openMenu();
2221+
fixture.detectChanges();
2222+
2223+
const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
2224+
const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect();
2225+
2226+
expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + 15);
2227+
});
2228+
22072229
it('should close all of the menus when an item is clicked', fakeAsync(() => {
22082230
compileTestComponent();
22092231
instance.rootTriggerEl.nativeElement.click();

‎tools/public_api_guard/material/menu.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
321321
static ɵfac: i0.ɵɵFactoryDeclaration<_MatMenuTriggerBase, [null, null, null, null, { optional: true; }, { optional: true; self: true; }, { optional: true; }, null, null]>;
322322
}
323323

324-
// @public
324+
// @public @deprecated
325325
const MENU_PANEL_TOP_PADDING = 8;
326326

327327
// @public

0 commit comments

Comments
 (0)