Item: remove isAction, use onClick to discriminate if it should render as button

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

@@ -29,12 +29,11 @@ function Example() {
 
 ## Props
 
-### `isAction`: `boolean`
+### `onClick`: `React.MouseEventHandler<HTMLDivElement>`
 
-Renders the item as an interactive `button` element.
+Even handler for processing `click` events. When defined, the `Item` component will render as a `button` (unless differently specified via the `as` prop).
 
 - Required: No
-- Default: `false`
 
 ### `size`: `'small' | 'medium' | 'large'`
 

packages/components/src/item-group/item/hook.ts

@@ -13,26 +13,47 @@ import { useItemGroupContext } from '../context';
 import { useCx } from '../../utils/hooks/use-cx';
 import type { ItemProps } from '../types';
 
+function useDeprecatedProps( {
+	as,
+	isAction = false,
+	...otherProps
+}: WordPressComponentProps< ItemProps, 'div' > ) {
+	let computedAs = as;
+
+	if ( isAction ) {
+		computedAs ??= 'button';
+	}
+
+	return {
+		...otherProps,
+		as: computedAs,
+	};
+}
+
 export function useItem( props: WordPressComponentProps< ItemProps, 'div' > ) {
 	const {
-		isAction = false,
 		as: asProp,
 		className,
+		onClick,
 		role = 'listitem',
 		size: sizeProp,
 		...otherProps
-	} = useContextSystem( props, 'Item' );
+	} = useContextSystem( useDeprecatedProps( props ), 'Item' );
 
 	const { spacedAround, size: contextSize } = useItemGroupContext();
 
 	const size = sizeProp || contextSize;
 
-	const as = ( asProp || isAction ? 'button' : 'div' ) as ElementType;
+	const as =
+		asProp ||
+		( ( typeof onClick !== 'undefined'
+			? 'button'
+			: 'div' ) as ElementType );
 
 	const cx = useCx();
 
 	const classes = cx(
-		isAction && styles.unstyledButton,
+		as === 'button' && styles.unstyledButton,
 		styles.itemSizes[ size ] || styles.itemSizes.medium,
 		styles.item,
 		spacedAround && styles.spacedAround,
@@ -44,6 +65,7 @@ export function useItem( props: WordPressComponentProps< ItemProps, 'div' > ) {
 	return {
 		as,
 		className: classes,
+		onClick,
 		wrapperClassName,
 		role,
 		...otherProps,

packages/components/src/item-group/stories/index.js

@@ -56,7 +56,6 @@ export const _default = () => {
 			},
 			PROP_UNSET
 		),
-		isAction: boolean( 'Item 1: isAction', true ),
 	};
 
 	// Do not pass the `size` prop when its value is `undefined`.
@@ -68,16 +67,14 @@ export const _default = () => {
 
 	return (
 		<ItemGroup style={ { width: '350px' } } { ...itemGroupProps }>
-			<Item { ...itemProps } onClick={ () => alert( 'WordPress.org' ) }>
+			<Item { ...itemProps }>Code is Poetry (no click handlers)</Item>
+			<Item onClick={ () => alert( 'WordPress.org' ) }>
 				Code is Poetry — Click me!
 			</Item>
-			<Item isAction onClick={ () => alert( 'WordPress.org' ) }>
+			<Item onClick={ () => alert( 'WordPress.org' ) }>
 				Code is Poetry — Click me!
 			</Item>
-			<Item isAction onClick={ () => alert( 'WordPress.org' ) }>
-				Code is Poetry — Click me!
-			</Item>
-			<Item isAction onClick={ () => alert( 'WordPress.org' ) }>
+			<Item onClick={ () => alert( 'WordPress.org' ) }>
 				Code is Poetry — Click me!
 			</Item>
 		</ItemGroup>
@@ -90,16 +87,14 @@ export const dropdown = () => (
 		trigger={ <Button>Open Popover</Button> }
 	>
 		<ItemGroup style={ { padding: 4 } }>
-			<Item isAction onClick={ () => alert( 'WordPress.org' ) }>
-				Code is Poetry — Click me!
-			</Item>
-			<Item isAction onClick={ () => alert( 'WordPress.org' ) }>
+			<Item>Code is Poetry (no click handlers)</Item>
+			<Item onClick={ () => alert( 'WordPress.org' ) }>
 				Code is Poetry — Click me!
 			</Item>
-			<Item isAction onClick={ () => alert( 'WordPress.org' ) }>
+			<Item onClick={ () => alert( 'WordPress.org' ) }>
 				Code is Poetry — Click me!
 			</Item>
-			<Item isAction onClick={ () => alert( 'WordPress.org' ) }>
+			<Item onClick={ () => alert( 'WordPress.org' ) }>
 				Code is Poetry — Click me!
 			</Item>
 		</ItemGroup>
@@ -141,7 +136,7 @@ export const complexLayouts = () => {
 
 	return (
 		<ItemGroup isBordered isSeparated style={ { width: '250px' } }>
-			<Item isAction onClick={ () => alert( 'Color palette' ) }>
+			<Item onClick={ () => alert( 'Color palette' ) }>
 				<HStack>
 					<FlexBlock>
 						<ZStack isLayered={ false } offset={ -8 }>
@@ -156,7 +151,7 @@ export const complexLayouts = () => {
 				</HStack>
 			</Item>
 
-			<Item isAction onClick={ () => alert( 'Single color setting' ) }>
+			<Item onClick={ () => alert( 'Single color setting' ) }>
 				<HStack justify="flex-start">
 					<FlexItem
 						as={ SimpleColorSwatch }
@@ -169,10 +164,7 @@ export const complexLayouts = () => {
 				</HStack>
 			</Item>
 
-			<Item
-				isAction
-				onClick={ () => alert( 'Single typography setting' ) }
-			>
+			<Item onClick={ () => alert( 'Single typography setting' ) }>
 				<HStack justify="flex-start">
 					<FlexItem>
 						<Icon icon={ typography } size={ 24 } />

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

@@ -33,28 +33,6 @@ Snapshot Diff:
       </div>
 `;
 
-exports[`ItemGroup Item should render a button with the isAction prop is true 1`] = `
-Snapshot Diff:
-- First value
-+ Second value
-
-  <div
-    class="css-dcjs67-itemWrapper"
-    role="listitem"
-  >
--   <div
--     class="components-item css-gqguxs-View-medium-itemWrapper em57xhy0"
-+   <button
-+     class="components-item css-1t39803-View-unstyledButton-medium-itemWrapper em57xhy0"
-      data-wp-c16t="true"
-      data-wp-component="Item"
-    >
-      Code is poetry
--   </div>
-+   </button>
-  </div>
-`;
-
 exports[`ItemGroup Item should use different amounts of padding depending on the value of the size prop 1`] = `
 Snapshot Diff:
 - First value

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

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { render } from '@testing-library/react';
+import { fireEvent, render, screen } from '@testing-library/react';
 
 /**
  * Internal dependencies
@@ -79,18 +79,36 @@ describe( 'ItemGroup', () => {
 	} );
 
 	describe( 'Item', () => {
-		it( 'should render a button with the isAction prop is true', () => {
-			// By default, `isAction` is `false`
-			const { container: normalItem } = render(
-				<Item>Code is poetry</Item>
-			);
-			const { container: actionItem } = render(
-				<Item isAction={ true }>Code is poetry</Item>
+		it( 'should render as a `button` if the `onClick` handler is specified', () => {
+			const spy = jest.fn();
+			render( <Item onClick={ spy }>Code is poetry</Item> );
+
+			const button = screen.getByRole( 'button' );
+
+			expect( button ).toBeInTheDocument();
+
+			fireEvent.click( button );
+
+			expect( spy ).toHaveBeenCalled();
+		} );
+
+		it( 'should give priority to the `as` prop even if the `onClick` handler is specified', () => {
+			const spy = jest.fn();
+			const { rerender } = render(
+				<Item onClick={ spy }>Code is poetry</Item>
 			);
 
-			expect( normalItem.firstChild ).toMatchDiffSnapshot(
-				actionItem.firstChild
+			expect( screen.getByRole( 'button' ) ).toBeInTheDocument();
+			expect( screen.queryByRole( 'label' ) ).not.toBeInTheDocument();
+
+			rerender(
+				<Item as="a" href="#" onClick={ spy }>
+					Code is poetry
+				</Item>
 			);
+
+			expect( screen.queryByRole( 'button' ) ).not.toBeInTheDocument();
+			expect( screen.getByRole( 'link' ) ).toBeInTheDocument();
 		} );
 
 		it( 'should use different amounts of padding depending on the value of the size prop', () => {
@@ -132,5 +150,32 @@ describe( 'ItemGroup', () => {
 				largeSize.firstChild
 			);
 		} );
+
+		it( 'should render a button with the isAction prop is true', () => {
+			// By default, `isAction` is `false`
+			const { rerender } = render( <Item>Code is poetry</Item> );
+
+			expect( screen.queryByRole( 'button' ) ).not.toBeInTheDocument();
+
+			rerender( <Item isAction>Code is poetry</Item> );
+
+			expect( screen.getByRole( 'button' ) ).toBeInTheDocument();
+		} );
+
+		it( 'should give priority to the `as` prop even if the isAction prop is true', () => {
+			const { rerender } = render( <Item isAction>Code is poetry</Item> );
+
+			expect( screen.getByRole( 'button' ) ).toBeInTheDocument();
+			expect( screen.queryByRole( 'label' ) ).not.toBeInTheDocument();
+
+			rerender(
+				<Item as="a" href="#" isAction>
+					Code is poetry
+				</Item>
+			);
+
+			expect( screen.queryByRole( 'button' ) ).not.toBeInTheDocument();
+			expect( screen.getByRole( 'link' ) ).toBeInTheDocument();
+		} );
 	} );
 } );

packages/components/src/item-group/types.ts

@@ -32,12 +32,6 @@ export interface ItemGroupProps {
 }
 
 export interface ItemProps {
-	/**
-	 * Renders the item as an interactive `button` element.
-	 *
-	 * @default false
-	 */
-	isAction?: boolean;
 	/**
 	 * Determines the amount of padding within the component.
 	 *
@@ -48,6 +42,13 @@ export interface ItemProps {
 	 * The children elements.
 	 */
 	children: React.ReactNode;
+	/**
+	 * Renders the item as an interactive `button` element.
+	 *
+	 * @default false
+	 * @deprecated
+	 */
+	isAction?: boolean;
 }
 
 export type ItemGroupContext = {

packages/edit-site/src/components/sidebar/global-styles-sidebar.js

@@ -206,11 +206,7 @@ function NavigationButton( {
 } ) {
 	const navigator = useNavigator();
 	return (
-		<Item
-			isAction
-			onClick={ () => navigator.push( path, { isBack } ) }
-			{ ...props }
-		>
+		<Item onClick={ () => navigator.push( path, { isBack } ) } { ...props }>
 			<HStack justify="flex-start">
 				{ icon && (
 					<FlexItem>