From 53209f4a029fe3fe2cb643a7956abb588e3491bb Mon Sep 17 00:00:00 2001
From: Edwin Cromley <cromleyedwin89@gmail.com>
Date: Thu, 8 Nov 2018 06:00:52 -0500
Subject: [PATCH] Change aria-checked to only be used on matching ARIA roles
 for MenuItem. (#11459)

* Fixes #11431. Adjust aria-checked attribute to only work on matching aria
roles.

https://www.w3.org/TR/wai-aria-1.1/#aria-checked

* Modify BlockNavigationList to make proper use of aria roles.

* Modify the editor CHANGELOG.md to reflect a11y changes.

* Modify changelog for components package to reflect a11y changes.

* Add additional test case for MenuItem when aria-checked should be used.

* Test fix for missing function toBeTrue, use toBe( true ) instead

* Update README.md as well as update MenuItem to better fit coding aesthetics.

* Modify changelog to reflect new feature.
---
 packages/components/CHANGELOG.md              |  6 +++++
 packages/components/src/menu-item/README.md   | 10 ++++++++-
 packages/components/src/menu-item/index.js    |  3 ++-
 .../components/src/menu-item/test/index.js    | 22 +++++++++++++++++++
 packages/editor/CHANGELOG.md                  |  6 +++++
 .../src/components/block-navigation/index.js  |  1 +
 6 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
index 2ceb51c459d3c4..4e91e6f7869dab 100644
--- a/packages/components/CHANGELOG.md
+++ b/packages/components/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 5.1.0 (unreleased)
+
+### New Features
+
+- Adjust a11y roles for MenuItem component, so that aria-checked is used properly, related change in Editor/Components/BlockNavigationList ([#11431](https://github.com/WordPress/gutenberg/issues/11431)).
+
 ## 5.0.2 (2018-11-03)
 
 ### Polish
diff --git a/packages/components/src/menu-item/README.md b/packages/components/src/menu-item/README.md
index c7f4777bd43b0d..76cff639fce511 100644
--- a/packages/components/src/menu-item/README.md
+++ b/packages/components/src/menu-item/README.md
@@ -25,7 +25,7 @@ const MyMenuItem = withState( {
 
 MenuItem supports the following props. Any additional props are passed through to the underlying [Button](../button) or [IconButton](../icon-button) component.
 
-### `children` 
+### `children`
 
 - Type: `WPElement`
 - Required: No
@@ -65,3 +65,11 @@ Refer to documentation for [IconButton's `icon` prop](../icon-button/README.md#i
 - Required: No
 
 Refer to documentation for [Shortcut's `shortcut` prop](../shortcut/README.md#shortcut).
+
+### `role`
+
+- Type: `string`
+- Require: No
+- Default: `'menuitem'`
+
+[Aria Spec](https://www.w3.org/TR/wai-aria-1.1/#aria-checked). If you need to have selectable menu items use menuitemradio for single select, and menuitemcheckbox for multiselect.
diff --git a/packages/components/src/menu-item/index.js b/packages/components/src/menu-item/index.js
index e281c6bf0ea657..fa8dd6a85d24fa 100644
--- a/packages/components/src/menu-item/index.js
+++ b/packages/components/src/menu-item/index.js
@@ -78,7 +78,8 @@ export function MenuItem( {
 		tagName,
 		{
 			'aria-label': label,
-			'aria-checked': isSelected,
+			// Make sure aria-checked matches spec https://www.w3.org/TR/wai-aria-1.1/#aria-checked
+			'aria-checked': ( role === 'menuitemcheckbox' || role === 'menuitemradio' ) ? isSelected : undefined,
 			role,
 			className,
 			...props,
diff --git a/packages/components/src/menu-item/test/index.js b/packages/components/src/menu-item/test/index.js
index 9fbd8b869ff470..f79f2da61be78e 100644
--- a/packages/components/src/menu-item/test/index.js
+++ b/packages/components/src/menu-item/test/index.js
@@ -72,4 +72,26 @@ describe( 'MenuItem', () => {
 
 		expect( wrapper.prop( 'aria-label' ) ).toBeUndefined();
 	} );
+
+	it( 'should avoid using aria-checked if only menuitem is set as aria-role', () => {
+		const wrapper = shallow(
+			<MenuItem role="menuitem" isSelected={ true }><div /></MenuItem>
+		);
+
+		expect( wrapper.prop( 'aria-checked' ) ).toBeUndefined();
+	} );
+
+	it( 'should use aria-checked if menuitemradio or menuitemcheckbox is set as aria-role', () => {
+		let wrapper = shallow(
+			<MenuItem role="menuitemradio" isSelected={ true }><div /></MenuItem>
+		);
+
+		expect( wrapper.prop( 'aria-checked' ) ).toBe( true );
+
+		wrapper = shallow(
+			<MenuItem role="menuitemcheckbox" isSelected={ true }><div /></MenuItem>
+		);
+
+		expect( wrapper.prop( 'aria-checked' ) ).toBe( true );
+	} );
 } );
diff --git a/packages/editor/CHANGELOG.md b/packages/editor/CHANGELOG.md
index 64475184f0431b..6c3f5dd6a8d750 100644
--- a/packages/editor/CHANGELOG.md
+++ b/packages/editor/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 6.2.0 (unreleased)
+
+### New Features
+
+- Adjust a11y roles for menu items, and make sure screen readers can properly use BlockNavigationList ([#11431](https://github.com/WordPress/gutenberg/issues/11431)).
+
 ## 6.1.1 (2018-11-03)
 
 ### Polish
diff --git a/packages/editor/src/components/block-navigation/index.js b/packages/editor/src/components/block-navigation/index.js
index 39ff985a2cc92b..d1c1b919ed7192 100644
--- a/packages/editor/src/components/block-navigation/index.js
+++ b/packages/editor/src/components/block-navigation/index.js
@@ -37,6 +37,7 @@ function BlockNavigationList( {
 								} ) }
 								onClick={ () => selectBlock( block.clientId ) }
 								isSelected={ block.clientId === selectedBlockClientId }
+								role="menuitemradio"
 							>
 								<BlockIcon icon={ blockType.icon } showColors />
 								{ blockType.title }