Skip to content

Commit

Permalink
fix(material/menu): update elevation logic for M3
Browse files Browse the repository at this point in the history
The menu has some logic to increase its elevation depending on it depth which hasn't worked since we introduced M3. These changes update it to account for a different base elevation.

(cherry picked from commit 03d00f5)
  • Loading branch information
crisbeto committed Jun 13, 2024
1 parent 93bc609 commit 9988ef2
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 15 deletions.
3 changes: 3 additions & 0 deletions src/material/core/tokens/m2/mat/_menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ $prefix: (mat, menu);
item-trailing-spacing: 16px,
item-with-icon-leading-spacing: 16px,
item-with-icon-trailing-spacing: 16px,
// Note that this uses a value, rather than a computed box-shadow, because we use
// the value at runtime to determine which shadow to set based on the menu's depth.
base-elevation-level: 8,
);
}

Expand Down
3 changes: 3 additions & 0 deletions src/material/core/tokens/m3/mat/_menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ $prefix: (mat, menu);
item-with-icon-leading-spacing: token-utils.hardcode(12px, $exclude-hardcoded),
item-with-icon-trailing-spacing: token-utils.hardcode(12px, $exclude-hardcoded),
container-color: map.get($systems, md-sys-color, surface-container),
// Note that this uses a value, rather than a computed box-shadow, because we use
// the value at runtime to determine which shadow to set based on the menu's depth.
base-elevation-level: token-utils.hardcode(2, $exclude-hardcoded),
)
);

Expand Down
28 changes: 14 additions & 14 deletions src/material/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ describe('MDC-based MatMenu', () => {
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;

expect(panel.classList).toContain('custom-one');
expect(panel.classList).toContain('mat-elevation-z8');
expect(panel.classList).toContain('mat-elevation-z2');

fixture.componentInstance.panelClass = 'custom-two';
fixture.detectChanges();
Expand All @@ -615,8 +615,8 @@ describe('MDC-based MatMenu', () => {
expect(panel.classList).toContain('custom-two');
expect(panel.classList).toContain('mat-mdc-elevation-specific');
expect(panel.classList)
.withContext('Expected mat-elevation-z8 not to be removed')
.toContain('mat-elevation-z8');
.withContext('Expected mat-elevation-z2 not to be removed')
.toContain('mat-elevation-z2');
}));

it('should set the "menu" role on the overlay panel', fakeAsync(() => {
Expand Down Expand Up @@ -2369,17 +2369,17 @@ describe('MDC-based MatMenu', () => {
expect(menus[0].classList).toContain('mat-mdc-elevation-specific');
expect(menus[0].classList)
.withContext('Expected root menu to have base elevation.')
.toContain('mat-elevation-z8');
.toContain('mat-elevation-z2');

expect(menus[1].classList).toContain('mat-mdc-elevation-specific');
expect(menus[1].classList)
.withContext('Expected first sub-menu to have base elevation + 1.')
.toContain('mat-elevation-z9');
.toContain('mat-elevation-z3');

expect(menus[2].classList).toContain('mat-mdc-elevation-specific');
expect(menus[2].classList)
.withContext('Expected second sub-menu to have base elevation + 2.')
.toContain('mat-elevation-z10');
.toContain('mat-elevation-z4');
}));

it('should update the elevation when the same menu is opened at a different depth', fakeAsync(() => {
Expand All @@ -2398,7 +2398,7 @@ describe('MDC-based MatMenu', () => {
expect(lastMenu.classList).toContain('mat-mdc-elevation-specific');
expect(lastMenu.classList)
.withContext('Expected menu to have the base elevation plus two.')
.toContain('mat-elevation-z10');
.toContain('mat-elevation-z4');

(overlay.querySelector('.cdk-overlay-backdrop')! as HTMLElement).click();
fixture.detectChanges();
Expand All @@ -2417,10 +2417,10 @@ describe('MDC-based MatMenu', () => {
expect(lastMenu.classList).toContain('mat-mdc-elevation-specific');
expect(lastMenu.classList)
.not.withContext('Expected menu not to maintain old elevation.')
.toContain('mat-elevation-z10');
.toContain('mat-elevation-z4');
expect(lastMenu.classList)
.withContext('Expected menu to have the proper updated elevation.')
.toContain('mat-elevation-z8');
.toContain('mat-elevation-z2');
}));

it('should not change focus origin if origin not specified for trigger', fakeAsync(() => {
Expand Down Expand Up @@ -2461,7 +2461,7 @@ describe('MDC-based MatMenu', () => {
expect(menuClasses)
.withContext('Expected user elevation to be maintained')
.toContain('mat-elevation-z24');
expect(menuClasses).not.toContain('mat-elevation-z8', 'Expected no stacked elevation.');
expect(menuClasses).not.toContain('mat-elevation-z2', 'Expected no stacked elevation.');
}));

it('should close all of the menus when the root is closed programmatically', fakeAsync(() => {
Expand Down Expand Up @@ -2762,7 +2762,7 @@ const SIMPLE_MENU_TEMPLATE = `
<button mat-menu-item>
<span>Item with text inside span</span>
</button>
@for (item of extraItems; track item) {
@for (item of extraItems; track $index) {
<button mat-menu-item> {{item}} </button>
}
</mat-menu>
Expand Down Expand Up @@ -2949,7 +2949,7 @@ class NestedMenuCustomElevation {
template: `
<button [matMenuTriggerFor]="root" #rootTriggerEl>Toggle menu</button>
<mat-menu #root="matMenu">
@for (item of items; track item) {
@for (item of items; track $index) {
<button
mat-menu-item
class="level-one-trigger"
Expand Down Expand Up @@ -3071,7 +3071,7 @@ class MenuWithCheckboxItems {
template: `
<button [matMenuTriggerFor]="menu">Toggle menu</button>
<mat-menu #menu="matMenu">
@for (item of items; track item) {
@for (item of items; track $index) {
<button [disabled]="item.disabled"mat-menu-item>{{item.label}}</button>
}
</mat-menu>
Expand All @@ -3092,7 +3092,7 @@ class SimpleMenuWithRepeater {
<button [matMenuTriggerFor]="menu">Toggle menu</button>
<mat-menu #menu="matMenu">
<ng-template matMenuContent>
@for (item of items; track item) {
@for (item of items; track $index) {
<button [disabled]="item.disabled" mat-menu-item>{{item.label}}</button>
}
</ng-template>
Expand Down
13 changes: 12 additions & 1 deletion src/material/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnI
private _firstItemFocusRef?: AfterRenderRef;
private _previousElevation: string;
private _elevationPrefix = 'mat-elevation-z';
private _baseElevation = 8;
private _baseElevation: number | null = null;

/** All items inside the menu. Includes items nested inside another menu. */
@ContentChildren(MatMenuItem, {descendants: true}) _allItems: QueryList<MatMenuItem>;
Expand Down Expand Up @@ -470,6 +470,17 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnI
* @param depth Number of parent menus that come before the menu.
*/
setElevation(depth: number): void {
// The base elevation depends on which version of the spec
// we're running so we have to resolve it at runtime.
if (this._baseElevation === null) {
const styles =
typeof getComputedStyle === 'function'
? getComputedStyle(this._elementRef.nativeElement)
: null;
const value = styles?.getPropertyValue('--mat-menu-base-elevation-level') || '8';
this._baseElevation = parseInt(value);
}

// The elevation starts at the base and increases by one for each level.
// Capped at 24 because that's the maximum elevation defined in the Material design spec.
const elevation = Math.min(this._baseElevation + depth, 24);
Expand Down

0 comments on commit 9988ef2

Please sign in to comment.