From de5e57ad1d9a76922091ca3adbab35bb9843b5fd Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 5 Nov 2024 13:57:20 +0100 Subject: [PATCH] fix(material/menu): use static elevation (#29968) Currently the menu's elevation is set using elevation classes, because earlier versions of the Material spec called for the elevation to increase for each level in a nested menu. That's no longer part of the spec and in v19 we won't include the elevation classes by default anymore. These changes remove the code that was handling the dynamic elevation and set the elevation using a token instead. (cherry picked from commit b072047b8a909ac51cf39bef2d5333264f05633d) --- src/material/core/tokens/m2/mat/_menu.scss | 8 +- src/material/core/tokens/m3/mat/_menu.scss | 9 +- src/material/menu/menu-panel.ts | 5 + src/material/menu/menu-trigger.ts | 16 --- src/material/menu/menu.html | 2 +- src/material/menu/menu.scss | 1 + src/material/menu/menu.spec.ts | 142 --------------------- src/material/menu/menu.ts | 40 +----- tools/public_api_guard/material/menu.md | 5 +- 9 files changed, 24 insertions(+), 204 deletions(-) diff --git a/src/material/core/tokens/m2/mat/_menu.scss b/src/material/core/tokens/m2/mat/_menu.scss index 73a9e8b138b0..bf3e68248b23 100644 --- a/src/material/core/tokens/m2/mat/_menu.scss +++ b/src/material/core/tokens/m2/mat/_menu.scss @@ -1,6 +1,7 @@ @use '../../token-definition'; @use '../../../theming/inspection'; @use '../../../style/sass-utils'; +@use '../../../style/elevation'; // The prefix used to generate the fully qualified name for tokens in this file. $prefix: (mat, menu); @@ -18,9 +19,10 @@ $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, + container-elevation-shadow: elevation.get-box-shadow(8), + + // Unused + base-elevation-level: null, ); } diff --git a/src/material/core/tokens/m3/mat/_menu.scss b/src/material/core/tokens/m3/mat/_menu.scss index 5df22d6dee4e..c341e9a40465 100644 --- a/src/material/core/tokens/m3/mat/_menu.scss +++ b/src/material/core/tokens/m3/mat/_menu.scss @@ -1,6 +1,7 @@ @use 'sass:map'; @use '../../../style/sass-utils'; @use '../../token-definition'; +@use '../../../style/elevation'; // The prefix used to generate the fully qualified name for tokens in this file. $prefix: (mat, menu); @@ -35,9 +36,11 @@ $prefix: (mat, menu); item-with-icon-leading-spacing: token-definition.hardcode(12px, $exclude-hardcoded), item-with-icon-trailing-spacing: token-definition.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-definition.hardcode(2, $exclude-hardcoded), + container-elevation-shadow: token-definition.hardcode( + elevation.get-box-shadow(2), $exclude-hardcoded), + + // Unused + base-elevation-level: null, ) ); diff --git a/src/material/menu/menu-panel.ts b/src/material/menu/menu-panel.ts index 8bf5cb06e4d5..b6814cbc2828 100644 --- a/src/material/menu/menu-panel.ts +++ b/src/material/menu/menu-panel.ts @@ -33,6 +33,11 @@ export interface MatMenuPanel { focusFirstItem: (origin?: FocusOrigin) => void; resetActiveItem: () => void; setPositionClasses?: (x: MenuPositionX, y: MenuPositionY) => void; + + /** + * @deprecated No longer used and will be removed. + * @breaking-change 21.0.0 + */ setElevation?(depth: number): void; lazyContent?: MatMenuContent; backdropClass?: string; diff --git a/src/material/menu/menu-trigger.ts b/src/material/menu/menu-trigger.ts index 9a8f519779ef..a0c0afc187e2 100644 --- a/src/material/menu/menu-trigger.ts +++ b/src/material/menu/menu-trigger.ts @@ -382,26 +382,10 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { private _initMenu(menu: MatMenuPanel): void { menu.parentMenu = this.triggersSubmenu() ? this._parentMaterialMenu : undefined; menu.direction = this.dir; - this._setMenuElevation(menu); menu.focusFirstItem(this._openedBy || 'program'); this._setIsMenuOpen(true); } - /** Updates the menu elevation based on the amount of parent menus that it has. */ - private _setMenuElevation(menu: MatMenuPanel): void { - if (menu.setElevation) { - let depth = 0; - let parentMenu = menu.parentMenu; - - while (parentMenu) { - depth++; - parentMenu = parentMenu.parentMenu; - } - - menu.setElevation(depth); - } - } - // set state rather than toggle to support triggers sharing a menu private _setIsMenuOpen(isOpen: boolean): void { if (isOpen !== this._menuOpen) { diff --git a/src/material/menu/menu.html b/src/material/menu/menu.html index ae46031d7f1a..77f78f1d71b6 100644 --- a/src/material/menu/menu.html +++ b/src/material/menu/menu.html @@ -1,6 +1,6 @@
{ expect(panel.classList).toContain('custom-two'); })); - it('should not remove mat-elevation class from overlay when panelClass is changed', fakeAsync(() => { - const fixture = createComponent(SimpleMenu, [], [FakeIcon]); - - fixture.componentInstance.panelClass = 'custom-one'; - fixture.detectChanges(); - fixture.componentInstance.trigger.openMenu(); - fixture.detectChanges(); - tick(500); - - const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!; - - expect(panel.classList).toContain('custom-one'); - expect(panel.classList).toContain('mat-elevation-z2'); - - fixture.componentInstance.panelClass = 'custom-two'; - fixture.changeDetectorRef.markForCheck(); - fixture.detectChanges(); - - expect(panel.classList).not.toContain('custom-one'); - expect(panel.classList).toContain('custom-two'); - expect(panel.classList).toContain('mat-mdc-elevation-specific'); - expect(panel.classList) - .withContext('Expected mat-elevation-z2 not to be removed') - .toContain('mat-elevation-z2'); - })); - it('should set the "menu" role on the overlay panel', fakeAsync(() => { const fixture = createComponent(SimpleMenu, [], [FakeIcon]); fixture.detectChanges(); @@ -2350,79 +2324,6 @@ describe('MatMenu', () => { expect(menuItems[0].querySelector('.mat-mdc-menu-submenu-icon')).toBeFalsy(); })); - it('should increase the sub-menu elevation based on its depth', fakeAsync(() => { - compileTestComponent(); - instance.rootTrigger.openMenu(); - fixture.detectChanges(); - tick(500); - - instance.levelOneTrigger.openMenu(); - fixture.detectChanges(); - tick(500); - - instance.levelTwoTrigger.openMenu(); - fixture.detectChanges(); - tick(500); - - const menus = overlay.querySelectorAll('.mat-mdc-menu-panel'); - - expect(menus[0].classList).toContain('mat-mdc-elevation-specific'); - expect(menus[0].classList) - .withContext('Expected root menu to have base elevation.') - .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-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-z4'); - })); - - it('should update the elevation when the same menu is opened at a different depth', fakeAsync(() => { - compileTestComponent(); - instance.rootTrigger.openMenu(); - fixture.detectChanges(); - - instance.levelOneTrigger.openMenu(); - fixture.detectChanges(); - - instance.levelTwoTrigger.openMenu(); - fixture.detectChanges(); - - let lastMenu = overlay.querySelectorAll('.mat-mdc-menu-panel')[2]; - - expect(lastMenu.classList).toContain('mat-mdc-elevation-specific'); - expect(lastMenu.classList) - .withContext('Expected menu to have the base elevation plus two.') - .toContain('mat-elevation-z4'); - - (overlay.querySelector('.cdk-overlay-backdrop')! as HTMLElement).click(); - fixture.detectChanges(); - tick(500); - - expect(overlay.querySelectorAll('.mat-mdc-menu-panel').length) - .withContext('Expected no open menus') - .toBe(0); - - instance.alternateTrigger.openMenu(); - fixture.detectChanges(); - tick(500); - - lastMenu = overlay.querySelector('.mat-mdc-menu-panel') as HTMLElement; - - expect(lastMenu.classList).toContain('mat-mdc-elevation-specific'); - expect(lastMenu.classList) - .not.withContext('Expected menu not to maintain old elevation.') - .toContain('mat-elevation-z4'); - expect(lastMenu.classList) - .withContext('Expected menu to have the proper updated elevation.') - .toContain('mat-elevation-z2'); - })); - it('should not change focus origin if origin not specified for trigger', fakeAsync(() => { compileTestComponent(); @@ -2442,28 +2343,6 @@ describe('MatMenu', () => { expect(levelTwoTrigger.classList).toContain('cdk-mouse-focused'); })); - it('should not increase the elevation if the user specified a custom one', fakeAsync(() => { - const elevationFixture = createComponent(NestedMenuCustomElevation); - - elevationFixture.detectChanges(); - elevationFixture.componentInstance.rootTrigger.openMenu(); - elevationFixture.detectChanges(); - tick(500); - - elevationFixture.componentInstance.levelOneTrigger.openMenu(); - elevationFixture.detectChanges(); - tick(500); - - const menuClasses = - overlayContainerElement.querySelectorAll('.mat-mdc-menu-panel')[1].classList; - - expect(menuClasses).toContain('mat-mdc-elevation-specific'); - expect(menuClasses) - .withContext('Expected user elevation to be maintained') - .toContain('mat-elevation-z24'); - 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(() => { compileTestComponent(); instance.rootTrigger.openMenu(); @@ -2934,27 +2813,6 @@ class NestedMenu { showLazy = false; } -@Component({ - template: ` - - - - - - - - - - `, - standalone: false, -}) -class NestedMenuCustomElevation { - @ViewChild('rootTrigger') rootTrigger: MatMenuTrigger; - @ViewChild('levelOneTrigger') levelOneTrigger: MatMenuTrigger; -} - @Component({ template: ` diff --git a/src/material/menu/menu.ts b/src/material/menu/menu.ts index 678f46fbabc7..809fe23c440c 100644 --- a/src/material/menu/menu.ts +++ b/src/material/menu/menu.ts @@ -116,9 +116,6 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI private _xPosition: MenuPositionX; private _yPosition: MenuPositionY; private _firstItemFocusRef?: AfterRenderRef; - private _previousElevation: string; - private _elevationPrefix = 'mat-elevation-z'; - private _baseElevation: number | null = null; /** All items inside the menu. Includes items nested inside another menu. */ @ContentChildren(MatMenuItem, {descendants: true}) _allItems: QueryList; @@ -439,41 +436,10 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI } /** - * Sets the menu panel elevation. - * @param depth Number of parent menus that come before the menu. + * @deprecated No longer used and will be removed. + * @breaking-change 21.0.0 */ - 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); - const newElevation = `${this._elevationPrefix}${elevation}`; - const customElevation = Object.keys(this._classList).find(className => { - return className.startsWith(this._elevationPrefix); - }); - - if (!customElevation || customElevation === this._previousElevation) { - const newClassList = {...this._classList}; - - if (this._previousElevation) { - newClassList[this._previousElevation] = false; - } - - newClassList[newElevation] = true; - this._previousElevation = newElevation; - this._classList = newClassList; - } - } + setElevation(_depth: number): void {} /** * Adds classes to the menu panel based on its position. Can be used by diff --git a/tools/public_api_guard/material/menu.md b/tools/public_api_guard/material/menu.md index 30587093ea5f..668b24addb2f 100644 --- a/tools/public_api_guard/material/menu.md +++ b/tools/public_api_guard/material/menu.md @@ -102,7 +102,8 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI removeItem(_item: MatMenuItem): void; resetActiveItem(): void; _resetAnimation(): void; - setElevation(depth: number): void; + // @deprecated (undocumented) + setElevation(_depth: number): void; setPositionClasses(posX?: MenuPositionX, posY?: MenuPositionY): void; _startAnimation(): void; templateRef: TemplateRef; @@ -222,7 +223,7 @@ export interface MatMenuPanel { removeItem?: (item: T) => void; // (undocumented) resetActiveItem: () => void; - // (undocumented) + // @deprecated (undocumented) setElevation?(depth: number): void; // (undocumented) setPositionClasses?: (x: MenuPositionX, y: MenuPositionY) => void;