Icons passed to Button component must be wrapped in a div for proper styling

[lmossman]

Tell us about the problem you're trying to solve

When providing both an icon to the Button component, and a label (as a child), the Button component does not always successfully apply the desired class names to the icon inside of the button.

However, if the icon is wrapped in a <div> before being passed to the Button, the styles are properly applied.

For example, we are currently doing this div-wrapping hack in the ConnectionStatusTab component:

airbyte/airbyte-webapp/src/pages/ConnectionPage/pages/ConnectionItemPage/ConnectionStatusTab.tsx

Lines 178 to 182 in 57068e3

	icon={
	<div className={styles.iconRotate}>
	<RotateIcon />
	</div>
	}

To demonstrate what this does, here is what the button and its styles look like currently on prod, with the icon wrapped in a div:

And here is what the button and styles look like when the icon is NOT wrapped in a div:

As you can see, the Button_buttonIcon_..., Button_positionLeft_..., and Button_withLabel_... styles are not being applied to the <svg> element, which messes up the formatting

Describe the solution you’d like

This may be happening because we are using React.cloneElement to create the button icon here:

airbyte/airbyte-webapp/src/components/ui/Button/Button.tsx

Lines 57 to 63 in a6974a7

	React.cloneElement(icon, {
	className: classNames(styles.buttonIcon, {
	[styles.positionLeft]: true,
	[styles.isRegularIcon]: true,
	[styles.withLabel]: Boolean(children),
	}),
	})}

And, it may only happen with specific icon elements like <svg>, which may have trouble having those styles applied.

One solution could be to always wrap the cloned element in a <div>, and apply the icon styles to that div instead, though this may cause us to end up with unnecessary <div> nesting.

[octavia-squidington-iv]

cc @airbytehq/frontend

[edmundito]

@lmossman Just looking into this to see if I should fix it but when I look at all the usages, the only icon that is having trouble is the rotate icon. Maybe this is something specific with the Rotate Icon itself? (or maybe we haven't caught it with the other custom icons?)

[lmossman]

Interesting. I thought it would be a problem with all icons, but if not then maybe we should try to understand what specifically about the rotate icon is causing the problem

[teallarson]

@edmundito I am seeing this with other custom icons as well, but not with FontAwesome icons.

To test it out, I went to a page that had a button with correct classNames (I looked at AllSourcesPage) and swapped out the icon with a custom one from our repo (I tried PencilIco, RotateIcon, and ModificationIcon) and the icon classnames were no longer being passed down from the Button component.

Interesting that for some reason they get properly applied to the svg element for FA but not for our custom ones.