Add gutterSize prop to EuiTimeline

[elizabetdev]

Summary

This PR adds a gutterSize prop to EuiTimelineItem and closes #5944.

Bug fix

This PR also fixes a bug in EuiTimelineItem where the vertical line was not showing on the last item when verticalAlign was set to center.

Logical CSS properties

This PR also introduces the use of logicalCSS to convert certain directional properties to logical properties.

Removed component prop from EuiTmelineItem which makes the EuiTimeline required (breaking change)

To control the gap from the parent EuiTimeline I decided to make this component required. EachEuiTimelineItem could accept a gutterSize prop to override the gutterSize from EuiTimeline. But this could introduce bad usages.

Also, I think there are other good reasons to make the EuiTimeline required:

• Before we could change the EuiTmelineItem HTML element from a default li to a div. This way consumers wouldn't need to wrap the items in a EuiTimeline (ol). But this wouldn't ensure good a11y. Now with the EuiTimeline required we can ensure the use of better semantic elements (ol -> li).
• I searched for this component usage in Kibana/Cloud and there's only one usage (index lifecycle management) and they are wrapping the items in a EuiTimeline. So no problem with removing the component prop.
• One of the reasons I introduced the component prop was because this component was meant to be used in EuiCommentList which didn't require the EuiComment's to be wrapped in a EuiCommentList. But the comment list will have some breaking changes ([EuiComment] Add "custom" option to EuiComment.type and more enhancements #5692) so let's make it also have better semantic elements.
• Also this component is still in beta and it's a good time to fix what is wrong.

Design enhancement

There was an icon min-witdh of 40px. I removed the min-width because for very small avatars the horizontal gap seemed very large. And forxl avatars the vertical line would be centered based on the 40px.

Checklist

• Checked in both light and dark modes
• Checked in mobile
• Checked in Chrome, Safari, Edge, and Firefox
• Props have proper autodocs and playground toggles
• Added documentation
• [ ] Checked Code Sandbox works for any docs examples
• Added or updated jest and cypress tests
• [ ] Checked for breaking changes and labeled appropriately
• [ ] Checked for accessibility including keyboard-only and screenreader modes
• Updated the Figma library counterpart
• A changelog entry exists and is marked appropriately

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5955/

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5955/

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5955/

[cchaos]

The code and style updates LGTM, but what's striking me as an anti-pattern is that a consumer can't apply the gap to the full EuiTimeline only at each EuiTimelineItem

This means for a more condensed display of items, the consumer has to manually pass gap onto each item render. It's also odd for the prop comment to explicitly says that this is spacing between items, but it's only applying to a single item.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5955/

[elizabetdev]

Thanks, @cchaos.

I moved the gap prop to the wrapper EuiTimeline. But this makes the wrapper required which makes sense to me. I added a note to the PR description explaining this change.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5955/

[thompsongl]

I moved the gap prop to the wrapper EuiTimeline. But this makes the wrapper required which makes sense to me.

Is it possible to have the gap on EuiTimeline simply passed through to each EuiTimelineItem?

<ol className={classes} role="list" {...rest}>
  {items.map((item, index) => (
    <EuiTimelineItem key={index} gap={gap} {...item} />
  ))}

EuiTimelineItem still accepts gap and can override gap from EuiTimeline. No breaking change in this case.

[elizabetdev]

Thanks, @thompsongl. I added a better explanation of the issue in the PR description where it says Removed component prop from EuiTmelineItem. Right now there's only one usage of this component in Kibana and Cloud and they're using the the wrapper so in practise nothing will happen.

Having the wrapper as required ensures better a11y. So I think is better if we follow this path.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5955/

[elizabetdev]

Yesterday I talked with @cchaos about this prop name. We agreed on calling it gap but I'm starting to have second thoughts.

The EuiFacetGroup, EuiBadgeGroup, EuiFlexGroup, EuiListGroup, and EuiHeaderLinks have a gutterSize prop.

Reading the discussion on #5575 there's a possibility of adding a prop called gapSize.

I'm thinking that gapSize is indeed more accurate. But gutterSize right now, creates more consistency.

@cchaos @thompsongl any thoughts on naming this prop?

[cchaos]

I flip flop on it honestly, so I'd just leave that up to you.

[thompsongl]

No real preference. If gutterSize seems appropriate and increases consistency, then that's likely a good choice.

[elizabetdev]

Thanks. I went for gutterSize.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5955/

[thompsongl]

Code changes LGTM!

[elizabetdev]

jenkins test this

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5955/

[cchaos]

Did a final pass on code and checked in Chrome. 👍

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5955/