Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Commit

Permalink
fix(menu): improve aria compliance
Browse files Browse the repository at this point in the history 
closes #4415
  • Loading branch information
@rschmukler @jelbourn
rschmukler authored and jelbourn committed Dec 2, 2015
1 parent 2a1de83 commit 097b799
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 77 deletions.
14 changes: 11 additions & 3 deletions src/components/menu/js/menuController.js
View file
Expand Up @@ -25,13 +25,19 @@ function MenuController($mdMenu, $attrs, $element, $scope, $mdUtil, $timeout, $r
// Default element for ARIA attributes has the ngClick or ngMouseenter expression
triggerElement = $element[0].querySelector('[ng-click],[ng-mouseenter]');

this.isInMenuBar = opts.isInMenuBar;
this.nestedMenus = $mdUtil.nodesToArray(menuContainer[0].querySelectorAll('.md-nested-menu'));

menuContainer.on('$mdInterimElementRemove', function() {
self.isOpen = false;
});

var menuContainerId = 'menu_container_' + $mdUtil.nextUid();
menuContainer.attr('id', menuContainerId);
angular.element(triggerElement).attr({
'aria-owns': menuContainerId,
'aria-haspopup': 'true'
});

$scope.$on('$destroy', this.disableHoverListener);
};

Expand Down Expand Up @@ -110,8 +116,8 @@ function MenuController($mdMenu, $attrs, $element, $scope, $mdUtil, $timeout, $r
nestLevel: self.nestLevel,
element: menuContainer,
target: triggerElement,
preserveElement: self.isInMenuBar || self.nestedMenus.length > 0,
parent: self.isInMenuBar ? $element : 'body'
preserveElement: true,
parent: $element
}).finally(function() {
self.disableHoverListener();
});
Expand All @@ -123,12 +129,14 @@ function MenuController($mdMenu, $attrs, $element, $scope, $mdUtil, $timeout, $r
$scope.$watch(function() { return self.isOpen; }, function(isOpen) {
if (isOpen) {
triggerElement.setAttribute('aria-expanded', 'true');
menuContainer.attr('aria-hidden', 'false');
$element[0].classList.add('md-open');
angular.forEach(self.nestedMenus, function(el) {
el.classList.remove('md-open');
});
} else {
triggerElement && triggerElement.setAttribute('aria-expanded', 'false');
menuContainer.attr('aria-hidden', 'true');
$element[0].classList.remove('md-open');
}
$scope.$mdMenuIsOpen = self.isOpen;
Expand Down
20 changes: 6 additions & 14 deletions src/components/menu/js/menuDirective.js
View file
Expand Up @@ -186,7 +186,6 @@ function MenuDirective($mdUtil) {
}
menuEl.classList.add('md-nested-menu');
menuEl.setAttribute('md-nest-level', nestingDepth + 1);
menuEl.setAttribute('role', 'menu');
});
}
return link;
Expand All @@ -200,20 +199,13 @@ function MenuDirective($mdUtil) {
'<div class="md-open-menu-container md-whiteframe-z2"></div>'
);
var menuContents = element.children()[1];
menuContainer.append(menuContents);
if (isInMenuBar) {
element.append(menuContainer);
menuContainer[0].style.display = 'none';
if (!menuContents.hasAttribute('role')) {
menuContents.setAttribute('role', 'menu');
}
mdMenuCtrl.init(menuContainer, { isInMenuBar: isInMenuBar });

scope.$on('$destroy', function() {
mdMenuCtrl
.destroy()
.finally(function(){
menuContainer.remove();
});
});
menuContainer.append(menuContents);

element.append(menuContainer);
menuContainer[0].style.display = 'none';
mdMenuCtrl.init(menuContainer, { isInMenuBar: isInMenuBar });
}
}
4 changes: 2 additions & 2 deletions src/components/menu/js/menuServiceProvider.js
View file
Expand Up @@ -56,7 +56,7 @@ function MenuProvider($$interimElementProvider) {
if (options.hasBackdrop) {
options.backdrop = $mdUtil.createBackdrop(scope, "md-menu-backdrop md-click-catcher");

$animate.enter(options.backdrop, options.parent);
$animate.enter(options.backdrop, $document[0].body);

This comment has been minimized.

Sign in to view

Copy link
@anatolyg

anatolyg Dec 29, 2015

if $animate.enter doesn't get passed an "after" element, it uses element.prepend to insert the backdrop into the DOM. Unfortunately, $document[0].body is a DOM element, which doesn't support the method prepend. This silently fails and leaves the document in a state where nothing is clickable.

Replacing $document[0].body with $document.find('body') fixes the issue.
}

/**
Expand Down Expand Up @@ -294,7 +294,7 @@ function MenuProvider($$interimElementProvider) {
if ((hasAnyAttribute(target, ['ng-click', 'ng-href', 'ui-sref']) ||
target.nodeName == 'BUTTON' || target.nodeName == 'MD-BUTTON') && !hasAnyAttribute(target, ['md-prevent-menu-close'])) {
var closestMenu = $mdUtil.getClosest(target, 'MD-MENU');
if (!target.hasAttribute('disabled') && (!closestMenu || closestMenu == opts.parent[0])) {
if (!target.hasAttribute('disabled') && (closestMenu == opts.parent[0])) {
close();
}
break;
Expand Down
92 changes: 43 additions & 49 deletions src/components/menu/menu.spec.js
View file
Expand Up @@ -28,12 +28,6 @@ describe('material.components.menu', function() {
expect(buildBadMenu).toThrow();
}));

it('removes everything but the first element', function() {
var menu = setup()[0];
expect(menu.children.length).toBe(1);
expect(menu.firstElementChild.nodeName).toBe('BUTTON');
});

it('specifies button type', inject(function($compile, $rootScope) {
var menu = setup()[0];
expect(menu.firstElementChild.getAttribute('type')).toBe('button');
Expand All @@ -42,35 +36,35 @@ describe('material.components.menu', function() {
it('opens on click', function () {
var menu = setup();
openMenu(menu);
expect(getOpenMenuContainer().length).toBe(1);
expect(getOpenMenuContainer(menu).length).toBe(1);
closeMenu(menu);
expect(getOpenMenuContainer().length).toBe(0);
expect(getOpenMenuContainer(menu).length).toBe(0);
});

it('opens on click without $event', function() {
var noEvent = true;
var menu = setup('ng-click', noEvent);
openMenu(menu);
expect(getOpenMenuContainer().length).toBe(1);
expect(getOpenMenuContainer(menu).length).toBe(1);
closeMenu(menu);
expect(getOpenMenuContainer().length).toBe(0);
expect(getOpenMenuContainer(menu).length).toBe(0);
});

it('opens on mouseEnter', function() {
var menu = setup('ng-mouseenter');
openMenu(menu, 'mouseenter');
expect(getOpenMenuContainer().length).toBe(1);
expect(getOpenMenuContainer(menu).length).toBe(1);
closeMenu(menu);
expect(getOpenMenuContainer().length).toBe(0);
expect(getOpenMenuContainer(menu).length).toBe(0);
});

it('opens on mouseEnter without $event', function() {
var noEvent = true;
var menu = setup('ng-mouseenter', noEvent);
openMenu(menu, 'mouseenter');
expect(getOpenMenuContainer().length).toBe(1);
expect(getOpenMenuContainer(menu).length).toBe(1);
closeMenu(menu);
expect(getOpenMenuContainer().length).toBe(0);
expect(getOpenMenuContainer(menu).length).toBe(0);
});

it('should not propagate the click event', function() {
Expand All @@ -87,56 +81,49 @@ describe('material.components.menu', function() {

it('closes on backdrop click', inject(function($document) {

openMenu(setup());
var menu = setup();
openMenu(menu);

expect(getOpenMenuContainer().length).toBe(1);
expect(getOpenMenuContainer(menu).length).toBe(1);

$document.find('md-backdrop').triggerHandler('click');
waitForMenuClose();

expect(getOpenMenuContainer().length).toBe(0);
expect(getOpenMenuContainer(menu).length).toBe(0);
}));


it('closes on escape', inject(function($document, $mdConstant) {
openMenu(setup());
expect(getOpenMenuContainer().length).toBe(1);
var menu = setup();
openMenu(menu);
expect(getOpenMenuContainer(menu).length).toBe(1);

var openMenuEl = $document[0].querySelector('md-menu-content');
var openMenuEl = menu[0].querySelector('md-menu-content');

pressKey(openMenuEl, $mdConstant.KEY_CODE.ESCAPE);
waitForMenuClose();

expect(getOpenMenuContainer().length).toBe(0);
}));

it('closes on $destroy', inject(function($document, $rootScope) {
var scope = $rootScope.$new();
openMenu( setup(null,false,scope) );

expect(getOpenMenuContainer().length).toBe(1);
scope.$destroy();

expect(getOpenMenuContainer().length).toBe(0);
expect(getOpenMenuContainer(menu).length).toBe(0);
}));

describe('closes with -', function() {
it('closes on normal option click', function() {
expect(getOpenMenuContainer().length).toBe(0);

openMenu(setup());
var menu = setup();
expect(getOpenMenuContainer(menu).length).toBe(0);
openMenu(menu);

expect(menuActionPerformed).toBeFalsy();
expect(getOpenMenuContainer().length).toBe(1);
expect(getOpenMenuContainer(menu).length).toBe(1);

var btn = getOpenMenuContainer()[0].querySelector('md-button');
var btn = getOpenMenuContainer(menu)[0].querySelector('md-button');
btn.click();

waitForMenuClose();

expect(menuActionPerformed).toBeTruthy();

expect(getOpenMenuContainer().length).toBe(0);
expect(getOpenMenuContainer(menu).length).toBe(0);
});

itClosesWithAttributes([
Expand All @@ -151,7 +138,7 @@ describe('material.components.menu', function() {
}

function testAttribute(attr) {
return inject(function($rootScope, $compile, $timeout, $browser, $animate) {
return inject(function($rootScope, $compile, $timeout, $browser) {
var template = '' +
'<md-menu>' +
' <button ng-click="$mdOpenMenu($event)">Hello World</button>' +
Expand All @@ -163,17 +150,18 @@ describe('material.components.menu', function() {
'</md-menu>';


openMenu($compile(template)($rootScope));
var menu = $compile(template)($rootScope);
openMenu(menu);

expect(getOpenMenuContainer().length).toBe(1);
expect(getOpenMenuContainer(menu).length).toBe(1);

$timeout.flush();
var btn = getOpenMenuContainer()[0].querySelector('md-button');
var btn = getOpenMenuContainer(menu)[0].querySelector('md-button');
btn.click();

waitForMenuClose();

expect(getOpenMenuContainer().length).toBe(0);
expect(getOpenMenuContainer(menu).length).toBe(0);
});
}
}
Expand Down Expand Up @@ -208,21 +196,27 @@ describe('material.components.menu', function() {
// Internal methods
// ********************************************

function getOpenMenuContainer() {
var res;
inject(function($document) {
res = angular.element($document[0].querySelector('.md-open-menu-container'));
});
return res;
function getOpenMenuContainer(el) {
el = (el instanceof angular.element) ? el[0] : el;
var container = el.querySelector('.md-open-menu-container');
if (container.style.display == 'none') {
return angular.element([]);
} else {
return angular.element(container);
}
}

function openMenu(el, triggerType) {
el.children().eq(0).triggerHandler(triggerType || 'click');
waitForMenuOpen();
inject(function($document) {
el.children().eq(0).triggerHandler(triggerType || 'click');
$document[0].body.appendChild(el[0]);
waitForMenuOpen();
});
}

function closeMenu() {
inject(function($document) {
$document.find('md-backdrop');
$document.find('md-backdrop').triggerHandler('click');
waitForMenuClose();
});
Expand Down
2 changes: 1 addition & 1 deletion src/components/menuBar/js/menuBarDirective.js
View file
Expand Up @@ -110,8 +110,8 @@ function MenuBarDirective($mdUtil, $mdTheming) {
if (menuEl.nodeName == 'MD-MENU') {
if (!menuEl.hasAttribute('md-position-mode')) {
menuEl.setAttribute('md-position-mode', 'left bottom');
menuEl.querySelector('button,a').setAttribute('role', 'menuitem');
}
menuEl.setAttribute('role', 'menu');
var contentEls = $mdUtil.nodesToArray(menuEl.querySelectorAll('md-menu-content'));
angular.forEach(contentEls, function(contentEl) {
contentEl.classList.add('md-menu-bar-menu');
Expand Down
14 changes: 9 additions & 5 deletions src/components/menuBar/js/menuItemDirective.js
View file
Expand Up @@ -22,11 +22,11 @@ function MenuItemDirective() {
templateEl.append(buttonEl);
templateEl[0].classList.add('md-indent');

setDefault('role', (templateAttrs.type == 'checkbox') ? 'menuitemcheckbox' : 'menuitemradio');
setDefault('role', (templateAttrs.type == 'checkbox') ? 'menuitemcheckbox' : 'menuitemradio', buttonEl);
angular.forEach(['ng-disabled'], moveAttrToButton);

} else {
setDefault('role', 'menuitem');
setDefault('role', 'menuitem', templateEl[0].querySelector('md-button,button,a'));
}


Expand All @@ -36,9 +36,13 @@ function MenuItemDirective() {
ctrl.init(ngModel);
};

function setDefault(attr, val) {
if (!templateEl[0].hasAttribute(attr)) {
templateEl[0].setAttribute(attr, val);
function setDefault(attr, val, el) {
el = el || templateEl;
if (el instanceof angular.element) {
el = el[0];
}
if (!el.hasAttribute(attr)) {
el.setAttribute(attr, val);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/menuBar/menu-bar.spec.js
View file
Expand Up @@ -19,7 +19,7 @@ describe('material.components.menuBar', function() {
});

describe('ARIA', function() {
it('sets aria-role="menubar" on the menubar', function() {
it('sets role="menubar" on the menubar', function() {
var menuBar = setup();
var ariaRole = menuBar[0].getAttribute('role');
expect(ariaRole).toBe('menubar');
Expand Down Expand Up @@ -207,7 +207,7 @@ describe('material.components.menuBar', function() {
expect(children[1].nodeName).toBe('MD-BUTTON');
});
it('sets aria role', function() {
var menuItem = setup()[0];
var menuItem = setup()[0].querySelector('md-button');
expect(menuItem.getAttribute('role')).toBe('menuitemcheckbox');
});
it('toggles on click', function() {
Expand Down Expand Up @@ -259,7 +259,7 @@ describe('material.components.menuBar', function() {
expect(children[1].nodeName).toBe('MD-BUTTON');
});
it('sets aria role', function() {
var menuItem = setup()[0];
var menuItem = setup()[0].querySelector('md-button');
expect(menuItem.getAttribute('role')).toBe('menuitemradio');
});
it('toggles on click', function() {
Expand Down

1 comment on commit 097b799

@PapyElGringo
Copy link

Choose a reason for hiding this comment

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

create #6049

Please sign in to comment.