From 03d00f57347bf6f0669dd2fc8fdb70fee4b22afd Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 12 Jun 2024 12:13:25 +0200 Subject: [PATCH] fix(material/menu): update elevation logic for M3 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. --- src/material/core/tokens/m2/mat/_menu.scss | 3 +++ src/material/core/tokens/m3/mat/_menu.scss | 3 +++ src/material/menu/menu.spec.ts | 28 +++++++++++----------- src/material/menu/menu.ts | 13 +++++++++- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/material/core/tokens/m2/mat/_menu.scss b/src/material/core/tokens/m2/mat/_menu.scss index 79c9155eeb26..e3b04cfe5fb5 100644 --- a/src/material/core/tokens/m2/mat/_menu.scss +++ b/src/material/core/tokens/m2/mat/_menu.scss @@ -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, ); } diff --git a/src/material/core/tokens/m3/mat/_menu.scss b/src/material/core/tokens/m3/mat/_menu.scss index bbb36c889c4f..ddc173329e55 100644 --- a/src/material/core/tokens/m3/mat/_menu.scss +++ b/src/material/core/tokens/m3/mat/_menu.scss @@ -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), ) ); diff --git a/src/material/menu/menu.spec.ts b/src/material/menu/menu.spec.ts index 7e538385988d..2de78b8f76e0 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -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(); @@ -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(() => { @@ -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(() => { @@ -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(); @@ -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(() => { @@ -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(() => { @@ -2762,7 +2762,7 @@ const SIMPLE_MENU_TEMPLATE = ` - @for (item of extraItems; track item) { + @for (item of extraItems; track $index) { } @@ -2949,7 +2949,7 @@ class NestedMenuCustomElevation { template: ` - @for (item of items; track item) { + @for (item of items; track $index) { - @for (item of items; track item) { + @for (item of items; track $index) { } @@ -3092,7 +3092,7 @@ class SimpleMenuWithRepeater { - @for (item of items; track item) { + @for (item of items; track $index) { } diff --git a/src/material/menu/menu.ts b/src/material/menu/menu.ts index c2e0716ec0fe..b625f64727e2 100644 --- a/src/material/menu/menu.ts +++ b/src/material/menu/menu.ts @@ -120,7 +120,7 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, 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; @@ -470,6 +470,17 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, 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);