Components: Improve ToolbarButton (#18931)

* Initial refactor on ToolbarButton

* Update storyshots

* Update ToolbarButton styles

* Update snapshots

* Remove unnecessary styles for components-toolbar-button

* Update Toolbar Button imports

* Refactors

packages/components/src/index.js

@@ -73,6 +73,7 @@ export { default as ToggleControl } from './toggle-control';
 export { default as Toolbar } from './toolbar';
 export { default as ToolbarButton } from './toolbar-button';
 export { default as ToolbarGroup } from './toolbar-group';
+export { default as __experimentalToolbarItem } from './toolbar-item';
 export { default as Tooltip } from './tooltip';
 export { default as TreeSelect } from './tree-select';
 export { default as VisuallyHidden } from './visually-hidden';

packages/components/src/index.native.js

@@ -7,6 +7,7 @@ export { default as Dropdown } from './dropdown';
 export { default as Toolbar } from './toolbar';
 export { default as ToolbarButton } from './toolbar-button';
 export { default as ToolbarGroup } from './toolbar-group';
+export { default as __experimentalToolbarItem } from './toolbar-item';
 export { default as Icon } from './icon';
 export { default as IconButton } from './button/deprecated';
 export { default as Spinner } from './spinner';

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

@@ -12,63 +12,55 @@ import { useContext } from '@wordpress/element';
  * Internal dependencies
  */
 import Button from '../button';
+import ToolbarItem from '../toolbar-item';
 import ToolbarContext from '../toolbar-context';
-import AccessibleToolbarButtonContainer from './accessible-toolbar-button-container';
 import ToolbarButtonContainer from './toolbar-button-container';
 
 function ToolbarButton( {
 	containerClassName,
-	icon,
-	title,
-	shortcut,
-	subscript,
-	onClick,
 	className,
-	isActive,
-	isDisabled,
 	extraProps,
 	children,
+	...props
 } ) {
-	// It'll contain state if `ToolbarButton` is being used within
-	// `<Toolbar __experimentalAccessibilityLabel="label" />`
 	const accessibleToolbarState = useContext( ToolbarContext );
 
-	const button = (
-		<Button
-			icon={ icon }
-			label={ title }
-			shortcut={ shortcut }
-			data-subscript={ subscript }
-			onClick={ ( event ) => {
-				event.stopPropagation();
-				if ( onClick ) {
-					onClick( event );
-				}
-			} }
-			className={ classnames(
-				'components-toolbar__control',
-				className,
-			) }
-			isPressed={ isActive }
-			disabled={ isDisabled }
-			{ ...extraProps }
-		/>
-	);
-
-	if ( accessibleToolbarState ) {
+	if ( ! accessibleToolbarState ) {
+		// This should be deprecated when <Toolbar __experimentalAccessibilityLabel="label">
+		// becomes stable.
 		return (
-			<AccessibleToolbarButtonContainer className={ containerClassName }>
-				{ button }
-			</AccessibleToolbarButtonContainer>
+			<ToolbarButtonContainer className={ containerClassName }>
+				<Button
+					icon={ props.icon }
+					label={ props.title }
+					shortcut={ props.shortcut }
+					data-subscript={ props.subscript }
+					onClick={ ( event ) => {
+						event.stopPropagation();
+						if ( props.onClick ) {
+							props.onClick( event );
+						}
+					} }
+					className={ classnames( 'components-toolbar__control', className ) }
+					isPressed={ props.isActive }
+					disabled={ props.isDisabled }
+					{ ...extraProps }
+				/>
+				{ children }
+			</ToolbarButtonContainer>
 		);
 	}
 
-	// ToolbarButton is being used outside of the accessible Toolbar
+	// ToobarItem will pass all props to the render prop child, which will pass
+	// all props to Button. This means that ToolbarButton has the same API as
+	// Button.
 	return (
-		<ToolbarButtonContainer className={ containerClassName }>
-			{ button }
-			{ children }
-		</ToolbarButtonContainer>
+		<ToolbarItem
+			className={ classnames( 'components-toolbar-button', className ) }
+			{ ...props }
+		>
+			{ ( toolbarItemProps ) => <Button { ...toolbarItemProps }>{ children }</Button> }
+		</ToolbarItem>
 	);
 }
 

packages/components/src/toolbar-group/index.js

@@ -53,9 +53,8 @@ function ToolbarGroup( {
 	children,
 	className,
 	isCollapsed,
-	icon,
 	title,
-	...otherProps
+	...props
 } ) {
 	// It'll contain state if `ToolbarGroup` is being used within
 	// `<Toolbar accessibilityLabel="label" />`
@@ -81,18 +80,17 @@ function ToolbarGroup( {
 	if ( isCollapsed ) {
 		return (
 			<ToolbarGroupCollapsed
-				icon={ icon }
 				label={ title }
 				controls={ controlSets }
 				className={ finalClassName }
 				children={ children }
-				{ ...otherProps }
+				{ ...props }
 			/>
 		);
 	}
 
 	return (
-		<ToolbarGroupContainer className={ finalClassName } { ...otherProps }>
+		<ToolbarGroupContainer className={ finalClassName } { ...props }>
 			{ flatMap( controlSets, ( controlSet, indexOfSet ) =>
 				controlSet.map( ( control, indexOfControl ) => (
 					<ToolbarButton

packages/components/src/toolbar-group/toolbar-group-collapsed.js

@@ -1,8 +1,3 @@
-/**
- * External dependencies
- */
-import { ToolbarItem } from 'reakit/Toolbar';
-
 /**
  * WordPress dependencies
  */
@@ -13,37 +8,24 @@ import { useContext } from '@wordpress/element';
  */
 import DropdownMenu from '../dropdown-menu';
 import ToolbarContext from '../toolbar-context';
+import ToolbarItem from '../toolbar-item';
 
-function ToolbarGroupCollapsed( {
-	controls = [],
-	className,
-	icon,
-	label,
-	...props
-} ) {
+function ToolbarGroupCollapsed( { controls = [], ...props } ) {
 	// It'll contain state if `ToolbarGroup` is being used within
 	// `<Toolbar __experimentalAccessibilityLabel="label" />`
 	const accessibleToolbarState = useContext( ToolbarContext );
 
 	const renderDropdownMenu = ( toggleProps ) => (
 		<DropdownMenu
 			hasArrowIndicator
-			icon={ icon }
-			label={ label }
 			controls={ controls }
-			className={ className }
 			toggleProps={ toggleProps }
 			{ ...props }
 		/>
 	);
 
 	if ( accessibleToolbarState ) {
-		return (
-			// https://reakit.io/docs/composition/#render-props
-			<ToolbarItem { ...accessibleToolbarState }>
-				{ ( toolbarItemHTMLProps ) => renderDropdownMenu( toolbarItemHTMLProps ) }
-			</ToolbarItem>
-		);
+		return <ToolbarItem>{ renderDropdownMenu }</ToolbarItem>;
 	}
 
 	return renderDropdownMenu();

packages/components/src/toolbar-group/toolbar-group-collapsed.native.js

@@ -3,16 +3,8 @@
  */
 import DropdownMenu from '../dropdown-menu';
 
-function ToolbarGroupCollapsed( { controls = [], className, icon, label } ) {
-	return (
-		<DropdownMenu
-			hasArrowIndicator
-			icon={ icon }
-			label={ label }
-			controls={ controls }
-			className={ className }
-		/>
-	);
+function ToolbarGroupCollapsed( { controls = [], ...props } ) {
+	return <DropdownMenu hasArrowIndicator controls={ controls } { ...props } />;
 }
 
 export default ToolbarGroupCollapsed;

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

@@ -0,0 +1,36 @@
+/**
+ * External dependencies
+ */
+import { useToolbarItem } from 'reakit/Toolbar';
+
+/**
+ * WordPress dependencies
+ */
+import { forwardRef, useContext } from '@wordpress/element';
+
+/**
+ * Internal dependencies
+ */
+import ToolbarContext from '../toolbar-context';
+
+function ToolbarItem( { children, ...props }, ref ) {
+	const accessibleToolbarState = useContext( ToolbarContext );
+	// https://reakit.io/docs/composition/#props-hooks
+	const itemProps = useToolbarItem( accessibleToolbarState, { ...props, ref } );
+
+	if ( typeof children !== 'function' ) {
+		// eslint-disable-next-line no-console
+		console.warn( '`ToolbarItem` is a generic headless component that accepts only function children props' );
+		return null;
+	}
+
+	if ( ! accessibleToolbarState ) {
+		// eslint-disable-next-line no-console
+		console.warn( '`ToolbarItem` should be rendered within `<Toolbar __experimentalAccessibilityLabel="label">`' );
+		return null;
+	}
+
+	return children( itemProps );
+}
+
+export default forwardRef( ToolbarItem );

packages/components/src/toolbar-item/index.native.js

@@ -0,0 +1,16 @@
+/**
+ * WordPress dependencies
+ */
+import { forwardRef } from '@wordpress/element';
+
+function ToolbarItem( { children, ...props }, ref ) {
+	if ( typeof children !== 'function' ) {
+		// eslint-disable-next-line no-console
+		console.warn( '`ToolbarItem` is a generic headless component that accepts only function children props' );
+		return null;
+	}
+
+	return children( { ...props, ref } );
+}
+
+export default forwardRef( ToolbarItem );

packages/components/src/toolbar/stories/index.js

@@ -2,7 +2,14 @@
  * Internal dependencies
  */
 import Toolbar from '../';
-import { SVG, Path, ToolbarButton, ToolbarGroup } from '../../';
+import {
+	SVG,
+	Path,
+	ToolbarButton,
+	ToolbarGroup,
+	__experimentalToolbarItem as ToolbarItem,
+	DropdownMenu,
+} from '../../';
 
 export default { title: 'Components|Toolbar', component: Toolbar };
 
@@ -20,22 +27,30 @@ export const _default = () => {
 		// id is required for server side rendering
 		<Toolbar __experimentalAccessibilityLabel="Options" id="options-toolbar">
 			<ToolbarGroup>
-				<ToolbarButton icon="editor-paragraph" title="Paragraph" />
+				<ToolbarButton icon="editor-paragraph" label="Paragraph" />
+			</ToolbarGroup>
+			<ToolbarGroup>
+				<ToolbarItem>
+					{ ( toggleProps ) => (
+						<DropdownMenu
+							hasArrowIndicator
+							icon="editor-alignleft"
+							label="Change text alignment"
+							controls={ [
+								{ icon: 'editor-alignleft', title: 'Align left', isActive: true },
+								{ icon: 'editor-aligncenter', title: 'Align center' },
+								{ icon: 'editor-alignright', title: 'Align right' },
+							] }
+							toggleProps={ toggleProps }
+						/>
+					) }
+				</ToolbarItem>
 			</ToolbarGroup>
-			<ToolbarGroup
-				icon="editor-alignleft"
-				label="Change text alignment"
-				isCollapsed
-				controls={ [
-					{ icon: 'editor-alignleft', title: 'Align left', isActive: true },
-					{ icon: 'editor-aligncenter', title: 'Align center' },
-					{ icon: 'editor-alignright', title: 'Align right' },
-				] }
-			/>
 			<ToolbarGroup>
-				<ToolbarButton icon="editor-bold" title="Bold" />
-				<ToolbarButton icon="editor-italic" title="Italic" />
-				<ToolbarButton icon="admin-links" title="Link" />
+				<ToolbarButton>Text</ToolbarButton>
+				<ToolbarButton icon="editor-bold" label="Bold" isPressed />
+				<ToolbarButton icon="editor-italic" label="Italic" />
+				<ToolbarButton icon="admin-links" label="Link" />
 				<ToolbarGroup
 					isCollapsed
 					icon={ false }
@@ -65,10 +80,13 @@ export const _default = () => {
 export const withoutGroup = () => {
 	return (
 		// id is required for server side rendering
-		<Toolbar __experimentalAccessibilityLabel="Options" id="options-toolbar-without-group">
-			<ToolbarButton icon="editor-bold" title="Bold" />
-			<ToolbarButton icon="editor-italic" title="Italic" />
-			<ToolbarButton icon="admin-links" title="Link" />
+		<Toolbar
+			__experimentalAccessibilityLabel="Options"
+			id="options-toolbar-without-group"
+		>
+			<ToolbarButton icon="editor-bold" label="Bold" isPressed />
+			<ToolbarButton icon="editor-italic" label="Italic" />
+			<ToolbarButton icon="admin-links" label="Link" />
 		</Toolbar>
 	);
 };

packages/components/src/toolbar/test/index.js

@@ -14,8 +14,8 @@ describe( 'Toolbar', () => {
 		it( 'should render a toolbar with toolbar buttons', () => {
 			const wrapper = mount(
 				<Toolbar __experimentalAccessibilityLabel="blocks">
-					<ToolbarButton title="control1" />
-					<ToolbarButton title="control2" />
+					<ToolbarButton label="control1" />
+					<ToolbarButton label="control2" />
 				</Toolbar>
 			);
 			const control1 = wrapper.find( 'button[aria-label="control1"]' );