fix(menubar): fix broken menubar accessability with checkbox and radio menuitems

closes #6151

Author: rschmukler

src/components/menu/js/menuController.js

@@ -24,6 +24,7 @@ function MenuController($mdMenu, $attrs, $element, $scope, $mdUtil, $timeout, $r
     menuContainer = setMenuContainer;
     // Default element for ARIA attributes has the ngClick or ngMouseenter expression
     triggerElement = $element[0].querySelector('[ng-click],[ng-mouseenter]');
+    triggerElement.setAttribute('aria-expanded', 'false');
 
     this.isInMenuBar = opts.isInMenuBar;
     this.nestedMenus = $mdUtil.nodesToArray(menuContainer[0].querySelectorAll('.md-nested-menu'));
@@ -110,6 +111,7 @@ function MenuController($mdMenu, $attrs, $element, $scope, $mdUtil, $timeout, $r
     self.enableHoverListener();
     self.isOpen = true;
     triggerElement = triggerElement || (ev ? ev.target : $element[0]);
+    triggerElement.setAttribute('aria-expanded', 'true');
     $scope.$emit('$mdMenuOpen', $element);
     $mdMenu.show({
       scope: $scope,
@@ -120,6 +122,7 @@ function MenuController($mdMenu, $attrs, $element, $scope, $mdUtil, $timeout, $r
       preserveElement: true,
       parent: 'body'
     }).finally(function() {
+      triggerElement.setAttribute('aria-expanded', 'false');
       self.disableHoverListener();
     });
   };

src/components/menu/js/menuServiceProvider.js

@@ -238,13 +238,13 @@ function MenuProvider($$interimElementProvider) {
               handled = true;
               break;
             case $mdConstant.KEY_CODE.UP_ARROW:
-              if (!focusMenuItem(ev, opts.menuContentEl, opts, -1)) {
+              if (!focusMenuItem(ev, opts.menuContentEl, opts, -1) && !opts.nestLevel) {
                 opts.mdMenuCtrl.triggerContainerProxy(ev);
               }
               handled = true;
               break;
             case $mdConstant.KEY_CODE.DOWN_ARROW:
-              if (!focusMenuItem(ev, opts.menuContentEl, opts, 1)) {
+              if (!focusMenuItem(ev, opts.menuContentEl, opts, 1) && !opts.nestLevel) {
                 opts.mdMenuCtrl.triggerContainerProxy(ev);
               }
               handled = true;

src/components/menu/menu.spec.js

@@ -1,4 +1,4 @@
-ddescribe('material.components.menu', function() {
+describe('material.components.menu', function() {
   var attachedElements = [];
   var $mdMenu, $timeout, menuActionPerformed, $mdUtil;
 

src/components/menuBar/js/menuItemController.js

@@ -22,11 +22,27 @@ MenuItemController.prototype.init = function(ngModel) {
     this.mode  = $attrs.type;
     this.iconEl = $element[0].children[0];
     this.buttonEl = $element[0].children[1];
-    if (ngModel) this.initClickListeners();
+    if (ngModel) {
+      // Clear ngAria set attributes
+      this.initClickListeners();
+    }
   }
 };
 
+// ngAria auto sets attributes on a menu-item with a ngModel.
+// We don't want this because our content (buttons) get the focus
+// and set their own aria attributes appropritately. Having both
+// breaks NVDA / JAWS. This undeoes ngAria's attrs.
+MenuItemController.prototype.clearNgAria = function() {
+  var el = this.$element[0];
+  var clearAttrs = ['role', 'tabindex', 'aria-invalid', 'aria-checked'];
+  angular.forEach(clearAttrs, function(attr) {
+    el.removeAttribute(attr);
+  });
+};
+
 MenuItemController.prototype.initClickListeners = function() {
+  var self = this;
   var ngModel = this.ngModel;
   var $scope = this.$scope;
   var $attrs = this.$attrs;
@@ -35,20 +51,21 @@ MenuItemController.prototype.initClickListeners = function() {
 
   this.handleClick = angular.bind(this, this.handleClick);
 
-  var icon = this.iconEl
+  var icon = this.iconEl;
   var button = angular.element(this.buttonEl);
   var handleClick = this.handleClick;
 
   $attrs.$observe('disabled', setDisabled);
   setDisabled($attrs.disabled);
 
   ngModel.$render = function render() {
+    self.clearNgAria();
     if (isSelected()) {
       icon.style.display = '';
-      $element.attr('aria-checked', 'true');
+      button.attr('aria-checked', 'true');
     } else {
       icon.style.display = 'none';
-      $element.attr('aria-checked', 'false');
+      button.attr('aria-checked', 'false');
     }
   };
 

src/components/menuBar/js/menuItemDirective.js

@@ -10,6 +10,7 @@ angular
 function MenuItemDirective() {
   return {
     require: ['mdMenuItem', '?ngModel'],
+    priority: 210, // ensure that our post link runs after ngAria
     compile: function(templateEl, templateAttrs) {
       if (templateAttrs.type == 'checkbox' || templateAttrs.type == 'radio') {
         var text = templateEl[0].textContent;

src/components/menuBar/menu-bar.spec.js

@@ -228,12 +228,13 @@ describe('material.components.menuBar', function() {
       });
       it('reflects the ng-model value', inject(function($rootScope) {
         var menuItem = setup('ng-model="test"')[0];
-        expect(menuItem.getAttribute('aria-checked')).toBe('false');
+        var button = menuItem.querySelector('md-button');
+        expect(button.getAttribute('aria-checked')).toBe('false');
         expect(menuItem.children[0].style.display).toBe('none');
         $rootScope.test = true;
         $rootScope.$digest();
         expect(menuItem.children[0].style.display).toBe('');
-        expect(menuItem.getAttribute('aria-checked')).toBe('true');
+        expect(button.getAttribute('aria-checked')).toBe('true');
       }));
 
       function setup(attrs) {
@@ -283,12 +284,13 @@ describe('material.components.menuBar', function() {
       it('reflects the ng-model value', inject(function($rootScope) {
         $rootScope.test = 'apple';
         var menuItem = setup('ng-model="test" value="hello"')[0];
-        expect(menuItem.getAttribute('aria-checked')).toBe('false');
+        var button = menuItem.querySelector('md-button');
+        expect(button.getAttribute('aria-checked')).toBe('false');
         expect(menuItem.children[0].style.display).toBe('none');
         $rootScope.test = 'hello';
         $rootScope.$digest();
         expect(menuItem.children[0].style.display).toBeFalsy();
-        expect(menuItem.getAttribute('aria-checked')).toBe('true');
+        expect(button.getAttribute('aria-checked')).toBe('true');
       }));
 
       function setup(attrs) {