chore: fix aria-* attirbutes

react-component · Apr 27, 2018 · f65a45a · f65a45a
1 parent c9c17ed
commit f65a45a
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 32 deletions.
diff --git a/src/MenuItem.jsx b/src/MenuItem.jsx
@@ -145,14 +145,27 @@ export class MenuItem extends React.Component {
       [this.getSelectedClassName()]: props.isSelected,
       [this.getDisabledClassName()]: props.disabled,
     });
-    const attrs = {
-      ...props.attribute,
+    let attrs = {
       title: props.title,
       className,
+      // set to menuitem by default
       role: 'menuitem',
-      'aria-selected': props.isSelected,
       'aria-disabled': props.disabled,
+      ...props.attribute,
     };
+
+    if (props.role === 'option') {
+      // overwrite to option
+      attrs = {
+        ...attrs,
+        role: 'option',
+        'aria-selected': props.isSelected,
+      }
+    } else if (attrs.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 = {

diff --git a/src/SubMenu.jsx b/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}
       >

diff --git a/src/SubPopupMenu.js b/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;

diff --git a/tests/__snapshots__/Menu.spec.js.snap b/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"
       >
@@ -80,7 +74,6 @@ exports[`Menu render renders horizontal menu correctly 1`] = `
     <div
       aria-expanded="false"
       aria-haspopup="true"
-      aria-owns="item_3$Menu"
       class="rc-menu-submenu-title"
       title="submenu"
     >
@@ -95,7 +88,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 +105,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 +115,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 +124,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 +143,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 +151,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"
@@ -178,7 +165,6 @@ exports[`Menu render renders inline menu correctly 1`] = `
     <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 +181,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 +198,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 +207,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 +215,6 @@ exports[`Menu render renders vertical menu correctly 1`] = `
     </ul>
   </li>
   <li
-    aria-selected="false"
     class="rc-menu-item"
     role="menuitem"
   >
@@ -251,15 +233,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"
       >
@@ -273,7 +253,6 @@ exports[`Menu render renders vertical menu correctly 1`] = `
     <div
       aria-expanded="false"
       aria-haspopup="true"
-      aria-owns="item_3$Menu"
       class="rc-menu-submenu-title"
       title="submenu"
     >

diff --git a/tests/__snapshots__/MenuItem.spec.js.snap b/tests/__snapshots__/MenuItem.spec.js.snap
@@ -2,13 +2,11 @@
 
 exports[`MenuItem rest props can render all props to sub component 1`] = `
 <ul
-  aria-activedescendant=""
   class="rc-menu  rc-menu-root rc-menu-inline"
   role="menu"
   tabindex="0"
 >
   <li
-    aria-selected="false"
     class="rc-menu-item className rc-menu-item-active"
     data-whatever="whatever"
     role="menuitem"
@@ -25,7 +23,6 @@ exports[`MenuItem rest props can render all props to sub component 1`] = `
     <div
       aria-expanded="false"
       aria-haspopup="true"
-      aria-owns="item_1$Menu"
       class="rc-menu-submenu-title"
       style="padding-left: 24px;"
       title="title"
@@ -52,7 +49,6 @@ exports[`MenuItem rest props can render all props to sub component 1`] = `
       class="rc-menu-item-group-list"
     >
       <li
-        aria-selected="false"
         class="rc-menu-item className"
         data-whatever="whatever"
         role="menuitem"