Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EuiButtonIcon is missing "secondary" color option #4836

Closed
masonlr opened this issue May 28, 2021 · 3 comments · Fixed by #4874
Closed

EuiButtonIcon is missing "secondary" color option #4836

masonlr opened this issue May 28, 2021 · 3 comments · Fixed by #4874
Assignees
Labels

Comments

@masonlr
Copy link

masonlr commented May 28, 2021

Looking at the v34.1.0 "Split buttons" example here https://elastic.github.io/eui/#/navigation/button, it's not possible to write something like:

<EuiFlexGroup responsive={false} gutterSize="s" alignItems="center">
  <EuiFlexItem grow={false}>
    <EuiButton size="s" color="secondary">
      Secondary action
    </EuiButton>
  </EuiFlexItem>
  <EuiFlexItem grow={false}>
    <EuiButtonIcon
      display="base"
      size="s"
      color="secondary"
      iconType="boxesVertical"
      aria-label="More"
    />
  </EuiFlexItem>
</EuiFlexGroup>

as the "secondary" option is not available on EuiButtonIcon, even though it's available on EuiButton.

To fix this, the color prop on EuiButtonIcon needs to support the "secondary" option.

@masonlr masonlr changed the title "secondary" color option missing for EuiButtonEmpty "secondary" color option missing on EuiButtonIcon May 28, 2021
@masonlr masonlr changed the title "secondary" color option missing on EuiButtonIcon EuiButtonIcon is missing "secondary" color option May 28, 2021
@masonlr
Copy link
Author

masonlr commented May 28, 2021

Additionally, there seems to be a color mismatch when using for example:

<EuiFlexGroup responsive={false} gutterSize="s" alignItems="center">
  <EuiFlexItem>
    <EuiButton
      color="warning"
      size="s"
      iconType="unlink"
    >
      Detach Selected Features
    </EuiButton>
  </EuiFlexItem>
  <EuiFlexItem grow={false}>
    <EuiButtonIcon
      display="base"
      color="warning"
      size="s"
      iconType="boxesVertical"
      aria-label="More"
    />
  </EuiFlexItem>
</EuiFlexGroup>

image

The color of the EuiButton element is different to the color of the EuiButtonIcon element, even though both have the "warning" option for the color prop. Maybe this is intended?

@miukimiu
Copy link
Contributor

Hi @masonlr,

The "secondary" color option is available on EuiButtonIcon as "success". In our new theme we want to deprecate the idea of secondary color and use instead "success".

So in your example you need to:

    <EuiFlexGroup responsive={false} gutterSize="s" alignItems="center">
      <EuiFlexItem grow={false}>
        <EuiButton size="s" color="secondary">
          Secondary action
        </EuiButton>
      </EuiFlexItem>
      <EuiFlexItem grow={false}>
        <EuiButtonIcon
          display="base"
          size="s"
          color="success"
          iconType="boxesVertical"
          aria-label="More"
        />
      </EuiFlexItem>
    </EuiFlexGroup>

I run some tests here and for both examples, the buttons don't quite match the background colors. We should fix this. All colors from the EuiButtonIcon should match the ones from the EuiButton. 😄

@masonlr
Copy link
Author

masonlr commented May 28, 2021

Thanks @miukimiu!

we want to deprecate the idea of secondary color

Good to know.

All colors from the EuiButtonIcon should match the ones from the EuiButton.

Thanks, I kind of moved on when I couldn't get any combination of secondary and success to match! Fixing this so that the EuiButtonIcon and EuiButton styles are consistent would make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants