From d3f63a3b452cda8eaa54637278ac712cfdb8f3e5 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 27 Aug 2019 06:28:12 +0200 Subject: [PATCH] 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. --- .../mdc-menu/menu.spec.ts | 56 +++++++++++++++++++ src/material/menu/menu.spec.ts | 19 ++++++- src/material/menu/menu.ts | 26 ++++++++- 3 files changed, 96 insertions(+), 5 deletions(-) diff --git a/src/material-experimental/mdc-menu/menu.spec.ts b/src/material-experimental/mdc-menu/menu.spec.ts index d4a289768ae5..7bf5039d5e5f 100644 --- a/src/material-experimental/mdc-menu/menu.spec.ts +++ b/src/material-experimental/mdc-menu/menu.spec.ts @@ -762,6 +762,44 @@ describe('MatMenu', () => { flush(); })); + it('should respect the DOM order, rather than insertion order, when moving focus using ' + + 'the arrow keys', fakeAsync(() => { + let fixture = createComponent(SimpleMenuWithRepeater); + + fixture.detectChanges(); + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + tick(500); + + let menuPanel = document.querySelector('.mat-mdc-menu-panel')!; + let items = menuPanel.querySelectorAll('.mat-mdc-menu-panel [mat-menu-item]'); + + expect(document.activeElement).toBe(items[0], 'Expected first item to be focused on open'); + + // Add a new item after the first one. + fixture.componentInstance.items.splice(1, 0, {label: 'Calzone', disabled: false}); + fixture.detectChanges(); + + items = menuPanel.querySelectorAll('.mat-mdc-menu-panel [mat-menu-item]'); + dispatchKeyboardEvent(menuPanel, 'keydown', DOWN_ARROW); + fixture.detectChanges(); + tick(); + + expect(document.activeElement).toBe(items[1], 'Expected second item to be focused'); + flush(); + })); + + it('should focus the menu panel if all items are disabled', fakeAsync(() => { + const fixture = createComponent(SimpleMenuWithRepeater, [], [FakeIcon]); + fixture.componentInstance.items.forEach(item => item.disabled = true); + fixture.detectChanges(); + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + + expect(document.activeElement) + .toBe(overlayContainerElement.querySelector('.mat-mdc-menu-panel')); + })); + describe('lazy rendering', () => { it('should be able to render the menu content lazily', fakeAsync(() => { const fixture = createComponent(SimpleLazyMenu); @@ -2361,3 +2399,21 @@ class DynamicPanelMenu { class MenuWithCheckboxItems { @ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger; } + + +@Component({ + template: ` + + + + + ` +}) +class SimpleMenuWithRepeater { + @ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger; + @ViewChild(MatMenu, {static: false}) menu: MatMenu; + items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}]; +} diff --git a/src/material/menu/menu.spec.ts b/src/material/menu/menu.spec.ts index e6517bfeaaab..73bce463d920 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -762,6 +762,16 @@ describe('MatMenu', () => { flush(); })); + it('should focus the menu panel if all items are disabled', fakeAsync(() => { + const fixture = createComponent(SimpleMenuWithRepeater, [], [FakeIcon]); + fixture.componentInstance.items.forEach(item => item.disabled = true); + fixture.detectChanges(); + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + + expect(document.activeElement).toBe(overlayContainerElement.querySelector('.mat-menu-panel')); + })); + describe('lazy rendering', () => { it('should be able to render the menu content lazily', fakeAsync(() => { const fixture = createComponent(SimpleLazyMenu); @@ -882,7 +892,7 @@ describe('MatMenu', () => { expect(document.activeElement).toBe(items[0], 'Expected first item to be focused on open'); // Add a new item after the first one. - fixture.componentInstance.items.splice(1, 0, 'Calzone'); + fixture.componentInstance.items.splice(1, 0, {label: 'Calzone', disabled: false}); fixture.detectChanges(); items = menuPanel.querySelectorAll('.mat-menu-panel [mat-menu-item]'); @@ -2383,14 +2393,17 @@ class MenuWithCheckboxItems { template: ` - + ` }) class SimpleMenuWithRepeater { @ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger; @ViewChild(MatMenu, {static: false}) menu: MatMenu; - items = ['Pizza', 'Pasta']; + items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}]; } @Component({ diff --git a/src/material/menu/menu.ts b/src/material/menu/menu.ts index e48c563bff1d..c49c6be17758 100644 --- a/src/material/menu/menu.ts +++ b/src/material/menu/menu.ts @@ -322,13 +322,35 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel * @param origin Action from which the focus originated. Used to set the correct styling. */ focusFirstItem(origin: FocusOrigin = 'program'): void { + const manager = this._keyManager; + // When the content is rendered lazily, it takes a bit before the items are inside the DOM. if (this.lazyContent) { this._ngZone.onStable.asObservable() .pipe(take(1)) - .subscribe(() => this._keyManager.setFocusOrigin(origin).setFirstItemActive()); + .subscribe(() => manager.setFocusOrigin(origin).setFirstItemActive()); } else { - this._keyManager.setFocusOrigin(origin).setFirstItemActive(); + manager.setFocusOrigin(origin).setFirstItemActive(); + } + + // If there's no active item at this point, it means that all the items are disabled. + // Move focus to the menu panel so keyboard events like Escape still work. Also this will + // give _some_ feedback to screen readers. + if (!manager.activeItem && this._directDescendantItems.length) { + let element = this._directDescendantItems.first._getHostElement().parentElement; + + // Because the `mat-menu` is at the DOM insertion point, not inside the overlay, we don't + // have a nice way of getting a hold of the menu panel. We can't use a `ViewChild` either + // because the panel is inside an `ng-template`. We work around it by starting from one of + // the items and walking up the DOM. + while (element) { + if (element.getAttribute('role') === 'menu') { + element.focus(); + break; + } else { + element = element.parentElement; + } + } } }