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: https://github.com/airbytehq/airbyte/blob/57068e3f2941698b9c44129b3527b646e5d4703d/airbyte-webapp/src/pages/ConnectionPage/pages/ConnectionItemPage/ConnectionStatusTab.tsx#L178-L182

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: https://github.com/airbytehq/airbyte/blob/a6974a7ba93b7f1c62f17b37ca45a0bfcb456061/airbyte-webapp/src/components/ui/Button/Button.tsx#L57-L63 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

[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.