feat(legend): render multiple columns on float config#1159
feat(legend): render multiple columns on float config#1159markov00 merged 8 commits intoelastic:masterfrom
Conversation
nickofthyme
left a comment
There was a problem hiding this comment.
LGTM, tested locally.
I think we should try to constrain the width to within the chart because this could get unwieldy pretty quickly. Constraining the overall width will start to truncate the longest strings to satisfy the constrain.
| @@ -9,12 +9,6 @@ | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Can we change the .echLegendList styles to include grid by default now?
.echLegendList {
display: grid;
}
&--horizontal {
.echLegendList {
grid-column-gap: $echLegendColumnGap;
grid-row-gap: $echLegendRowGap;
margin-top: $echLegendRowGap;
margin-bottom: $echLegendRowGap;
}
}
src/components/legend/style_utils.ts
Outdated
| paddingTop, | ||
| paddingBottom, | ||
| ...(floating && { | ||
| display: 'grid', |
There was a problem hiding this comment.
Make default, see previous comment
| display: 'grid', |
src/components/legend/style_utils.ts
Outdated
| paddingBottom, | ||
| ...(floating && { | ||
| display: 'grid', | ||
| gridTemplateColumns: `repeat(${clamp(floatingColumns ?? 1, 1, totalItems)}, 1fr`, |
There was a problem hiding this comment.
1fr will cause the largest string to control the width for all columns like this....
If we use auto instead of 1fr then each column width will be independent. Not sure which you think is desirable but I like the independent widths.
| gridTemplateColumns: `repeat(${clamp(floatingColumns ?? 1, 1, totalItems)}, 1fr`, | |
| gridTemplateColumns: `repeat(${clamp(floatingColumns ?? 1, 1, totalItems)}, auto`, |
nickofthyme
left a comment
There was a problem hiding this comment.
LGTM just one comment.
src/components/legend/style_utils.ts
Outdated
| paddingLeft, | ||
| paddingRight, | ||
| gridTemplateColumns: `repeat(auto-fill, minmax(${legendStyle.verticalWidth}px, 1fr))`, | ||
| gridTemplateColumns: `repeat(auto-fill, minmax(${legendStyle.verticalWidth}px, auto))`, |
There was a problem hiding this comment.
I don't think this should be auto. The auto with the auto-fill acts differently than without, like below on line 73. I don't see any noticeable difference here so I think we should revert this and revisit how the horizontal legend sizes the columns is a future PR.
| gridTemplateColumns: `repeat(auto-fill, minmax(${legendStyle.verticalWidth}px, auto))`, | |
| gridTemplateColumns: `repeat(auto-fill, minmax(${legendStyle.verticalWidth}px, 1fr))`, |
# [29.2.0](v29.1.0...v29.2.0) (2021-05-25) ### Bug Fixes * **legend:** disable handleLabelClick for one legend item ([#1134](#1134)) ([a7242af](a7242af)), closes [#1055](#1055) ### Features * **a11y:** add alt text for all chart types ([#1118](#1118)) ([9e42229](9e42229)), closes [#1107](#1107) * **legend:** specify number of columns on floating legend ([#1159](#1159)) ([c2e4652](c2e4652)), closes [#1158](#1158) * simple screenspace constraint solver ([#1141](#1141)) ([eb11480](eb11480))
|
🎉 This PR is included in version 29.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [29.2.0](elastic/elastic-charts@v29.1.0...v29.2.0) (2021-05-25) ### Bug Fixes * **legend:** disable handleLabelClick for one legend item ([opensearch-project#1134](elastic/elastic-charts#1134)) ([e485174](elastic/elastic-charts@e485174)), closes [opensearch-project#1055](elastic/elastic-charts#1055) ### Features * **a11y:** add alt text for all chart types ([opensearch-project#1118](elastic/elastic-charts#1118)) ([e1c7489](elastic/elastic-charts@e1c7489)), closes [opensearch-project#1107](elastic/elastic-charts#1107) * **legend:** specify number of columns on floating legend ([opensearch-project#1159](elastic/elastic-charts#1159)) ([ed3736e](elastic/elastic-charts@ed3736e)), closes [opensearch-project#1158](elastic/elastic-charts#1158) * simple screenspace constraint solver ([opensearch-project#1141](elastic/elastic-charts#1141)) ([af9dd96](elastic/elastic-charts@af9dd96))
Summary
When rendering a floating legend inside the chart (
Settings.legendPosition.floating:true), the addedSettings.legendPosition.floatingColumnsproperty allows you to specify the number of columns used to distributed the legend items.Connected issues
This feature was added as part of the Kibana Timelion chart library replacement.
fix #1158
Checklist
Delete any items that are not applicable to this PR.
src/index.ts(and stories only import from../srcexcept for test data & storybook)