Fix ContextualMenuItemButton's secondary text never read by screen-reader

common/changes/office-ui-fabric-react/keco-ctx-menu-secondary-txt-aria_2018-10-11-23-57.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "office-ui-fabric-react",
+      "comment": "Fix ContextualMenuItem secondary text not read by screen reader due to aria-label containing only primary text.\"",
+      "type": "patch"
+    }
+  ],
+  "packageName": "office-ui-fabric-react",
+  "email": "keco@microsoft.com"
+}

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

@@ -20,6 +20,43 @@ describe('ContextualMenu', () => {
     }
   });
 
+  it('allows setting aria-label per ContextualMenuItem', () => {
+    const items: IContextualMenuItem[] = [{ text: 'Later Today', key: 'Today', secondaryText: '7:00 PM', ariaLabel: 'foo' }];
+
+    ReactTestUtils.renderIntoDocument<IContextualMenuProps>(<ContextualMenu items={items} />);
+
+    const menuItemButtonEl = document.querySelector('.ms-ContextualMenu-link') as HTMLButtonElement;
+    const ariaLabel = menuItemButtonEl.getAttribute('aria-label');
+
+    expect(ariaLabel).toBe('foo');
+  });
+
+  // By default, do not set aria-label with value set to item text only.
+  // This is to ensure that screen-readers read both primary and (optional) secondary text,
+  // unless aria-label set explicitly. See https://github.com/OfficeDev/office-ui-fabric-react/pull/6670.
+  it('by default aria-label is undefined per ContextualMenuItem', () => {
+    const items: IContextualMenuItem[] = [{ text: 'Later Today', key: 'Today', secondaryText: '7:00 PM' }];
+
+    ReactTestUtils.renderIntoDocument<IContextualMenuProps>(<ContextualMenu items={items} />);
+
+    const menuItemButtonEl = document.querySelector('.ms-ContextualMenu-link') as HTMLButtonElement;
+    const hasAriaLabel = menuItemButtonEl.hasAttribute('aria-label');
+
+    expect(hasAriaLabel).toBe(false);
+  });
+
+  it('renders secondary text if provided per ContextualMenuItem', () => {
+    const items: IContextualMenuItem[] = [{ text: 'Later Today', key: 'Today', secondaryText: 'foo' }];
+
+    ReactTestUtils.renderIntoDocument<IContextualMenuProps>(<ContextualMenu items={items} />);
+
+    const menuItemPrimaryText = document.querySelector('.ms-ContextualMenu-itemText') as HTMLSpanElement;
+    const menuItemSecondaryText = document.querySelector('.ms-ContextualMenu-secondaryText') as HTMLSpanElement;
+
+    expect(menuItemPrimaryText.textContent).toBe('Later Today');
+    expect(menuItemSecondaryText.textContent).toBe('foo');
+  });
+
   it('does not have a scrollbar due to an overflowing icon', () => {
     const items: IContextualMenuItem[] = [
       { text: 'TestText 1', key: 'TestKey1', canCheck: true, isChecked: true },
@@ -48,9 +85,7 @@ describe('ContextualMenu', () => {
       spyCalled = true;
     };
 
-    ReactTestUtils.renderIntoDocument<IContextualMenuProps>(
-      <ContextualMenu items={items} isSubMenu={true} onDismiss={onDismissSpy} />
-    );
+    ReactTestUtils.renderIntoDocument<IContextualMenuProps>(<ContextualMenu items={items} isSubMenu={true} onDismiss={onDismissSpy} />);
 
     const menuList = document.querySelector('ul.ms-ContextualMenu-list') as HTMLUListElement;
     ReactTestUtils.Simulate.keyDown(menuList, { which: KeyCodes.left });
@@ -918,9 +953,7 @@ describe('ContextualMenu', () => {
     ];
     const customRenderer = jest.fn(() => null);
 
-    ReactTestUtils.renderIntoDocument<IContextualMenuProps>(
-      <ContextualMenu items={items} contextualMenuItemAs={customRenderer} />
-    );
+    ReactTestUtils.renderIntoDocument<IContextualMenuProps>(<ContextualMenu items={items} contextualMenuItemAs={customRenderer} />);
 
     const menuItem = document.querySelector('button.ms-ContextualMenu-link') as HTMLButtonElement;
     ReactTestUtils.Simulate.click(menuItem);
@@ -964,11 +997,7 @@ describe('ContextualMenu', () => {
           key: 'Item 3',
           sectionProps: {
             key: 'Section1',
-            items: [
-              { text: 'Item 1', key: 'Item 1' },
-              { text: 'Item 2', key: 'Item 2', canCheck: true },
-              { text: 'Item 3', key: 'Item 3' }
-            ]
+            items: [{ text: 'Item 1', key: 'Item 1' }, { text: 'Item 2', key: 'Item 2', canCheck: true }, { text: 'Item 3', key: 'Item 3' }]
           }
         }
       ];

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

@@ -27,13 +27,12 @@ export class ContextualMenuButton extends ContextualMenuItemWrapper {
     } = this.props;
 
     const subMenuId = this._getSubMenuId(item);
-    const ariaLabel = item.ariaLabel || item.text || item.name || '';
 
     const isChecked: boolean | null | undefined = getIsChecked(item);
     const canCheck: boolean = isChecked !== null;
     const defaultRole = canCheck ? 'menuitemcheckbox' : 'menuitem';
     const itemHasSubmenu = hasSubmenu(item);
-    const { itemProps } = item;
+    const { itemProps, ariaLabel } = item;
 
     const buttonNativeProperties = getNativeProps(item, buttonProperties);
     // Do not add the disabled attribute to the button so that it is focusable

packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenuItemWrapper/__snapshots__/ContextualMenuButton.deprecated.test.tsx.snap

@@ -3,7 +3,6 @@
 exports[`ContextualMenuButton creates a normal button renders the contextual menu split button correctly 1`] = `
 <button
   aria-disabled={false}
-  aria-label=""
   aria-posinset={1}
   aria-setsize={1}
   className="root"

packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenuItemWrapper/__snapshots__/ContextualMenuButton.test.tsx.snap

@@ -3,7 +3,6 @@
 exports[`ContextualMenuButton creates a normal button renders the contextual menu split button correctly 1`] = `
 <button
   aria-disabled={false}
-  aria-label=""
   aria-posinset={1}
   aria-setsize={1}
   className="root"