Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Legend should not show values from the chart #246

Closed
wylieconlon opened this issue Jun 19, 2019 · 15 comments · Fixed by #553 or elastic/kibana#60578
Closed

Legend should not show values from the chart #246

wylieconlon opened this issue Jun 19, 2019 · 15 comments · Fixed by #553 or elastic/kibana#60578
Labels
discuss To be discussed :legend Legend related issue released Issue released publicly

Comments

@wylieconlon
Copy link

Describe the bug
The legend shows either the right-most value from the chart or the currently-hovered series. It would make more sense to hide the value, giving more space for the legend or chart.

To Reproduce

This behavior is shown in all of the chart samples, including: https://elastic.github.io/elastic-charts/?selectedKind=Bar%20Chart&selectedStory=with%20axis%20and%20legend&full=0&addons=1&stories=1&panelRight=1&addonPanel=storybook%2Fstories%2Fstories-panel

Jun-19-2019 16-33-37

Expected behavior
Because there is already the same information available elsewhere, I would expect the numeric value to be hidden on the legend.

Checklist

  • [ ] every related Kibana issue is listed under Kibana Cross Issues list
  • [ ] kibana cross issue tag is associated to the issue if any kibana cross issue is present
@wylieconlon wylieconlon added discuss To be discussed :legend Legend related issue labels Jun 19, 2019
@wylieconlon
Copy link
Author

Looks like this is potentially also a way to avoid #163

@markov00
Copy link
Member

The reason is exactly the one @emmacunningham wrote in slack: in a time series visualization is useful to see the last bucket value directly on the chart (legend in this case), instead of move your mouse to every single chart and aim the last data point to see its value.

For sure there is a bit of duplication between the tooltip and the legend where, on hover, we are showing all the series values either on the legend and on the tooltip. This doesn’t happen if you are using a TooltipType.Follow tooltip, that will shows only the value of the hovered element (not all the values on the same x value) that is more or less what TSVB is currently doing.

We for sure have to go through our defaults and configure them in a way to avoid as mush as possible code by consumers for sure (there is an issue to follow for that discussion: #121)

I think @AlonaNadler can give us more insight for from a customer point of view. It will be great to have the rationale behind that behaviour in TSVB and it will also help us if we have any ER for that functionality

@AlonaNadler
Copy link

I agree with Emma

in a time series visualization is useful to see the last bucket value directly on the chart (legend in this case), instead of move your mouse to every single chart and aim the last data point to see its value.

@wylie in case users can hide the legend and get more space for the chart, do you still have this concern?

@wylieconlon
Copy link
Author

I don't personally see the value in showing the last value, even in a time series. It feels redundant to what's already shown in two other ways- on the tooltip and by the axis values. It's also not useful for bars. So the ideal change for me is to completely remove this from the legend, and the second best option is to change the default so that this is not shown by default.

@markov00
Copy link
Member

I'm ok to disable the showLegendDisplayValue by default. Currently TSVB and most of the API charts are displaying that the last bucket value, so they can enable it separately.

We will not remove completely that value from the legend, it's a requested feature. We can fix some use cases like when using non temporal data, but we can't remove it completely.

I just want to show you the value of this label on the legend with some use cases:

  1. dashboard full of charts that depict the last x minutes of logs
  2. visualization with 4/5 or more lines/elements displayed
  3. on visualization with stacked values
  4. on a dashboard displayed on a big screen (control center or anything similar)

Let me try to give you some examples on those:

  1. in systems like APM or SIEM tools where the metrics displayed are used to identify as early as possible a problem, it's very useful to check the current/last value of the metrics as fast as possible. It's too time consuming going through all the charts with the mouse to get the last value (specially if you have to report that value to someone else). There are some request in kibana repo about that, where Ops people want to see the data value right away without interactions.
    For example if I look at the following graph my eyes have to go first to the last bar or point on the right, then draw an imaginary line to the left to find the axis, and then back and forth 1 or two times to be completely sure the "interval" of the data value.

Screenshot 2019-06-20 at 18 39 10

Screenshot 2019-06-20 at 18 38 42

  1. if the number of lines increase, it increase also the effort to understand that value from the axis per single chart

  2. on stacked values it also much more difficult to get a nice overview of the value of each single stacked bar, in that case, and if we are on the Operation/APM use case, it can be useful to have a clear understanding of the last bucket of data

  3. on big screen (a led wall or similar) you can get the general overview of the data, but you can't interact. Having a value there can help understand the situation

Same Grafana user also ask us to introduce more values on the legend like avg, min, max

Screenshot 2019-06-20 at 19 10 40

@wylieconlon
Copy link
Author

Okay, so I can see how it would be useful to get an "at a glance" overview of the data, but I don't think that putting the last value in the legend is the most effective way of accomplishing this. It's not obviously connected to the right-most value, and the live-updating on hover makes it more confusing.

In the Grafana example, it has extra text "Current: 103 milliseconds" which gives context for the value. I can think of several alternatives to placing the last value in the legend:

  • Add more text, like "Current: 6". This probably requires more space
  • Use grid lines by default for line charts
  • Create an "at a glance" section which summarizes the data- potentially also useful for accessibility
  • Render a label on the last value, attached to the line. This can be done with Vega, for example:
    Screenshot 2019-06-20 13 38 25

Would you be open to any of those approaches?

@markov00
Copy link
Member

markov00 commented Jun 20, 2019

Absolutely, actually we are just trying to replicate the existing features of the libraries we have on Kibana, so in that case it's a main feature for TSVB.

Add more text, like "Current: 6". This probably requires more space

We can find the required space if necessary, maybe it's worth having a meeting with the design team on that (we actually don't have any specific design decision on the legend)

Use grid lines by default for line charts

We are going to add them, we have a style document that we are following to configure the default style of the charts https://docs.google.com/document/d/1bEK70zhXxM-t64H7rzohtUvo-fMmDy4qG0H7BD48lGo/edit #232

Create an "at a glance" section which summarizes the data- potentially also useful for accessibility

Yep, we also considered that, when we will start planning for accessibility we will consider display some part of the summary of specified by the user configurations.

Render a label on the last value, attached to the line. This can be done with Vega.

That's another idea. There are few small problems here that needs to be solved: labels can overlaps and can happens most of the time for specific type metric charts, and labels moves up and down and you always have to visually link the line color to the color of the line to know what value you are looking at.

So yes, there isn't a best practice, or there isn't a good way or a bad way to accomplish it. As first step we just reimplemented the current solution, we are absolutely open to add and include different solutions to that problem.

@markov00
Copy link
Member

@wylieconlon I think I've just found a more "robust" reason for that value on the legend:
if you add more than one TSVB chart on the same dashboard and you move the cursor over one chart, all the other TSVB chart will shows a line at the same position of the mouse cursor on the other chart. They not only shows that line, but they also update the value on the legend: this allows to quickly check values on the same X position along multiple chart without moving the mouse to each single chart on the same X position.

Jun-26-2019 15-22-17

I'm completely with you about the fact that it's not intuitive at the first look and also redundant when comes to a single visualization, but can be useful when multiple visualization are displayed.

@cchaos
Copy link
Contributor

cchaos commented Jul 26, 2019

For me, the problem I had with the value in the legend was not that it was there at all but that what the value was representing. I would have assumed that while not hovering on the chart, the legend values would be accumulative (or whatever the aggregation is) for the whole series across the x-axis. Basically, a total. I don't think it's clear at all that this number would indicate the last bucket in the series.

Perhaps that's just a small change?

@markov00
Copy link
Member

@cchaos so for sure we have to disable that for ordinal x axis series because we don't have a linear ordering there, so the last bucket in that case doesn't mean nothing.
TSVB was built with that legend feature, so we have to keep it for TSVB.
We should implement various options there like: show last bucket, show total, hide completely.
We should also find a way to describe that value, maybe a small title on top-right of the legend saying: last value, total sum etc?

@cchaos
Copy link
Contributor

cchaos commented Jul 29, 2019

It would be nice not to have to include more text to the chart. I'm thinking that simply using the same highlighting scheme could help clarify. For instance, when indicating "last bucket", when the user is not hovering on the chart, always highlight the last bucket:

@markov00
Copy link
Member

yes that is a good idea, but we have also to consider how this will affect an already existing feature in kibana: we have a rectangular annotation similar to the highlight that highlight if the time window selected doesn't cover fully the first and last buckets
Screenshot 2019-07-29 at 21 32 14

@cchaos
Copy link
Contributor

cchaos commented Jul 29, 2019

Just make sure they're not styled the same, haha 😄

@nickofthyme
Copy link
Collaborator

To add more ideas to this. I'm generally not a fan of this last value thing at all. But I noticed that if we have a split series and one of the series doesn't have a value for the "last" bucket, then the next-to-last value is checked and so on. Potentially resulting in the FIRST value if a given series only has a value in the first bucket. Resulting in a melting pot of first to last values thoroughly confusing the user.

Notice in the gif below how PH has a value for zip (2nd-to-last bucket) but not deb (the last bucket) and the none selected state shows the zip value of 2????

Screen Recording 2019-08-27 at 08 47 AM

Code Example playground.tsx

See code

Copy/paste into playground!

import React from 'react';

import { Axis, Chart, getAxisId, getSpecId, niceTimeFormatter, Position, ScaleType, Settings, BarSeries } from '../src';
import { KIBANA_METRICS } from '../src/utils/data_samples/test_dataset_kibana';
import { CursorEvent } from '../src/specs/settings';
import { CursorUpdateListener } from '../src/chart_types/xy_chart/store/chart_state';

const DATA = [
  {
    'extension.keyword': '',
    'geo.src': 'IN',
    count: 23,
  },
  {
    'extension.keyword': '',
    'geo.src': 'CN',
    count: 17,
  },
  {
    'extension.keyword': '',
    'geo.src': 'US',
    count: 6,
  },
  {
    'extension.keyword': '',
    'geo.src': 'BR',
    count: 3,
  },
  {
    'extension.keyword': '',
    'geo.src': 'FR',
    count: 3,
  },
  {
    'extension.keyword': 'gz',
    'geo.src': 'CN',
    count: 9,
  },
  {
    'extension.keyword': 'gz',
    'geo.src': 'US',
    count: 5,
  },
  {
    'extension.keyword': 'gz',
    'geo.src': 'IN',
    count: 4,
  },
  {
    'extension.keyword': 'gz',
    'geo.src': 'BD',
    count: 3,
  },
  {
    'extension.keyword': 'gz',
    'geo.src': 'AR',
    count: 2,
  },
  {
    'extension.keyword': 'css',
    'geo.src': 'CN',
    count: 7,
  },
  {
    'extension.keyword': 'css',
    'geo.src': 'VE',
    count: 3,
  },
  {
    'extension.keyword': 'css',
    'geo.src': 'IN',
    count: 2,
  },
  {
    'extension.keyword': 'css',
    'geo.src': 'US',
    count: 2,
  },
  {
    'extension.keyword': 'css',
    'geo.src': 'AF',
    count: 1,
  },
  {
    'extension.keyword': 'zip',
    'geo.src': 'CN',
    count: 8,
  },
  {
    'extension.keyword': 'zip',
    'geo.src': 'IN',
    count: 7,
  },
  {
    'extension.keyword': 'zip',
    'geo.src': 'US',
    count: 4,
  },
  {
    'extension.keyword': 'zip',
    'geo.src': 'PH',
    count: 2,
  },
  {
    'extension.keyword': 'zip',
    'geo.src': 'BD',
    count: 1,
  },
  {
    'extension.keyword': 'deb',
    'geo.src': 'CN',
    count: 5,
  },
  {
    'extension.keyword': 'deb',
    'geo.src': 'SA',
    count: 3,
  },
  {
    'extension.keyword': 'deb',
    'geo.src': 'IT',
    count: 2,
  },
  {
    'extension.keyword': 'deb',
    'geo.src': 'PK',
    count: 2,
  },
  {
    'extension.keyword': 'deb',
    'geo.src': 'CG',
    count: 1,
  },
];

export class Playground extends React.Component {
  render() {
    return (
      <Chart>
        <Settings tooltip={{ type: 'vertical' }} debug={false} legendPosition={Position.Right} showLegend />
        <Axis id={getAxisId('timestamp')} title="timestamp" position={Position.Bottom} />
        <Axis id={getAxisId('count')} title="count" position={Position.Left} tickFormat={(d) => d.toFixed(2)} />
        <BarSeries
          id={getSpecId('Testing')}
          xScaleType={ScaleType.Ordinal}
          yScaleType={ScaleType.Log}
          data={DATA}
          xAccessor="extension.keyword"
          yAccessors={['count']}
          splitSeriesAccessors={['geo.src']}
        />
      </Chart>
    );
  }
}

markov00 added a commit to markov00/elastic-charts that referenced this issue Feb 20, 2020
change the default value of `showLegendExtra` to `false`

BREAKING CHANGE: Previously `showLegendExtra` was true by default always showing the legend extra
value. Now it becomes always hidden by default

close elastic#246
markov00 added a commit that referenced this issue Feb 20, 2020
This commit will decouple the tooltip component from the XY chart to allow Partition and other chart type an ease use.

BREAKING CHANGE: the `SeriesIdentifier` type is generalized into a simplified object with two values in common: `specId` and `key`. A specialized `XYChartSeriesIdentifier` extends now the base `SeriesIdentifier`. The `SettingsSpec` prop `showLegendDisplayValue` is renamed to `showLegendExtra` and its default value is now `false` hiding the current/last value on the legend by default.

close #246
markov00 added a commit that referenced this issue Feb 21, 2020
This commit will decouple the tooltip component from the XY chart to allow Partition and other chart type an ease use.

BREAKING CHANGE: the `SeriesIdentifier` type is generalized into a simplified object with two values in common: `specId` and `key`. A specialized `XYChartSeriesIdentifier` extends now the base `SeriesIdentifier`. The `SettingsSpec` prop `showLegendDisplayValue` is renamed to `showLegendExtra` and its default value is now `false` hiding the current/last value on the legend by default.

close #246
markov00 added a commit that referenced this issue Mar 2, 2020
This commit will decouple the tooltip component from the XY chart to allow Partition and other chart type an ease use.

BREAKING CHANGE: the `SeriesIdentifier` type is generalized into a simplified object with two values in common: `specId` and `key`. A specialized `XYChartSeriesIdentifier` extends now the base `SeriesIdentifier`. The `SettingsSpec` prop `showLegendDisplayValue` is renamed to `showLegendExtra` and its default value is now `false` hiding the current/last value on the legend by default.

close #246
markov00 added a commit that referenced this issue Mar 2, 2020
This commit moves the `Datum`, `Rotation`, `Position` and `Color` into `utils/commons`. It decouples the legend from axis position and moves the `scales` to `utils/scales`.
It decouples the tooltip component from the XY chart to allow Partition charts and other chart to use it.

close #246

Co-authored-by: Marco Vettorello <vettorello.marco@gmail.com>
markov00 pushed a commit that referenced this issue Mar 17, 2020
# [18.0.0](v17.1.1...v18.0.0) (2020-03-17)

### Code Refactoring

* clean up TS types ([#554](#554)) ([22f7635](22f7635)), closes [#547](#547) [#547](#547)
* decouple tooltip from XY chart ([#553](#553)) ([e70792e](e70792e)), closes [#246](#246)

### Features

* cleaner color API on SeriesSpec ([#571](#571)) ([f769f7c](f769f7c))
* **legend:** allow color picker component render prop ([#545](#545)) ([90f4b95](90f4b95))
* **partition:** add element click, over and out events ([#578](#578)) ([103df02](103df02))
* **partition:** add tooltip ([#544](#544)) ([6bf9a69](6bf9a69)), closes [#246](#246)
* percentage display in partitioning charts ([#558](#558)) ([d6aa8d7](d6aa8d7))
* specify series name with a function on SeriesSpec ([#539](#539)) ([358455a](358455a))
* xAccessor can be a function accessor ([#574](#574)) ([bcc3d63](bcc3d63))

### BREAKING CHANGES

* The `getSpecId`, `getGroupId`, `getAxisId` and `getAnnotationId` are no longer available. Use a simple `string` instead.
* `customSeriesColors` prop on `SeriesSpec` is now `color`. The `CustomSeriesColors` type is  replaced with `SeriesColorAccessor`.
* Remove `customSubSeriesName` prop on series specs in favor of cleaner api using just the `name` prop on `SeriesSpec`. The types `SeriesStringPredicate`, `SubSeriesStringPredicate` have been removed.
* the `SeriesIdentifier` type is generalized into a simplified object with two values in common: `specId` and `key`. A specialized `XYChartSeriesIdentifier` extends now the base `SeriesIdentifier`. The `SettingsSpec` prop `showLegendDisplayValue` is renamed to `showLegendExtra` and its default value is now `false` hiding the current/last value on the legend by default.
@markov00
Copy link
Member

🎉 This issue has been resolved in version 18.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Mar 17, 2020
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this issue Feb 10, 2022
# [18.0.0](elastic/elastic-charts@v17.1.1...v18.0.0) (2020-03-17)

### Code Refactoring

* clean up TS types ([opensearch-project#554](elastic/elastic-charts#554)) ([6ce56fa](elastic/elastic-charts@6ce56fa)), closes [opensearch-project#547](elastic/elastic-charts#547) [opensearch-project#547](elastic/elastic-charts#547)
* decouple tooltip from XY chart ([opensearch-project#553](elastic/elastic-charts#553)) ([cb02cd0](elastic/elastic-charts@cb02cd0)), closes [opensearch-project#246](elastic/elastic-charts#246)

### Features

* cleaner color API on SeriesSpec ([opensearch-project#571](elastic/elastic-charts#571)) ([6a78c4e](elastic/elastic-charts@6a78c4e))
* **legend:** allow color picker component render prop ([opensearch-project#545](elastic/elastic-charts#545)) ([22ef1e6](elastic/elastic-charts@22ef1e6))
* **partition:** add element click, over and out events ([opensearch-project#578](elastic/elastic-charts#578)) ([4189573](elastic/elastic-charts@4189573))
* **partition:** add tooltip ([opensearch-project#544](elastic/elastic-charts#544)) ([0cffed4](elastic/elastic-charts@0cffed4)), closes [opensearch-project#246](elastic/elastic-charts#246)
* percentage display in partitioning charts ([opensearch-project#558](elastic/elastic-charts#558)) ([993a448](elastic/elastic-charts@993a448))
* specify series name with a function on SeriesSpec ([opensearch-project#539](elastic/elastic-charts#539)) ([fc6430b](elastic/elastic-charts@fc6430b))
* xAccessor can be a function accessor ([opensearch-project#574](elastic/elastic-charts#574)) ([f702e2c](elastic/elastic-charts@f702e2c))

### BREAKING CHANGES

* The `getSpecId`, `getGroupId`, `getAxisId` and `getAnnotationId` are no longer available. Use a simple `string` instead.
* `customSeriesColors` prop on `SeriesSpec` is now `color`. The `CustomSeriesColors` type is  replaced with `SeriesColorAccessor`.
* Remove `customSubSeriesName` prop on series specs in favor of cleaner api using just the `name` prop on `SeriesSpec`. The types `SeriesStringPredicate`, `SubSeriesStringPredicate` have been removed.
* the `SeriesIdentifier` type is generalized into a simplified object with two values in common: `specId` and `key`. A specialized `XYChartSeriesIdentifier` extends now the base `SeriesIdentifier`. The `SettingsSpec` prop `showLegendDisplayValue` is renamed to `showLegendExtra` and its default value is now `false` hiding the current/last value on the legend by default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss To be discussed :legend Legend related issue released Issue released publicly
Projects
None yet
5 participants