With Gutenberg 6.6, the pinned plugins menu icons are not scaled properly

[youknowriad]

Try installing the dropit plugin. In previous versions the icon was showing properly, it Gutenberg 6.6, the icons are not scaled down to the button height.

Probably due to the buttons refactoring cc @jasmussen @talldan

[youknowriad]

Same in the more menu

[jasmussen]

I don't understand how this could have regressed.

Looking at 5.2 sans plugin, I see the following CSS:

This is coming from here: http://moc.co/wp-content/plugins/dropit/scripts/sidebar/build/style.css?ver=1534242899

And if I toggle that off, the icons explode:

I'm not suggesting this isn't something we should fix, it'd be nice to apply a max-width to SVGs there. But are you sure it's a regression in Gutenberg?

[youknowriad]

So I noticed this as soon as I updated my blog to 6.6 and I'm pretty sure I didn't touch dropit in the meantime, I can debug deeper later though.

[youknowriad]

I suspect buttons were flex aligned which also affects the size of their children and it's not the case anymore.

[jasmussen]

Oh I know what it is now.

On 5.2 without plugin, the markup is this:

<span class="dropit-sidebar-icon is-small color-giphy"><svg width="115px" height="159px" viewBox="0 0 115 159"> ... </svg></span>

On 5.2 with plugin 6.6 enabled, the markup is this:

<span class="dropit-sidebar-icon is-24 color-giphy"><svg width="115px" height="159px" viewBox="0 0 115 159"> ... </svg></span>

So at some point, is-small became replaced by is-24.

[etoledom]

@youknowriad - I've been checking this out following the changes on #17228.

Previous to this change:

gutenberg/packages/components/src/icon-button/index.js

Line 58 in eb3a82f

<Icon icon={ icon } />

We were passing down a Dashicon if icon was a string, or the icon object directly otherwise.
{ isString( icon ) ? <Dashicon icon={ icon } ariaPressed={ ariaPressed } /> : icon }

Now we are always passing the icon (objec or string) to the Icon component, and Icon has a default size value of 24.

Probably if we keep this logic and just change Dashicon with Icon, it will work as before:
{ isString( icon ) ? <Icon icon={ icon } /> : icon }

Did a fast test on web and mobile and it seems to work.
What do you think?

cc @Tug

[youknowriad]

I'm not certain that's the issue here. Since that plugin icon is always rendered with Icon anyway (before and after). The icon there is an element not a string.

[gziolo]

There is also a different margin when the pinned icon is toggled:

[etoledom]

I'm not certain that's the issue here. Since that plugin icon is always rendered with Icon anyway (before and after). The icon there is an element not a string.

This change do solve the issue with the buttons though. I don't see other possible change on #17228 that could have triggered this problem

-    <Icon icon={ icon } />
+    { isString( icon ) ? <Icon icon={ icon } />  : icon }

Maybe it could at least give some lights?

[youknowriad]

ohhh, actually you're right @etoledom let's just restore the "if" then