Ensure tooltips are labeled with percentage values#13212
Ensure tooltips are labeled with percentage values#13212thomasneirynck wants to merge 2 commits intoelastic:6.0from
Conversation
| } else { | ||
| return false; | ||
| } | ||
| return (scale.invert) ? true : false; |
There was a problem hiding this comment.
how is this relatedto this PR ?
you could also write it as return !!scale.invert
| import _ from 'lodash'; | ||
| import d3 from 'd3'; | ||
| import { SCALE_MODES } from './scale_modes'; | ||
| import { SCALE_MODES } from '../../../vis/scale_modes'; |
There was a problem hiding this comment.
scale modes is something thats really vislib specific, should not be used from outside, and should not be part of vis
| @@ -0,0 +1,10 @@ | |||
| import { SCALE_MODES } from '../vis/scale_modes'; | |||
|
|
|||
| export function isPercentage(aggConfigId, seriesParams, valueAxes) { | |||
There was a problem hiding this comment.
this is already implemented on the value axis.
handler.charts[0].series[0].getValueAxis().axisConfig.isPercentage()
what about adding a getSeries method to handler, which will return the right series based on id ?
handler.getSeries(serieId).getValueAxis().axisConfig.isPercentage()
|
|
||
| if (!this.type) return ''; | ||
| let pre = (_.get(this.vis, 'params.mode') === 'percentage') ? 'Percentage of ' : ''; | ||
| let pre = (this.isPercentage()) ? 'Percentage of ' : ''; |
There was a problem hiding this comment.
why not handle both the same ? (label and the value)
so you already have set the eventData.percent when you want your value to be rendered as percent,
so you only need to print different label in tooltip formatter when that happens (check is already there), no need to polute aggConfigs
| "addLegend": true, | ||
| "addTimeMarker": false, | ||
| "addTooltip": true, | ||
| "categoryAxes": [ |
There was a problem hiding this comment.
extract the config to fixtures ? (i think there are already some configs there) ... also i don't think the whole config is needed ?
|
i created a spin off, take a look at #13217 |
|
Closing this. This fix is a little too convoluted. We decided that we want to match pre 5.4 behavior, which means that we do not need to display a label like "Percentage of ....". The added advantage of that is that we can just read out if an axis is in percentage mode from the |
Closes #12391.
this is a reprise of #12746. Opened against 6.0 (some issues with ES/Kibana right now on master. Will forward and backport).
This is a spot fix, suitable for v5 and v6. #12661, which is still a WIP, may introduce another approach, but cannot be backported to v5 and may not be ready for v6 release.