Component: Order and spacing issues for text and icon in Button component

[t-hamano]

Description

On Button component with the icon, there are perhaps unanticipated issues regarding the order and spacing of text and icon.

Button text can be specified with the text prop or as a child element, but in that variation I see the following problem:

• Button with text prop: no space between text and buttons
• Button with text children: iconPosition prop is not reflected in the case of child text
• Button with TextProps and text children: the icon is placed between the text when iconPosition is right

In my opinion, the expected behavior is as follows:

• Button with text prop: no space between text and buttons: should be a space between text and icon
• Button with text children: iconPosition prop should also be reflected if in the case of child text
• Button with TextProps and text children: the icon should be placed after the whole text when iconPosition is right

Is the current behavior expected?
Also, is my expected behavior reasonable?

cc @ciampo @mirka

Step-by-step reproduction instructions

To check the behavior of the current button component, I used the following code in the core/code block.

/**
 * WordPress dependencies
 */
import { useBlockProps } from '@wordpress/block-editor';
import { Button, Flex } from '@wordpress/components';
import { help } from '@wordpress/icons';

export default function CodeEdit() {
	const blockProps = useBlockProps();
	return (
		<div { ...blockProps }>
			<p>
				Button with text prop
				<br />
				(No space between text and buttons)
			</p>
			<Flex justify="flex-start" gap={ 3 }>
				<Button
					icon={ help }
					text="TextProp"
					variant="primary"
					iconPosition="left"
				/>
				<Button
					icon={ help }
					text="TextProp"
					variant="primary"
					iconPosition="right"
				/>
			</Flex>
			<p>
				Button with text children
				<br />
				(iconPosition prop is not reflected in the case of child text)
			</p>
			<Flex justify="flex-start" gap={ 3 }>
				<Button icon={ help } variant="primary" iconPosition="left">
					TextChildren
				</Button>
				<Button icon={ help } variant="primary" iconPosition="right">
					TextChildren
				</Button>
			</Flex>
			<p>
				Button with text props and text children
				<br />
				(the icon is placed between the text when iconPosition is right)
			</p>
			<Flex justify="flex-start" gap={ 3 }>
				<Button
					icon={ help }
					text="TextProp"
					variant="primary"
					iconPosition="left"
				>
					TextChildren
				</Button>
				<Button
					icon={ help }
					text="TextProp"
					variant="primary"
					iconPosition="right"
				>
					TextChildren
				</Button>
			</Flex>
		</div>
	);
}

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[ciampo]

there are perhaps unanticipated issues

The Button component definitely feels a bit out of date and could do with some tweaking of the styles, especially about the different combination of text and icons, and with all of its variants.

It's a tricky set of changes because the component is so widely used across the WordPress ecosystem, and I'm sure in many instances with style overrides.

I wonder what the best approach could look like here

[ciampo]

Thinking about it, I'd be tempted to see if we can deprecate all icon-related props on Button (icon, iconSize, iconPosition) — consumers of the component can easily just add and position icons as children, which would definitely encourage composability and remove complexity from Button.

@mirka what do you think?

[mirka]

In my opinion, the expected behavior is as follows:

• Button with text prop: no space between text and buttons: should be a space between text and icon
• Button with text children: iconPosition prop should also be reflected if in the case of child text
• Button with TextProps and text children: the icon should be placed after the whole text when iconPosition is right

@t-hamano I agree with this spec 100% 👍

Thinking about it, I'd be tempted to see if we can deprecate all icon-related props on Button (icon, iconSize, iconPosition) — consumers of the component can easily just add and position icons as children, which would definitely encourage composability and remove complexity from Button.

@ciampo Hard to say without taking better inventory of the current in-app usage, but I imagine the "icon + text" pattern to be common enough that it warrants some kind of designated API, if only for consistent styling.

I'm hoping that, by streamlining the existing CSS and providing padding wrappers like we did for Dropdown, we can improve the resizability (important!) and internal composability of Button without making API changes 🤞

[t-hamano]

In my experience, the icon prop is very useful when I want to add text with an icon to the Button component. This is because we don't need to use the <Icon /> component as a child. I think many developers might believe that only dashicons can be specified for the icon prop 😅

As shown in the query results on WP Directory, iconSize and iconPosition also appear to be used by many consumers (However, this query might not extract correctly).

• iconSize
• iconPosition

I think many developers seem to use only button text as a children, as shown in the usage examples in the component reference.
I suspect that many developers who look at this reference will use the icon prop instead of the children when they want to add an icon.

Ideally, the Button component might not have icon props, but I would expect the icon properties to survive 🙂

[mirka]

By the way @t-hamano, we have a visual regression testing setup in #43393 that would be great to use when cleaning up these Button styles. I'll try and get it merged soon so you can add a test file with all your awesome test fixtures 😄

[t-hamano]

Update:

I have confirmed that gap is now used for button spacing in #50277. This slightly changes the rendering results from the code first presented in this issue:

I think this at least means that we no longer have to worry about padding and margin.