Skip to content

Commit

Permalink
fix(material/menu): handle keyboard events through dispatcher (#29997)
Browse files Browse the repository at this point in the history
Currently `mat-menu` handles it keyboard events in the template, however this ignores the overlay's stacking context which can capture some events that it shouldn't.

These changes switch the menu to handling the events through the common dispatcher instead.

Fixes #29996.

(cherry picked from commit 42f6a4a)
  • Loading branch information
crisbeto committed Nov 14, 2024
1 parent 5345a87 commit dbcb921
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 16 deletions.
10 changes: 5 additions & 5 deletions src/material/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,11 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
config.positionStrategy as FlexibleConnectedPositionStrategy,
);
this._overlayRef = this._overlay.create(config);

// Consume the `keydownEvents` in order to prevent them from going to another overlay.
// Ideally we'd also have our keyboard event logic in here, however doing so will
// break anybody that may have implemented the `MatMenuPanel` themselves.
this._overlayRef.keydownEvents().subscribe();
this._overlayRef.keydownEvents().subscribe(event => {
if (this.menu instanceof MatMenu) {
this.menu._handleKeydown(event);
}
});
}

return this._overlayRef;
Expand Down
1 change: 0 additions & 1 deletion src/material/menu/menu.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
class="mat-mdc-menu-panel"
[id]="panelId"
[class]="_classList"
(keydown)="_handleKeydown($event)"
(click)="closed.emit('click')"
[@transformMenu]="_panelAnimationState"
(@transformMenu.start)="_onAnimationStart($event)"
Expand Down
5 changes: 1 addition & 4 deletions src/material/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -473,16 +473,13 @@ describe('MatMenu', () => {
fixture.componentInstance.trigger.openMenu();

const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
const event = createKeyboardEvent('keydown', ESCAPE);
spyOn(event, 'stopPropagation').and.callThrough();
const event = dispatchKeyboardEvent(panel, 'keydown', ESCAPE);

dispatchEvent(panel, event);
fixture.detectChanges();
tick(500);

expect(overlayContainerElement.textContent).toBe('');
expect(event.defaultPrevented).toBe(true);
expect(event.stopPropagation).toHaveBeenCalled();
}));

it('should not close the menu when pressing ESCAPE with a modifier', fakeAsync(() => {
Expand Down
4 changes: 0 additions & 4 deletions src/material/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,10 +385,6 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnI
manager.onKeydown(event);
return;
}

// Don't allow the event to propagate if we've already handled it, or it may
// end up reaching other overlays that were opened earlier (see #22694).
event.stopPropagation();
}

/**
Expand Down
3 changes: 1 addition & 2 deletions src/material/menu/testing/menu-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
HarnessLoader,
HarnessPredicate,
TestElement,
TestKey,
} from '@angular/cdk/testing';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {MenuHarnessFilters, MenuItemHarnessFilters} from './menu-harness-filters';
Expand Down Expand Up @@ -82,7 +81,7 @@ export class MatMenuHarness extends ContentContainerComponentHarness<string> {
async close(): Promise<void> {
const panel = await this._getMenuPanel();
if (panel) {
return panel.sendKeys(TestKey.ESCAPE);
return panel.click();
}
}

Expand Down

0 comments on commit dbcb921

Please sign in to comment.