fix(menuButton) - accessibility - add aria-controls attribute, when e…

…xists (#2107)

* fixing the bug related to the aria-controls pointing to not existing elements

* code improve

* fix the test, always take proper DOM element, after click element was not updated

* changelog updated

* Apply suggestions from code review

Co-Authored-By: Marija Najdova <mnajdova@gmail.com>

CHANGELOG.md

@@ -25,6 +25,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Apply unhandled props of `Ref` to the children if used @jurokapsiar ([#2105](https://github.com/microsoft/fluent-ui-react/pull/2105))
 - Add necessary `-ms-grid-` styles to `Layout` for IE11 @jurokapsiar ([#2106](https://github.com/microsoft/fluent-ui-react/pull/2106))
 - Accessibility `splitButton` & `menuButton` - screen reader fixes @kolaps33 ([#2090](https://github.com/microsoft/fluent-ui-react/pull/2090))
+- Accessibility `menuButton` add aria-controls attribute based on `open` prop @kolaps33 ([#2107](https://github.com/microsoft/fluent-ui-react/pull/2107))
 
 ### Features
 - Add `Table` component base implementation @pompomon ([#2099](https://github.com/microsoft/fluent-ui-react/pull/2099))

packages/accessibility/src/behaviors/MenuButton/menuButtonBehavior.ts

@@ -7,11 +7,11 @@ import popupBehavior, { PopupBehaviorProps } from '../Popup/popupBehavior'
 /**
  * @description
  * Implements ARIA [MenuButton](https://www.w3.org/TR/wai-aria-practices/#menubutton) design pattern.
+ * Adds attribute 'aria-controls=menu-id' based on the property 'open' to 'trigger' slot.
  *
  * @specification
  * Adds attribute 'aria-haspopup=true' to 'trigger' slot if 'contextMenu' property is not set.
  * Adds attribute 'tabIndex=-1' based on the property 'open' to 'trigger' slot.
- * Adds attribute 'aria-controls=menu-id' based on the property 'menuId' to 'trigger' slot.
  * Adds attribute 'aria-expanded=true' based on the property 'open' to 'trigger' slot.
  * Adds attribute 'id=trigger-id' based on the property 'triggerId' to 'trigger' slot.
  * Adds attribute 'id=menu-id' based on the property 'menuId' to 'menu' slot.
@@ -22,7 +22,7 @@ const menuButtonBehavior: Accessibility<MenuButtonBehaviorProps> = props => {
   return _.merge(behavior, {
     attributes: {
       trigger: {
-        'aria-controls': props.menuId,
+        'aria-controls': props.open ? props.menuId : undefined,
         'aria-expanded': props.open || undefined,
         'aria-haspopup': props.contextMenu ? undefined : 'true',
         id: props.triggerId,

packages/accessibility/test/behaviors/menuButtonBehavior-test.tsx

@@ -0,0 +1,22 @@
+import { menuButtonBehavior } from '@stardust-ui/accessibility'
+
+const mockedMenuId = 'anyMenuId'
+
+describe('MenuButtonBehavior.ts', () => {
+  test('aria-controls are NOT defined, when menu is NOT open', () => {
+    const property = {
+      open: false,
+      menuId: mockedMenuId,
+    }
+    const expectedResult = menuButtonBehavior(property)
+    expect(expectedResult.attributes.trigger['aria-controls']).toBe(undefined)
+  })
+  test('aria-controls are defined, when menu is open', () => {
+    const property = {
+      open: true,
+      menuId: mockedMenuId,
+    }
+    const expectedResult = menuButtonBehavior(property)
+    expect(expectedResult.attributes.trigger['aria-controls']).toBe(mockedMenuId)
+  })
+})

packages/react/test/specs/components/MenuButton/MenuButton-test.tsx

@@ -71,27 +71,23 @@ describe('MenuButton', () => {
       })
 
       test('menu id is used', () => {
+        const menuId = 'test-id'
         const menuButton = mountWithProvider(
-          <MenuButton trigger={<button />} menu={{ ...mockMenu, id: 'test-id' }} />,
+          <MenuButton trigger={<button />} menu={{ ...mockMenu, id: menuId }} />,
         )
-        const button = menuButton.find('button')
-        button.simulate('click')
-        const menu = menuButton.find('ul')
-        const menuId = menu.prop('id')
+        menuButton.find('button').simulate('click')
 
-        expect(menuId).toEqual('test-id')
-        expect(button.prop('aria-controls')).toEqual(menuId)
+        expect(menuButton.find('ul').prop('id')).toEqual(menuId)
+        expect(menuButton.find('button').prop('aria-controls')).toEqual(menuId)
       })
 
       test('menu id is generated if not provided', () => {
         const menuButton = mountWithProvider(<MenuButton trigger={<button />} menu={mockMenu} />)
-        const button = menuButton.find('button')
-        button.simulate('click')
-        const menu = menuButton.find('ul')
-        const menuId = menu.prop('id')
+        menuButton.find('button').simulate('click')
+        const menuId = menuButton.find('ul').prop('id')
 
         expect(menuId).toMatch(/menubutton-menu-\d+/)
-        expect(button.prop('aria-controls')).toEqual(menuId)
+        expect(menuButton.find('button').prop('aria-controls')).toEqual(menuId)
       })
     })
   })