Merge pull request #155 from cdeutsch/menuitem-role-none

On MenuItem, set the `role` to `none` if the role is `null` or `none`.

Author: picodoth

src/MenuItem.jsx

@@ -150,7 +150,7 @@ export class MenuItem extends React.Component {
       title: props.title,
       className,
       // set to menuitem by default
-      role: 'menuitem',
+      role: props.role || 'menuitem',
       'aria-disabled': props.disabled,
     };
 
@@ -161,10 +161,13 @@ export class MenuItem extends React.Component {
         role: 'option',
         'aria-selected': props.isSelected,
       };
-    } else if (props.role === null) {
+    } else if (props.role === null || props.role === 'none') {
       // sometimes we want to specify role inside <li/> element
       // <li><a role='menuitem'>Link</a></li> would be a good example
-      delete attrs.role;
+      // in this case the role on <li/> should be "none" to
+      // remove the implied listitem role.
+      // https://www.w3.org/TR/wai-aria-practices-1.1/examples/menubar/menubar-1/menubar-1.html
+      attrs.role = 'none';
     }
     // In case that onClick/onMouseLeave/onMouseEnter is passed down from owner
     const mouseEvent = {

tests/MenuItem.spec.js

@@ -117,13 +117,25 @@ describe('MenuItem', () => {
   });
 
   describe('overwrite default role', () => {
-    it('should set empty role', () => {
+    it('should set role to none if null', () => {
       const wrapper = shallow(<NakedMenuItem role={null}>test</NakedMenuItem>);
 
       expect(wrapper.render()).toMatchSnapshot();
     });
 
-    it('should set specific role', () => {
+    it('should set role to none if none', () => {
+      const wrapper = shallow(<NakedMenuItem role="none">test</NakedMenuItem>);
+
+      expect(wrapper.render()).toMatchSnapshot();
+    });
+
+    it('should set role to listitem', () => {
+      const wrapper = shallow(<NakedMenuItem role="listitem">test</NakedMenuItem>);
+
+      expect(wrapper.render()).toMatchSnapshot();
+    });
+
+    it('should set role to option', () => {
       const wrapper = shallow(<NakedMenuItem role="option">test</NakedMenuItem>);
 
       expect(wrapper.render()).toMatchSnapshot();

tests/__snapshots__/MenuItem.spec.js.snap

@@ -1,14 +1,33 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`MenuItem overwrite default role should set empty role 1`] = `
+exports[`MenuItem overwrite default role should set role to listitem 1`] = `
 <li
   class="undefined-item"
+  role="listitem"
 >
   test
 </li>
 `;
 
-exports[`MenuItem overwrite default role should set specific role 1`] = `
+exports[`MenuItem overwrite default role should set role to none if none 1`] = `
+<li
+  class="undefined-item"
+  role="none"
+>
+  test
+</li>
+`;
+
+exports[`MenuItem overwrite default role should set role to none if null 1`] = `
+<li
+  class="undefined-item"
+  role="none"
+>
+  test
+</li>
+`;
+
+exports[`MenuItem overwrite default role should set role to option 1`] = `
 <li
   class="undefined-item"
   role="option"