eui
eui copied to clipboard
[EuiDescriptionList] Add option for vertical spacing
Summary
There have been several instances when using EuiDescriptionList where the default 16px spacing feels too generous. I've run into this when working on flyouts, expanded table rows, or dashboard-esque panels where we provide summary information. In these cases, i've found that 8px feels more appropriate. This could also work well on smaller devices / breakpoints. I've seen engineers work around this by using multiple EuiDescriptionLists with just a single title/description inside so they can control the spacing with EuiSpacer or custom CSS. This feels a little messy and not very semantic. Ideally all items are rendered inside a single <dl>
Proposal
Add a prop to EuiDescriptionList to enable this. The prop should default to our current 16px spacing so current implementations are unaffected. I imagine this prop would be ignored when using the condensed layout option. I'm open to ideas on what we call this prop. I like spacing, which would accept sizes such as s (8px) and m (16px, default). However, this prop name isn't used anywhere else (AFAIK) and might be opening a can of worms. Open to ideas!

Next steps
Curious how others feel about this. If there's consensus, I'd be happy to take this on / assign myself. Shouldn't be too hard and it's been awhile since I've contributed :)
I think it's a fair proposal and I would name the prop gap and use the actual CSS gap property.
Ah good call. gap sounds perfect. Thanks for the tip! I'll add this to my back burner for when I have time.
@hbharding We're currently converting our components to CSS-in-JS. This one will likely be on the early side since it's one of the simpler components. Should we consider this issue actually open for grabs so if we get to the conversion before you've tackled this issue, should we go ahead and implement it?
Good point. Prime candidate for CSS-in-JS conversion. This isn't urgent, so I'll hold off until then and let someone else tackle it. Thanks!
While you're in here I would :100: deep dive on how these things wrap as well. Most of the ugly usages of this component are due to people stuffing way too much in there. In some cases some line zebraing would go a long way to legibility.
Happy to dive in! I can't count the number of times I've witnessed what you're describing @snide. Sometimes our team has opted instead to use a "headerless" 2 column EuiTable which provides control over column width and comes with horizontal dividers and a hover effect on each row... Probably not the most semantic implementation, and all the dividers sometimes feel a bit heavy.
Some other ideas I might consider:
- For column layouts, an option (or built in behavior) to adjust the column ratio. It's currently 1:1, which isn't always ideal. It can cause excessive word wrapping and a large gap between the title and description when the title is short and the descriptions are long. When the content is known, this option could be useful.
- Adding an option to
EuiDescriptionListDescriptionso that the value can be copied. This is a pretty common pattern across Kibana that has a lot of different implementations. Sometimes a tooltip appears that says "Copied" and another says "Copied to clipboard". I think I've even seen a toast appear.
With regard to bullet one, I just implemented a similar thing for EuiDescribedFormGroup where I added a ratio prop and then simply used the flex-grow property to adjust the scale of each column: https://github.com/elastic/eui/pull/5756/files#diff-ceeb6d2fc8ab3fb3e87ecbb3c5f9be85ee2f2132d2e9192d197acbcbb116b5f5
Your second bullet though, needs more spec. I'm not sure we'd want it to be directly part of this component. We already have the EuiCopy wrapping component that creates those tooltips as you mention.
Your second bullet though, needs more spec. I'm not sure we'd want it to be directly part of this component. We already have the EuiCopy wrapping component that creates those tooltips as you mention.
I was in the middle of editing my comment and you beat me to it haha. I was going to suggest a wrapper component, and forgot EuiCopy exists. Anyway, agree on needing more spec. Just an early thought.