[ContextualMenu] Made disabled buttons focusable

common/changes/office-ui-fabric-react/magellan-focusableDisabledItems_2018-02-22-23-48.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "office-ui-fabric-react",
+      "comment": "[ContextualMenu] Disabled buttons are focusable",
+      "type": "patch"
+    }
+  ],
+  "packageName": "office-ui-fabric-react",
+  "email": "law@microsoft.com"
+}

packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx

@@ -239,13 +239,22 @@ describe('ContextualMenu', () => {
     expect(document.querySelector('.SubMenuClass')).toBeDefined();
   });
 
-  it('applies disabled property when `disabled` is true', () => {
+  it('can focus on disabled items', () => {
     const items: IContextualMenuItem[] = [
       {
         name: 'TestText 1',
         key: 'TestKey1',
+      },
+      {
+        name: 'TestText 2',
+        key: 'TestKey2',
         disabled: true,
       },
+      {
+        name: 'TestText 3',
+        key: 'TestKey3',
+        isDisabled: true,
+      },
     ];
 
     ReactTestUtils.renderIntoDocument<ContextualMenu>(
@@ -254,17 +263,51 @@ describe('ContextualMenu', () => {
       />
     );
 
-    const menuItem = document.querySelector('button.ms-ContextualMenu-link') as HTMLButtonElement;
+    const menuItems = document.querySelectorAll('button.ms-ContextualMenu-link') as NodeListOf<HTMLButtonElement>;
+    expect(menuItems.length).toEqual(3);
+
+    menuItems[0].focus();
+    expect(document.activeElement.textContent).toEqual('TestText 1');
+    expect(document.activeElement.className.split(' ')).not.toContain('is-disabled');
 
-    expect(menuItem.disabled).toBeTruthy();
+    menuItems[1].focus();
+    expect(document.activeElement.textContent).toEqual('TestText 2');
+    expect(document.activeElement.className.split(' ')).toContain('is-disabled');
+
+    menuItems[2].focus();
+    expect(document.activeElement.textContent).toEqual('TestText 3');
+    expect(document.activeElement.className.split(' ')).toContain('is-disabled');
   });
 
-  it('applies disabled property when deprecated property `isDisabled` is true', () => {
+  it('cannot click on disabled items', () => {
+    const itemsClicked = [
+      false,
+      false,
+      false
+    ];
     const items: IContextualMenuItem[] = [
       {
         name: 'TestText 1',
         key: 'TestKey1',
+        onClick: () => itemsClicked[0] = true
+      },
+      {
+        name: 'TestText 2',
+        key: 'TestKey2',
+        disabled: true,
+        onClick: () => {
+          itemsClicked[1] = true;
+          fail('Disabled item should not be clickable');
+        }
+      },
+      {
+        name: 'TestText 3',
+        key: 'TestKey3',
         isDisabled: true,
+        onClick: () => {
+          itemsClicked[2] = true;
+          fail('Disabled item should not be clickable');
+        }
       },
     ];
 
@@ -274,9 +317,17 @@ describe('ContextualMenu', () => {
       />
     );
 
-    const menuItem = document.querySelector('button.ms-ContextualMenu-link') as HTMLButtonElement;
+    const menuItems = document.querySelectorAll('button.ms-ContextualMenu-link') as NodeListOf<HTMLButtonElement>;
+    expect(menuItems.length).toEqual(3);
+
+    menuItems[0].click();
+    expect(itemsClicked[0]).toEqual(true);
+
+    menuItems[1].click();
+    expect(itemsClicked[1]).toEqual(false);
 
-    expect(menuItem.disabled).toBeTruthy();
+    menuItems[2].click();
+    expect(itemsClicked[2]).toEqual(false);
   });
 
   it('renders headers properly', () => {

packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx

@@ -290,7 +290,7 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
             { title && <div className={ this._classNames.title } role='heading' aria-level={ 1 }> { title } </div> }
             { (items && items.length) ? (
               <FocusZone
-                {...this._adjustedFocusZoneProps }
+                { ...this._adjustedFocusZoneProps }
                 className={ this._classNames.root }
                 isCircularNavigation={ true }
                 allowTabKey={ true }
@@ -466,11 +466,11 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
     return (
       <div className={ this._classNames.header } style={ item.style } role='heading' aria-level={ this.props.title ? 2 : 1 }>
         <ChildrenRenderer
-          item={item}
-          classNames={classNames}
-          index={index}
-          onCheckmarkClick={hasCheckmarks? this._onItemClick : undefined}
-          hasIcons={hasIcons}
+          item={ item }
+          classNames={ classNames }
+          index={ index }
+          onCheckmarkClick={ hasCheckmarks ? this._onItemClick : undefined }
+          hasIcons={ hasIcons }
         />
       </div>);
   }
@@ -492,11 +492,11 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
           onClick={ this._onAnchorClick.bind(this, item) }
         >
           <ChildrenRenderer
-            item={item}
-            classNames={classNames}
-            index={index}
-            onCheckmarkClick={hasCheckmarks? this._onItemClick : undefined}
-            hasIcons={hasIcons}
+            item={ item }
+            classNames={ classNames }
+            index={ index }
+            onCheckmarkClick={ hasCheckmarks ? this._onItemClick : undefined }
+            hasIcons={ hasIcons }
           />
         </a>
       </div>);
@@ -531,6 +531,10 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
     const defaultRole = canCheck ? 'menuitemcheckbox' : 'menuitem';
     const itemHasSubmenu = hasSubmenu(item);
 
+    const buttonNativeProperties = getNativeProps(item, buttonProperties);
+    // Do not add the deleted attribute to the button so that it is focusable
+    delete (buttonNativeProperties as any).disabled;
+
     const itemButtonProperties = {
       className: classNames.root,
       onClick: this._onItemClick.bind(this, item),
@@ -539,7 +543,6 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
       onMouseLeave: this._onMouseItemLeave.bind(this, item),
       onMouseDown: (ev: any) => this._onItemMouseDown(item, ev),
       onMouseMove: this._onItemMouseMove.bind(this, item),
-      disabled: this._isItemDisabled(item),
       href: item.href,
       title: item.title,
       'aria-label': ariaLabel,
@@ -556,10 +559,16 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
 
     return (
       <button
-        { ...getNativeProps(item, buttonProperties) }
+        { ...buttonNativeProperties }
         { ...itemButtonProperties }
       >
-        <ChildrenRenderer item={item} classNames={classNames} index={index} onCheckmarkClick={hasCheckmarks? this._onItemClick : undefined} hasIcons={hasIcons} />
+        <ChildrenRenderer
+          item={ item }
+          classNames={ classNames }
+          index={ index }
+          onCheckmarkClick={ hasCheckmarks ? this._onItemClick : undefined }
+          hasIcons={ hasIcons }
+        />
       </button>
     );
   }
@@ -617,7 +626,7 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
     } as IContextualMenuItem;
     return React.createElement('button',
       getNativeProps(itemProps, buttonProperties),
-      <ChildrenRenderer item={item} classNames={classNames} index={index} onCheckmarkClick={hasCheckmarks? this._onItemClick : undefined} hasIcons={hasIcons} />,
+      <ChildrenRenderer item={ item } classNames={ classNames } index={ index } onCheckmarkClick={ hasCheckmarks ? this._onItemClick : undefined } hasIcons={ hasIcons } />,
     );
   }
 
@@ -640,7 +649,7 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
         onMouseDown: (ev: any) => this._onItemMouseDown(item, ev),
         onMouseMove: this._onItemMouseMove.bind(this, item)
       }),
-      <ChildrenRenderer item={item} classNames={classNames} index={index} hasIcons={false} />
+      <ChildrenRenderer item={ item } classNames={ classNames } index={ index } hasIcons={ false } />
     );
   }
 
@@ -821,6 +830,9 @@ export class ContextualMenu extends BaseComponent<IContextualMenuProps, IContext
   }
 
   private _executeItemClick(item: IContextualMenuItem, ev: React.MouseEvent<HTMLElement>) {
+    if (item.disabled || item.isDisabled) {
+      return;
+    }
     if (item.onClick) {
       item.onClick(ev, item);
     } else if (this.props.onItemClick) {

packages/office-ui-fabric-react/src/components/ContextualMenu/examples/ContextualMenu.Basic.Example.tsx

@@ -23,28 +23,33 @@ export class ContextualMenuBasicExample extends React.Component {
             items: [
               {
                 key: 'newItem',
-                name: 'New'
+                name: 'New',
+                onClick: () => console.log('New clicked')
               },
               {
                 key: 'divider_1',
                 itemType: ContextualMenuItemType.Divider
               },
               {
                 key: 'rename',
-                name: 'Rename'
+                name: 'Rename',
+                onClick: () => console.log('Rename clicked')
               },
               {
                 key: 'edit',
-                name: 'Edit'
+                name: 'Edit',
+                onClick: () => console.log('Edit clicked')
               },
               {
                 key: 'properties',
-                name: 'Properties'
+                name: 'Properties',
+                onClick: () => console.log('Properties clicked')
               },
               {
                 key: 'disabled',
                 name: 'Disabled item',
-                disabled: true
+                disabled: true,
+                onClick: () => console.error('Disabled item should not be clickable.')
               }
             ]
           } }