[Elastic Charts] Added value labels styles for bar charts #4845

elizabetdev · 2021-06-01T10:33:22Z

Summary

Closes #4267.

This PR adds better value labels default styles for bar charts.

By default the value labels once enabled are going to:

Be vertical and horizontal centered. The ideal positions would be based on the rotation of the bars. But this can't be achieved because we can't pass to the theme object what is the current rotation. So vertical and horizontal centered labels seem a good compromise.
The color will adapt to black or white according to the background (textInvertible)

The following demo #/elastic-charts/categorical was updated and a "Show value labels" switch was added. The value labels adapt the position according to the configuration. They are positioned close to the edge of the bars.

When it's multi-series and stacked the value labels are positioned vertically and horizontally in the center.

Checklist

Check against all themes for compatibility 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 the any docs examples~~
[ ] Added or updated jest tests
~~[ ] Checked for breaking changes and labeled appropriately~~
~~[ ] Checked for accessibility including keyboard-only and screenreader modes~~
A changelog entry exists and is marked appropriately

src/themes/charts/themes.ts

kibanamachine · 2021-06-01T10:55:32Z

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

…lue-labels

kibanamachine · 2021-07-26T17:25:00Z

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

kibanamachine · 2021-07-27T12:19:04Z

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

kibanamachine · 2021-07-27T12:59:56Z

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

kibanamachine · 2021-07-27T14:47:47Z

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

kibanamachine · 2021-07-27T16:21:06Z

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

cchaos

👍 💯 Much better defaults for sure!

I have mainly suggestions around documentation and giving more to the consumer than just a toggle.

CHANGELOG.md

src-docs/src/views/elastic_charts/category_chart.js

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

kibanamachine · 2021-07-28T11:38:19Z

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

…lue-labels

kibanamachine · 2021-07-28T17:55:05Z

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

markov00

I've only one comment:
when rendering stacked bars, I agree with the choice of having the labels centered in the bars, the value corresponds to the size of the bar and is not directly connected with the Y-axis.
Instead, when you only have a single series, the label could be placed near the top edge of the bar. In that way, you link the label to its corresponding position on the Y-axis. This also limits the eye movements when reading the bar, reducing the gap between the top edge and the value label.
Using your screenshot as an example, I'm noticing that my eyes are focused or on the value labels (centered) or on the bars' shape/trend. Moving the label closer to the edge makes the readability easier

monfera · 2021-07-29T13:16:28Z

I agree it'd be better for the numbers in the non-stacked case to be at the top. Also, the custom is that if there's room past the single bar (in the positive or negative direction, wherever the bar extends toward) then the label would be outside the bar. There's more text contrast there. Example elastic/elastic-charts#1277

This may not be the place to raise this issue, but our bar charts, and charts in general are configured with a suboptimal balance of font size and data glyph (bar rectangle, etc.) shape. The fonts are small and the data shapes are large. It looks like a functional prototype not the final product. It's also an issue from an accessibility viewpoint: the user gets a huge piece of help in determining the screenspace size of the data shapes, but it's at the expense of font size. Also, even if there's ample room and the user has something like 3 bars, it shouldn't take up all that space. There ought to be a ratio of font size to bar width. For example, on the above images, the bar thickness is easily 3x that of the category labels, and the slightly taller value label inside the bar is also barely over 1/3rd of the height. 1.618x (golden ratio) to around 2x ratio would work but 3x is visually and accessibility wise unbalanced. Eg. this uses around 2x, despite few bars (if there are many bars, it's OK for the text to be about as tall as the bar is thick). This page has a couple of bad examples or unsupported advice, but most charts have a good font/shape balance. Big bars, small fonts is the main way to recognize Kibana visualizations in the wild
elastic/elastic-charts#1276

elizabetdev · 2021-07-29T13:44:40Z

Instead, when you only have a single series, the label could be placed near the top edge of the bar. In that way, you link the label to its corresponding position on the Y-axis. This also limits the eye movements when reading the bar, reducing the gap between the top edge and the value label.

@markov00 I agree. But how can I get this as default in the theme? The theme is static and doesn't know what is the current rotation state. Inside the theme object, we don't know if the chart is rotated or if it is stacked. That's why I had to pick one default alignment that would make more sense to most use cases.

Then in the demo, we're showing that you don't need to use the defaults but implement an alignment that makes sense:

The first time a value label is enabled it is aligned at the end of the bar chart. But then you have the option to center align. The idea is that users can play around with the toggles and copy the configuration that makes sense for their use case. We even recommend just use value labels when it's strictly necessary.

…lue-labels

kibanamachine · 2021-09-09T16:30:01Z

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

kibanamachine · 2021-09-09T17:33:14Z

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

…lue-labels

elizabetdev · 2021-09-09T17:49:18Z

Default theme

@markov00 as we agreed, the default theme keeps the value labels vertically/horizontally centered.

Demo (https://eui.elastic.co/pr_4845/#/elastic-charts/categorical)

@cchaos and @markov00 for the demo in our docs, I removed the center value labels switch option. Now according to the configuration, the value labels adapt.

when rendering stacked bars, I agree with the choice of having the labels centered in the bars

✔️ When stacked the labels get centered

Instead, when you only have a single series, the label could be placed near the top edge of the bar.

✔️ They are now positioned near the edge of the bar

Generated configuration

I also improved the data of the "Copy code of current configuration" and removed the orderBy (lodash) that was included in the generated code.

suggestion: usually the reason why you want to show numbers on a chart is that you want to clearly present the exact number (all digits/decimal digits are important). If that is the case, a suggestion could be to include, on the side, a small table that definitely shows clearly all the numbers and allow the user to sort them by value or categories.

markov00 · 2021-09-13T08:09:37Z

src-docs/src/views/elastic_charts/category_chart.js

+        offsetX: rotated ? 2 : 0,
+        offsetY: rotated ? 0 : -2,


From what I see, this is used as padding on the text. We can probably introduce a default or custom padding and we can get rid of this part

elastic/elastic-charts#1375

I agree 👍🏽

Optional, but I'd love to see even more separation.

Suggested change

offsetX: rotated ? 2 : 0,

offsetY: rotated ? 0 : -2,

offsetX: rotated ? 4 : 0,

offsetY: rotated ? 0 : -4,

markov00 · 2021-09-13T08:23:39Z

src/themes/charts/themes.ts

@@ -100,9 +100,17 @@ function createTheme(colors: any): EuiChartThemeType {
      },
      barSeriesStyle: {
        displayValue: {
-          fontSize: 8,
+          fontSize: 10,


You can also set a min and max font size, the code automatically uses the right size depending on the available space.

cchaos

Overall the new toggle and theme settings LGTM. Had a couple questions and suggestions, but ok to merge in as-is.

CHANGELOG.md

cchaos · 2021-09-14T15:55:42Z

src-docs/src/views/elastic_charts/category_chart.js

+  <Axis
+    id="left-axis"
+    position={${rotated ? '"bottom"' : '"left"'}}
+    ${formatted ? 'tickFormat={d => `${round(Number(d) / 1000, 2)}k`}' : ''}


@markov00 The one thing I noticed is that when we format the tick labels on a particular axis, it does also get applied to the showValueLabel setting, except when the chart gets rotated. Is this a known issue in elastic-charts?

thank you @cchaos you are right, this is an issue in our code. I will check and fix the bug soon.

elastic/elastic-charts#1387

cchaos · 2021-09-14T15:59:26Z

src-docs/src/views/elastic_charts/category_chart.js

+        offsetX: rotated ? 2 : 0,
+        offsetY: rotated ? 0 : -2,


Optional, but I'd love to see even more separation.

Suggested change

offsetX: rotated ? 2 : 0,

offsetY: rotated ? 0 : -2,

offsetX: rotated ? 4 : 0,

offsetY: rotated ? 0 : -4,

src-docs/src/views/elastic_charts/category_chart.js

…lue-labels

kibanamachine · 2021-09-16T15:09:20Z

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

Adding new theme value labels styling

2cd5539

elizabetdev commented Jun 1, 2021

View reviewed changes

src/themes/charts/themes.ts Outdated Show resolved Hide resolved

elizabetdev added 4 commits July 16, 2021 18:06

Merge remote-tracking branch 'upstream/master' into xy-chart-theme-va…

c09bfdf

…lue-labels

Merge remote-tracking branch 'upstream/master' into xy-chart-theme-va…

6c4e731

…lue-labels

Setting textBorder to zero

e1f3f7c

Merge remote-tracking branch 'upstream/master' into xy-chart-theme-va…

9382384

…lue-labels

elizabetdev added 2 commits July 26, 2021 20:31

Adding showValueLabels field in MultiChartCard

56a8aab

Increase category chart height

7a3f23d

elizabetdev added 2 commits July 27, 2021 13:32

Adding center alignment

20e86ea

CL

90166c3

elizabetdev changed the title ~~[Elastic Charts] Adding new theme value labels styling~~ [Elastic Charts] Added value labels styles for bar charts Jul 27, 2021

Shared comp improvement

69e1ab3

elizabetdev marked this pull request as ready for review July 27, 2021 17:12

elizabetdev requested review from markov00 and cchaos July 27, 2021 17:13

cchaos reviewed Jul 27, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

src-docs/src/views/elastic_charts/category_chart.js Show resolved Hide resolved

src-docs/src/views/elastic_charts/category_chart.js Outdated Show resolved Hide resolved

Update CHANGELOG.md

8d1242f

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

elizabetdev added 3 commits July 28, 2021 12:39

Merge remote-tracking branch 'upstream/master' into xy-chart-theme-va…

f63de66

…lue-labels

Adding value labels card and config text to copy.

7973a88

Merge remote-tracking branch 'upstream/master' into xy-chart-theme-va…

a5c0094

…lue-labels

elizabetdev requested a review from cchaos July 28, 2021 17:48

markov00 reviewed Jul 29, 2021

View reviewed changes

markov00 requested a review from monfera July 29, 2021 12:53

monfera mentioned this pull request Jul 29, 2021

Improve font-size/data glyph balance elastic/elastic-charts#1276

Open

elizabetdev added 3 commits September 9, 2021 16:31

Merge remote-tracking branch 'upstream/master' into xy-chart-theme-va…

e82929f

…lue-labels

Adding multi and stacked centered

9ea28f2

CL

924eaf8

Adding better data for multi series and removing lodash orderBy

91e91f5

elizabetdev added 2 commits September 9, 2021 18:42

Better indentation

d3c215c

Merge remote-tracking branch 'upstream/master' into xy-chart-theme-va…

2833783

…lue-labels

markov00 mentioned this pull request Sep 13, 2021

Replace offsetX and offsetY with padding in ValueLabels elastic/elastic-charts#1375

Open

markov00 approved these changes Sep 13, 2021

View reviewed changes

cchaos approved these changes Sep 14, 2021

View reviewed changes

elizabetdev added 2 commits September 16, 2021 15:39

Custom theme only when value labels. Small styles fixes.

9a19371

Merge remote-tracking branch 'upstream/master' into xy-chart-theme-va…

467370e

…lue-labels

elizabetdev merged commit a4de4d8 into elastic:master Sep 16, 2021

markov00 mentioned this pull request Sep 20, 2021

Wrong tickFormat applied to the value labels elastic/elastic-charts#1387

Closed

breehall mentioned this pull request Oct 1, 2021

Bumping EUI to 39.0.0 elastic/kibana#113633

Merged

+                      <EuiFlexItem>
+                        <ChartCard
+                          title="Value labels"
+                          description="Value labels can add too much detail and make categorical charts more difficult to interpret. Consider showing them only when the values are of extreme importance."

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Elastic Charts] Added value labels styles for bar charts #4845

[Elastic Charts] Added value labels styles for bar charts #4845

elizabetdev commented Jun 1, 2021 •

edited

Loading

kibanamachine commented Jun 1, 2021

kibanamachine commented Jul 26, 2021

kibanamachine commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

cchaos left a comment

kibanamachine commented Jul 28, 2021

kibanamachine commented Jul 28, 2021

markov00 left a comment

monfera commented Jul 29, 2021 •

edited by markov00

Loading

elizabetdev commented Jul 29, 2021

kibanamachine commented Sep 9, 2021

kibanamachine commented Sep 9, 2021

elizabetdev commented Sep 9, 2021

kibanamachine commented Sep 9, 2021

markov00 left a comment

markov00 Sep 13, 2021

markov00 Sep 13, 2021

markov00 Sep 13, 2021

elizabetdev Sep 13, 2021

cchaos Sep 14, 2021

markov00 Sep 13, 2021

cchaos left a comment

cchaos Sep 14, 2021

markov00 Sep 20, 2021

markov00 Sep 20, 2021

cchaos Sep 14, 2021

kibanamachine commented Sep 16, 2021

[Elastic Charts] Added value labels styles for bar charts #4845

[Elastic Charts] Added value labels styles for bar charts #4845

Conversation

elizabetdev commented Jun 1, 2021 • edited Loading

Summary

Checklist

kibanamachine commented Jun 1, 2021

kibanamachine commented Jul 26, 2021

kibanamachine commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

cchaos left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 28, 2021

kibanamachine commented Jul 28, 2021

markov00 left a comment

Choose a reason for hiding this comment

monfera commented Jul 29, 2021 • edited by markov00 Loading

elizabetdev commented Jul 29, 2021

kibanamachine commented Sep 9, 2021

kibanamachine commented Sep 9, 2021

elizabetdev commented Sep 9, 2021

Default theme

Demo (https://eui.elastic.co/pr_4845/#/elastic-charts/categorical)

Generated configuration

Next

kibanamachine commented Sep 9, 2021

markov00 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Sep 16, 2021

elizabetdev commented Jun 1, 2021 •

edited

Loading

monfera commented Jul 29, 2021 •

edited by markov00

Loading