chore: fix aria-* attirbutes

src/MenuItem.jsx

@@ -11,6 +11,7 @@ import { noop, menuAllProps } from './util';
 
 export class MenuItem extends React.Component {
   static propTypes = {
+    attribute: PropTypes.object,
     rootPrefixCls: PropTypes.string,
     eventKey: PropTypes.string,
     active: PropTypes.bool,
@@ -145,14 +146,27 @@ export class MenuItem extends React.Component {
       [this.getSelectedClassName()]: props.isSelected,
       [this.getDisabledClassName()]: props.disabled,
     });
-    const attrs = {
+    let attrs = {
       ...props.attribute,
       title: props.title,
       className,
+      // set to menuitem by default
       role: 'menuitem',
-      'aria-selected': props.isSelected,
       'aria-disabled': props.disabled,
     };
+
+    if (props.role === 'option') {
+      // overwrite to option
+      attrs = {
+        ...attrs,
+        role: 'option',
+        'aria-selected': props.isSelected,
+      };
+    } else if (props.role === null) {
+      // sometimes we want to specify role inside <li/> element
+      // <li><a role='menuitem'>Link</a></li> would be a good example
+      delete attrs.role;
+    }
     let mouseEvent = {};
     if (!props.disabled) {
       mouseEvent = {
@@ -181,11 +195,11 @@ export class MenuItem extends React.Component {
   }
 }
 
+MenuItem.isMenuItem = true;
+
 const connected = connect(({ activeKey, selectedKeys }, { eventKey, subMenuKey }) => ({
   active: activeKey[subMenuKey] === eventKey,
   isSelected: selectedKeys.indexOf(eventKey) !== -1,
 }))(MenuItem);
 
-connected.isMenuItem = true;
-
 export default connected;

src/SubMenu.jsx

@@ -399,7 +399,7 @@ export class SubMenu extends React.Component {
         component=""
         transitionAppear={transitionAppear}
       >
-        <SubPopupMenu {...baseProps}>{children}</SubPopupMenu>
+        <SubPopupMenu {...baseProps} id={this._menuId}>{children}</SubPopupMenu>
       </Animate>
     );
   }
@@ -448,6 +448,17 @@ export class SubMenu extends React.Component {
     if (isInlineMode) {
       style.paddingLeft = props.inlineIndent * props.level;
     }
+
+    let ariaOwns = {};
+    // only set aria-owns when menu is open
+    // otherwise it would be an invalid aria-owns value
+    // since corresponding node cannot be found
+    if (this.props.isOpen) {
+      ariaOwns = {
+        'aria-owns': this._menuId,
+      };
+    }
+
     const title = (
       <div
         ref={this.saveSubMenuTitle}
@@ -456,7 +467,7 @@ export class SubMenu extends React.Component {
         {...titleMouseEvents}
         {...titleClickEvents}
         aria-expanded={isOpen}
-        aria-owns={this._menuId}
+        {...ariaOwns}
         aria-haspopup="true"
         title={typeof props.title === 'string' ? props.title : undefined}
       >
@@ -479,7 +490,7 @@ export class SubMenu extends React.Component {
     } = props;
     menuAllProps.forEach(key => delete props[key]);
     return (
-      <li {...props} {...mouseEvents} className={className}>
+      <li {...props} {...mouseEvents} className={className} role="menuitem">
         {isInlineMode && title}
         {isInlineMode && children}
         {!isInlineMode && (

src/SubPopupMenu.js

@@ -316,8 +316,8 @@ export class SubPopupMenu extends React.Component {
     );
     const domProps = {
       className,
-      role: 'menu',
-      'aria-activedescendant': '',
+      // role could be 'select' and by default set to menu
+      role: props.role || 'menu',
     };
     if (props.id) {
       domProps.id = props.id;

tests/Menu.spec.js

@@ -39,6 +39,27 @@ describe('Menu', () => {
     });
   });
 
+  describe('render role listbox', () => {
+    function createMenu() {
+      return (
+        <Menu
+          className="myMenu"
+          openAnimation="fade"
+          role="listbox"
+        >
+          <MenuItem key="1" role="option">1</MenuItem>
+          <MenuItem key="2" role="option">2</MenuItem>
+          <MenuItem key="3" role="option">3</MenuItem>
+        </Menu>
+      );
+    }
+
+    it(`renders menu correctly`, () => {
+      const wrapper = render(createMenu());
+      expect(renderToJson(wrapper)).toMatchSnapshot();
+    });
+  });
+
   it('set activeKey', () => {
     const wrapper = mount(
       <Menu activeKey="1">

tests/MenuItem.spec.js

@@ -112,4 +112,18 @@ describe('MenuItem', () => {
       expect(onClick).toHaveBeenCalledTimes(3);
     });
   });
+
+  describe('overwrite default role', () => {
+    it('should set empty role', () => {
+      const wrapper = shallow(<NakedMenuItem role={null}>test</NakedMenuItem>);
+
+      expect(wrapper.render()).toMatchSnapshot();
+    });
+
+    it('should set specific role', () => {
+      const wrapper = shallow(<NakedMenuItem role="option">test</NakedMenuItem>);
+
+      expect(wrapper.render()).toMatchSnapshot();
+    });
+  });
 });

tests/__snapshots__/Menu.spec.js.snap

@@ -2,7 +2,6 @@
 
 exports[`Menu render renders horizontal menu correctly 1`] = `
 <ul
-  aria-activedescendant=""
   class="rc-menu myMenu rc-menu-root rc-menu-horizontal"
   role="menu"
   tabindex="0"
@@ -20,7 +19,6 @@ exports[`Menu render renders horizontal menu correctly 1`] = `
       class="rc-menu-item-group-list"
     >
       <li
-        aria-selected="false"
         class="rc-menu-item"
         role="menuitem"
       >
@@ -30,7 +28,6 @@ exports[`Menu render renders horizontal menu correctly 1`] = `
         class=" rc-menu-item-divider"
       />
       <li
-        aria-selected="false"
         class="rc-menu-item"
         role="menuitem"
       >
@@ -39,7 +36,6 @@ exports[`Menu render renders horizontal menu correctly 1`] = `
     </ul>
   </li>
   <li
-    aria-selected="false"
     class="rc-menu-item"
     role="menuitem"
   >
@@ -58,15 +54,13 @@ exports[`Menu render renders horizontal menu correctly 1`] = `
       class="rc-menu-item-group-list"
     >
       <li
-        aria-selected="false"
         class="rc-menu-item"
         role="menuitem"
       >
         4
       </li>
       <li
         aria-disabled="true"
-        aria-selected="false"
         class="rc-menu-item rc-menu-item-disabled"
         role="menuitem"
       >
@@ -76,11 +70,11 @@ exports[`Menu render renders horizontal menu correctly 1`] = `
   </li>
   <li
     class="rc-menu-submenu rc-menu-submenu-horizontal"
+    role="menuitem"
   >
     <div
       aria-expanded="false"
       aria-haspopup="true"
-      aria-owns="item_3$Menu"
       class="rc-menu-submenu-title"
       title="submenu"
     >
@@ -95,7 +89,6 @@ exports[`Menu render renders horizontal menu correctly 1`] = `
 
 exports[`Menu render renders inline menu correctly 1`] = `
 <ul
-  aria-activedescendant=""
   class="rc-menu myMenu rc-menu-root rc-menu-inline"
   role="menu"
   tabindex="0"
@@ -113,7 +106,6 @@ exports[`Menu render renders inline menu correctly 1`] = `
       class="rc-menu-item-group-list"
     >
       <li
-        aria-selected="false"
         class="rc-menu-item"
         role="menuitem"
         style="padding-left:24px"
@@ -124,7 +116,6 @@ exports[`Menu render renders inline menu correctly 1`] = `
         class=" rc-menu-item-divider"
       />
       <li
-        aria-selected="false"
         class="rc-menu-item"
         role="menuitem"
         style="padding-left:24px"
@@ -134,7 +125,6 @@ exports[`Menu render renders inline menu correctly 1`] = `
     </ul>
   </li>
   <li
-    aria-selected="false"
     class="rc-menu-item"
     role="menuitem"
     style="padding-left:24px"
@@ -154,7 +144,6 @@ exports[`Menu render renders inline menu correctly 1`] = `
       class="rc-menu-item-group-list"
     >
       <li
-        aria-selected="false"
         class="rc-menu-item"
         role="menuitem"
         style="padding-left:24px"
@@ -163,7 +152,6 @@ exports[`Menu render renders inline menu correctly 1`] = `
       </li>
       <li
         aria-disabled="true"
-        aria-selected="false"
         class="rc-menu-item rc-menu-item-disabled"
         role="menuitem"
         style="padding-left:24px"
@@ -174,11 +162,11 @@ exports[`Menu render renders inline menu correctly 1`] = `
   </li>
   <li
     class="rc-menu-submenu rc-menu-submenu-inline"
+    role="menuitem"
   >
     <div
       aria-expanded="false"
       aria-haspopup="true"
-      aria-owns="item_3$Menu"
       class="rc-menu-submenu-title"
       style="padding-left:24px"
       title="submenu"
@@ -195,7 +183,6 @@ exports[`Menu render renders inline menu correctly 1`] = `
 
 exports[`Menu render renders vertical menu correctly 1`] = `
 <ul
-  aria-activedescendant=""
   class="rc-menu myMenu rc-menu-root rc-menu-vertical"
   role="menu"
   tabindex="0"
@@ -213,7 +200,6 @@ exports[`Menu render renders vertical menu correctly 1`] = `
       class="rc-menu-item-group-list"
     >
       <li
-        aria-selected="false"
         class="rc-menu-item"
         role="menuitem"
       >
@@ -223,7 +209,6 @@ exports[`Menu render renders vertical menu correctly 1`] = `
         class=" rc-menu-item-divider"
       />
       <li
-        aria-selected="false"
         class="rc-menu-item"
         role="menuitem"
       >
@@ -232,7 +217,6 @@ exports[`Menu render renders vertical menu correctly 1`] = `
     </ul>
   </li>
   <li
-    aria-selected="false"
     class="rc-menu-item"
     role="menuitem"
   >
@@ -251,15 +235,13 @@ exports[`Menu render renders vertical menu correctly 1`] = `
       class="rc-menu-item-group-list"
     >
       <li
-        aria-selected="false"
         class="rc-menu-item"
         role="menuitem"
       >
         4
       </li>
       <li
         aria-disabled="true"
-        aria-selected="false"
         class="rc-menu-item rc-menu-item-disabled"
         role="menuitem"
       >
@@ -269,11 +251,11 @@ exports[`Menu render renders vertical menu correctly 1`] = `
   </li>
   <li
     class="rc-menu-submenu rc-menu-submenu-vertical"
+    role="menuitem"
   >
     <div
       aria-expanded="false"
       aria-haspopup="true"
-      aria-owns="item_3$Menu"
       class="rc-menu-submenu-title"
       title="submenu"
     >
@@ -285,3 +267,33 @@ exports[`Menu render renders vertical menu correctly 1`] = `
   </li>
 </ul>
 `;
+
+exports[`Menu render role listbox renders menu correctly 1`] = `
+<ul
+  class="rc-menu myMenu rc-menu-root rc-menu-vertical"
+  role="listbox"
+  tabindex="0"
+>
+  <li
+    aria-selected="false"
+    class="rc-menu-item"
+    role="option"
+  >
+    1
+  </li>
+  <li
+    aria-selected="false"
+    class="rc-menu-item"
+    role="option"
+  >
+    2
+  </li>
+  <li
+    aria-selected="false"
+    class="rc-menu-item"
+    role="option"
+  >
+    3
+  </li>
+</ul>
+`;