ButtonGroup with Icon and Text children are missaligned

[indiragp]

Issue summary

When you create a ButtonGroup with size="slim" where one Button is text and the other button is an Icon, in a full-screen view the buttons are not-aligned. This works well for any bigger button sizes and when the screen is small (inspecting on chrome on a phone view).

Expected behavior

When creating a ButtonGroup if one Button is text and another button is an Icon, they should be aligned

Actual behavior

There is a 1px missalignment between the button with text and the button with an Icon

Steps to reproduce the problem

Reduced test case

CodeSandbox to reproduce the issue.

Specifications

• Are you using the React components? (Y/N):Y
• Polaris version number: 6.1.0
• Browser: Chrome:89.0.4389.90
• Device: MacBook Pro
• Operating System: macOS 10.15.7

👋 Thanks for opening your first issue. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines.

[indiragp]

Some notes about the issue:
The height for the text button is 28px, where as the height for the button with the icon is 29px.
Another differences are that the Polaris_Button_Content for the text one is 16px and. the Polaris_Button_Content for the icon is 20px.
Some of the things that fix this alignment are:

1. Updating the size of the Polaris-Icon to 19px
2. Setting the Polaris-ButtonGroup__Item height to 28px
3. From the Polaris-ButtonGroup__Item removing the html, body line-height: 2rem;
4. Adding a margin top of 1px to the last button.
5. In Polaris-ButtonGroup__Item there is an applied css:

@media (min-width: 40em)
html, body {
     font-size: 1.4rem;
}

Removing this applied style the rest of the applied font-size applied is 1.5 rem and the buttons get aligned again.

This bug was originally reported as: #2194
I've reported this on Slack but didn't get a response.

[tylernoseworthy]

Hey I am just revisiting this bug that is also outlined in this issue https://github.com/Shopify/core-issues/issues/28798. Is anyone on the team able to take a look at it? or should I just put up a PR? cc @nickpresta

[tylernoseworthy]

Going to pick up this issue.

[tylernoseworthy]

Some notes about the issue: The height for the text button is 28px, where as the height for the button with the icon is 29px. Another differences are that the Polaris_Button_Content for the text one is 16px and. the Polaris_Button_Content for the icon is 20px. Some of the things that fix this alignment are:

1. Updating the size of the Polaris-Icon to 19px
2. Setting the Polaris-ButtonGroup__Item height to 28px
3. From the Polaris-ButtonGroup__Item removing the html, body line-height: 2rem;
4. Adding a margin top of 1px to the last button.
5. In Polaris-ButtonGroup__Item there is an applied css:

@media (min-width: 40em)
html, body {
     font-size: 1.4rem;
}

Removing this applied style the rest of the applied font-size applied is 1.5 rem and the buttons get aligned again.

This bug was originally reported as: #2194 I've reported this on Slack but didn't get a response.

@chloerice out of these above potential fixes, what do you think would be the best solution?

[tylernoseworthy]

This only really seems to be an issue with Buttons that have the slim variant.

[chloerice]

@chloerice out of these above potential fixes, what do you think would be the best solution?

Hey @tylernoseworthy!! 👋🏽 I checked out the updates you pushed to the PR and the negative margin does work, but feels like a deep reach (but then again, so does all of the Sass in that Button stylesheet 😬) that may not be effective for custom icons that might not adhere to the sizing we use internally. I tested adding line-height: normal; to .Item in the ButtonGroup stylesheet and it fixes the issue as well so I would vote for that route.

(Click to view playground code for the screenshot below 👀)

import React from 'react';
import {DeleteMinor} from '@shopify/polaris-icons';

import {Page, Layout, Card, Icon, Button, ButtonGroup} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      <Layout>
        <Layout.Section>
          <Card>
            <Card.Section title="Slim buttons">
              <ButtonGroup segmented>
                <Button size="slim">Remove</Button>
                <Button icon={<Icon source={DeleteMinor} />} size="slim" />
                <Button icon={<Icon source={DeleteMinor} />} size="slim">
                  Remove
                </Button>
                <Button disclosure size="slim">
                  More actions
                </Button>
              </ButtonGroup>
            </Card.Section>

            <Card.Section title="Default buttons">
              <ButtonGroup segmented>
                <Button>Remove</Button>
                <Button icon={<Icon source={DeleteMinor} />} />
                <Button icon={<Icon source={DeleteMinor} />}>Remove</Button>
                <Button disclosure>More actions</Button>
              </ButtonGroup>
            </Card.Section>

            <Card.Section title="Large buttons">
              <ButtonGroup segmented>
                <Button size="large">Remove</Button>
                <Button icon={<Icon source={DeleteMinor} />} size="large" />
                <Button icon={<Icon source={DeleteMinor} />} size="large">
                  Remove
                </Button>
                <Button disclosure size="large">
                  More actions
                </Button>
              </ButtonGroup>
            </Card.Section>

            <Card.Section title="Full width buttons">
              <ButtonGroup fullWidth segmented>
                <Button>Remove</Button>
                <Button icon={<Icon source={DeleteMinor} />} />
                <Button icon={<Icon source={DeleteMinor} />}>Remove</Button>
                <Button disclosure>More actions</Button>
              </ButtonGroup>
            </Card.Section>

            <Card.Section title="Outline buttons">
              <ButtonGroup segmented>
                <Button outline>Remove</Button>
                <Button outline icon={<Icon source={DeleteMinor} />} />
                <Button outline icon={<Icon source={DeleteMinor} />}>
                  Remove
                </Button>
                <Button outline disclosure>
                  More actions
                </Button>
              </ButtonGroup>
            </Card.Section>

            <Card.Section title="Plain buttons">
              <ButtonGroup segmented>
                <Button plain>Remove</Button>
                <Button plain icon={<Icon source={DeleteMinor} />} />
                <Button plain icon={<Icon source={DeleteMinor} />}>
                  Remove
                </Button>
                <Button plain disclosure>
                  More actions
                </Button>
              </ButtonGroup>
            </Card.Section>
          </Card>
        </Layout.Section>
      </Layout>
    </Page>
  );
}

[tylernoseworthy]

@chloerice thanks for looking into those solutions!! I agree, wasn't overly happy on doing any styles within Button. It seemed like all the fixes were simple enough, but may have had impacted other components negatively. Going to update the PR with the line-height: normal fix