fix(Menu): correctly handle disabled item

CHANGELOG.md

@@ -22,7 +22,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Add Dropdown Single Selection variant @silviuavram ([#584](https://github.com/stardust-ui/react/pull/584))
 
 ### Fixes
-- Fix unicode arrow characters to be RTL aware ([#690](https://github.com/stardust-ui/react/pull/690))
+- Fix unicode arrow characters to be RTL aware @mnajdova ([#690](https://github.com/stardust-ui/react/pull/690))
+- Correctly handle disabled `Menu.Item` in styles and accessibility @miroslavstastny ([#694](https://github.com/stardust-ui/react/pull/694))
 
 <!--------------------------------[ v0.16.0 ]------------------------------- -->
 ## [v0.16.0](https://github.com/stardust-ui/react/tree/v0.16.0) (2019-01-07)

docs/src/examples/components/Menu/States/MenuExampleDisabled.shorthand.tsx

@@ -0,0 +1,12 @@
+import * as React from 'react'
+import { Menu } from '@stardust-ui/react'
+
+const items = [
+  { key: 'editorials', content: 'Editorials' },
+  { key: 'review', content: 'Disabled Item', disabled: true },
+  { key: 'events', content: 'Upcoming Events' },
+]
+
+const MenuExampleDisabled = () => <Menu defaultActiveIndex={0} items={items} />
+
+export default MenuExampleDisabled

docs/src/examples/components/Menu/States/index.tsx

@@ -0,0 +1,15 @@
+import * as React from 'react'
+import ComponentExample from 'docs/src/components/ComponentDoc/ComponentExample'
+import ExampleSection from 'docs/src/components/ComponentDoc/ExampleSection'
+
+const States = () => (
+  <ExampleSection title="States">
+    <ComponentExample
+      title="Disabled"
+      description="A menu item can be disabled to show it is currently unable to be interacted with."
+      examplePath="components/Menu/States/MenuExampleDisabled"
+    />
+  </ExampleSection>
+)
+
+export default States

docs/src/examples/components/Menu/index.tsx

@@ -1,10 +1,12 @@
 import * as React from 'react'
 import Types from './Types'
+import States from './States'
 import Variations from './Variations'
 
 const MenuExamples = () => (
   <div>
     <Types />
+    <States />
     <Variations />
   </div>
 )

src/components/Menu/MenuItem.tsx

@@ -172,7 +172,17 @@ class MenuItem extends AutoControlledComponent<ReactProps<MenuItemProps>, MenuIt
   }
 
   renderComponent({ ElementType, classes, accessibility, rest, styles }) {
-    const { children, content, icon, wrapper, menu, primary, secondary, active } = this.props
+    const {
+      children,
+      content,
+      icon,
+      wrapper,
+      menu,
+      primary,
+      secondary,
+      active,
+      disabled,
+    } = this.props
 
     const { menuOpen } = this.state
 
@@ -182,6 +192,7 @@ class MenuItem extends AutoControlledComponent<ReactProps<MenuItemProps>, MenuIt
       <Ref innerRef={this.itemRef}>
         <ElementType
           className={classes.root}
+          disabled={disabled}
           onBlur={this.handleBlur}
           onFocus={this.handleFocus}
           {...accessibility.attributes.anchor}
@@ -283,6 +294,13 @@ class MenuItem extends AutoControlledComponent<ReactProps<MenuItemProps>, MenuIt
   }
 
   private handleClick = e => {
+    const { disabled } = this.props
+
+    if (disabled) {
+      e.preventDefault()
+      return
+    }
+
     this.performClick(e)
     _.invoke(this.props, 'onClick', e, this.props)
   }

src/lib/accessibility/Behaviors/Dialog/dialogBehavior.ts

@@ -8,7 +8,7 @@ import popupFocusTrapBehavior from '../Popup/popupFocusTrapBehavior'
  * Adds tabIndex='0' to 'trigger' component's part, if it is not tabbable element and no tabIndex attribute provided.
  *
  * @specification
- * Adds attribute 'aria-disabled=true' to 'trigger' component's part based on the property 'disabled'.
+ * Adds attribute 'aria-disabled=true' to 'trigger' component's part if 'disabled' property is true. Does not set the attribute otherwise.
  * Adds attribute 'aria-modal=true' to 'popup' component's part.
  * Adds attribute 'role=dialog' to 'popup' component's part.
  * Traps focus inside component.

src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts

@@ -1,6 +1,7 @@
 import { Accessibility } from '../../types'
 import { IS_FOCUSABLE_ATTRIBUTE } from '../../FocusZone/focusUtilities'
 import * as keyboardKey from 'keyboard-key'
+import * as _ from 'lodash'
 
 /**
  * @description
@@ -10,10 +11,11 @@ import * as keyboardKey from 'keyboard-key'
  * Adds role 'presentation' to 'root' component's part.
  * Adds role 'menuitem' to 'anchor' component's part.
  * Adds attribute 'tabIndex=0' to 'anchor' component's part.
- * Adds attribute 'data-is-focusable=true' to 'anchor' component's part.
+ * Adds attribute 'data-is-focusable=false' to 'anchor' component's part if 'disabled' property is true. Sets the attribute to 'true' otherwise.
  * Adds attribute 'aria-label' based on the property 'aria-label' to 'anchor' component's part.
  * Adds attribute 'aria-labelledby' based on the property 'aria-labelledby' to 'anchor' component's part.
  * Adds attribute 'aria-describedby' based on the property 'aria-describedby' to 'anchor' component's part.
+ * Adds attribute 'aria-disabled=true' to 'anchor' component's part based on the property 'disabled'. This can be overriden by providing 'aria-disabled' property directly to the component.
  */
 
 const menuItemBehavior: Accessibility = (props: any) => ({
@@ -27,11 +29,16 @@ const menuItemBehavior: Accessibility = (props: any) => ({
       'aria-label': props['aria-label'],
       'aria-labelledby': props['aria-labelledby'],
       'aria-describedby': props['aria-describedby'],
-      [IS_FOCUSABLE_ATTRIBUTE]: true,
+      'aria-disabled': !_.isNil(props['aria-disabled'])
+        ? props['aria-disabled']
+        : !!props['disabled']
+        ? true
+        : undefined,
+      [IS_FOCUSABLE_ATTRIBUTE]: !props['disabled'],
     },
   },
 
-  handledProps: ['aria-label', 'aria-labelledby', 'aria-describedby'],
+  handledProps: ['aria-label', 'aria-labelledby', 'aria-describedby', 'aria-disabled'],
 
   keyActions: {
     root: {

src/lib/accessibility/Behaviors/Popup/popupBehavior.ts

@@ -8,7 +8,7 @@ import * as _ from 'lodash'
  * Adds tabIndex='0' to 'trigger' component's part, if it is not tabbable element and no tabIndex attribute provided.
  *
  * @specification
- * Adds attribute 'aria-disabled=true' to 'trigger' component's part based on the property 'disabled'.
+ * Adds attribute 'aria-disabled=true' to 'trigger' component's part if 'disabled' property is true. Does not set the attribute otherwise.
  */
 const popupBehavior: Accessibility = (props: any) => {
   return {
@@ -18,7 +18,9 @@ const popupBehavior: Accessibility = (props: any) => {
         tabIndex: getAriaAttributeFromProps('tabIndex', props, '0'),
         'aria-disabled': !_.isNil(props['aria-disabled'])
           ? props['aria-disabled']
-          : !!props['disabled'],
+          : !!props['disabled']
+          ? true
+          : undefined,
       },
     },
     keyActions: {

src/lib/accessibility/Behaviors/Popup/popupFocusTrapBehavior.ts

@@ -7,7 +7,7 @@ import popupBehavior from './popupBehavior'
  * Adds tabIndex='0' to 'trigger' component's part, if it is not tabbable element and no tabIndex attribute provided.
  *
  * @specification
- * Adds attribute 'aria-disabled=true' to 'trigger' component's part based on the property 'disabled'.
+ * Adds attribute 'aria-disabled=true' to 'trigger' component's part if 'disabled' property is true. Does not set the attribute otherwise.
  * Traps focus inside component.
  */
 const popupFocusTrapBehavior: Accessibility = (props: any) => ({

src/lib/accessibility/Behaviors/Tab/tabBehavior.ts

@@ -8,11 +8,13 @@ import * as _ from 'lodash'
  * Adds role 'presentation' to 'root' component's part.
  * Adds role 'tab' to 'anchor' component's part.
  * Adds attribute 'tabIndex=0' to 'anchor' component's part.
+ * Adds attribute 'data-is-focusable=false' to 'anchor' component's part if 'disabled' property is true. Sets the attribute to 'true' otherwise.
  * Adds attribute 'aria-selected=true' to 'anchor' component's part based on the property 'active'. This can be overriden by providing 'aria-selected' property directly to the component.
  * Adds attribute 'aria-label' based on the property 'aria-label' to 'anchor' component's part.
  * Adds attribute 'aria-labelledby' based on the property 'aria-labelledby' to 'anchor' component's part.
  * Adds attribute 'aria-describedby' based on the property 'aria-describedby' to 'anchor' component's part.
  * Adds attribute 'aria-controls' based on the property 'aria-controls' to 'anchor' component's part.
+ * Adds attribute 'aria-disabled=true' to 'anchor' component's part based on the property 'disabled'. This can be overriden by providing 'aria-disabled' property directly to the component.
  * Performs click action with 'Enter' and 'Spacebar' on 'anchor'.
  */
 const tabBehavior: Accessibility = (props: any) => ({
@@ -30,7 +32,12 @@ const tabBehavior: Accessibility = (props: any) => ({
       'aria-labelledby': props['aria-labelledby'],
       'aria-describedby': props['aria-describedby'],
       'aria-controls': props['aria-controls'],
-      [IS_FOCUSABLE_ATTRIBUTE]: true,
+      'aria-disabled': !_.isNil(props['aria-disabled'])
+        ? props['aria-disabled']
+        : !!props['disabled']
+        ? true
+        : undefined,
+      [IS_FOCUSABLE_ATTRIBUTE]: !props['disabled'],
     },
   },
   handledProps: [
@@ -39,6 +46,7 @@ const tabBehavior: Accessibility = (props: any) => ({
     'aria-describedby',
     'aria-controls',
     'aria-selected',
+    'aria-disabled',
   ],
   keyActions: {
     anchor: {

src/lib/accessibility/Behaviors/Toolbar/toolbarButtonBehavior.ts

@@ -11,6 +11,7 @@ import * as _ from 'lodash'
  * Adds role 'presentation' to 'root' component's part.
  * Adds role 'button' to 'anchor' component's part.
  * Adds attribute 'tabIndex=0' to 'anchor' component's part.
+ * Adds attribute 'data-is-focusable=false' to 'anchor' component's part if 'disabled' property is true. Sets the attribute to 'true' otherwise.
  * Adds attribute 'aria-label' based on the property 'aria-label' to 'anchor' component's part.
  * Adds attribute 'aria-labelledby' based on the property 'aria-labelledby' to 'anchor' component's part.
  * Adds attribute 'aria-describedby' based on the property 'aria-describedby' to 'anchor' component's part.
@@ -27,11 +28,13 @@ const toolbarButtonBehavior: Accessibility = (props: any) => ({
       tabIndex: '0',
       'aria-disabled': !_.isNil(props['aria-disabled'])
         ? props['aria-disabled']
-        : !!props['disabled'],
+        : !!props['disabled']
+        ? true
+        : undefined,
       'aria-label': props['aria-label'],
       'aria-labelledby': props['aria-labelledby'],
       'aria-describedby': props['aria-describedby'],
-      [IS_FOCUSABLE_ATTRIBUTE]: true,
+      [IS_FOCUSABLE_ATTRIBUTE]: !props['disabled'],
     },
   },
 

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

@@ -144,6 +144,7 @@ const menuItemStyles: ComponentSlotStylesInput<MenuItemPropsAndState, MenuVariab
   wrapper: ({ props, variables: v, theme }): ICSSInJSStyle => {
     const {
       active,
+      disabled,
       iconOnly,
       isFromKeyboard,
       pills,
@@ -247,11 +248,27 @@ const menuItemStyles: ComponentSlotStylesInput<MenuItemPropsAndState, MenuVariab
             }),
           }),
       },
+
+      ...(disabled && {
+        color: v.disabledColor,
+        ':hover': {
+          // empty - overwrite all existing hover styles
+        },
+      }),
     }
   },
 
   root: ({ props, variables: v, theme }): ICSSInJSStyle => {
-    const { active, iconOnly, isFromKeyboard, pointing, primary, underlined, vertical } = props
+    const {
+      active,
+      iconOnly,
+      isFromKeyboard,
+      pointing,
+      primary,
+      underlined,
+      vertical,
+      disabled,
+    } = props
     const { arrowDown } = theme.siteVariables
     const sideArrow = getSideArrow(theme)
 
@@ -333,6 +350,14 @@ const menuItemStyles: ComponentSlotStylesInput<MenuItemPropsAndState, MenuVariab
           : !active && underlined && underlinedItem(v.activeBackgroundColor)),
       },
 
+      ...(disabled && {
+        cursor: 'default',
+        ':hover': {
+          // reset all existing hover styles
+          color: 'inherit',
+        },
+      }),
+
       '::after': {
         ...(props.menu && {
           position: 'relative',

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

@@ -20,6 +20,8 @@ export interface MenuVariables {
   primaryHoverBorderColor: string
   primaryUnderlinedBorderColor: string
 
+  disabledColor: string
+
   circularRadius: string
   lineHeightBase: string
 }
@@ -45,6 +47,8 @@ export default (siteVars: any): MenuVariables => {
     primaryHoverBorderColor: siteVars.gray08,
     primaryUnderlinedBorderColor: siteVars.gray08,
 
+    disabledColor: siteVars.gray06,
+
     circularRadius: pxToRem(999),
     lineHeightBase: siteVars.lineHeightBase,
   }

test/specs/behaviors/testDefinitions.ts

@@ -90,7 +90,7 @@ definitions.push({
 
 // Adds attribute 'aria-selected=true' to 'anchor' component's part based on the property 'active'. This can be overriden by directly providing 'aria-selected' property to the component.
 definitions.push({
-  regexp: /Adds attribute '([a-z A-Z -]+)=([a-z 0-9]+)' to '([a-z -]+)' component's part based on the property '[a-z]+'\. This can be overriden by providing '([a-z -]+)' property directly to the component\./g,
+  regexp: /Adds attribute '([a-zA-Z-]+)=([a-z0-9]+)' to '([a-z-]+)' component's part based on the property '[a-z]+'\. This can be overriden by providing '([a-z-]+)' property directly to the component\./g,
   testMethod: (parameters: TestMethod) => {
     const [attributeToBeAdded, valueOfAttributeToBeAdded, component, overridingProperty] = [
       ...parameters.props,
@@ -124,20 +124,68 @@ definitions.push({
   },
 })
 
-// Example: Adds attribute 'aria-disabled=true' to 'trigger' component's part based on the property 'disabled'.
+function testMethodConditionallyAddAttribute(
+  parameters,
+  attributeToBeAdded,
+  valueOfAttributeToBeAddedIfTrue,
+  component,
+  propertyDependsOn,
+  valueOfAttributeToBeAddedOtherwise,
+) {
+  const propertyWithAriaSelected = {}
+  const expectedResultAttributeNotDefined = parameters.behavior(propertyWithAriaSelected)
+    .attributes[component][attributeToBeAdded]
+  expect(testHelper.convertToBooleanIfApplicable(expectedResultAttributeNotDefined)).toEqual(
+    testHelper.convertToBooleanIfApplicable(valueOfAttributeToBeAddedOtherwise),
+  )
+
+  propertyWithAriaSelected[propertyDependsOn] = true
+  const expectedResultAttributeDefined = parameters.behavior(propertyWithAriaSelected).attributes[
+    component
+  ][attributeToBeAdded]
+  expect(testHelper.convertToBooleanIfApplicable(expectedResultAttributeDefined)).toEqual(
+    testHelper.convertToBooleanIfApplicable(valueOfAttributeToBeAddedIfTrue),
+  )
+}
+
+// Example: Adds attribute 'aria-disabled=true' to 'trigger' component's part if 'disabled' property is true. Does not set the attribute otherwise.
 definitions.push({
-  regexp: /Adds attribute '([a-z A-Z -]+)=([a-z 0-9]+)' to '([a-z -]+)' component's part based on the property '([a-z -]+)'\./g,
+  regexp: /Adds attribute '([a-zA-Z-]+)=([a-z0-9]+)' to '([a-z-]+)' component's part if '([a-z -]+)' property is true\. Does not set the attribute otherwise\./g,
   testMethod: (parameters: TestMethod) => {
     const [attributeToBeAdded, valueOfAttributeToBeAdded, component, propertyDependsOn] = [
       ...parameters.props,
     ]
-    const propertyWithAriaSelected = {}
-    propertyWithAriaSelected[propertyDependsOn] = true
-    const expectedResultAttributeDefined = parameters.behavior(propertyWithAriaSelected).attributes[
-      component
-    ][attributeToBeAdded]
-    expect(testHelper.convertToBooleanIfApplicable(expectedResultAttributeDefined)).toEqual(
-      testHelper.convertToBooleanIfApplicable(valueOfAttributeToBeAdded),
+
+    testMethodConditionallyAddAttribute(
+      parameters,
+      attributeToBeAdded,
+      valueOfAttributeToBeAdded,
+      component,
+      propertyDependsOn,
+      undefined,
+    )
+  },
+})
+
+// Example: Adds attribute 'aria-disabled=true' to 'trigger' component's part if 'disabled' property is true. Sets the attribute to 'false' otherwise.
+definitions.push({
+  regexp: /Adds attribute '([a-zA-Z-]+)=([a-z0-9]+)' to '([a-z-]+)' component's part if '([a-z -]+)' property is true\. Sets the attribute to '([a-z0-9]+)' otherwise\./g,
+  testMethod: (parameters: TestMethod) => {
+    const [
+      attributeToBeAdded,
+      valueOfAttributeToBeAddedIfTrue,
+      component,
+      propertyDependsOn,
+      valueOfAttributeToBeAddedOtherwise,
+    ] = [...parameters.props]
+
+    testMethodConditionallyAddAttribute(
+      parameters,
+      attributeToBeAdded,
+      valueOfAttributeToBeAddedIfTrue,
+      component,
+      propertyDependsOn,
+      valueOfAttributeToBeAddedOtherwise,
     )
   },
 })