Also we would have to invent an icon-only ToggleButton class.

FWIW, we do need this eventually, for T313835: Add icon-only support to ToggleButton

On 3. Having a span in a button is weird, but the layout need might overweigh the markup confusion. Noting that we could run in a number of problems with nesting text in spans as we don't have all control over button markup in every place.

Note that the used font is also responsible here, as different font families feature different letter boxes.

As brought up by @AnneT on the most recent patch on using text-top:
It would move the icons down by half a pixel or a pixel at 14px base (Chrome/macOS):

vertical-align: middle	vertical-align: text-top

Therefore I'd prefer middle.
Revisiting in Firefox/macOS, it does something completely different due to the Helvetica letter boxes.

@Volker_E using vertical-align: text-top compared to middle moves the icons up, not down

Sorry I'm missing a lot of context here, but whats the reason why we cant use flexbox? In my quick testing, something like this seems to look alright:

display: flex; // or inline-flex for the button groups
align-items: center;
justify-content: center;
column-gap: 4px; // arbitrary value

The only obvious case where this looks a little off to me is with certain icons that aren't symetrical i.e. the chat icon is technically perfectly centered, but it due to the icon it almost appears vertically not centered

For completion, in a dev meeting we've compared what's happening on our very own machines, and there's no clear advantageous path with any of the vertical-align values right now including text-top. We'll make this a topic in future design/eng sync.

@lwatson FYI, this is the component-library-wide vertical alignment issue I was talking about

This problem has been around for a while now. I think that it's pretty glaring in Icon-only buttons in particular.

We could separate out rules for icon-only buttons for perfect rendering with Flexbox. But then we'd have different positioning of icon-only and icon + text buttons. Also we would have to invent an icon-only ToggleButton class. Maybe not a bad idea anyways, to be equal to Button. But the patch would grow and grow

I'd like to propose that we go ahead and do this – vertical-align: middle is simply not working for icon buttons (an extremely common UI element) and we can fix this right away using Flexbox.