elastic / elastic-charts

https://elastic.github.io/elastic-charts/storybook
Other
366 stars 116 forks source link

[Legend] Actions icon misaligned when using Eui components #1832

Open PhilippeOberti opened 1 year ago

PhilippeOberti commented 1 year ago

Describe the issue The icon shown when using the legendAction prop can be sometimes misaligned. Using for example eui components (like EuiToolTip, EuiButtonIcon...) breaks a bit the UI:

Image 2022-09-30 at 1 51 13 PM

Notice the misalignment of the legend action icon relative to the legend label

To Reproduce See demo here which is based off of the 11_legend_actions.story and modify the getLegendAction method (found here). The Settings.legendAction prop expects ComponentType<LegendActionProps> which can be implemented with the EuiPopover. The EuiPopover expects a button prop to trigger the popover. The storybook example uses an html button tag like below...

// button as html button
const Button = (
  <button
    type="button"
    ref={ref}
    style={{
      display: 'flex',
      justifyContent: 'center',
      alignItems: 'center',
      paddingLeft: 2,
      paddingRight: 2,
    }}
    onClick={() => setPopoverOpen(!popoverOpen)}
  >
    <EuiIcon size="s" type="pencil" />
  </button>
);

However, when you instead use an EuiToolTip with EuiButtonIcon as the button, this causes the shift in the action alignment.

// button as EuiToolTip with EuiButtonIcon
const Button = (
  <EuiToolTip content="Test">
    <EuiButtonIcon
      aria-label="Test"
      iconType="boxesHorizontal"
      iconSize="s"
      size="xs"
      onClick={() => setPopoverOpen(!popoverOpen)}
    />
  </EuiToolTip>
);

This would result in something like this...

bad

Expected behaviour Using EUI components shouldn't break the way the icons are displayed.

PhilippeOberti commented 1 year ago

The main issue here is that we limit the hight of the legend action to 18px and the EuiButtonIcon height is controlled by the size prop.

It seems that there are a couple of ways to get around this issue:

First approach consists of applying some styles to the EuiButtonIcon, like

const Button = (
  <EuiToolTip content="Test">
    <EuiButtonIcon
      aria-label="Test"
      iconType="boxesHorizontal"
      iconSize="s"
      size="xs"
      onClick={() => setPopoverOpen(!popoverOpen)}
      // Add this style here 👇
      style={{ height: "100%" }}
    />
  </EuiToolTip>
);

Before

image

After

image

Another approach is to not use the EuiButtonIcon at all. This would avoid EUI highjacking the chart styles, and follow what's already done in the getLegendAction used in the story.