Skip to content

Commit d3f63a3

Browse files
crisbetojelbourn
authored andcommitted
fix(menu): keyboard controls not working if all items are disabled (#16572)
Currently we depend on `focusFirstItem` to bring focus into the menu, however if all the items are disabled, focus will stay on the trigger and the user won't get feedback that something has happened. Furthermore, the keyboard shortcuts that are implemented in the menu won't work either, because the keyboard event listener is on the menu. These changes work around the issue by setting focus on the `role="menu"` if none of the items were focusable. Fixes #16565.
1 parent 399ed29 commit d3f63a3

File tree

3 files changed

+96
-5
lines changed

3 files changed

+96
-5
lines changed

src/material-experimental/mdc-menu/menu.spec.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -762,6 +762,44 @@ describe('MatMenu', () => {
762762
flush();
763763
}));
764764

765+
it('should respect the DOM order, rather than insertion order, when moving focus using ' +
766+
'the arrow keys', fakeAsync(() => {
767+
let fixture = createComponent(SimpleMenuWithRepeater);
768+
769+
fixture.detectChanges();
770+
fixture.componentInstance.trigger.openMenu();
771+
fixture.detectChanges();
772+
tick(500);
773+
774+
let menuPanel = document.querySelector('.mat-mdc-menu-panel')!;
775+
let items = menuPanel.querySelectorAll('.mat-mdc-menu-panel [mat-menu-item]');
776+
777+
expect(document.activeElement).toBe(items[0], 'Expected first item to be focused on open');
778+
779+
// Add a new item after the first one.
780+
fixture.componentInstance.items.splice(1, 0, {label: 'Calzone', disabled: false});
781+
fixture.detectChanges();
782+
783+
items = menuPanel.querySelectorAll('.mat-mdc-menu-panel [mat-menu-item]');
784+
dispatchKeyboardEvent(menuPanel, 'keydown', DOWN_ARROW);
785+
fixture.detectChanges();
786+
tick();
787+
788+
expect(document.activeElement).toBe(items[1], 'Expected second item to be focused');
789+
flush();
790+
}));
791+
792+
it('should focus the menu panel if all items are disabled', fakeAsync(() => {
793+
const fixture = createComponent(SimpleMenuWithRepeater, [], [FakeIcon]);
794+
fixture.componentInstance.items.forEach(item => item.disabled = true);
795+
fixture.detectChanges();
796+
fixture.componentInstance.trigger.openMenu();
797+
fixture.detectChanges();
798+
799+
expect(document.activeElement)
800+
.toBe(overlayContainerElement.querySelector('.mat-mdc-menu-panel'));
801+
}));
802+
765803
describe('lazy rendering', () => {
766804
it('should be able to render the menu content lazily', fakeAsync(() => {
767805
const fixture = createComponent(SimpleLazyMenu);
@@ -2361,3 +2399,21 @@ class DynamicPanelMenu {
23612399
class MenuWithCheckboxItems {
23622400
@ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
23632401
}
2402+
2403+
2404+
@Component({
2405+
template: `
2406+
<button [matMenuTriggerFor]="menu">Toggle menu</button>
2407+
<mat-menu #menu="matMenu">
2408+
<button
2409+
*ngFor="let item of items"
2410+
[disabled]="item.disabled"
2411+
mat-menu-item>{{item.label}}</button>
2412+
</mat-menu>
2413+
`
2414+
})
2415+
class SimpleMenuWithRepeater {
2416+
@ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
2417+
@ViewChild(MatMenu, {static: false}) menu: MatMenu;
2418+
items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}];
2419+
}

src/material/menu/menu.spec.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -762,6 +762,16 @@ describe('MatMenu', () => {
762762
flush();
763763
}));
764764

765+
it('should focus the menu panel if all items are disabled', fakeAsync(() => {
766+
const fixture = createComponent(SimpleMenuWithRepeater, [], [FakeIcon]);
767+
fixture.componentInstance.items.forEach(item => item.disabled = true);
768+
fixture.detectChanges();
769+
fixture.componentInstance.trigger.openMenu();
770+
fixture.detectChanges();
771+
772+
expect(document.activeElement).toBe(overlayContainerElement.querySelector('.mat-menu-panel'));
773+
}));
774+
765775
describe('lazy rendering', () => {
766776
it('should be able to render the menu content lazily', fakeAsync(() => {
767777
const fixture = createComponent(SimpleLazyMenu);
@@ -882,7 +892,7 @@ describe('MatMenu', () => {
882892
expect(document.activeElement).toBe(items[0], 'Expected first item to be focused on open');
883893

884894
// Add a new item after the first one.
885-
fixture.componentInstance.items.splice(1, 0, 'Calzone');
895+
fixture.componentInstance.items.splice(1, 0, {label: 'Calzone', disabled: false});
886896
fixture.detectChanges();
887897

888898
items = menuPanel.querySelectorAll('.mat-menu-panel [mat-menu-item]');
@@ -2383,14 +2393,17 @@ class MenuWithCheckboxItems {
23832393
template: `
23842394
<button [matMenuTriggerFor]="menu">Toggle menu</button>
23852395
<mat-menu #menu="matMenu">
2386-
<button *ngFor="let item of items" mat-menu-item>{{item}}</button>
2396+
<button
2397+
*ngFor="let item of items"
2398+
[disabled]="item.disabled"
2399+
mat-menu-item>{{item.label}}</button>
23872400
</mat-menu>
23882401
`
23892402
})
23902403
class SimpleMenuWithRepeater {
23912404
@ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
23922405
@ViewChild(MatMenu, {static: false}) menu: MatMenu;
2393-
items = ['Pizza', 'Pasta'];
2406+
items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}];
23942407
}
23952408

23962409
@Component({

src/material/menu/menu.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,13 +322,35 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
322322
* @param origin Action from which the focus originated. Used to set the correct styling.
323323
*/
324324
focusFirstItem(origin: FocusOrigin = 'program'): void {
325+
const manager = this._keyManager;
326+
325327
// When the content is rendered lazily, it takes a bit before the items are inside the DOM.
326328
if (this.lazyContent) {
327329
this._ngZone.onStable.asObservable()
328330
.pipe(take(1))
329-
.subscribe(() => this._keyManager.setFocusOrigin(origin).setFirstItemActive());
331+
.subscribe(() => manager.setFocusOrigin(origin).setFirstItemActive());
330332
} else {
331-
this._keyManager.setFocusOrigin(origin).setFirstItemActive();
333+
manager.setFocusOrigin(origin).setFirstItemActive();
334+
}
335+
336+
// If there's no active item at this point, it means that all the items are disabled.
337+
// Move focus to the menu panel so keyboard events like Escape still work. Also this will
338+
// give _some_ feedback to screen readers.
339+
if (!manager.activeItem && this._directDescendantItems.length) {
340+
let element = this._directDescendantItems.first._getHostElement().parentElement;
341+
342+
// Because the `mat-menu` is at the DOM insertion point, not inside the overlay, we don't
343+
// have a nice way of getting a hold of the menu panel. We can't use a `ViewChild` either
344+
// because the panel is inside an `ng-template`. We work around it by starting from one of
345+
// the items and walking up the DOM.
346+
while (element) {
347+
if (element.getAttribute('role') === 'menu') {
348+
element.focus();
349+
break;
350+
} else {
351+
element = element.parentElement;
352+
}
353+
}
332354
}
333355
}
334356

0 commit comments

Comments
 (0)