Skip to content

Commit

Permalink
fix(material/menu): use static elevation (#29968)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto authored Nov 5, 2024
1 parent a2dc657 commit b072047
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 204 deletions.
8 changes: 5 additions & 3 deletions src/material/core/tokens/m2/mat/_menu.scss
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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,
);
}

Expand Down
9 changes: 6 additions & 3 deletions src/material/core/tokens/m3/mat/_menu.scss
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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,
)
);

Expand Down
5 changes: 5 additions & 0 deletions src/material/menu/menu-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ export interface MatMenuPanel<T = any> {
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;
Expand Down
16 changes: 0 additions & 16 deletions src/material/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/material/menu/menu.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<ng-template>
<div
class="mat-mdc-menu-panel mat-mdc-elevation-specific"
class="mat-mdc-menu-panel"
[id]="panelId"
[class]="_classList"
(keydown)="_handleKeydown($event)"
Expand Down
1 change: 1 addition & 0 deletions src/material/menu/menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ mat-menu {
@include token-utils.use-tokens(tokens-mat-menu.$prefix, tokens-mat-menu.get-token-slots()) {
@include token-utils.create-token-slot(border-radius, container-shape);
@include token-utils.create-token-slot(background-color, container-color);
@include token-utils.create-token-slot(box-shadow, container-elevation-shadow);
}

// TODO(crisbeto): we don't need this for anything, but it was there when
Expand Down
142 changes: 0 additions & 142 deletions src/material/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,32 +594,6 @@ describe('MatMenu', () => {
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();
Expand Down Expand Up @@ -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();

Expand All @@ -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();
Expand Down Expand Up @@ -2934,27 +2813,6 @@ class NestedMenu {
showLazy = false;
}

@Component({
template: `
<button [matMenuTriggerFor]="root" #rootTrigger="matMenuTrigger">Toggle menu</button>
<mat-menu #root="matMenu">
<button mat-menu-item
[matMenuTriggerFor]="levelOne"
#levelOneTrigger="matMenuTrigger">One</button>
</mat-menu>
<mat-menu #levelOne="matMenu" class="mat-elevation-z24">
<button mat-menu-item>Two</button>
</mat-menu>
`,
standalone: false,
})
class NestedMenuCustomElevation {
@ViewChild('rootTrigger') rootTrigger: MatMenuTrigger;
@ViewChild('levelOneTrigger') levelOneTrigger: MatMenuTrigger;
}

@Component({
template: `
<button [matMenuTriggerFor]="root" #rootTriggerEl>Toggle menu</button>
Expand Down
40 changes: 3 additions & 37 deletions src/material/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,6 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, 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<MatMenuItem>;
Expand Down Expand Up @@ -439,41 +436,10 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, 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
Expand Down
5 changes: 3 additions & 2 deletions tools/public_api_guard/material/menu.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, 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<any>;
Expand Down Expand Up @@ -222,7 +223,7 @@ export interface MatMenuPanel<T = any> {
removeItem?: (item: T) => void;
// (undocumented)
resetActiveItem: () => void;
// (undocumented)
// @deprecated (undocumented)
setElevation?(depth: number): void;
// (undocumented)
setPositionClasses?: (x: MenuPositionX, y: MenuPositionY) => void;
Expand Down

0 comments on commit b072047

Please sign in to comment.