Components: Add info prop support to MenuItem, FeatureToggle (WordP…

…ress#10802)

* Components: Allow aria-label override on IconButton

* Components: Add `info` prop support to MenuItem

* Edit Post: Add `info` prop support to FeatureToggle

* Edit Post: Add feature toggle info texts

* Components: Menu Item: Break text beyond mobile toolbar

* Components: Icon Button: Indent text by SVG margin

Avoids offset indent on wrapped text

* Tweak menu item styling

* Avoid italic text

packages/components/CHANGELOG.md

@@ -3,10 +3,15 @@
 ### New Feature
 
 - Added a new `ColorPicker` component ([#10564](https://github.com/WordPress/gutenberg/pull/10564)).
+- `MenuItem` now accepts an `info` prop for including an extended description.
+
+### Bug Fix
+
+- `IconButton` correctly respects a passed `aria-label` prop.
 
 ### Deprecation
 
-- `wp.components.PanelColor` has been deprecated in favor of `wp.editor.PanelColorSettings`.
+- `PanelColor` has been deprecated in favor of `wp.editor.PanelColorSettings`.
 
 ## 4.1.0 (2018-10-10)
 
@@ -18,7 +23,7 @@
 
 ### Breaking Change
 
-- `wp.components.Draggable` as a DOM node drag handler has been removed. Please, use `wp.components.Draggable` as a wrap component for your DOM node drag handler.
+- `Draggable` as a DOM node drag handler has been removed. Please, use `Draggable` as a wrap component for your DOM node drag handler.
 
 ### Deprecation
 
@@ -29,9 +34,9 @@
 ### Breaking Change
 
 - `withAPIData` has been removed. Please use the Core Data module or `@wordpress/api-fetch` directly instead.
-- `wp.components.Draggable` as a DOM node drag handler has been deprecated. Please, use `wp.components.Draggable` as a wrap component for your DOM node drag handler.
+- `Draggable` as a DOM node drag handler has been deprecated. Please, use `Draggable` as a wrap component for your DOM node drag handler.
 - Change how required built-ins are polyfilled with Babel 7 ([#9171](https://github.com/WordPress/gutenberg/pull/9171)).  If you're using an environment that has limited or no support for ES2015+ such as lower versions of IE then using [core-js](https://github.com/zloirock/core-js) or [@babel/polyfill](https://babeljs.io/docs/en/next/babel-polyfill) will add support for these methods.
-- `wp.components.withContext` has been removed. Please use `wp.element.createContext` instead. See: https://reactjs.org/docs/context.html.
+- `withContext` has been removed. Please use `wp.element.createContext` instead. See: https://reactjs.org/docs/context.html.
 
 ### New Feature
 

packages/components/src/icon-button/index.js

@@ -41,7 +41,7 @@ class IconButton extends Component {
 		);
 
 		let element = (
-			<Button { ...additionalProps } aria-label={ label } className={ classes }>
+			<Button aria-label={ label } { ...additionalProps } className={ classes }>
 				{ isString( icon ) ? <Dashicon icon={ icon } /> : icon }
 				{ children }
 			</Button>

packages/components/src/icon-button/style.scss

@@ -8,7 +8,6 @@
 	color: $dark-gray-500;
 	position: relative;
 	overflow: hidden;
-	text-indent: 4px;
 	border-radius: $radius-round-rectangle;
 
 	.dashicon {
@@ -20,6 +19,10 @@
 	svg {
 		fill: currentColor;
 		outline: none;
+
+		&:not:only-child {
+			margin-right: 4px;
+		}
 	}
 
 	&:not(:disabled):not([aria-disabled="true"]):not(.is-default):hover {

packages/components/src/icon-button/test/index.js

@@ -41,6 +41,11 @@ describe( 'IconButton', () => {
 			expect( iconButton.find( 'Button' ).prop( 'aria-label' ) ).toBe( 'WordPress' );
 		} );
 
+		it( 'should support explicit aria-label override', () => {
+			const iconButton = shallow( <IconButton aria-label="Custom" /> );
+			expect( iconButton.prop( 'aria-label' ) ).toBe( 'Custom' );
+		} );
+
 		it( 'should add an additional className', () => {
 			const iconButton = shallow( <IconButton className="test" /> );
 			expect( iconButton.hasClass( 'test' ) ).toBe( true );

packages/components/src/menu-item/README.md

@@ -1,5 +1,7 @@
 # MenuItem
 
+MenuItem is a component which renders a button intended to be used in combination with the [DropdownMenu component](../dropdown-menu).
+
 ## Usage
 
 ```jsx
@@ -18,3 +20,48 @@ const MyMenuItem = withState( {
 	</MenuItem>
 ) );
 ```
+
+## Props
+
+MenuItem supports the following props. Any additional props are passed through to the underlying [Button](../button) or [IconButton](../icon-button) component.
+
+### `children` 
+
+- Type: `WPElement`
+- Required: No
+
+Element to render as child of button.
+
+Element
+
+### `label`
+
+- Type: `string`
+- Required: No
+
+String to use as primary button label text, applied as `aria-label`. Useful in cases where an `info` prop is passed, where `label` should be the minimal text of the button, described in further detail by `info`.
+
+Defaults to the value of `children`, if `children` is passed as a string.
+
+### `info`
+
+- Type: `string`
+- Required: No
+
+Text to use as description for button text.
+
+Refer to documentation for [`label`](#label).
+
+### `icon`
+
+- Type: `string`
+- Required: No
+
+Refer to documentation for [IconButton's `icon` prop](../icon-button/README.md#icon).
+
+### `shortcut`
+
+- Type: `string`
+- Required: No
+
+Refer to documentation for [Shortcut's `shortcut` prop](../shortcut/README.md#shortcut).

packages/components/src/menu-item/index.js

@@ -7,7 +7,8 @@ import { isString } from 'lodash';
 /**
  * WordPress dependencies
  */
-import { cloneElement } from '@wordpress/element';
+import { createElement, cloneElement } from '@wordpress/element';
+import { withInstanceId } from '@wordpress/compose';
 
 /**
  * Internal dependencies
@@ -21,11 +22,45 @@ import IconButton from '../icon-button';
  *
  * @return {WPElement} More menu item.
  */
-function MenuItem( { children, className, icon, shortcut, isSelected, role = 'menuitem', ...props } ) {
+export function MenuItem( {
+	children,
+	label = children,
+	info,
+	className,
+	icon,
+	shortcut,
+	isSelected,
+	role = 'menuitem',
+	instanceId,
+	...props
+} ) {
 	className = classnames( 'components-menu-item__button', className, {
 		'has-icon': icon,
 	} );
 
+	// Avoid using label if it is passed as non-string children.
+	label = isString( label ) ? label : undefined;
+
+	if ( info ) {
+		const infoId = 'edit-post-feature-toggle__info-' + instanceId;
+
+		// Deconstructed props is scoped to the function; mutation is fine.
+		props[ 'aria-describedby' ] = infoId;
+
+		children = (
+			<span className="components-menu-item__info-wrapper">
+				{ children }
+				<span
+					id={ infoId }
+					className="components-menu-item__info">
+					{ info }
+				</span>
+			</span>
+		);
+	}
+
+	let tagName = Button;
+
 	if ( icon ) {
 		if ( ! isString( icon ) ) {
 			icon = cloneElement( icon, {
@@ -35,31 +70,22 @@ function MenuItem( { children, className, icon, shortcut, isSelected, role = 'me
 			} );
 		}
 
-		return (
-			<IconButton
-				className={ className }
-				icon={ icon }
-				aria-checked={ isSelected }
-				role={ role }
-				{ ...props }
-			>
-				{ children }
-				<Shortcut className="components-menu-item__shortcut" shortcut={ shortcut } />
-			</IconButton>
-		);
+		tagName = IconButton;
+		props.icon = icon;
 	}
 
-	return (
-		<Button
-			className={ className }
-			aria-checked={ isSelected }
-			role={ role }
-			{ ...props }
-		>
-			{ children }
-			<Shortcut className="components-menu-item__shortcut" shortcut={ shortcut } />
-		</Button>
+	return createElement(
+		tagName,
+		{
+			'aria-label': label,
+			'aria-checked': isSelected,
+			role,
+			className,
+			...props,
+		},
+		children,
+		<Shortcut className="components-menu-item__shortcut" shortcut={ shortcut } />
 	);
 }
 
-export default MenuItem;
+export default withInstanceId( MenuItem );

packages/components/src/menu-item/style.scss

@@ -3,7 +3,7 @@
 	width: 100%;
 	padding: 8px;
 	text-align: left;
-	color: $dark-gray-500;
+	color: $dark-gray-600;
 
 	// Target plugin icons that can have arbitrary classes by using an aggressive selector.
 	.dashicon,
@@ -24,13 +24,17 @@
 	&:focus:not(:disabled):not([aria-disabled="true"]) {
 		@include menu-style__focus;
 	}
+}
 
-	// Don't wrap text until viewport is beyond the mobile breakpoint.
-	@include break-mobile() {
-		.components-popover:not(.is-mobile) & {
-			white-space: nowrap;
-		}
-	}
+.components-menu-item__info-wrapper {
+	display: flex;
+	flex-direction: column;
+}
+
+.components-menu-item__info {
+	margin-top: $grid-size-small;
+	font-size: $default-font-size - 1px;
+	opacity: 0.82;
 }
 
 .components-menu-item__shortcut {

packages/components/src/menu-item/test/__snapshots__/index.js.snap

@@ -3,6 +3,7 @@
 exports[`MenuItem should match snapshot when all props provided 1`] = `
 <IconButton
   aria-checked={true}
+  aria-label="My item"
   className="components-menu-item__button my-class has-icon"
   icon="wordpress"
   onClick={[Function]}
@@ -16,8 +17,33 @@ exports[`MenuItem should match snapshot when all props provided 1`] = `
 </IconButton>
 `;
 
+exports[`MenuItem should match snapshot when info is provided 1`] = `
+<Button
+  aria-describedby="edit-post-feature-toggle__info-1"
+  aria-label="My item"
+  className="components-menu-item__button"
+  role="menuitem"
+>
+  <span
+    className="components-menu-item__info-wrapper"
+  >
+    My item
+    <span
+      className="components-menu-item__info"
+      id="edit-post-feature-toggle__info-1"
+    >
+      Extended description of My Item
+    </span>
+  </span>
+  <Shortcut
+    className="components-menu-item__shortcut"
+  />
+</Button>
+`;
+
 exports[`MenuItem should match snapshot when isSelected and role are optionally provided 1`] = `
 <IconButton
+  aria-label="My item"
   className="components-menu-item__button my-class has-icon"
   icon="wordpress"
   onClick={[Function]}
@@ -33,6 +59,7 @@ exports[`MenuItem should match snapshot when isSelected and role are optionally
 
 exports[`MenuItem should match snapshot when only label provided 1`] = `
 <Button
+  aria-label="My item"
   className="components-menu-item__button"
   role="menuitem"
 >

packages/components/src/menu-item/test/index.js

@@ -7,12 +7,12 @@ import { noop } from 'lodash';
 /**
  * Internal dependencies
  */
-import MenuItem from '../';
+import { MenuItem } from '../';
 
 jest.mock( '../../button' );
 
 describe( 'MenuItem', () => {
-	test( 'should match snapshot when only label provided', () => {
+	it( 'should match snapshot when only label provided', () => {
 		const wrapper = shallow(
 			<MenuItem>
 				My item
@@ -22,7 +22,7 @@ describe( 'MenuItem', () => {
 		expect( wrapper ).toMatchSnapshot();
 	} );
 
-	test( 'should match snapshot when all props provided', () => {
+	it( 'should match snapshot when all props provided', () => {
 		const wrapper = shallow(
 			<MenuItem
 				className="my-class"
@@ -39,7 +39,7 @@ describe( 'MenuItem', () => {
 		expect( wrapper ).toMatchSnapshot();
 	} );
 
-	test( 'should match snapshot when isSelected and role are optionally provided', () => {
+	it( 'should match snapshot when isSelected and role are optionally provided', () => {
 		const wrapper = shallow(
 			<MenuItem
 				className="my-class"
@@ -53,4 +53,23 @@ describe( 'MenuItem', () => {
 
 		expect( wrapper ).toMatchSnapshot();
 	} );
+
+	it( 'should match snapshot when info is provided', () => {
+		const wrapper = shallow(
+			<MenuItem info="Extended description of My Item" instanceId={ 1 }>
+				My item
+			</MenuItem>
+		);
+
+		expect( wrapper.prop( 'aria-label' ) ).not.toBeUndefined();
+		expect( wrapper ).toMatchSnapshot();
+	} );
+
+	it( 'should avoid using aria-label if only has non-string children', () => {
+		const wrapper = shallow(
+			<MenuItem><div /></MenuItem>
+		);
+
+		expect( wrapper.prop( 'aria-label' ) ).toBeUndefined();
+	} );
 } );

packages/edit-post/src/components/header/feature-toggle/index.js

@@ -5,13 +5,15 @@ import { withSelect, withDispatch } from '@wordpress/data';
 import { compose } from '@wordpress/compose';
 import { MenuItem } from '@wordpress/components';
 
-function FeatureToggle( { onToggle, isActive, label } ) {
+function FeatureToggle( { onToggle, isActive, label, info } ) {
 	return (
 		<MenuItem
 			icon={ isActive && 'yes' }
 			isSelected={ isActive }
 			onClick={ onToggle }
 			role="menuitemcheckbox"
+			label={ label }
+			info={ info }
 		>
 			{ label }
 		</MenuItem>