Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions src/material-experimental/mdc-menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,44 @@ describe('MatMenu', () => {
flush();
}));

it('should respect the DOM order, rather than insertion order, when moving focus using ' +
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ported this test over, because it got in through a very old PR that got in recently which was opened before the MDC menu existed.

'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);
Expand Down Expand Up @@ -2361,3 +2399,21 @@ class DynamicPanelMenu {
class MenuWithCheckboxItems {
@ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
}


@Component({
template: `
<button [matMenuTriggerFor]="menu">Toggle menu</button>
<mat-menu #menu="matMenu">
<button
*ngFor="let item of items"
[disabled]="item.disabled"
mat-menu-item>{{item.label}}</button>
</mat-menu>
`
})
class SimpleMenuWithRepeater {
@ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
@ViewChild(MatMenu, {static: false}) menu: MatMenu;
items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}];
}
19 changes: 16 additions & 3 deletions src/material/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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]');
Expand Down Expand Up @@ -2383,14 +2393,17 @@ class MenuWithCheckboxItems {
template: `
<button [matMenuTriggerFor]="menu">Toggle menu</button>
<mat-menu #menu="matMenu">
<button *ngFor="let item of items" mat-menu-item>{{item}}</button>
<button
*ngFor="let item of items"
[disabled]="item.disabled"
mat-menu-item>{{item.label}}</button>
</mat-menu>
`
})
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({
Expand Down
26 changes: 24 additions & 2 deletions src/material/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,35 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
* @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();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: it felt a little weird to have this logic here since the method is called focusFirstItem, however the alternative is to do it inside the menu trigger which would require another method to be added to the MatMenuPanel interface which will increase the API surface. Furthermore, doing it through the trigger may no be very reliable, because we'd need to check if the document.activeElement is inside the menu and some browsers move focus asynchronously.

break;
} else {
element = element.parentElement;
}
}
}
}

Expand Down