Skip to content

Commit

Permalink
fix(menu): allow alternate roles to be set on menu item (#14165)
Browse files Browse the repository at this point in the history
Based on the a11y guidelines, the menu items can have the `menuitemradio` and `menuitemcheckbox` roles, instead of `menuitem`. These changes allow for the consumer to set an alternate role.

Fixes #14163.
  • Loading branch information
crisbeto authored and jelbourn committed Dec 3, 2018
1 parent c552504 commit 3f1588f
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/lib/menu/menu-item.ts
Expand Up @@ -15,6 +15,7 @@ import {
ViewEncapsulation,
Inject,
Optional,
Input,
} from '@angular/core';
import {
CanDisable, CanDisableCtor,
Expand Down Expand Up @@ -42,7 +43,7 @@ export const _MatMenuItemMixinBase: CanDisableRippleCtor & CanDisableCtor & type
exportAs: 'matMenuItem',
inputs: ['disabled', 'disableRipple'],
host: {
'role': 'menuitem',
'[attr.role]': 'role',
'class': 'mat-menu-item',
'[class.mat-menu-item-highlighted]': '_highlighted',
'[class.mat-menu-item-submenu-trigger]': '_triggersSubmenu',
Expand All @@ -59,6 +60,9 @@ export const _MatMenuItemMixinBase: CanDisableRippleCtor & CanDisableCtor & type
export class MatMenuItem extends _MatMenuItemMixinBase
implements FocusableOption, CanDisable, CanDisableRipple, OnDestroy {

/** ARIA role for the menu item. */
@Input() role: 'menuitem' | 'menuitemradio' | 'menuitemcheckbox' = 'menuitem';

private _document: Document;

/** Stream that emits when the menu item is hovered. */
Expand Down
39 changes: 39 additions & 0 deletions src/lib/menu/menu.spec.ts
Expand Up @@ -385,6 +385,30 @@ describe('MatMenu', () => {
expect(role).toBe('menu', 'Expected panel to have the "menu" role.');
});

it('should set the "menuitem" role on the items by default', () => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
fixture.componentInstance.trigger.openMenu();
fixture.detectChanges();

const items = Array.from(overlayContainerElement.querySelectorAll('.mat-menu-item'));

expect(items.length).toBeGreaterThan(0);
expect(items.every(item => item.getAttribute('role') === 'menuitem')).toBe(true);
});

it('should be able to set an alternate role on the menu items', () => {
const fixture = createComponent(MenuWithCheckboxItems);
fixture.detectChanges();
fixture.componentInstance.trigger.openMenu();
fixture.detectChanges();

const items = Array.from(overlayContainerElement.querySelectorAll('.mat-menu-item'));

expect(items.length).toBeGreaterThan(0);
expect(items.every(item => item.getAttribute('role') === 'menuitemcheckbox')).toBe(true);
});

it('should not throw an error on destroy', () => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
expect(fixture.destroy.bind(fixture)).not.toThrow();
Expand Down Expand Up @@ -2100,3 +2124,18 @@ class DynamicPanelMenu {
@ViewChild('one') firstMenu: MatMenu;
@ViewChild('two') secondMenu: MatMenu;
}


@Component({
template: `
<button [matMenuTriggerFor]="menu">Toggle menu</button>
<mat-menu #menu="matMenu">
<button mat-menu-item role="menuitemcheckbox" aria-checked="true">Checked</button>
<button mat-menu-item role="menuitemcheckbox" aria-checked="false">Not checked</button>
</mat-menu>
`
})
class MenuWithCheckboxItems {
@ViewChild(MatMenuTrigger) trigger: MatMenuTrigger;
}

0 comments on commit 3f1588f

Please sign in to comment.