feat(Menu): Accessibility - move MenuItem onClick handler from li to a

CHANGELOG.md

@@ -23,6 +23,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Fixed issues ([#31](https://github.com/stardust-ui/react/issues/31)) and ([#32](https://github.com/stardust-ui/react/issues/32)) @mnajdova ([#38](https://github.com/stardust-ui/react/pull/38))
 - Changing the default styles for Input component @alinais ([#25](https://github.com/stardust-ui/react/pull/25))
 - Upgrade Typescript to version 3.0.1 @luzhon ([#67](https://github.com/stardust-ui/react/pull/67))
+- Menu `onClick` handler moved from `li` to `a` (accessibility) ([#61](https://github.com/stardust-ui/react/pull/61))
 
 ### Features
 - Behaviors for accessibility roles and other ARIA attributes @smykhailov, @jurokapsiar, @sophieH29 ([#29](https://github.com/stardust-ui/react/pull/29))

src/components/Menu/MenuItem.tsx

@@ -86,17 +86,13 @@ class MenuItem extends UIComponent<any, any> {
     const { children, content } = this.props
 
     return (
-      <ElementType
-        className={classes.root}
-        onClick={this.handleClick}
-        {...accessibility.attributes.root}
-        {...rest}
-      >
+      <ElementType className={classes.root} {...accessibility.attributes.root} {...rest}>
         {childrenExist(children) ? (
           children
         ) : (
           <a
             className={cx('ui-menu__item__anchor', classes.anchor)}
+            onClick={this.handleClick}
             {...accessibility.attributes.anchor}
           >
             {content}

src/themes/teams/components/Menu/menuItemStyles.ts

@@ -37,8 +37,6 @@ export default {
       lineHeight: 1,
       position: 'relative',
       verticalAlign: 'middle',
-      padding: `${pxToRem(14)} ${pxToRem(18)}`,
-      cursor: 'pointer',
       display: 'block',
       ...(shape === 'pills' && {
         ...(vertical ? { margin: `0 0 ${pxToRem(5)} 0` } : { margin: `0 ${pxToRem(8)} 0 0` }),
@@ -65,12 +63,6 @@ export default {
             background: variables.typePrimaryActiveBackgroundColor,
           }),
         }),
-        ...(shape === 'underlined' && {
-          ...underlinedItem(variables.defaultActiveBackgroundColor),
-          ...(type === 'primary' && {
-            ...underlinedItem(variables.typePrimaryActiveBorderColor),
-          }),
-        }),
       },
 
       ...(active && {
@@ -114,8 +106,37 @@ export default {
             }),
           },
         }),
+      }),
+    }
+  },
+
+  anchor: ({ props, variables }) => {
+    const { active, shape, type } = props
+
+    return {
+      color: 'inherit',
+      display: 'block',
+      ...(shape === 'underlined'
+        ? { padding: '0 0 8px 0' }
+        : { padding: `${pxToRem(14)} ${pxToRem(18)}` }),
+      cursor: 'pointer',
+
+      ':hover': {
+        color: 'inherit',
         ...(shape === 'underlined' && {
+          paddingBottom: '4px',
+          ...underlinedItem(variables.defaultActiveBackgroundColor),
+          ...(type === 'primary' && {
+            ...underlinedItem(variables.typePrimaryActiveBorderColor),
+          }),
+        }),
+      },
+
+      ...(active &&
+        shape === 'underlined' && {
           color: variables.defaultColor,
+          paddingBottom: '4px',
+          ':hover': {},
           ...underlinedItem(variables.defaultActiveColor),
           ...(type === 'primary'
             ? {
@@ -126,13 +147,6 @@ export default {
                 fontWeight: '700',
               }),
         }),
-      }),
     }
   },
-  anchor: () => ({
-    color: 'inherit',
-    ':hover': {
-      color: 'inherit',
-    },
-  }),
 }

test/specs/components/Menu/Menu-test.tsx

@@ -27,7 +27,11 @@ describe('Menu', () => {
       const items = getItems()
       const menuItems = mountWithProvider(<Menu items={items} />).find('MenuItem')
 
-      menuItems.first().simulate('click')
+      menuItems
+        .first()
+        .find('a')
+        .first()
+        .simulate('click')
       expect(items[0].onClick).toHaveBeenCalled()
     })
 
@@ -48,7 +52,11 @@ describe('Menu', () => {
         const wrapper = mountWithProvider(<Menu items={getItems()} />)
         const menuItems = wrapper.find('MenuItem')
 
-        menuItems.at(1).simulate('click')
+        menuItems
+          .at(1)
+          .find('a')
+          .first()
+          .simulate('click')
 
         const updatedItems = wrapper.find('MenuItem')
 

test/specs/components/Menu/MenuItem-test.tsx

@@ -5,7 +5,11 @@ import { getTestingRenderedComponent } from 'test/utils'
 import MenuItem from 'src/components/Menu/MenuItem'
 
 describe('MenuItem', () => {
-  isConformant(MenuItem)
+  isConformant(MenuItem, {
+    eventTargets: {
+      onClick: 'a',
+    },
+  })
   handlesAccessibility(MenuItem, { defaultRootRole: 'presentation' })
   handlesAccessibility(MenuItem, { defaultRootRole: 'menuitem', partSelector: 'a' })