Skip to content

Commit

Permalink
fix(material/menu): not interrupting keyboard events to other overlays (
Browse files Browse the repository at this point in the history
#23310)

This is a resubmit of #22856 which had some issues in g3.

For historical reasons, `mat-menu` doesn't use the same keyboard event dispatcher as the other overlays. To work around it, previously we added a dummy subscription so that the menu would still show up in the overlay keyboard stack.

This works for most events, but it breaks down for the escape key, because closing the menu removes it from the stack immediately, allowing the event to bubble up to the document and be dispatched to the next overlay in the stack.

These changes resolve the issue by adding a stopPropagation call.

Fixes #22694.

(cherry picked from commit 864b8ae)
  • Loading branch information
crisbeto authored and andrewseguin committed Jan 15, 2022
1 parent 64dd8ed commit 4182717
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/material-experimental/mdc-menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,13 +428,15 @@ describe('MDC-based MatMenu', () => {

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

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
2 changes: 2 additions & 0 deletions src/material/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,13 +428,15 @@ describe('MatMenu', () => {

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

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
5 changes: 5 additions & 0 deletions src/material/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,12 @@ export class _MatMenuBase
}

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

0 comments on commit 4182717

Please sign in to comment.